From bc8ff9590c319ef74da1faa504e250a45ed3f455 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 30 Jul 2013 11:08:03 -0500 Subject: [PATCH] SEC-2230: Defaults when using only Previously an error occurred when no child elements were specified with . Now all the explicitly supported header elements are added with their default settings. --- .../http/HeadersBeanDefinitionParser.java | 91 ++++++++++++------- .../config/http/HttpHeadersConfigTests.groovy | 14 ++- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java index 6e068024e4..c24a0b948c 100644 --- a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java @@ -20,6 +20,7 @@ import java.net.URISyntaxException; import java.util.List; import java.util.regex.PatternSyntaxException; +import org.springframework.beans.BeanMetadataElement; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -76,10 +77,10 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private static final String ALLOW_FROM = "ALLOW-FROM"; - private ManagedList headerWriters; + private ManagedList headerWriters; public BeanDefinition parse(Element element, ParserContext parserContext) { - headerWriters = new ManagedList(); + headerWriters = new ManagedList(); BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(HeadersFilter.class); parseCacheControlElement(element); @@ -91,8 +92,18 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { parseHeaderElements(element); if(headerWriters.isEmpty()) { - parserContext.getReaderContext().error("At least one type of header must be specified when using ", - element); + addCacheControl(); + addHsts(null); + addContentTypeOptions(); + + BeanDefinitionBuilder frameOptions = BeanDefinitionBuilder.genericBeanDefinition(XFrameOptionsHeaderWriter.class); + frameOptions.addConstructorArgValue("DENY"); + headerWriters.add(frameOptions.getBeanDefinition()); + + BeanDefinitionBuilder xss = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); + xss.addConstructorArgValue(XSS_PROTECTION_HEADER); + xss.addConstructorArgValue("1; mode=block"); + headerWriters.add(xss.getBeanDefinition()); } builder.addConstructorArgValue(headerWriters); return builder.getBeanDefinition(); @@ -101,36 +112,46 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private void parseCacheControlElement(Element element) { Element cacheControlElement = DomUtils.getChildElementByTagName(element, CACHE_CONTROL_ELEMENT); if (cacheControlElement != null) { - ManagedList headers = new ManagedList(); - - BeanDefinitionBuilder pragmaHeader = BeanDefinitionBuilder.genericBeanDefinition(Header.class); - pragmaHeader.addConstructorArgValue("Pragma"); - ManagedList pragmaValues = new ManagedList(); - pragmaValues.add("no-cache"); - pragmaHeader.addConstructorArgValue(pragmaValues); - headers.add(pragmaHeader.getBeanDefinition()); - - BeanDefinitionBuilder cacheControlHeader = BeanDefinitionBuilder.genericBeanDefinition(Header.class); - cacheControlHeader.addConstructorArgValue("Cache-Control"); - ManagedList cacheControlValues = new ManagedList(); - cacheControlValues.add("no-cache"); - cacheControlValues.add("no-store"); - cacheControlValues.add("max-age=0"); - cacheControlValues.add("must-revalidate"); - cacheControlHeader.addConstructorArgValue(cacheControlValues); - headers.add(cacheControlHeader.getBeanDefinition()); - - BeanDefinitionBuilder headersWriter = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); - headersWriter.addConstructorArgValue(headers); - - headerWriters.add(headersWriter.getBeanDefinition()); + addCacheControl(); } } + private void addCacheControl() { + ManagedList headers = new ManagedList(); + + BeanDefinitionBuilder pragmaHeader = BeanDefinitionBuilder.genericBeanDefinition(Header.class); + pragmaHeader.addConstructorArgValue("Pragma"); + ManagedList pragmaValues = new ManagedList(); + pragmaValues.add("no-cache"); + pragmaHeader.addConstructorArgValue(pragmaValues); + headers.add(pragmaHeader.getBeanDefinition()); + + BeanDefinitionBuilder cacheControlHeader = BeanDefinitionBuilder.genericBeanDefinition(Header.class); + cacheControlHeader.addConstructorArgValue("Cache-Control"); + ManagedList cacheControlValues = new ManagedList(); + cacheControlValues.add("no-cache"); + cacheControlValues.add("no-store"); + cacheControlValues.add("max-age=0"); + cacheControlValues.add("must-revalidate"); + cacheControlHeader.addConstructorArgValue(cacheControlValues); + headers.add(cacheControlHeader.getBeanDefinition()); + + BeanDefinitionBuilder headersWriter = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); + headersWriter.addConstructorArgValue(headers); + + headerWriters.add(headersWriter.getBeanDefinition()); + } + private void parseHstsElement(Element element) { Element hstsElement = DomUtils.getChildElementByTagName(element, HSTS_ELEMENT); if (hstsElement != null) { - BeanDefinitionBuilder headersWriter = BeanDefinitionBuilder.genericBeanDefinition(HstsHeaderWriter.class); + addHsts(hstsElement); + } + } + + private void addHsts(Element hstsElement) { + BeanDefinitionBuilder headersWriter = BeanDefinitionBuilder.genericBeanDefinition(HstsHeaderWriter.class); + if(hstsElement != null) { String includeSubDomains = hstsElement.getAttribute(ATT_INCLUDE_SUBDOMAINS); if(StringUtils.hasText(includeSubDomains)) { headersWriter.addPropertyValue("includeSubDomains", includeSubDomains); @@ -143,8 +164,8 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { if(StringUtils.hasText(requestMatcherRef)) { headersWriter.addPropertyReference("requestMatcher", requestMatcherRef); } - headerWriters.add(headersWriter.getBeanDefinition()); } + headerWriters.add(headersWriter.getBeanDefinition()); } private void parseHeaderElements(Element element) { @@ -165,13 +186,17 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private void parseContentTypeOptionsElement(Element element) { Element contentTypeElt = DomUtils.getChildElementByTagName(element, CONTENT_TYPE_ELEMENT); if (contentTypeElt != null) { - BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); - builder.addConstructorArgValue(CONTENT_TYPE_OPTIONS_HEADER); - builder.addConstructorArgValue("nosniff"); - headerWriters.add(builder.getBeanDefinition()); + addContentTypeOptions(); } } + private void addContentTypeOptions() { + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(StaticHeadersWriter.class); + builder.addConstructorArgValue(CONTENT_TYPE_OPTIONS_HEADER); + builder.addConstructorArgValue("nosniff"); + headerWriters.add(builder.getBeanDefinition()); + } + private void parseFrameOptionsElement(Element element, ParserContext parserContext) { BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(XFrameOptionsHeaderWriter.class); diff --git a/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy index 7879c72469..2cf22879ec 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/HttpHeadersConfigTests.groovy @@ -55,14 +55,22 @@ class HttpHeadersConfigTests extends AbstractHttpConfigTests { } def 'http headers with empty headers'() { - when: + setup: httpAutoConfig { 'headers'() } createAppContext() + when: + def hf = getFilter(HeadersFilter) + MockHttpServletResponse response = new MockHttpServletResponse() + hf.doFilter(new MockHttpServletRequest(secure:true), response, new MockFilterChain()) then: - BeanDefinitionParsingException success = thrown() - success.message.contains "At least one type of header must be specified when using " + assertHeaders(response, ['X-Content-Type-Options':'nosniff', + 'X-Frame-Options':'DENY', + 'Strict-Transport-Security': 'max-age=31536000 ; includeSubDomains', + 'Cache-Control': 'no-cache,no-store,max-age=0,must-revalidate', + 'Pragma':'no-cache', + 'X-XSS-Protection' : '1; mode=block']) } def 'http headers content-type-options'() {