From 4709f4374c3e4c9a3683bd9008b77b365257bd02 Mon Sep 17 00:00:00 2001 From: Kai Zimmermann Date: Wed, 7 Jun 2017 16:32:52 +0200 Subject: [PATCH] Fix to many request filter usage in DDI (#526) * Fix DOS filter usage in DDI Signed-off-by: kaizimmerm * Add optional CSP definition. Signed-off-by: kaizimmerm * Fix for empty case. Signed-off-by: kaizimmerm * readability and ensure that manual enforcement is also possible in timeforced active but no hit yet. Signed-off-by: kaizimmerm * Class order to bean order. Signed-off-by: kaizimmerm * Fix exception propagation. Signed-off-by: kaizimmerm --- .../SecurityManagedConfiguration.java | 149 ++++++++++-------- .../main/resources/hawkbitdefaults.properties | 3 - .../jpa/JpaDeploymentManagement.java | 2 +- .../test/util/AbstractIntegrationTest.java | 2 +- .../eclipse/hawkbit/security/DosFilter.java | 27 +++- .../ExcludePathAwareShallowETagFilter.java | 3 +- .../security/HawkbitSecurityProperties.java | 13 ++ .../security/SystemSecurityContext.java | 2 + 8 files changed, 124 insertions(+), 77 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 92dd637ba..20e8fbfa4 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 @@ -10,6 +10,8 @@ package org.eclipse.hawkbit.autoconfigure.security; import java.io.IOException; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import javax.annotation.PostConstruct; import javax.servlet.Filter; @@ -80,6 +82,7 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationEn import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.security.web.session.HttpSessionEventPublisher; import org.springframework.security.web.session.SessionManagementFilter; +import org.springframework.util.StringUtils; import org.vaadin.spring.security.VaadinSecurityContext; import org.vaadin.spring.security.annotation.EnableVaadinSecurity; import org.vaadin.spring.security.web.VaadinDefaultRedirectStrategy; @@ -99,7 +102,7 @@ public class SecurityManagedConfiguration { private static final Logger LOG = LoggerFactory.getLogger(SecurityManagedConfiguration.class); - private static final int DOS_FILTER_ORDER = 1; + private static final int DOS_FILTER_ORDER = -200; @Autowired private AuthenticationConfiguration configuration; @@ -136,6 +139,8 @@ public class SecurityManagedConfiguration { @ConditionalOnClass(DdiApiConfiguration.class) static class ControllerSecurityConfigurationAdapter extends WebSecurityConfigurerAdapter { + private static final String DDI_ANT_MATCHER = "/{tenant}/controller/**"; + @Autowired private ControllerManagement controllerManagement; @@ -165,12 +170,10 @@ public class SecurityManagedConfiguration { * of service protection filter in the filter chain */ @Bean - @ConditionalOnClass(DdiApiConfiguration.class) public FilterRegistrationBean dosDDiFilter(final HawkbitSecurityProperties securityProperties) { - final FilterRegistrationBean filterRegBean = dosFilter(securityProperties.getDos().getFilter(), - securityProperties.getClients()); - filterRegBean.addUrlPatterns("/{tenant}/controller/v1/*"); + final FilterRegistrationBean filterRegBean = dosFilter(Arrays.asList(DDI_ANT_MATCHER), + securityProperties.getDos().getFilter(), securityProperties.getClients()); filterRegBean.setOrder(DOS_FILTER_ORDER); filterRegBean.setName("dosDDiFilter"); @@ -224,13 +227,13 @@ public class SecurityManagedConfiguration { Arrays.asList(new SimpleGrantedAuthority(SpringEvalExpressions.CONTROLLER_ROLE_ANONYMOUS), new SimpleGrantedAuthority(SpringEvalExpressions.CONTROLLER_DOWNLOAD_ROLE))); anoymousFilter.setAuthenticationDetailsSource(authenticationDetailsSource); - httpSec.requestMatchers().antMatchers("/*/controller/v1/**", "/*/controller/artifacts/v1/**").and() - .securityContext().disable().anonymous().authenticationFilter(anoymousFilter); + httpSec.requestMatchers().antMatchers(DDI_ANT_MATCHER).and().securityContext().disable().anonymous() + .authenticationFilter(anoymousFilter); } else { httpSec.addFilter(securityHeaderFilter).addFilter(securityTokenFilter) .addFilter(gatewaySecurityTokenFilter).addFilter(controllerAnonymousDownloadFilter) - .antMatcher("/*/controller/**").anonymous().disable().authorizeRequests().anyRequest() + .antMatcher(DDI_ANT_MATCHER).anonymous().disable().authorizeRequests().anyRequest() .authenticated().and().exceptionHandling() .authenticationEntryPoint((request, response, authException) -> response .setStatus(HttpStatus.UNAUTHORIZED.value())) @@ -259,8 +262,8 @@ public class SecurityManagedConfiguration { @Bean public FilterRegistrationBean dosSystemFilter(final HawkbitSecurityProperties securityProperties) { - final FilterRegistrationBean filterRegBean = dosFilter(securityProperties.getDos().getFilter(), - securityProperties.getClients()); + final FilterRegistrationBean filterRegBean = dosFilter(Collections.emptyList(), + securityProperties.getDos().getFilter(), securityProperties.getClients()); filterRegBean.setUrlPatterns(Arrays.asList("/system/*")); filterRegBean.setOrder(DOS_FILTER_ORDER); filterRegBean.setName("dosSystemFilter"); @@ -268,39 +271,54 @@ public class SecurityManagedConfiguration { return filterRegBean; } - private static FilterRegistrationBean dosFilter(final HawkbitSecurityProperties.Dos.Filter filterProperties, + private static FilterRegistrationBean dosFilter(final Collection includeAntPaths, + final HawkbitSecurityProperties.Dos.Filter filterProperties, final HawkbitSecurityProperties.Clients clientProperties) { final FilterRegistrationBean filterRegBean = new FilterRegistrationBean(); - filterRegBean.setFilter(new DosFilter(filterProperties.getMaxRead(), filterProperties.getMaxWrite(), - filterProperties.getWhitelist(), clientProperties.getBlacklist(), + filterRegBean.setFilter(new DosFilter(includeAntPaths, filterProperties.getMaxRead(), + filterProperties.getMaxWrite(), filterProperties.getWhitelist(), clientProperties.getBlacklist(), clientProperties.getRemoteIpHeader())); return filterRegBean; } /** - * Filter registration bean for spring etag filter. - * - * @return the spring filter registration bean for registering an etag - * filter in the filter chain + * A Websecruity config to handle and filter the download ids. */ - @Bean - @Order(100) - public FilterRegistrationBean eTagFilter() { + @Configuration + @EnableWebSecurity + @Order(320) + @ConditionalOnClass(MgmtApiConfiguration.class) + public static class IdRestSecurityConfigurationAdapter extends WebSecurityConfigurerAdapter { - final FilterRegistrationBean filterRegBean = new FilterRegistrationBean(); - // Exclude the URLs for downloading artifacts, so no eTag is generated - // in the ShallowEtagHeaderFilter, just using the SH1 hash of the - // artifact itself as 'ETag', because otherwise the file will be copied - // in memory! - filterRegBean.setFilter( - new ExcludePathAwareShallowETagFilter("/rest/v1/softwaremodules/{smId}/artifacts/{artId}/download", - "/{tenant}/controller/v1/{controllerId}/softwaremodules/{softwareModuleId}/artifacts/**", - "/api/v1/downloadserver/**")); + @Autowired + private DdiSecurityProperties ddiSecurityConfiguration; - return filterRegBean; + @Autowired + private DownloadIdCache downloadIdCache; + + @Override + protected void configure(final HttpSecurity http) throws Exception { + + final HttpDownloadAuthenticationFilter downloadIdAuthenticationFilter = new HttpDownloadAuthenticationFilter( + downloadIdCache); + downloadIdAuthenticationFilter.setAuthenticationManager(authenticationManager()); + + http.csrf().disable(); + http.anonymous().disable(); + + http.regexMatcher(HttpDownloadAuthenticationFilter.REQUEST_ID_REGEX_PATTERN) + .addFilterBefore(downloadIdAuthenticationFilter, FilterSecurityInterceptor.class); + http.authorizeRequests().anyRequest().authenticated(); + } + + @Override + protected void configure(final AuthenticationManagerBuilder auth) throws Exception { + auth.authenticationProvider(new PreAuthTokenSourceTrustAuthenticationProvider( + ddiSecurityConfiguration.getRp().getTrustedIPs())); + } } /** @@ -336,7 +354,7 @@ public class SecurityManagedConfiguration { @Bean public FilterRegistrationBean dosMgmtFilter(final HawkbitSecurityProperties securityProperties) { - final FilterRegistrationBean filterRegBean = dosFilter(securityProperties.getDos().getFilter(), + final FilterRegistrationBean filterRegBean = dosFilter(null, securityProperties.getDos().getFilter(), securityProperties.getClients()); filterRegBean.setUrlPatterns(Arrays.asList("/rest/*", "/api/*")); filterRegBean.setOrder(DOS_FILTER_ORDER); @@ -384,6 +402,29 @@ public class SecurityManagedConfiguration { } } + /** + * Filter registration bean for spring etag filter. + * + * @return the spring filter registration bean for registering an etag + * filter in the filter chain + */ + @Bean + @Order(380) + public FilterRegistrationBean eTagFilter() { + + final FilterRegistrationBean filterRegBean = new FilterRegistrationBean(); + // Exclude the URLs for downloading artifacts, so no eTag is generated + // in the ShallowEtagHeaderFilter, just using the SH1 hash of the + // artifact itself as 'ETag', because otherwise the file will be copied + // in memory! + filterRegBean.setFilter(new ExcludePathAwareShallowETagFilter("/UI/**", + "/rest/v1/softwaremodules/{smId}/artifacts/{artId}/download", + "/{tenant}/controller/v1/{controllerId}/softwaremodules/{softwareModuleId}/artifacts/**", + "/api/v1/downloadserver/**")); + + return filterRegBean; + } + /** * {@link WebSecurityConfigurer} for external (management) access. */ @@ -399,6 +440,9 @@ public class SecurityManagedConfiguration { @Autowired private SecurityProperties springSecurityProperties; + @Autowired + private HawkbitSecurityProperties hawkbitSecurityProperties; + /** * Filter to protect the hawkBit management UI against to many requests. * @@ -411,7 +455,7 @@ public class SecurityManagedConfiguration { @Bean public FilterRegistrationBean dosMgmtUiFilter(final HawkbitSecurityProperties securityProperties) { - final FilterRegistrationBean filterRegBean = dosFilter(securityProperties.getDos().getUiFilter(), + final FilterRegistrationBean filterRegBean = dosFilter(null, securityProperties.getDos().getUiFilter(), securityProperties.getClients()); // All URLs that can be called anonymous filterRegBean.setUrlPatterns(Arrays.asList("/UI/login", "/UI/login/*", "/UI/logout", "/UI/logout/*")); @@ -490,6 +534,10 @@ public class SecurityManagedConfiguration { "\"******************\\n** Requires HTTPS Security has been disabled for UI, should only be used for developing purposes **\\n******************\""); } + if (!StringUtils.isEmpty(hawkbitSecurityProperties.getContentSecurityPolicy())) { + httpSec.headers().contentSecurityPolicy(hawkbitSecurityProperties.getContentSecurityPolicy()); + } + httpSec // UI .authorizeRequests().antMatchers("/UI/login/**").permitAll().antMatchers("/UI/UIDL/**").permitAll() @@ -501,46 +549,11 @@ public class SecurityManagedConfiguration { @Override public void configure(final WebSecurity webSecurity) throws Exception { + // Not security for static content webSecurity.ignoring().antMatchers("/documentation/**", "/VAADIN/**", "/*.*", "/docs/**"); } } - /** - * A Websecruity config to handle and filter the download ids. - */ - @Configuration - @EnableWebSecurity - @Order(200) - @ConditionalOnClass(DdiApiConfiguration.class) - public static class IdRestSecurityConfigurationAdapter extends WebSecurityConfigurerAdapter { - - @Autowired - private DdiSecurityProperties ddiSecurityConfiguration; - - @Autowired - private DownloadIdCache downloadIdCache; - - @Override - protected void configure(final HttpSecurity http) throws Exception { - - final HttpDownloadAuthenticationFilter downloadIdAuthenticationFilter = new HttpDownloadAuthenticationFilter( - downloadIdCache); - downloadIdAuthenticationFilter.setAuthenticationManager(authenticationManager()); - - http.csrf().disable(); - http.anonymous().disable(); - - http.regexMatcher(HttpDownloadAuthenticationFilter.REQUEST_ID_REGEX_PATTERN) - .addFilterBefore(downloadIdAuthenticationFilter, FilterSecurityInterceptor.class); - http.authorizeRequests().anyRequest().authenticated(); - } - - @Override - protected void configure(final AuthenticationManagerBuilder auth) throws Exception { - auth.authenticationProvider(new PreAuthTokenSourceTrustAuthenticationProvider( - ddiSecurityConfiguration.getRp().getTrustedIPs())); - } - } } /** diff --git a/hawkbit-autoconfigure/src/main/resources/hawkbitdefaults.properties b/hawkbit-autoconfigure/src/main/resources/hawkbitdefaults.properties index fe951eda9..cd97b7c49 100644 --- a/hawkbit-autoconfigure/src/main/resources/hawkbitdefaults.properties +++ b/hawkbit-autoconfigure/src/main/resources/hawkbitdefaults.properties @@ -15,9 +15,6 @@ security.basic.realm=HawkBit security.user.name=admin security.user.password=admin -# Ensure that DosFilter runs before Spring Security -security.filter-order=5 - # Spring cloud bus and stream spring.cloud.bus.enabled=false # Disable Cloud Bus default events diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java index 5ca02376c..2c256682c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaDeploymentManagement.java @@ -614,7 +614,7 @@ public class JpaDeploymentManagement implements DeploymentManagement { final JpaAction action = actionRepository.findById(actionId) .orElseThrow(() -> new EntityNotFoundException(Action.class, actionId)); - if (!action.isForced()) { + if (!action.isForce()) { action.setActionType(ActionType.FORCED); return actionRepository.save(action); } diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java index 18305f3a3..8e41819e6 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java @@ -313,7 +313,7 @@ public abstract class AbstractIntegrationTest implements EnvironmentAware { protected DefaultMockMvcBuilder createMvcWebAppContext() { return MockMvcBuilders.webAppContextSetup(context) - .addFilter(new DosFilter(100, 10, "127\\.0\\.0\\.1|\\[0:0:0:0:0:0:0:1\\]", "(^192\\.168\\.)", + .addFilter(new DosFilter(null, 100, 10, "127\\.0\\.0\\.1|\\[0:0:0:0:0:0:0:1\\]", "(^192\\.168\\.)", "X-Forwarded-For")) .addFilter(new ExcludePathAwareShallowETagFilter( "/rest/v1/softwaremodules/{smId}/artifacts/{artId}/download", diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java index ecb85394b..468e522ee 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/DosFilter.java @@ -9,6 +9,7 @@ package org.eclipse.hawkbit.security; import java.io.IOException; +import java.util.Collection; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -23,6 +24,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.util.AntPathMatcher; import org.springframework.web.filter.OncePerRequestFilter; import com.github.benmanes.caffeine.cache.Cache; @@ -40,6 +42,9 @@ public class DosFilter extends OncePerRequestFilter { private static final Logger LOG_BLACKLIST = LoggerFactory .getLogger(SecurityConstants.SECURITY_LOG_PREFIX + ".blacklist"); + private final AntPathMatcher antMatcher = new AntPathMatcher(); + private final Collection includeAntPaths; + private final Pattern ipAdressBlacklist; private final Cache readCountCache = Caffeine.newBuilder() @@ -57,6 +62,9 @@ public class DosFilter extends OncePerRequestFilter { /** * Filter constructor including configuration. + * + * @param includeAntPaths + * paths where filter should hit * * @param maxRead * Maximum number of allowed REST read/GET requests per second @@ -73,9 +81,10 @@ public class DosFilter extends OncePerRequestFilter { * the header containing the forwarded IP address e.g. * {@code x-forwarded-for} */ - public DosFilter(final int maxRead, final int maxWrite, final String ipDosWhiteListPattern, - final String ipBlackListPattern, final String forwardHeader) { + public DosFilter(final Collection includeAntPaths, final int maxRead, final int maxWrite, + final String ipDosWhiteListPattern, final String ipBlackListPattern, final String forwardHeader) { + this.includeAntPaths = includeAntPaths; this.maxRead = maxRead; this.maxWrite = maxWrite; this.forwardHeader = forwardHeader; @@ -93,10 +102,24 @@ public class DosFilter extends OncePerRequestFilter { } } + private boolean shouldInclude(final HttpServletRequest request) { + if (includeAntPaths == null || includeAntPaths.isEmpty()) { + return true; + } + + return includeAntPaths.stream() + .anyMatch(pattern -> antMatcher.match(request.getContextPath() + pattern, request.getRequestURI())); + } + @Override protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain) throws ServletException, IOException { + if (!shouldInclude(request)) { + filterChain.doFilter(request, response); + return; + } + boolean processChain; final String ip = IpUtil.getClientIpFromRequest(request, forwardHeader).getHost(); diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/ExcludePathAwareShallowETagFilter.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/ExcludePathAwareShallowETagFilter.java index 3f51c7cce..3727902b4 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/ExcludePathAwareShallowETagFilter.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/ExcludePathAwareShallowETagFilter.java @@ -27,14 +27,13 @@ import org.springframework.web.filter.ShallowEtagHeaderFilter; public class ExcludePathAwareShallowETagFilter extends ShallowEtagHeaderFilter { private final String[] excludeAntPaths; - private final AntPathMatcher antMatcher; + private final AntPathMatcher antMatcher = new AntPathMatcher(); /** * @param excludeAntPaths */ public ExcludePathAwareShallowETagFilter(final String... excludeAntPaths) { this.excludeAntPaths = excludeAntPaths; - this.antMatcher = new AntPathMatcher(); } @Override 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 f5fe893ad..5f78fc9cb 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 @@ -20,6 +20,19 @@ public class HawkbitSecurityProperties { private final Clients clients = new Clients(); private final Dos dos = new Dos(); + /** + * Content Security policy Header for Manager UI. + */ + private String contentSecurityPolicy; + + public String getContentSecurityPolicy() { + return contentSecurityPolicy; + } + + public void setContentSecurityPolicy(final String contentSecurityPolicy) { + this.contentSecurityPolicy = contentSecurityPolicy; + } + public Dos getDos() { return dos; } diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java index 5d9281de0..d86d5a134 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java @@ -98,6 +98,8 @@ public class SystemSecurityContext { setSystemContext(SecurityContextHolder.getContext()); return callable.call(); + } catch (final RuntimeException e) { + throw e; } catch (final Exception e) { throw new RuntimeException(e); }