From 188742a82febd30b1604356333a0cef463c446f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andor=20Moln=C3=A1r?= Date: Sun, 8 Dec 2019 14:06:40 +0100 Subject: [PATCH] HBASE-23303 Add security headers to REST server/info page (#843) Signed-off-by: Toshihiro Suzuki Signed-off-by: Sean Busbey --- .../http/ClickjackingPreventionFilter.java | 11 ++ .../apache/hadoop/hbase/http/HttpServer.java | 11 +- .../hbase/http/SecurityHeadersFilter.java | 81 +++++++++++++ .../hbase/http/TestSecurityHeadersFilter.java | 106 +++++++++++++++++ .../apache/hadoop/hbase/rest/RESTServer.java | 70 ++++++----- .../hbase/rest/TestSecurityHeadersFilter.java | 110 ++++++++++++++++++ 6 files changed, 359 insertions(+), 30 deletions(-) create mode 100644 hbase-http/src/main/java/org/apache/hadoop/hbase/http/SecurityHeadersFilter.java create mode 100644 hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSecurityHeadersFilter.java create mode 100644 hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecurityHeadersFilter.java diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ClickjackingPreventionFilter.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ClickjackingPreventionFilter.java index 7ce13012a40..0f0c7150c41 100644 --- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ClickjackingPreventionFilter.java +++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/ClickjackingPreventionFilter.java @@ -18,6 +18,8 @@ package org.apache.hadoop.hbase.http; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -27,6 +29,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.yetus.audience.InterfaceAudience; @@ -34,6 +37,7 @@ import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) public class ClickjackingPreventionFilter implements Filter { private FilterConfig filterConfig; + private static final String DEFAULT_XFRAMEOPTIONS = "DENY"; @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -51,4 +55,11 @@ public class ClickjackingPreventionFilter implements Filter { @Override public void destroy() { } + + public static Map getDefaultParameters(Configuration conf) { + Map params = new HashMap<>(); + params.put("xframeoptions", conf.get("hbase.http.filter.xframeoptions.mode", + DEFAULT_XFRAMEOPTIONS)); + return params; + } } diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java index e96e057b439..661af4a49b4 100644 --- a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java +++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/HttpServer.java @@ -596,10 +596,15 @@ public class HttpServer implements FilterContainer { addDefaultApps(contexts, appDir, conf); addGlobalFilter("safety", QuotingInputFilter.class.getName(), null); - Map params = new HashMap<>(); - params.put("xframeoptions", conf.get("hbase.http.filter.xframeoptions.mode", "DENY")); + addGlobalFilter("clickjackingprevention", - ClickjackingPreventionFilter.class.getName(), params); + ClickjackingPreventionFilter.class.getName(), + ClickjackingPreventionFilter.getDefaultParameters(conf)); + + addGlobalFilter("securityheaders", + SecurityHeadersFilter.class.getName(), + SecurityHeadersFilter.getDefaultParameters(conf)); + final FilterInitializer[] initializers = getFilterInitializers(conf); if (initializers != null) { conf = new Configuration(conf); diff --git a/hbase-http/src/main/java/org/apache/hadoop/hbase/http/SecurityHeadersFilter.java b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/SecurityHeadersFilter.java new file mode 100644 index 00000000000..b83fef16719 --- /dev/null +++ b/hbase-http/src/main/java/org/apache/hadoop/hbase/http/SecurityHeadersFilter.java @@ -0,0 +1,81 @@ +/** + * 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.hadoop.hbase.http; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseInterfaceAudience; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) +public class SecurityHeadersFilter implements Filter { + private static final Logger LOG = + LoggerFactory.getLogger(SecurityHeadersFilter.class); + private static final String DEFAULT_HSTS = ""; + private static final String DEFAULT_CSP = ""; + private FilterConfig filterConfig; + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + this.filterConfig = filterConfig; + LOG.info("Added security headers filter"); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + HttpServletResponse httpResponse = (HttpServletResponse) response; + httpResponse.addHeader("X-Content-Type-Options", "nosniff"); + httpResponse.addHeader("X-XSS-Protection", "1; mode=block"); + String hsts = filterConfig.getInitParameter("hsts"); + if (StringUtils.isNotBlank(hsts)) { + httpResponse.addHeader("Strict-Transport-Security", hsts); + } + String csp = filterConfig.getInitParameter("csp"); + if (StringUtils.isNotBlank(csp)) { + httpResponse.addHeader("Content-Security-Policy", csp); + } + chain.doFilter(request, response); + } + + @Override + public void destroy() { + } + + public static Map getDefaultParameters(Configuration conf) { + Map params = new HashMap<>(); + params.put("hsts", conf.get("hbase.http.filter.hsts.value", + DEFAULT_HSTS)); + params.put("csp", conf.get("hbase.http.filter.csp.value", + DEFAULT_CSP)); + return params; + } +} diff --git a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSecurityHeadersFilter.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSecurityHeadersFilter.java new file mode 100644 index 00000000000..41a1235baaf --- /dev/null +++ b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSecurityHeadersFilter.java @@ -0,0 +1,106 @@ +/** + * 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.hadoop.hbase.http; + +import static org.apache.hadoop.hbase.http.HttpServerFunctionalTest.createTestServer; +import static org.apache.hadoop.hbase.http.HttpServerFunctionalTest.getServerURL; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.hamcrest.core.Is; +import org.hamcrest.core.IsEqual; +import org.junit.After; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({HttpServerFunctionalTest.class, MediumTests.class}) +public class TestSecurityHeadersFilter { + private static URL baseUrl; + private HttpServer http; + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestSecurityHeadersFilter.class); + + @After + public void tearDown() throws Exception { + http.stop(); + } + + @Test + public void testDefaultValues() throws Exception { + http = createTestServer(); + http.start(); + baseUrl = getServerURL(http); + + URL url = new URL(baseUrl, "/"); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + assertThat(conn.getResponseCode(), equalTo(HttpURLConnection.HTTP_OK)); + + assertThat("Header 'X-Content-Type-Options' is missing", + conn.getHeaderField("X-Content-Type-Options"), is(not((String)null))); + assertThat(conn.getHeaderField("X-Content-Type-Options"), equalTo("nosniff")); + assertThat("Header 'X-XSS-Protection' is missing", + conn.getHeaderField("X-XSS-Protection"), is(not((String)null))); + assertThat("Header 'X-XSS-Protection' has invalid value", + conn.getHeaderField("X-XSS-Protection"), equalTo("1; mode=block")); + + assertThat("Header 'Strict-Transport-Security' should be missing from response," + + "but it's present", + conn.getHeaderField("Strict-Transport-Security"), is((String)null)); + assertThat("Header 'Content-Security-Policy' should be missing from response," + + "but it's present", + conn.getHeaderField("Content-Security-Policy"), is((String)null)); + } + + @Test + public void testHstsAndCspSettings() throws IOException { + Configuration conf = new Configuration(); + conf.set("hbase.http.filter.hsts.value", + "max-age=63072000;includeSubDomains;preload"); + conf.set("hbase.http.filter.csp.value", + "default-src https: data: 'unsafe-inline' 'unsafe-eval'"); + http = createTestServer(conf); + http.start(); + baseUrl = getServerURL(http); + + URL url = new URL(baseUrl, "/"); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + assertThat(conn.getResponseCode(), equalTo(HttpURLConnection.HTTP_OK)); + + assertThat("Header 'Strict-Transport-Security' is missing from Rest response", + conn.getHeaderField("Strict-Transport-Security"), Is.is(not((String)null))); + assertThat("Header 'Strict-Transport-Security' has invalid value", + conn.getHeaderField("Strict-Transport-Security"), + IsEqual.equalTo("max-age=63072000;includeSubDomains;preload")); + + assertThat("Header 'Content-Security-Policy' is missing from Rest response", + conn.getHeaderField("Content-Security-Policy"), Is.is(not((String)null))); + assertThat("Header 'Content-Security-Policy' has invalid value", + conn.getHeaderField("Content-Security-Policy"), + IsEqual.equalTo("default-src https: data: 'unsafe-inline' 'unsafe-eval'")); + } +} diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java index a73f28064fa..c3d16ee4c91 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.java @@ -18,32 +18,50 @@ package org.apache.hadoop.hbase.rest; +import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider; import java.lang.management.ManagementFactory; import java.util.ArrayList; +import java.util.EnumSet; import java.util.List; import java.util.Map; -import java.util.EnumSet; import java.util.concurrent.ArrayBlockingQueue; - -import com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider; +import javax.servlet.DispatcherType; import org.apache.commons.lang3.ArrayUtils; -import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseInterfaceAudience; +import org.apache.hadoop.hbase.http.ClickjackingPreventionFilter; +import org.apache.hadoop.hbase.http.HttpServerUtil; import org.apache.hadoop.hbase.http.InfoServer; +import org.apache.hadoop.hbase.http.SecurityHeadersFilter; import org.apache.hadoop.hbase.log.HBaseMarkers; import org.apache.hadoop.hbase.rest.filter.AuthFilter; import org.apache.hadoop.hbase.rest.filter.GzipFilter; import org.apache.hadoop.hbase.rest.filter.RestCsrfPreventionFilter; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.util.DNS; -import org.apache.hadoop.hbase.http.HttpServerUtil; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.ReflectionUtils; import org.apache.hadoop.hbase.util.Strings; import org.apache.hadoop.hbase.util.VersionInfo; - +import org.apache.yetus.audience.InterfaceAudience; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.jmx.MBeanContainer; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.SecureRequestCustomizer; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.servlet.ServletContainer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; import org.apache.hbase.thirdparty.org.apache.commons.cli.CommandLine; import org.apache.hbase.thirdparty.org.apache.commons.cli.HelpFormatter; @@ -51,27 +69,6 @@ import org.apache.hbase.thirdparty.org.apache.commons.cli.Options; import org.apache.hbase.thirdparty.org.apache.commons.cli.ParseException; import org.apache.hbase.thirdparty.org.apache.commons.cli.PosixParser; -import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.HttpConnectionFactory; -import org.eclipse.jetty.server.SslConnectionFactory; -import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.server.SecureRequestCustomizer; -import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.servlet.ServletHolder; -import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.eclipse.jetty.jmx.MBeanContainer; -import org.eclipse.jetty.servlet.FilterHolder; - -import org.glassfish.jersey.server.ResourceConfig; -import org.glassfish.jersey.servlet.ServletContainer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.servlet.DispatcherType; - /** * Main class for launching REST gateway as a servlet hosted by Jetty. *

@@ -137,6 +134,23 @@ public class RESTServer implements Constants { } } + private void addClickjackingPreventionFilter(ServletContextHandler ctxHandler, + Configuration conf) { + FilterHolder holder = new FilterHolder(); + holder.setName("clickjackingprevention"); + holder.setClassName(ClickjackingPreventionFilter.class.getName()); + holder.setInitParameters(ClickjackingPreventionFilter.getDefaultParameters(conf)); + ctxHandler.addFilter(holder, PATH_SPEC_ANY, EnumSet.allOf(DispatcherType.class)); + } + + private void addSecurityHeadersFilter(ServletContextHandler ctxHandler, Configuration conf) { + FilterHolder holder = new FilterHolder(); + holder.setName("securityheaders"); + holder.setClassName(SecurityHeadersFilter.class.getName()); + holder.setInitParameters(SecurityHeadersFilter.getDefaultParameters(conf)); + ctxHandler.addFilter(holder, PATH_SPEC_ANY, EnumSet.allOf(DispatcherType.class)); + } + // login the server principal (if using secure Hadoop) private static Pair> loginServerPrincipal( UserProvider userProvider, Configuration conf) throws Exception { @@ -352,6 +366,8 @@ public class RESTServer implements Constants { ctxHandler.addFilter(filter, PATH_SPEC_ANY, EnumSet.of(DispatcherType.REQUEST)); } addCSRFFilter(ctxHandler, conf); + addClickjackingPreventionFilter(ctxHandler, conf); + addSecurityHeadersFilter(ctxHandler, conf); HttpServerUtil.constrainHttpMethods(ctxHandler, servlet.getConfiguration() .getBoolean(REST_HTTP_ALLOW_OPTIONS_METHOD, REST_HTTP_ALLOW_OPTIONS_METHOD_DEFAULT)); diff --git a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecurityHeadersFilter.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecurityHeadersFilter.java new file mode 100644 index 00000000000..bf0c69502d5 --- /dev/null +++ b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecurityHeadersFilter.java @@ -0,0 +1,110 @@ +/** + * 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.hadoop.hbase.rest; + +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertThat; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.rest.client.Client; +import org.apache.hadoop.hbase.rest.client.Cluster; +import org.apache.hadoop.hbase.rest.client.Response; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.RestTests; +import org.junit.After; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({RestTests.class, MediumTests.class}) +public class TestSecurityHeadersFilter { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestSecurityHeadersFilter.class); + + private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static final HBaseRESTTestingUtility REST_TEST_UTIL = + new HBaseRESTTestingUtility(); + private static Client client; + + @After + public void tearDown() throws Exception { + REST_TEST_UTIL.shutdownServletContainer(); + TEST_UTIL.shutdownMiniCluster(); + } + + @Test + public void testDefaultValues() throws Exception { + TEST_UTIL.startMiniCluster(); + REST_TEST_UTIL.startServletContainer(TEST_UTIL.getConfiguration()); + client = new Client(new Cluster().add("localhost", + REST_TEST_UTIL.getServletPort())); + + String path = "/version/cluster"; + Response response = client.get(path); + assertThat(response.getCode(), equalTo(200)); + + assertThat("Header 'X-Content-Type-Options' is missing from Rest response", + response.getHeader("X-Content-Type-Options"), is(not((String)null))); + assertThat("Header 'X-Content-Type-Options' has invalid default value", + response.getHeader("X-Content-Type-Options"), equalTo("nosniff")); + + assertThat("Header 'X-XSS-Protection' is missing from Rest response", + response.getHeader("X-XSS-Protection"), is(not((String)null))); + assertThat("Header 'X-XSS-Protection' has invalid default value", + response.getHeader("X-XSS-Protection"), equalTo("1; mode=block")); + + assertThat("Header 'Strict-Transport-Security' should be missing from Rest response," + + "but it's present", + response.getHeader("Strict-Transport-Security"), is((String)null)); + assertThat("Header 'Content-Security-Policy' should be missing from Rest response," + + "but it's present", + response.getHeader("Content-Security-Policy"), is((String)null)); + } + + @Test + public void testHstsAndCspSettings() throws Exception { + TEST_UTIL.getConfiguration().set("hbase.http.filter.hsts.value", + "max-age=63072000;includeSubDomains;preload"); + TEST_UTIL.getConfiguration().set("hbase.http.filter.csp.value", + "default-src https: data: 'unsafe-inline' 'unsafe-eval'"); + TEST_UTIL.startMiniCluster(); + REST_TEST_UTIL.startServletContainer(TEST_UTIL.getConfiguration()); + client = new Client(new Cluster().add("localhost", + REST_TEST_UTIL.getServletPort())); + + String path = "/version/cluster"; + Response response = client.get(path); + assertThat(response.getCode(), equalTo(200)); + + assertThat("Header 'Strict-Transport-Security' is missing from Rest response", + response.getHeader("Strict-Transport-Security"), is(not((String)null))); + assertThat("Header 'Strict-Transport-Security' has invalid value", + response.getHeader("Strict-Transport-Security"), + equalTo("max-age=63072000;includeSubDomains;preload")); + + assertThat("Header 'Content-Security-Policy' is missing from Rest response", + response.getHeader("Content-Security-Policy"), is(not((String)null))); + assertThat("Header 'Content-Security-Policy' has invalid value", + response.getHeader("Content-Security-Policy"), + equalTo("default-src https: data: 'unsafe-inline' 'unsafe-eval'")); + } +}