From 98f7a5b9f36aded8375bbfd775edef4ee5593e4e Mon Sep 17 00:00:00 2001 From: Ammar Bikic Date: Fri, 4 Dec 2020 13:33:59 +0100 Subject: [PATCH] Host header attack implementation improvements and tests Signed-off-by: Ammar Bikic --- .../SecurityManagedConfiguration.java | 34 ++++++++++++++++--- .../hawkbit/app/AbstractSecurityTest.java | 8 +++++ .../hawkbit/app/AllowedHostNamesTest.java | 23 ++++++++----- .../org/eclipse/hawkbit/app/CorsTest.java | 3 +- .../security/HawkbitSecurityProperties.java | 17 ++++++++++ 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java index 5ad3e161c..bcc7530ec 100644 --- a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java +++ b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java @@ -20,6 +20,7 @@ import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; import org.eclipse.hawkbit.cache.DownloadIdCache; import org.eclipse.hawkbit.ddi.rest.api.DdiRestConstants; @@ -92,6 +93,7 @@ import org.springframework.security.web.authentication.logout.LogoutSuccessHandl import org.springframework.security.web.authentication.preauth.RequestHeaderAuthenticationFilter; import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; +import org.springframework.security.web.firewall.FirewalledRequest; import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.session.HttpSessionEventPublisher; @@ -725,17 +727,39 @@ public class SecurityManagedConfiguration { @Bean public HttpFirewall httpFirewall() { final List allowedHostNames = hawkbitSecurityProperties.getAllowedHostNames(); - final StrictHttpFirewall firewall = new StrictHttpFirewall(); + final IgnorePathsStrictHttpFirewall firewall = new IgnorePathsStrictHttpFirewall( + hawkbitSecurityProperties.getHttpFirewallIgnoredPaths()); - if (allowedHostNames != null && !CollectionUtils.isEmpty(allowedHostNames)) { + if (!CollectionUtils.isEmpty(allowedHostNames)) { firewall.setAllowedHostnames(hostName -> { - LOG.info("Firewall check host: {}, allowed: {}", hostName, allowedHostNames.contains(hostName)); - return allowedHostNames.stream() - .anyMatch(allowedHostName -> allowedHostName.equals(hostName));}); + LOG.debug("Firewall check host: {}, allowed: {}", hostName, allowedHostNames.contains(hostName)); + return allowedHostNames.contains(hostName);}); } return firewall; } + private static class IgnorePathsStrictHttpFirewall extends StrictHttpFirewall { + + private final Collection pathsToIgnore; + + public IgnorePathsStrictHttpFirewall(final Collection pathsToIgnore) { + super(); + this.pathsToIgnore = pathsToIgnore; + } + + @Override + public FirewalledRequest getFirewalledRequest(final HttpServletRequest request) { + if (pathsToIgnore != null && pathsToIgnore.contains(request.getRequestURI())) { + return new FirewalledRequest(request) { + @Override + public void reset() { + } + }; + } + return super.getFirewalledRequest(request); + } + } + @Override public void configure(final WebSecurity webSecurity) throws Exception { // No security for static content diff --git a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java index de8d224c7..9b4114eb2 100644 --- a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java +++ b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java @@ -1,3 +1,11 @@ +/** + * Copyright (c) 2020 Bosch.IO GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ package org.eclipse.hawkbit.app; import org.eclipse.hawkbit.repository.test.util.MsSqlTestDatabase; diff --git a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java index 94f3c63b4..165d96fbd 100644 --- a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java +++ b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java @@ -8,33 +8,38 @@ */ package org.eclipse.hawkbit.app; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import org.junit.Test; -import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.HttpHeaders; import org.springframework.security.web.firewall.RequestRejectedException; import io.qameta.allure.Feature; import io.qameta.allure.Story; +import org.springframework.test.context.TestPropertySource; -@SpringBootTest(properties = { "hawkbit.server.security.allowedHostNames=localhost" }) +@TestPropertySource(properties = { "hawkbit.server.security.allowedHostNames=localhost", + "hawkbit.server.security.httpFirewallIgnoredPaths=/index.html" }) @Feature("Integration Test - Security") @Story("Allowed Host Names") public class AllowedHostNamesTest extends AbstractSecurityTest { @Test - public void allowedHostNameWithNotAllowedHost() throws Exception { - try { - mvc.perform(get("/").header(HttpHeaders.HOST, "www.google.com")); - } catch (final RequestRejectedException e) { - // do nothing as this exception is expected - } + public void allowedHostNameWithNotAllowedHost() { + assertThatExceptionOfType(RequestRejectedException.class).isThrownBy( + () -> mvc.perform(get("/").header(HttpHeaders.HOST, "www.google.com"))); } @Test public void allowedHostNameWithAllowedHost() throws Exception { mvc.perform(get("/").header(HttpHeaders.HOST, "localhost")).andExpect(status().is3xxRedirection()); } -} \ No newline at end of file + + @Test + public void notAllowedHostnameWithIgnoredPath() throws Exception { + mvc.perform(get("/index.html").header(HttpHeaders.HOST, "www.google.com")) + .andExpect(status().is4xxClientError()); + } +} \ No newline at end of file diff --git a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/CorsTest.java b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/CorsTest.java index 6c20c193a..8a576df45 100644 --- a/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/CorsTest.java +++ b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/CorsTest.java @@ -24,6 +24,7 @@ import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.TestExecutionListeners; +import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; @@ -36,7 +37,7 @@ import io.qameta.allure.Description; import io.qameta.allure.Feature; import io.qameta.allure.Story; -@SpringBootTest(properties = { "hawkbit.server.security.cors.enabled=true", +@TestPropertySource(properties = { "hawkbit.server.security.cors.enabled=true", "hawkbit.server.security.cors.allowedOrigins=" + CorsTest.ALLOWED_ORIGIN_FIRST + "," + CorsTest.ALLOWED_ORIGIN_SECOND }) @Feature("Integration Test - Security") diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java index db8a43f7c..420f1e828 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/HawkbitSecurityProperties.java @@ -35,8 +35,17 @@ public class HawkbitSecurityProperties { */ private boolean requireSsl; + /** + * With this property a list of allowed hostnames can be configured. All + * requests with different Host headers will be rejected. + */ private List allowedHostNames; + /** + * Add paths that will be ignored by {@link StrictHttpFirewall}. + */ + private List httpFirewallIgnoredPaths; + /** * Basic authentication realm, see * https://tools.ietf.org/html/rfc2617#page-3 . @@ -59,6 +68,14 @@ public class HawkbitSecurityProperties { this.allowedHostNames = allowedHostNames; } + public List getHttpFirewallIgnoredPaths() { + return httpFirewallIgnoredPaths; + } + + public void setHttpFirewallIgnoredPaths(final List httpFirewallIgnoredPaths) { + this.httpFirewallIgnoredPaths = httpFirewallIgnoredPaths; + } + public String getBasicRealm() { return basicRealm; }