From 75d906252e8b4a3f1fda7fe18bb7e6ec205489d2 Mon Sep 17 00:00:00 2001 From: Ammar Bikic Date: Mon, 30 Nov 2020 16:25:43 +0100 Subject: [PATCH] Fix host header attack Signed-off-by: Ammar Bikic --- .../SecurityManagedConfiguration.java | 16 ++++++++ .../hawkbit/app/AbstractSecurityTest.java | 38 ++++++++++++++++++ .../hawkbit/app/AllowedHostNamesTest.java | 40 +++++++++++++++++++ .../org/eclipse/hawkbit/app/CorsTest.java | 22 ++-------- .../security/HawkbitSecurityProperties.java | 10 +++++ 5 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java create mode 100644 hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java 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 d237ab18f..85b70adb4 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 @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.List; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -91,9 +92,12 @@ 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.HttpFirewall; +import org.springframework.security.web.firewall.StrictHttpFirewall; import org.springframework.security.web.session.HttpSessionEventPublisher; import org.springframework.security.web.session.SessionManagementFilter; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; @@ -718,6 +722,18 @@ public class SecurityManagedConfiguration { .logoutSuccessHandler(logoutSuccessHandler); } + @Bean + public HttpFirewall httpFirewall() { + final List allowedHostNames = hawkbitSecurityProperties.getAllowedHostNames(); + final StrictHttpFirewall firewall = new StrictHttpFirewall(); + + if (allowedHostNames != null && !CollectionUtils.isEmpty(allowedHostNames)) { + firewall.setAllowedHostnames(hostName -> allowedHostNames.stream() + .anyMatch(allowedHostName -> allowedHostName.equals(hostName))); + } + return firewall; + } + @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 new file mode 100644 index 000000000..de8d224c7 --- /dev/null +++ b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AbstractSecurityTest.java @@ -0,0 +1,38 @@ +package org.eclipse.hawkbit.app; + +import org.eclipse.hawkbit.repository.test.util.MsSqlTestDatabase; +import org.eclipse.hawkbit.repository.test.util.MySqlTestDatabase; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +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.TestExecutionListeners.MergeMode; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; + +@RunWith(SpringRunner.class) +@SpringBootTest(properties = { "hawkbit.dmf.rabbitmq.enabled=false" }) +@TestExecutionListeners(listeners = { MySqlTestDatabase.class, + MsSqlTestDatabase.class }, mergeMode = MergeMode.MERGE_WITH_DEFAULTS) +@DirtiesContext +public abstract class AbstractSecurityTest { + + @Autowired + private WebApplicationContext context; + + protected MockMvc mvc; + + @Before + public void setup() { + final DefaultMockMvcBuilder builder = MockMvcBuilders.webAppContextSetup(context) + .apply(SecurityMockMvcConfigurers.springSecurity()).dispatchOptions(true); + mvc = builder.build(); + } + +} \ No newline at end of file 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 new file mode 100644 index 000000000..94f3c63b4 --- /dev/null +++ b/hawkbit-runtime/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/AllowedHostNamesTest.java @@ -0,0 +1,40 @@ +/** + * Copyright (c) 2019 Bosch Software Innovations 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 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; + +@SpringBootTest(properties = { "hawkbit.server.security.allowedHostNames=localhost" }) +@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 + } + } + + @Test + public void allowedHostNameWithAllowedHost() throws Exception { + mvc.perform(get("/").header(HttpHeaders.HOST, "localhost")).andExpect(status().is3xxRedirection()); + } +} \ 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 7dd67eb55..6c20c193a 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 @@ -36,14 +36,12 @@ import io.qameta.allure.Description; import io.qameta.allure.Feature; import io.qameta.allure.Story; -@RunWith(SpringRunner.class) -@SpringBootTest(properties = {"hawkbit.dmf.rabbitmq.enabled=false", "hawkbit.server.security.cors.enabled=true", - "hawkbit.server.security.cors.allowedOrigins=" + CorsTest.ALLOWED_ORIGIN_FIRST + "," + CorsTest.ALLOWED_ORIGIN_SECOND}) -@TestExecutionListeners(listeners = { MySqlTestDatabase.class, MsSqlTestDatabase.class }, - mergeMode = MergeMode.MERGE_WITH_DEFAULTS) +@SpringBootTest(properties = { "hawkbit.server.security.cors.enabled=true", + "hawkbit.server.security.cors.allowedOrigins=" + CorsTest.ALLOWED_ORIGIN_FIRST + "," + + CorsTest.ALLOWED_ORIGIN_SECOND }) @Feature("Integration Test - Security") @Story("CORS") -public class CorsTest { +public class CorsTest extends AbstractSecurityTest { final static String ALLOWED_ORIGIN_FIRST = "http://test.first.origin"; final static String ALLOWED_ORIGIN_SECOND = "http://test.second.origin"; @@ -51,18 +49,6 @@ public class CorsTest { private final static String INVALID_ORIGIN = "http://test.invalid.origin"; private final static String INVALID_CORS_REQUEST = "Invalid CORS request"; - @Autowired - private WebApplicationContext context; - - private MockMvc mvc; - - @Before - public void setup() { - final DefaultMockMvcBuilder builder = MockMvcBuilders.webAppContextSetup(context) - .apply(SecurityMockMvcConfigurers.springSecurity()).dispatchOptions(true); - mvc = builder.build(); - } - @WithUserDetails("admin") @Test @Description("Ensures that Cors is working.") 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 5e0bed358..db8a43f7c 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,6 +35,8 @@ public class HawkbitSecurityProperties { */ private boolean requireSsl; + private List allowedHostNames; + /** * Basic authentication realm, see * https://tools.ietf.org/html/rfc2617#page-3 . @@ -49,6 +51,14 @@ public class HawkbitSecurityProperties { this.requireSsl = requireSsl; } + public List getAllowedHostNames() { + return allowedHostNames; + } + + public void setAllowedHostNames(final List allowedHostNames) { + this.allowedHostNames = allowedHostNames; + } + public String getBasicRealm() { return basicRealm; }