From 1090072fff420852cd4e46d40f10c508c6158334 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 24 Apr 2008 01:59:19 +0000 Subject: [PATCH] SEC-795: Add check for protected login page when using namespace http://jira.springframework.org/browse/SEC-795. I've added checks for the various scenarios which will result in a protected login page and suitable warning messages. --- .../config/FilterChainProxyPostProcessor.java | 110 +++++++++++++++++- ...faultFilterInvocationDefinitionSource.java | 2 +- .../security/util/FilterChainProxy.java | 2 +- ...HttpSecurityBeanDefinitionParserTests.java | 54 +++++++++ 4 files changed, 163 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java b/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java index 998a1d5275..df3dc25335 100644 --- a/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java +++ b/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java @@ -16,8 +16,21 @@ import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.core.OrderComparator; import org.springframework.core.Ordered; +import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.config.ConfigUtils.FilterChainList; +import org.springframework.security.context.HttpSessionContextIntegrationFilter; +import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource; +import org.springframework.security.intercept.web.FilterSecurityInterceptor; +import org.springframework.security.providers.anonymous.AnonymousAuthenticationToken; +import org.springframework.security.providers.anonymous.AnonymousProcessingFilter; +import org.springframework.security.ui.ExceptionTranslationFilter; +import org.springframework.security.ui.SessionFixationProtectionFilter; +import org.springframework.security.ui.basicauth.BasicProcessingFilter; +import org.springframework.security.ui.webapp.AuthenticationProcessingFilter; +import org.springframework.security.ui.webapp.AuthenticationProcessingFilterEntryPoint; +import org.springframework.security.ui.webapp.DefaultLoginPageGeneratingFilter; import org.springframework.security.util.FilterChainProxy; +import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter; /** * @@ -59,23 +72,114 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac } logger.info("Filter chain..."); - for(int i=0; i < filters.size(); i++) { + for (int i=0; i < filters.size(); i++) { // Remove the ordered wrapper from the filter and put it back in the chain at the same position. Filter filter = unwrapFilter(filters.get(i)); logger.info("[" + i + "] - " + filter); filters.set(i, filter); } + checkFilterStack(filters); + // Note that this returns a copy Map filterMap = filterChainProxy.getFilterChainMap(); filterMap.put(filterChainProxy.getMatcher().getUniversalMatchPattern(), filters); filterChainProxy.setFilterChainMap(filterMap); - - logger.info("FilterChainProxy: " + filterChainProxy); + + checkLoginPageIsntProtected(filterChainProxy); + + logger.info("FilterChainProxy: " + filterChainProxy); return bean; } + /** + * Checks the filter list for possible errors and logs them + */ + private void checkFilterStack(List filters) { + checkForDuplicates(HttpSessionContextIntegrationFilter.class, filters); + checkForDuplicates(AuthenticationProcessingFilter.class, filters); + checkForDuplicates(SessionFixationProtectionFilter.class, filters); + checkForDuplicates(BasicProcessingFilter.class, filters); + checkForDuplicates(SecurityContextHolderAwareRequestFilter.class, filters); + checkForDuplicates(ExceptionTranslationFilter.class, filters); + checkForDuplicates(FilterSecurityInterceptor.class, filters); + } + + private void checkForDuplicates(Class clazz, List filters) { + for (int i=0; i < filters.size(); i++) { + Filter f1 = (Filter)filters.get(i); + if (clazz.isAssignableFrom(f1.getClass())) { + // Found the first one, check remaining for another + for (int j=i+1; j < filters.size(); j++) { + Filter f2 = (Filter)filters.get(j); + if (clazz.isAssignableFrom(f2.getClass())) { + logger.warn("Possible error: Filters at position " + i + " and " + j + " are both " + + "instances of " + clazz.getName()); + return; + } + } + } + } + } + + /* Checks for the common error of having a login page URL protected by the security interceptor */ + private void checkLoginPageIsntProtected(FilterChainProxy fcp) { + ExceptionTranslationFilter etf = (ExceptionTranslationFilter) beanFactory.getBean(BeanIds.EXCEPTION_TRANSLATION_FILTER); + + if (etf.getAuthenticationEntryPoint() instanceof AuthenticationProcessingFilterEntryPoint) { + String loginPage = + ((AuthenticationProcessingFilterEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl(); + List filters = fcp.getFilters(loginPage); + logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration"); + + if (filters == null || filters.isEmpty()) { + logger.debug("Filter chain is empty for the login page"); + return; + } + + if (loginPage.equals(DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL) && + beanFactory.containsBean(BeanIds.DEFAULT_LOGIN_PAGE_GENERATING_FILTER)) { + logger.debug("Default generated login page is in use"); + return; + } + + FilterSecurityInterceptor fsi = + ((FilterSecurityInterceptor)beanFactory.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR)); + DefaultFilterInvocationDefinitionSource fids = + (DefaultFilterInvocationDefinitionSource) fsi.getObjectDefinitionSource(); + ConfigAttributeDefinition cad = fids.lookupAttributes(loginPage, "POST"); + + if (cad == null) { + logger.debug("No access attributes defined for login page URL"); + if (fsi.isRejectPublicInvocations()) { + logger.warn("FilterSecurityInterceptor is configured to reject public invocations." + + " Your login page may not be accessible."); + } + return; + } + + if (!beanFactory.containsBean(BeanIds.ANONYMOUS_PROCESSING_FILTER)) { + logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + + " anonymous authentication enabled. This is almost certainly an error."); + return; + } + + // Simulate an anonymous access with the supplied attributes. + AnonymousProcessingFilter anonPF = (AnonymousProcessingFilter) beanFactory.getBean(BeanIds.ANONYMOUS_PROCESSING_FILTER); + AnonymousAuthenticationToken token = + new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), + anonPF.getUserAttribute().getAuthorities()); + try { + fsi.getAccessDecisionManager().decide(token, new Object(), cad); + } catch (Exception e) { + logger.warn("Anonymous access to the login page doesn't appear to be enabled. This is almost certainly " + + "an error. Please check your configuration allows unauthenticated access to the configured " + + "login page. (Simulated access was rejected: " + e + ")"); + } + } + } + /** * Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped. */ diff --git a/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java index 507762dca9..0d3bab2a5c 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java @@ -179,7 +179,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation * @return the ConfigAttributeDefinition that applies to the specified FilterInvocation * or null if no match is foud */ - protected ConfigAttributeDefinition lookupAttributes(String url, String method) { + public ConfigAttributeDefinition lookupAttributes(String url, String method) { if (stripQueryStringFromUrls) { // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321 int firstQuestionMarkIndex = url.indexOf("?"); diff --git a/core/src/main/java/org/springframework/security/util/FilterChainProxy.java b/core/src/main/java/org/springframework/security/util/FilterChainProxy.java index b1e864ac6b..75435b61ad 100644 --- a/core/src/main/java/org/springframework/security/util/FilterChainProxy.java +++ b/core/src/main/java/org/springframework/security/util/FilterChainProxy.java @@ -180,7 +180,7 @@ public class FilterChainProxy implements Filter, InitializingBean, ApplicationCo * @param url the request URL * @return an ordered array of Filters defining the filter chain */ - List getFilters(String url) { + public List getFilters(String url) { Iterator filterChains = filterChainMap.entrySet().iterator(); while (filterChains.hasNext()) { diff --git a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java index 8199ba88ed..e05608fb76 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -8,6 +8,7 @@ import java.util.List; import org.junit.After; import org.junit.Test; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.support.AbstractXmlApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; @@ -282,6 +283,15 @@ public class HttpSecurityBeanDefinitionParserTests { assertTrue(filters.get(1) instanceof SecurityContextHolderAwareRequestFilter); assertTrue(filters.get(5) instanceof SecurityContextHolderAwareRequestFilter); } + + @Test(expected=BeanCreationException.class) + public void twoFiltersWithSameOrderAreRejected() { + setContext( + "" + AUTH_PROVIDER_XML + + "" + + " " + + ""); + } @Test public void rememberMeServiceWorksWithTokenRepoRef() { @@ -414,6 +424,50 @@ public class HttpSecurityBeanDefinitionParserTests { assertEquals("Hello from the post processor!", service.getPostProcessorWasHere()); } + /** + * SEC-795. Two methods that exercise the scenarios that will or won't result in a protected login page warning. + * Check the log. + */ + @Test + public void unprotectedLoginPageDoesntResultInWarning() { + // Anonymous access configured + setContext( + " " + + " " + + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + closeAppContext(); + // No filters applied to login page + setContext( + " " + + " " + + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + } + + @Test + public void protectedLoginPageResultsInWarning() { + // Protected, no anonymous filter configured. + setContext( + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + closeAppContext(); + // Protected, anonymous provider but no access + setContext( + " " + + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + } + + private void setContext(String context) { appContext = new InMemoryXmlApplicationContext(context); }