SLING-10147 block unauthorized access to SlingBindingsVariablesListJsonServlet (#7)
scripting variables implementation details were exposed to not authorized users
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.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.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 @@
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 @@
@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");
+ }
+ }
+
+ }
+
+}
