From: Julien Lancelot Date: Thu, 10 Nov 2016 11:29:23 +0000 (+0100) Subject: SONAR-8247 Fix security headers X-Git-Tag: 6.2-RC1~110 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=901b8810a4d2c237395eced14e467be415218249;p=sonarqube.git SONAR-8247 Fix security headers --- 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/http/HttpHeadersTest.java deleted file mode 100644 index bf06af57216..00000000000 --- a/it/it-tests/src/test/java/it/http/HttpHeadersTest.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact 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 it.http; - -import com.google.common.base.Throwables; -import com.sonar.orchestrator.Orchestrator; -import it.Category4Suite; -import java.io.IOException; -import java.util.concurrent.TimeUnit; -import okhttp3.CacheControl; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; -import org.junit.ClassRule; -import org.junit.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -public class HttpHeadersTest { - - @ClassRule - public static final Orchestrator orchestrator = Category4Suite.ORCHESTRATOR; - - /** - * SONAR-6964 - */ - @Test - public void no_browser_cache_for_pages() { - Response httpResponse = call(orchestrator.getServer().getUrl() + "/"); - - assertNoCacheInBrowser(httpResponse); - } - - @Test - public void no_browser_cache_for_ws() { - Response httpResponse = call(orchestrator.getServer().getUrl() + "/api/issues/search"); - - assertNoCacheInBrowser(httpResponse); - } - - @Test - public void no_browser_cache_in_ruby_ws() { - Response httpResponse = call(orchestrator.getServer().getUrl() + "/api/resources/index"); - - assertNoCacheInBrowser(httpResponse); - } - - @Test - public void browser_cache_on_images() { - Response httpResponse = call(orchestrator.getServer().getUrl() + "/images/logo.svg"); - - assertCacheInBrowser(httpResponse); - } - - @Test - public void browser_cache_on_css() { - Response httpResponse = call(orchestrator.getServer().getUrl() + "/css/sonar.css"); - - assertCacheInBrowser(httpResponse); - } - - private static void assertCacheInBrowser(Response httpResponse) { - CacheControl cacheControl = httpResponse.cacheControl(); - assertThat(cacheControl.mustRevalidate()).isFalse(); - assertThat(cacheControl.noCache()).isFalse(); - assertThat(cacheControl.noStore()).isFalse(); - } - - private static void assertNoCacheInBrowser(Response httpResponse) { - CacheControl cacheControl = httpResponse.cacheControl(); - assertThat(cacheControl.mustRevalidate()).isTrue(); - assertThat(cacheControl.noCache()).isTrue(); - assertThat(cacheControl.noStore()).isTrue(); - } - - private static Response call(String url) { - Request request = new Request.Builder().get().url(url).build(); - try { - // SonarQube ws-client cannot be used as it does not - // expose HTTP headers - return new OkHttpClient.Builder() - .connectTimeout(30, TimeUnit.SECONDS) - .readTimeout(30, TimeUnit.SECONDS) - .build() - .newCall(request).execute(); - } catch (IOException e) { - throw Throwables.propagate(e); - } - } -} diff --git a/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java new file mode 100644 index 00000000000..79f539a8a80 --- /dev/null +++ b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java @@ -0,0 +1,147 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact 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 it.serverSystem; + +import com.google.common.base.Throwables; +import com.sonar.orchestrator.Orchestrator; +import it.Category4Suite; +import java.io.IOException; +import java.util.concurrent.TimeUnit; +import okhttp3.CacheControl; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; +import org.junit.ClassRule; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HttpHeadersTest { + + @ClassRule + public static final Orchestrator orchestrator = Category4Suite.ORCHESTRATOR; + + /** + * SONAR-6964 + */ + @Test + public void no_browser_cache_for_pages() { + Response httpResponse = call(orchestrator.getServer().getUrl() + "/"); + + assertNoCacheInBrowser(httpResponse); + } + + @Test + public void no_browser_cache_for_ws() { + Response httpResponse = call(orchestrator.getServer().getUrl() + "/api/issues/search"); + + assertNoCacheInBrowser(httpResponse); + } + + @Test + public void no_browser_cache_in_ruby_ws() { + Response httpResponse = call(orchestrator.getServer().getUrl() + "/api/resources/index"); + + assertNoCacheInBrowser(httpResponse); + } + + @Test + public void browser_cache_on_images() { + Response httpResponse = call(orchestrator.getServer().getUrl() + "/images/logo.svg"); + + assertCacheInBrowser(httpResponse); + } + + @Test + public void browser_cache_on_css() { + Response httpResponse = call(orchestrator.getServer().getUrl() + "/css/sonar.css"); + + 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(); + assertThat(cacheControl.noCache()).isFalse(); + assertThat(cacheControl.noStore()).isFalse(); + } + + private static void assertNoCacheInBrowser(Response httpResponse) { + CacheControl cacheControl = httpResponse.cacheControl(); + assertThat(cacheControl.mustRevalidate()).isTrue(); + assertThat(cacheControl.noCache()).isTrue(); + 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 { + // SonarQube ws-client cannot be used as it does not + // expose HTTP headers + return new OkHttpClient.Builder() + .connectTimeout(30, TimeUnit.SECONDS) + .readTimeout(30, TimeUnit.SECONDS) + .build() + .newCall(request).execute(); + } catch (IOException e) { + throw Throwables.propagate(e); + } + } +} 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 /*