From f33307f612a0decb9f087b191db55e133b3db7d4 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Sat, 26 Oct 2024 21:51:55 +0800 Subject: [PATCH] Revert "HBASE-23303 Add default security headers if SSL is enabled (#4128)" This reverts commit 7f30aa8b63895a2df13af55a663269cbc23fd142. --- .../apache/hadoop/hbase/http/HttpServer.java | 4 +-- .../hbase/http/SecurityHeadersFilter.java | 26 +++++-------------- .../hadoop/hbase/http/TestSSLHttpServer.java | 14 ---------- .../apache/hadoop/hbase/rest/RESTServer.java | 6 ++--- .../hadoop/hbase/rest/TestRESTServerSSL.java | 6 ----- 5 files changed, 10 insertions(+), 46 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 d5af8df1c7fd..8aa177c2ab79 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 @@ -639,10 +639,8 @@ private void initializeWebServer(String name, String hostName, Configuration con addGlobalFilter("clickjackingprevention", ClickjackingPreventionFilter.class.getName(), ClickjackingPreventionFilter.getDefaultParameters(conf)); - HttpConfig httpConfig = new HttpConfig(conf); - addGlobalFilter("securityheaders", SecurityHeadersFilter.class.getName(), - SecurityHeadersFilter.getDefaultParameters(conf, httpConfig.isSecure())); + SecurityHeadersFilter.getDefaultParameters(conf)); // 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 08e34085a0c8..2a9fc7f7e129 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 @@ -36,10 +36,10 @@ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) public class SecurityHeadersFilter implements Filter { + private static final Logger LOG = LoggerFactory.getLogger(SecurityHeadersFilter.class); - 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 static final String DEFAULT_HSTS = ""; + private static final String DEFAULT_CSP = ""; private FilterConfig filterConfig; @Override @@ -69,27 +69,15 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha public void destroy() { } - /** - * @param conf configuration - * @param isSecure use secure defaults if 'true' - * @return default parameters, as a map - */ - public static Map getDefaultParameters(Configuration conf, boolean isSecure) { - Map params = new HashMap<>(); - 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; - } - /** * @param conf configuration * @return default parameters, as a map - * @deprecated Use {@link SecurityHeadersFilter#getDefaultParameters(Configuration, boolean)} - * instead. */ - @Deprecated public static Map getDefaultParameters(Configuration conf) { - return getDefaultParameters(conf, false); + 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/TestSSLHttpServer.java b/hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestSSLHttpServer.java index 4654a27d87ae..9c0af5f4215c 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 @@ -21,11 +21,9 @@ 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,7 +71,6 @@ 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(); @@ -120,17 +117,6 @@ public void testEcho() throws Exception { assertEquals("a:b\nc<:d\ne:>\n", readOut(new URL(baseUrl, "/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 dea6f2909875..535ab0672328 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 @@ -291,9 +291,7 @@ 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()); @@ -380,8 +378,8 @@ public synchronized void run() throws Exception { ctxHandler.addFilter(filter, PATH_SPEC_ANY, EnumSet.of(DispatcherType.REQUEST)); } addCSRFFilter(ctxHandler, conf); - HttpServerUtil.addClickjackingPreventionFilter(ctxHandler, conf, PATH_SPEC_ANY); - HttpServerUtil.addSecurityHeadersFilter(ctxHandler, conf, isSecure, PATH_SPEC_ANY); + HttpServerUtil.addClickjackingPreventionFilter(ctxHandler, conf); + HttpServerUtil.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/TestRESTServerSSL.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestRESTServerSSL.java index 5731dd94fc66..294a8f8d02a2 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 @@ -121,12 +121,6 @@ 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)