From 1d0d70e641f6d8b58d039b5a5a9c79295ef237bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andor=20Moln=C3=A1r?= Date: Wed, 2 Mar 2022 15:25:57 +0100 Subject: [PATCH] HBASE-23303 Add default security headers if SSL is enabled (#4128) Signed-off-by: Balazs Meszaros --- .../org/apache/hadoop/hbase/http/HttpServer.java | 4 +++- .../hadoop/hbase/http/SecurityHeadersFilter.java | 12 +++++------- .../hadoop/hbase/http/TestSSLHttpServer.java | 14 ++++++++++++++ .../org/apache/hadoop/hbase/rest/RESTServer.java | 9 ++++++--- .../hadoop/hbase/rest/TestRESTServerSSL.java | 8 ++++++-- 5 files changed, 34 insertions(+), 13 deletions(-) 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 d534e15180e7..884780cecc30 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 @@ -626,9 +626,11 @@ private void initializeWebServer(String name, String hostName, ClickjackingPreventionFilter.class.getName(), ClickjackingPreventionFilter.getDefaultParameters(conf)); + HttpConfig httpConfig = new HttpConfig(conf); + addGlobalFilter("securityheaders", SecurityHeadersFilter.class.getName(), - SecurityHeadersFilter.getDefaultParameters(conf)); + SecurityHeadersFilter.getDefaultParameters(conf, httpConfig.isSecure())); // But security needs to be enabled prior to adding the other servlets if (authenticationEnabled) { 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 index b83fef167196..f00f2a195af0 100644 --- 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 @@ -39,8 +39,8 @@ 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 static final String DEFAULT_HSTS = "max-age=63072000;includeSubDomains;preload"; + private static final String DEFAULT_CSP = "default-src https: data: 'unsafe-inline' 'unsafe-eval'"; private FilterConfig filterConfig; @Override @@ -70,12 +70,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha public void destroy() { } - public static Map getDefaultParameters(Configuration conf) { + public static Map getDefaultParameters(Configuration conf, boolean isSecure) { 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)); + params.put("hsts", conf.get("hbase.http.filter.hsts.value", isSecure ? DEFAULT_HSTS : "")); + params.put("csp", conf.get("hbase.http.filter.csp.value", isSecure ? DEFAULT_CSP : "")); return params; } } diff --git a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java index 382118d3a2db..2b72793a690f 100644 --- a/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java +++ b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java @@ -19,9 +19,11 @@ import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URL; +import java.security.GeneralSecurityException; import javax.net.ssl.HttpsURLConnection; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileUtil; @@ -73,6 +75,7 @@ public static void setup() throws Exception { serverConf = HTU.getConfiguration(); serverConf.setInt(HttpServer.HTTP_MAX_THREADS, TestHttpServer.MAX_THREADS); + serverConf.setBoolean(ServerConfigurationKeys.HBASE_SSL_ENABLED_KEY, true); keystoresDir = new File(HTU.getDataTestDir("keystore").toString()); keystoresDir.mkdirs(); @@ -122,6 +125,17 @@ public void testEcho() throws Exception { "/echo?a=b&c<=d&e=>"))); } + @Test + public void testSecurityHeaders() throws IOException, GeneralSecurityException { + HttpsURLConnection conn = (HttpsURLConnection) baseUrl.openConnection(); + conn.setSSLSocketFactory(clientSslFactory.createSSLSocketFactory()); + assertEquals(HttpsURLConnection.HTTP_OK, conn.getResponseCode()); + assertEquals("max-age=63072000;includeSubDomains;preload", + conn.getHeaderField("Strict-Transport-Security")); + assertEquals("default-src https: data: 'unsafe-inline' 'unsafe-eval'", + conn.getHeaderField("Content-Security-Policy")); + } + private static String readOut(URL url) throws Exception { HttpsURLConnection conn = (HttpsURLConnection) url.openConnection(); conn.setSSLSocketFactory(clientSslFactory.createSSLSocketFactory()); 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 40dfa90691bb..1ff59c245ca3 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 @@ -148,11 +148,12 @@ private void addClickjackingPreventionFilter(ServletContextHandler ctxHandler, ctxHandler.addFilter(holder, PATH_SPEC_ANY, EnumSet.allOf(DispatcherType.class)); } - private void addSecurityHeadersFilter(ServletContextHandler ctxHandler, Configuration conf) { + private void addSecurityHeadersFilter(ServletContextHandler ctxHandler, + Configuration conf, boolean isSecure) { FilterHolder holder = new FilterHolder(); holder.setName("securityheaders"); holder.setClassName(SecurityHeadersFilter.class.getName()); - holder.setInitParameters(SecurityHeadersFilter.getDefaultParameters(conf)); + holder.setInitParameters(SecurityHeadersFilter.getDefaultParameters(conf, isSecure)); ctxHandler.addFilter(holder, PATH_SPEC_ANY, EnumSet.allOf(DispatcherType.class)); } @@ -307,7 +308,9 @@ public synchronized void run() throws Exception { httpConfig.setSendDateHeader(false); ServerConnector serverConnector; + boolean isSecure = false; if (conf.getBoolean(REST_SSL_ENABLED, false)) { + isSecure = true; HttpConfiguration httpsConfig = new HttpConfiguration(httpConfig); httpsConfig.addCustomizer(new SecureRequestCustomizer()); @@ -395,7 +398,7 @@ public synchronized void run() throws Exception { } addCSRFFilter(ctxHandler, conf); addClickjackingPreventionFilter(ctxHandler, conf); - addSecurityHeadersFilter(ctxHandler, conf); + addSecurityHeadersFilter(ctxHandler, conf, isSecure); 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/TestRESTServerSSL.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestRESTServerSSL.java index ea13360dae15..1a55996fd8c4 100644 --- a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestRESTServerSSL.java +++ b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestRESTServerSSL.java @@ -18,8 +18,6 @@ package org.apache.hadoop.hbase.rest; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - import java.io.File; import java.security.KeyPair; import java.security.cert.X509Certificate; @@ -107,6 +105,12 @@ public void testSslConnection() throws Exception { Response response = sslClient.get("/version", Constants.MIMETYPE_TEXT); assertEquals(200, response.getCode()); + + // Default security headers + assertEquals("max-age=63072000;includeSubDomains;preload", + response.getHeader("Strict-Transport-Security")); + assertEquals("default-src https: data: 'unsafe-inline' 'unsafe-eval'", + response.getHeader("Content-Security-Policy")); } @Test(expected = org.apache.http.client.ClientProtocolException.class)