SONAR-17343 Add security headers in error HTTP code responses

6 files changed, 146 insertions, 18 deletions

diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/app/SecureErrorReportValve.java b/server/sonar-webserver/src/main/java/org/sonar/server/app/SecureErrorReportValve.java
new file mode 100644
index 00000000000..3c8db3332c0
--- /dev/null
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/app/SecureErrorReportValve.java
@@ -0,0 +1,40 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonar.server.app;
+
+import java.io.IOException;
+import javax.servlet.ServletException;
+import org.apache.catalina.connector.Request;
+import org.apache.catalina.connector.Response;
+import org.apache.catalina.valves.ErrorReportValve;
+import org.sonar.server.platform.web.SecurityServletFilter;
+
+/**
+ * Extending the ErrorReportValve to add security HTTP headers in all responses.
+ */
+public class SecureErrorReportValve extends ErrorReportValve {
+
+  @Override
+  public void invoke(Request request, Response response) throws IOException, ServletException {
+    SecurityServletFilter.addSecurityHeaders(request, response);
+    super.invoke(request, response);
+  }
+}
diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/app/TomcatErrorHandling.java b/server/sonar-webserver/src/main/java/org/sonar/server/app/TomcatErrorHandling.java
index cdba2e4fb42..1089d1a1e6e 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/app/TomcatErrorHandling.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/app/TomcatErrorHandling.java
@@ -24,7 +24,7 @@ import org.apache.catalina.valves.ErrorReportValve;
 
 public class TomcatErrorHandling {
   void configure(Tomcat tomcat) {
-    ErrorReportValve valve = new ErrorReportValve();
+    ErrorReportValve valve = new SecureErrorReportValve();
     valve.setShowServerInfo(false);
     valve.setShowReport(false);
     tomcat.getHost().getPipeline().addValve(valve);
diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
index 30193cd0acc..3e4f10d0fbb 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
@@ -55,24 +55,35 @@ public class SecurityServletFilter implements Filter {
     }
 
     // WARNING, headers must be added before the doFilter, otherwise they won't be added when response is already committed (for instance when a WS is called)
+    addSecurityHeaders(httpRequest, httpResponse);
 
+    chain.doFilter(httpRequest, httpResponse);
+  }
+
+  /**
+   * Adds security HTTP headers in the response. The headers are added using {@code setHeader()}, which overwrites existing headers.
+   */
+  public static void addSecurityHeaders(HttpServletRequest httpRequest, HttpServletResponse httpResponse) {
     // Clickjacking protection
     // See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE
     // The protection is disabled on purpose for integration in external systems like Github (/integration/github).
     String path = httpRequest.getRequestURI().replaceFirst(httpRequest.getContextPath(), "");
     if (!path.startsWith("/integration/")) {
-      httpResponse.addHeader("X-Frame-Options", "SAMEORIGIN");
+      httpResponse.setHeader("X-Frame-Options", "SAMEORIGIN");
+    }
+
+    // If the request is secure, the Strict-Transport-Security header is added.
+    if ("https".equals(httpRequest.getHeader("x-forwarded-proto"))) {
+      httpResponse.setHeader("Strict-Transport-Security", "max-age=31536000; includeSubDomains;");
     }
 
     // Cross-site scripting
     // See https://www.owasp.org/index.php/List_of_useful_HTTP_headers
-    httpResponse.addHeader("X-XSS-Protection", "1; mode=block");
+    httpResponse.setHeader("X-XSS-Protection", "1; mode=block");
 
     // MIME-sniffing
     // See https://www.owasp.org/index.php/List_of_useful_HTTP_headers
-    httpResponse.addHeader("X-Content-Type-Options", "nosniff");
-
-    chain.doFilter(httpRequest, httpResponse);
+    httpResponse.setHeader("X-Content-Type-Options", "nosniff");
   }
 
   @Override
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/app/SecurityErrorReportValveTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/app/SecurityErrorReportValveTest.java
new file mode 100644
index 00000000000..062cd972d98
--- /dev/null
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/app/SecurityErrorReportValveTest.java
@@ -0,0 +1,62 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonar.server.app;
+
+import java.io.IOException;
+import javax.servlet.ServletException;
+import org.apache.catalina.connector.Request;
+import org.apache.catalina.connector.Response;
+import org.apache.catalina.valves.ValveBase;
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class SecurityErrorReportValveTest {
+
+  SecureErrorReportValve underTest = spy(SecureErrorReportValve.class);
+
+  @Test
+  public void add_security_headers() throws ServletException, IOException {
+    var request = mock(Request.class);
+    var response = mock(Response.class);
+
+    underTest.setNext(new ValveBase() {
+      @Override
+      public void invoke(Request request, Response response) {
+      }
+    });
+
+    when(request.getMethod()).thenReturn("GET");
+    when(request.getRequestURI()).thenReturn("/");
+    when(request.getContextPath()).thenReturn("");
+    when(request.getHeader("x-forwarded-proto")).thenReturn("https");
+
+    underTest.invoke(request, response);
+
+    verify(response).setHeader("X-Frame-Options", "SAMEORIGIN");
+    verify(response).setHeader("X-XSS-Protection", "1; mode=block");
+    verify(response).setHeader("X-Content-Type-Options", "nosniff");
+    verify(response).setHeader("Strict-Transport-Security", "max-age=31536000; includeSubDomains;");
+  }
+}
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/app/TomcatErrorHandlingTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/app/TomcatErrorHandlingTest.java
index fa999482938..1c1d31dbcca 100644
--- a/server/sonar-webserver/src/test/java/org/sonar/server/app/TomcatErrorHandlingTest.java
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/app/TomcatErrorHandlingTest.java
@@ -20,7 +20,6 @@
 package org.sonar.server.app;
 
 import org.apache.catalina.startup.Tomcat;
-import org.apache.catalina.valves.ErrorReportValve;
 import org.junit.Test;
 import org.mockito.Mockito;
 
@@ -35,6 +34,6 @@ public class TomcatErrorHandlingTest {
   public void enable_access_logs_by_Default() {
     Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS);
     underTest.configure(tomcat);
-    verify(tomcat.getHost().getPipeline()).addValve(any(ErrorReportValve.class));
+    verify(tomcat.getHost().getPipeline()).addValve(any(SecureErrorReportValve.class));
   }
 }
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/SecurityServletFilterTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/SecurityServletFilterTest.java
index eb0254c331e..b73e0c67fa4 100644
--- a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/SecurityServletFilterTest.java
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/SecurityServletFilterTest.java
@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.junit.Test;
 
