This is an automated email from the ASF dual-hosted git repository.
enorman pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-core.git
The following commit(s) were added to refs/heads/master by this push:
new e4481ee SLING-10147 block unauthorized access to
SlingBindingsVariablesListJsonServlet (#7)
e4481ee is described below
commit e4481eeafdd5a6b1c1114f5746fc13cd7d55d808
Author: Eric Norman <[email protected]>
AuthorDate: Fri Mar 26 11:13:02 2021 -0700
SLING-10147 block unauthorized access to
SlingBindingsVariablesListJsonServlet (#7)
scripting variables implementation details were exposed to not authorized
users
---
pom.xml | 13 +
.../SlingBindingsVariablesListJsonServlet.java | 32 ++
.../sling/scripting/core/it/SLING_10147IT.java | 352 +++++++++++++++++++++
3 files changed, 397 insertions(+)
diff --git a/pom.xml b/pom.xml
index fdac974..c813149 100644
--- a/pom.xml
+++ b/pom.xml
@@ -272,6 +272,13 @@
</dependency>
<dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient-osgi</artifactId>
+ <version>4.5.8</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.testing.osgi-mock</artifactId>
<version>2.3.6</version>
@@ -376,5 +383,11 @@
<version>1.2.6</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+
<artifactId>org.apache.sling.extensions.webconsolesecurityprovider</artifactId>
+ <version>1.2.4</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/src/main/java/org/apache/sling/scripting/core/impl/SlingBindingsVariablesListJsonServlet.java
b/src/main/java/org/apache/sling/scripting/core/impl/SlingBindingsVariablesListJsonServlet.java
index 7e99689..76ca403 100644
---
a/src/main/java/org/apache/sling/scripting/core/impl/SlingBindingsVariablesListJsonServlet.java
+++
b/src/main/java/org/apache/sling/scripting/core/impl/SlingBindingsVariablesListJsonServlet.java
@@ -26,8 +26,11 @@ import javax.script.ScriptEngineFactory;
import javax.script.ScriptEngineManager;
import javax.servlet.Servlet;
import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletResponse;
import org.apache.felix.utils.json.JSONWriter;
+import org.apache.felix.webconsole.WebConsoleSecurityProvider;
+import org.apache.felix.webconsole.WebConsoleSecurityProvider2;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.resource.NonExistingResource;
@@ -43,6 +46,8 @@ import org.osgi.service.component.ComponentContext;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicyOption;
/**
* Return all scripting variables for all registered scripting languages for
the default context (=request).
@@ -68,6 +73,12 @@ public class SlingBindingsVariablesListJsonServlet extends
SlingSafeMethodsServl
private static final long serialVersionUID = -6744726829737263875L;
/**
+ * The webconsole security provider
+ */
+ @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption =
ReferencePolicyOption.GREEDY)
+ private WebConsoleSecurityProvider webconsoleSecurity;
+
+ /**
* The script engine manager.
*/
@Reference
@@ -91,6 +102,27 @@ public class SlingBindingsVariablesListJsonServlet extends
SlingSafeMethodsServl
@Override
protected void doGet(SlingHttpServletRequest request,
SlingHttpServletResponse response)
throws ServletException, IOException {
+ boolean allowed = true;
+ if (webconsoleSecurity == null) {
+ log("Access forbidden as the WebConsoleSecurity reference is not
set");
+ allowed = false;
+ } else if (!(webconsoleSecurity instanceof
WebConsoleSecurityProvider2)) {
+ log("Access forbidden as the WebConsoleSecurity reference does not
implement WebConsoleSecurityProvider2");
+ allowed = false;
+ } else if
(!((WebConsoleSecurityProvider2)webconsoleSecurity).authenticate(request,
response)) {
+ log("Access forbidden as the WebConsoleSecurity component returned
false");
+ // the request is terminated without any more response sent back
to the client.
+ // The WebConsoleSecurityProvider2 implementation may have sent
auth challenge to the client
+ // in the case of anonymous access.
+ allowed = false;
+ }
+ if (!allowed) {
+ if (!response.isCommitted()) {
+ response.sendError(HttpServletResponse.SC_FORBIDDEN);
+ }
+ return;
+ }
+
response.setContentType("application/json");
JSONWriter jsonWriter = new JSONWriter(response.getWriter());
jsonWriter.array();
diff --git
a/src/test/java/org/apache/sling/scripting/core/it/SLING_10147IT.java
b/src/test/java/org/apache/sling/scripting/core/it/SLING_10147IT.java
new file mode 100644
index 0000000..4312e36
--- /dev/null
+++ b/src/test/java/org/apache/sling/scripting/core/it/SLING_10147IT.java
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.scripting.core.it;
+
+import static org.apache.sling.testing.paxexam.SlingOptions.slingAuthForm;
+import static
org.apache.sling.testing.paxexam.SlingOptions.slingQuickstartOakTar;
+import static
org.apache.sling.testing.paxexam.SlingOptions.slingScriptingJavascript;
+import static org.apache.sling.testing.paxexam.SlingOptions.versionResolver;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.ops4j.pax.exam.CoreOptions.composite;
+import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
+import static org.ops4j.pax.exam.CoreOptions.vmOption;
+import static
org.ops4j.pax.exam.cm.ConfigurationAdminOptions.factoryConfiguration;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.util.concurrent.Callable;
+
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.http.Header;
+import org.apache.http.HttpEntity;
+import org.apache.http.auth.AuthenticationException;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.auth.BasicScheme;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.client.LaxRedirectStrategy;
+import org.awaitility.Awaitility;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.rules.TestRule;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.Configuration;
+import org.ops4j.pax.exam.Option;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.options.ModifiableCompositeOption;
+import org.ops4j.pax.exam.options.extra.VMOption;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerClass;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.ServiceReference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests for SLING-10147 - verify scripting variables implementation details
are not
+ * exposed to not authorized users
+ */
+@RunWith(Enclosed.class)
+public class SLING_10147IT {
+
+ private SLING_10147IT() {
+ // private constructor to hide the implicit public one
+ }
+
+ /**
+ * Base class to share the common parts of LoadedIT and NotLoadedIT
+ * tests
+ */
+ public abstract static class BaseIT extends ScriptingCoreTestSupport {
+ protected final Logger logger = LoggerFactory.getLogger(getClass());
+
+ protected CloseableHttpClient httpClient;
+
+ @Rule
+ public TestRule watcher = new TestWatcher() {
+
+ /* (non-Javadoc)
+ * @see
org.junit.rules.TestWatcher#starting(org.junit.runner.Description)
+ */
+ @Override
+ protected void starting(Description description) {
+ logger.info("Starting test: {}", description.getMethodName());
+ }
+
+ /* (non-Javadoc)
+ * @see
org.junit.rules.TestWatcher#finished(org.junit.runner.Description)
+ */
+ @Override
+ protected void finished(Description description) {
+ logger.info("Finished test: {}", description.getMethodName());
+ }
+
+ };
+
+ @Before
+ public void before() {
+ waitForServices();
+
+ int timeout = 15; // seconds
+ RequestConfig config = RequestConfig.custom()
+ .setConnectTimeout(timeout * 1000)
+ .setConnectionRequestTimeout(timeout * 1000)
+ .setSocketTimeout(timeout * 1000).build();
+
+ HttpClientBuilder httpClientBuilder = HttpClientBuilder.create()
+ .setRedirectStrategy(new LaxRedirectStrategy())
+ .setDefaultRequestConfig(config);
+ httpClient = httpClientBuilder.build();
+ }
+
+ /**
+ * Wait for services to be available
+ */
+ protected void waitForServices() {
+ final BundleContext bundleContext =
FrameworkUtil.getBundle(SLING_10147IT.class).getBundleContext();
+ Awaitility.await()
+ .atMost(Duration.ofMinutes(5))
+ .pollInterval(Duration.ofSeconds(5))
+ .until(new Callable<Boolean>() {
+
+ private String [] servicesLists = new String[] {
+ "org.apache.sling.jcr.api.SlingRepository",
+ "org.apache.sling.engine.auth.Authenticator",
+
"org.apache.sling.api.resource.ResourceResolverFactory",
+ "org.apache.sling.api.servlets.ServletResolver",
+ "javax.script.ScriptEngineManager"
+ };
+
+ @Override
+ public Boolean call() throws Exception {
+ boolean foundAllServices = true;
+ for (String serviceClass : servicesLists) {
+ ServiceReference<?> serviceReference =
bundleContext.getServiceReference(serviceClass);
+ if (serviceReference == null) {
+ foundAllServices = false;
+ break;
+ }
+ }
+ return foundAllServices;
+ }
+
+ });
+ }
+
+ @After
+ public void after() throws IOException {
+ if (httpClient != null) {
+ httpClient.close();
+ httpClient = null;
+ }
+ }
+
+ @Configuration
+ public Option[] configuration() {
+ versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.api");
+ versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.resourceresolver");
+ versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.servlets.resolver");
+ versionResolver.setVersionFromProject("org.apache.sling",
"org.apache.sling.scripting.api");
+
+ final Option webconsolesecurityprovider =
mavenBundle().groupId("org.apache.sling").artifactId("org.apache.sling.extensions.webconsolesecurityprovider").versionAsInProject();
+ return composite(
+ slingQuickstartOakTar(String.format("target/%s",
getClass().getSimpleName()), httpPort),
+ slingScriptingJavascript(),
+ slingAuthForm(),
+ baseConfiguration(),
+
mavenBundle().groupId("org.apache.httpcomponents").artifactId("httpcore-osgi").version(versionResolver),
+
mavenBundle().groupId("org.apache.httpcomponents").artifactId("httpclient-osgi").version(versionResolver),
+ webconsolesecurityprovider,
+ optionalRemoteDebug()
+ ).remove(
+ scriptingCore // remove the old version
+ ).getOptions();
+ }
+
+ /**
+ * Optionally configure remote debugging on the port supplied by the
"debugPort"
+ * system property.
+ */
+ protected ModifiableCompositeOption optionalRemoteDebug() {
+ VMOption option = null;
+ String property = System.getProperty("debugPort");
+ if (property != null) {
+ option =
vmOption(String.format("-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=%s",
property));
+ }
+ return composite(option);
+ }
+
+ protected void checkContentType(CloseableHttpResponse response ,String
expected) {
+ // Remove whatever follows semicolon in content-type
+ HttpEntity entity = response.getEntity();
+ Header contentTypeHeader = entity.getContentType();
+ String contentType = contentTypeHeader == null ? null :
contentTypeHeader.getValue();
+ if (contentType != null) {
+ contentType = contentType.split(";")[0].trim();
+ }
+ // check for match
+ assertEquals(expected, contentType);
+ }
+
+ }
+
+ /**
+ * Verify that when authorized that the scripting variables is accessible
+ */
+ @RunWith(PaxExam.class)
+ @ExamReactorStrategy(PerClass.class)
+ public static class AuthorizedIT extends BaseIT {
+
+ @Test
+ public void testGetSlingBindingsVariablesListJsonWithAuthorizedUser()
throws IOException, AuthenticationException {
+ // make GET request and verify it returned a 200 ok
+ HttpGet request = new
HttpGet(String.format("http://localhost:%d/.SLING_availablebindings.json?extension=esp";,
httpPort()));
+ // pre-emptive basic authentication
+ request.addHeader(new BasicScheme().authenticate(new
UsernamePasswordCredentials("admin", "admin"), request, null));
+
+ if (logger.isInfoEnabled()) {
+ logger.info("Executing GET Request to: {}", request.getURI());
+ }
+ try (CloseableHttpResponse response = httpClient.execute(request))
{
+ if (logger.isInfoEnabled()) {
+ logger.info("Response Content\r\n: {}",
IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8));
+ }
+ // should have passed all security checks and returned the
JSON payload
+ assertEquals(HttpServletResponse.SC_OK,
response.getStatusLine().getStatusCode());
+ checkContentType(response, "application/json");
+ }
+ }
+ }
+
+ /**
+ * Verify that when not authorized that the scripting variables is not
accessible
+ */
+ @RunWith(PaxExam.class)
+ @ExamReactorStrategy(PerClass.class)
+ public static class NotAuthorizedIT extends BaseIT {
+
+ @Configuration
+ @Override
+ public Option[] configuration() {
+ return composite(super.configuration())
+ .add(// testing - add a user to use to login and verify
+
factoryConfiguration("org.apache.sling.jcr.repoinit.RepositoryInitializer")
+ .put("scripts", new String[] {
+ "create user sling10147 with password
sling10147"
+ })
+ .asOption()
+ )
+ .getOptions();
+ }
+
+ @Test
+ public void
testGetSlingBindingsVariablesListJsonWithNotAuthorizedUser() throws
IOException, AuthenticationException {
+ // make GET request and verify it is denied for user without
sufficient rights
+ HttpGet request = new
HttpGet(String.format("http://localhost:%d/.SLING_availablebindings.json?extension=esp";,
httpPort()));
+ // pre-emptive basic authentication
+ request.addHeader(new BasicScheme().authenticate(new
UsernamePasswordCredentials("sling10147", "sling10147"), request, null));
+
+ if (logger.isInfoEnabled()) {
+ logger.info("Executing GET Request to: {}", request.getURI());
+ }
+ try (CloseableHttpResponse response = httpClient.execute(request))
{
+ if (logger.isInfoEnabled()) {
+ logger.info("Response Content\r\n: {}",
IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8));
+ }
+ // should have been denied access by
WebConsoleSecurityProvider2 service access not granted to the user
+ assertEquals(HttpServletResponse.SC_FORBIDDEN,
response.getStatusLine().getStatusCode());
+ checkContentType(response, "text/html");
+ }
+ }
+
+ @Test
+ public void testGetSlingBindingsVariablesListJsonWithAnonymousUser()
throws IOException, AuthenticationException {
+ // make GET request and verify it returned a forms login page
challenge
+ HttpGet request = new
HttpGet(String.format("http://localhost:%d/.SLING_availablebindings.json?extension=esp";,
httpPort()));
+
+ if (logger.isInfoEnabled()) {
+ logger.info("Executing GET Request to: {}", request.getURI());
+ }
+ try (CloseableHttpResponse response = httpClient.execute(request))
{
+ String content =
IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
+ if (logger.isInfoEnabled()) {
+ logger.info("Response Content\r\n: {}", content);
+ }
+ // should have been challenged for authentication by the
WebConsoleSecurity2#authenitcate call
+ assertEquals(HttpServletResponse.SC_OK,
response.getStatusLine().getStatusCode());
+ checkContentType(response, "text/html");
+ assertTrue("Expected forms based login page",
content.contains("Login to Apache Sling"));
+ }
+ }
+ }
+
+ /**
+ * Verify that when no WebConsoleSecurityProvider2 service is available
that
+ * the scripting variables is not accessible
+ */
+ @RunWith(PaxExam.class)
+ @ExamReactorStrategy(PerClass.class)
+ public static class HasNoWebConsoleSecurityProvider2IT extends BaseIT {
+
+ @Configuration
+ @Override
+ public Option[] configuration() {
+ // remove the security provider bundle from the configuration
+ final Option webconsolesecurityprovider =
mavenBundle().groupId("org.apache.sling").artifactId("org.apache.sling.extensions.webconsolesecurityprovider").versionAsInProject();
+ return composite(super.configuration())
+ .remove(webconsolesecurityprovider)
+ .getOptions();
+ }
+
+ @Test
+ public void
testGetSlingBindingsVariablesListJsonWithoutWebConsoleSecurityProvider2()
throws IOException, AuthenticationException {
+ // make GET request and verify it returned a 403 error
+ HttpGet request = new
HttpGet(String.format("http://localhost:%d/.SLING_availablebindings.json?extension=esp";,
httpPort()));
+ // pre-emptive basic authentication
+ request.addHeader(new BasicScheme().authenticate(new
UsernamePasswordCredentials("admin", "admin"), request, null));
+
+ if (logger.isInfoEnabled()) {
+ logger.info("Executing GET Request to: {}", request.getURI());
+ }
+ try (CloseableHttpResponse response = httpClient.execute(request))
{
+ if (logger.isInfoEnabled()) {
+ logger.info("Response Content\r\n: {}",
IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8));
+ }
+ // should have failed fast due to the missing
WebConsoleSecurityProvider2 service
+ assertEquals(HttpServletResponse.SC_FORBIDDEN,
response.getStatusLine().getStatusCode());
+ checkContentType(response, "text/html");
+ }
+ }
+
+ }
+
+}
