From 901b8810a4d2c237395eced14e467be415218249 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 10 Nov 2016 12:29:23 +0100 Subject: [PATCH] SONAR-8247 Fix security headers --- .../src/test/java/it/Category4Suite.java | 2 +- .../HttpHeadersTest.java | 43 ++++++++++++++++++- .../platform/web/SecurityServletFilter.java | 4 +- .../main/webapp/WEB-INF/config/environment.rb | 34 --------------- .../sonar-web/src/main/webapp/WEB-INF/web.xml | 6 +-- 5 files changed, 48 insertions(+), 41 deletions(-) rename it/it-tests/src/test/java/it/{http => serverSystem}/HttpHeadersTest.java (69%) diff --git a/it/it-tests/src/test/java/it/Category4Suite.java b/it/it-tests/src/test/java/it/Category4Suite.java index 9d9c8b61501..41c4373f731 100644 --- a/it/it-tests/src/test/java/it/Category4Suite.java +++ b/it/it-tests/src/test/java/it/Category4Suite.java @@ -29,7 +29,7 @@ import it.duplication.CrossProjectDuplicationsOnRemoveFileTest; import it.duplication.CrossProjectDuplicationsTest; import it.duplication.DuplicationsTest; import it.duplication.NewDuplicationsTest; -import it.http.HttpHeadersTest; +import it.serverSystem.HttpHeadersTest; import it.projectComparison.ProjectComparisonTest; import it.projectEvent.EventTest; import it.projectSearch.SearchProjectsTest; diff --git a/it/it-tests/src/test/java/it/http/HttpHeadersTest.java b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java similarity index 69% rename from it/it-tests/src/test/java/it/http/HttpHeadersTest.java rename to it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java index bf06af57216..79f539a8a80 100644 --- a/it/it-tests/src/test/java/it/http/HttpHeadersTest.java +++ b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java @@ -17,8 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - -package it.http; +package it.serverSystem; import com.google.common.base.Throwables; import com.sonar.orchestrator.Orchestrator; @@ -77,6 +76,36 @@ public class HttpHeadersTest { assertCacheInBrowser(httpResponse); } + @Test + public void verify_security_headers_on_base_url() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/")); + } + + @Test + public void verify_security_headers_on_ws() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/api/issues/search")); + } + + @Test + public void verify_security_headers_on_ruby_ws() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/api/resources/index")); + } + + @Test + public void verify_security_headers_on_images() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/images/logo.svg")); + } + + @Test + public void verify_security_headers_on_css() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/css/sonar.css")); + } + + @Test + public void verify_security_headers_on_js() throws Exception { + verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/js/bundles/main.js")); + } + private static void assertCacheInBrowser(Response httpResponse) { CacheControl cacheControl = httpResponse.cacheControl(); assertThat(cacheControl.mustRevalidate()).isFalse(); @@ -91,6 +120,16 @@ public class HttpHeadersTest { assertThat(cacheControl.noStore()).isTrue(); } + /** + * SONAR-8247 + */ + private static void verifySecurityHeaders(Response httpResponse) { + assertThat(httpResponse.isSuccessful()).isTrue(); + assertThat(httpResponse.headers().get("X-Frame-Options")).isEqualTo("SAMEORIGIN"); + assertThat(httpResponse.headers().get("X-XSS-Protection")).isEqualTo("1; mode=block"); + assertThat(httpResponse.headers().get("X-Content-Type-Options")).isEqualTo("nosniff"); + } + private static Response call(String url) { Request request = new Request.Builder().get().url(url).build(); try { diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java index a91d29e0262..c533f7e1ec1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java @@ -56,7 +56,7 @@ public class SecurityServletFilter implements Filter { return; } - chain.doFilter(httpRequest, httpResponse); + // 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) // Clickjacking protection // See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE @@ -69,6 +69,8 @@ public class SecurityServletFilter implements Filter { // 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); } @Override diff --git a/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb b/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb index cd529e41506..3c375e4607b 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb @@ -52,37 +52,6 @@ class EagerPluginLoader < Rails::Plugin::Loader end end - -# -# Put response headers on all HTTP calls. This is done by the Java SecurityServlerFilter, -# but for some reason Rack swallows the headers set on Java side. -# See middleware configuration below. -# -class SecurityHeaders - def initialize(app) - @app = app - end - - def call(env) - status, headers, body = @app.call(env) - - # Clickjacking protection - # See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE - headers['X-Frame-Options']='SAMEORIGIN' - - # Cross-site scripting - # See https://www.owasp.org/index.php/List_of_useful_HTTP_headers - headers['X-XSS-Protection']='1; mode=block' - - # MIME-sniffing - # See https://www.owasp.org/index.php/List_of_useful_HTTP_headers - headers['X-Content-Type-Options']='nosniff'; - - [status, headers, body] - end -end - - Rails::Initializer.run do |config| # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers @@ -139,9 +108,6 @@ Rails::Initializer.run do |config| # Activate observers that should always be running # Please note that observers generated using script/generate observer need to have an _observer suffix # config.active_record.observers = :cacher, :garbage_collector, :forum_observer - - # Add security related headers - config.middleware.use SecurityHeaders end diff --git a/server/sonar-web/src/main/webapp/WEB-INF/web.xml b/server/sonar-web/src/main/webapp/WEB-INF/web.xml index ab60f08440a..1a8aa2dc8ad 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/web.xml +++ b/server/sonar-web/src/main/webapp/WEB-INF/web.xml @@ -75,15 +75,15 @@ /* - UserSessionFilter + SecurityFilter /* - ServletFilters + UserSessionFilter /* - SecurityFilter + ServletFilters /* -- 2.39.5