+import static org.junit.Assert.assertNull;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -82,14 +83,29 @@ public class SecurityServletFilterTest {
   }
 
   @Test
-  public void set_security_headers() throws Exception {
+  public void set_security_headers_non_secure_request() throws Exception {
     HttpServletRequest request = newRequest("GET", "/");
+    when(request.getHeader("x-forwarded-proto")).thenReturn("https");
 
     underTest.doFilter(request, response, chain);
 
-    verify(response).addHeader("X-Frame-Options", "SAMEORIGIN");
-    verify(response).addHeader("X-XSS-Protection", "1; mode=block");
-    verify(response).addHeader("X-Content-Type-Options", "nosniff");
+    verify(response).setHeader("X-Frame-Options", "SAMEORIGIN");
+    verify(response).setHeader("X-XSS-Protection", "1; mode=block");
+    verify(response).setHeader("X-Content-Type-Options", "nosniff");
+    assertNull(response.getHeader("Strict-Transport-Security"));
+  }
+
+  @Test
+  public void set_security_headers_secure_request() throws ServletException, IOException {
+    HttpServletRequest request = newRequest("GET", "/");
+    when(request.getHeader("x-forwarded-proto")).thenReturn("https");
+
+    underTest.doFilter(request, response, chain);
+
+    verify(response).setHeader("X-Frame-Options", "SAMEORIGIN");
+    verify(response).setHeader("X-XSS-Protection", "1; mode=block");
+    verify(response).setHeader("X-Content-Type-Options", "nosniff");
+    verify(response).setHeader("Strict-Transport-Security", "max-age=31536000; includeSubDomains;");
   }
 
   @Test
@@ -98,9 +114,9 @@ public class SecurityServletFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(response, never()).addHeader(eq("X-Frame-Options"), anyString());
-    verify(response).addHeader("X-XSS-Protection", "1; mode=block");
-    verify(response).addHeader("X-Content-Type-Options", "nosniff");
+    verify(response, never()).setHeader(eq("X-Frame-Options"), anyString());
+    verify(response).setHeader("X-XSS-Protection", "1; mode=block");
+    verify(response).setHeader("X-Content-Type-Options", "nosniff");
   }
 
   @Test
@@ -112,9 +128,9 @@ public class SecurityServletFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(response, never()).addHeader(eq("X-Frame-Options"), anyString());
-    verify(response).addHeader("X-XSS-Protection", "1; mode=block");
-    verify(response).addHeader("X-Content-Type-Options", "nosniff");
+    verify(response, never()).setHeader(eq("X-Frame-Options"), anyString());
+    verify(response).setHeader("X-XSS-Protection", "1; mode=block");
+    verify(response).setHeader("X-Content-Type-Options", "nosniff");
   }
 
   private static HttpServletRequest newRequest(String httpMethod, String path) {