Repository: ambari Updated Branches: refs/heads/trunk 89ef3b5c7 -> 05c8eb5f6
AMBARI-14292. Security filters set the X-Frame-Options header value properly for view resources api calls (Laszlo Puskas via rlevas) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/05c8eb5f Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/05c8eb5f Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/05c8eb5f Branch: refs/heads/trunk Commit: 05c8eb5f61ec70cd15e0772290a2e44a6502a8c0 Parents: 89ef3b5 Author: Laszlo Puskas <lpus...@hortonworks.com> Authored: Wed Dec 9 14:22:59 2015 -0500 Committer: Robert Levas <rle...@hortonworks.com> Committed: Wed Dec 9 14:23:09 2015 -0500 ---------------------------------------------------------------------- .../ambari/server/controller/AmbariServer.java | 6 ++ .../security/AbstractSecurityHeaderFilter.java | 68 +++++++++++++------- .../AmbariServerSecurityHeaderFilter.java | 17 ++++- .../AmbariViewsSecurityHeaderFilter.java | 12 +++- .../AbstractSecurityHeaderFilterTest.java | 33 ++++++---- .../AmbariServerSecurityHeaderFilterTest.java | 13 +++- .../AmbariViewsSecurityHeaderFilterTest.java | 15 ++++- 7 files changed, 124 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java index 2909c31..c6f90c0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java @@ -85,6 +85,7 @@ import org.apache.ambari.server.resources.ResourceManager; import org.apache.ambari.server.resources.api.rest.GetResource; import org.apache.ambari.server.scheduler.ExecutionScheduleManager; import org.apache.ambari.server.security.AmbariServerSecurityHeaderFilter; +import org.apache.ambari.server.security.AmbariViewsSecurityHeaderFilter; import org.apache.ambari.server.security.CertificateManager; import org.apache.ambari.server.security.SecurityFilter; import org.apache.ambari.server.security.authorization.AmbariAuthorizationFilter; @@ -328,6 +329,11 @@ public class AmbariServer { // Conditionally adds security-related headers to all HTTP responses. root.addFilter(new FilterHolder(injector.getInstance(AmbariServerSecurityHeaderFilter.class)), "/*", DISPATCHER_TYPES); + // The security header filter - conditionally adds security-related headers to the HTTP response for Ambari Views + // requests. + root.addFilter(new FilterHolder(injector.getInstance(AmbariViewsSecurityHeaderFilter.class)), "/api/v1/views/*", + DISPATCHER_TYPES); + // session-per-request strategy for api root.addFilter(new FilterHolder(injector.getInstance(AmbariPersistFilter.class)), "/api/*", DISPATCHER_TYPES); root.addFilter(new FilterHolder(new MethodOverrideFilter()), "/api/*", DISPATCHER_TYPES); http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/main/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilter.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilter.java b/ambari-server/src/main/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilter.java index 36f691c..10aa6ea 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilter.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilter.java @@ -18,11 +18,7 @@ package org.apache.ambari.server.security; -import com.google.inject.Inject; -import org.apache.ambari.server.configuration.Configuration; -import org.apache.commons.lang.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.io.IOException; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -31,7 +27,13 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; + +import org.apache.ambari.server.configuration.Configuration; +import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Inject; /** * AbstractSecurityHeaderFilter is an abstract class used to help add security-related headers to @@ -56,6 +58,11 @@ public abstract class AbstractSecurityHeaderFilter implements Filter { * The logger. */ private final static Logger LOG = LoggerFactory.getLogger(AbstractSecurityHeaderFilter.class); + + /** + * Signals whether subsequent filters are allowed to override security headers + */ + protected final static String DENY_HEADER_OVERRIDES_FLAG = "deny.header.overrides.flag"; /** * The Configuration object used to determine how Ambari is configured */ @@ -95,28 +102,21 @@ public abstract class AbstractSecurityHeaderFilter implements Filter { @Override public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { - if (servletResponse instanceof HttpServletResponse) { - HttpServletResponse httpServletResponse = (HttpServletResponse) servletResponse; - // Conditionally set the Strict-Transport-Security HTTP response header if SSL is enabled and - // a value is supplied - if (sslEnabled && !StringUtils.isEmpty(strictTransportSecurity)) { - httpServletResponse.setHeader(STRICT_TRANSPORT_HEADER, strictTransportSecurity); - } - - // Conditionally set the X-Frame-Options HTTP response header if a value is supplied - if (!StringUtils.isEmpty(xFrameOptionsHeader)) { - httpServletResponse.setHeader(X_FRAME_OPTIONS_HEADER, xFrameOptionsHeader); - } - - // Conditionally set the X-XSS-Protection HTTP response header if a value is supplied - if (!StringUtils.isEmpty(xXSSProtectionHeader)) { - httpServletResponse.setHeader(X_XSS_PROTECTION_HEADER, xXSSProtectionHeader); - } + if (checkPrerequisites(servletRequest)) { + doFilterInternal(servletRequest, servletResponse); } filterChain.doFilter(servletRequest, servletResponse); } + /** + * Checks whether the security headers need to be set, if so signals it in a request parameter. + * + * @param servletRequest the incoming request + * @return true if headers need to be set, false otherwise + */ + protected abstract boolean checkPrerequisites(ServletRequest servletRequest); + @Override public void destroy() { LOG.debug("Destroying {}", this.getClass().getName()); @@ -140,4 +140,26 @@ public abstract class AbstractSecurityHeaderFilter implements Filter { protected void setxXSSProtectionHeader(String xXSSProtectionHeader) { this.xXSSProtectionHeader = xXSSProtectionHeader; } + + private void doFilterInternal(ServletRequest servletRequest, ServletResponse servletResponse) { + if (servletResponse instanceof HttpServletResponse) { + HttpServletResponse httpServletResponse = (HttpServletResponse) servletResponse; + // Conditionally set the Strict-Transport-Security HTTP response header if SSL is enabled and + // a value is supplied + if (sslEnabled && !StringUtils.isEmpty(strictTransportSecurity)) { + httpServletResponse.setHeader(STRICT_TRANSPORT_HEADER, strictTransportSecurity); + } + + if (!StringUtils.isEmpty(xFrameOptionsHeader)) { + // perform filter specific logic related to the X-Frame-Options HTTP response header + httpServletResponse.setHeader(X_FRAME_OPTIONS_HEADER, xFrameOptionsHeader); + } + + // Conditionally set the X-XSS-Protection HTTP response header if a value is supplied + if (!StringUtils.isEmpty(xXSSProtectionHeader)) { + httpServletResponse.setHeader(X_XSS_PROTECTION_HEADER, xXSSProtectionHeader); + } + } + } + } http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilter.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilter.java b/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilter.java index 86c28ea..b40953b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilter.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilter.java @@ -18,9 +18,12 @@ package org.apache.ambari.server.security; -import com.google.inject.Singleton; +import javax.servlet.ServletRequest; + import org.apache.ambari.server.configuration.Configuration; +import com.google.inject.Singleton; + /** * AmbariServerSecurityHeaderFilter adds security-related headers to HTTP response messages for Ambari Server UI */ @@ -28,10 +31,22 @@ import org.apache.ambari.server.configuration.Configuration; public class AmbariServerSecurityHeaderFilter extends AbstractSecurityHeaderFilter { @Override + protected boolean checkPrerequisites(ServletRequest servletRequest) { + boolean retVal = false; + if (null == servletRequest.getAttribute(AbstractSecurityHeaderFilter.DENY_HEADER_OVERRIDES_FLAG)) { + retVal = true; + } else { + servletRequest.removeAttribute(AbstractSecurityHeaderFilter.DENY_HEADER_OVERRIDES_FLAG); + } + return retVal; + } + + @Override protected void processConfig(Configuration configuration) { setSslEnabled(configuration.getApiSSLAuthentication()); setStrictTransportSecurity(configuration.getStrictTransportSecurityHTTPResponseHeader()); setxFrameOptionsHeader(configuration.getXFrameOptionsHTTPResponseHeader()); setxXSSProtectionHeader(configuration.getXXSSProtectionHTTPResponseHeader()); } + } http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilter.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilter.java b/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilter.java index 63137fb..5bff4e3 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilter.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilter.java @@ -18,15 +18,25 @@ package org.apache.ambari.server.security; -import com.google.inject.Singleton; +import javax.servlet.ServletRequest; + import org.apache.ambari.server.configuration.Configuration; +import com.google.inject.Singleton; + /** * AmbariViewsSecurityHeaderFilter adds security-related headers to HTTP response messages for Ambari Views */ @Singleton public class AmbariViewsSecurityHeaderFilter extends AbstractSecurityHeaderFilter { + + @Override + protected boolean checkPrerequisites(ServletRequest servletRequest) { + servletRequest.setAttribute(AbstractSecurityHeaderFilter.DENY_HEADER_OVERRIDES_FLAG, "true"); + return true; + } + @Override protected void processConfig(Configuration configuration) { setSslEnabled(configuration.getApiSSLAuthentication()); http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/test/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilterTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilterTest.java index 3481092..48231b7 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilterTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/AbstractSecurityHeaderFilterTest.java @@ -18,10 +18,15 @@ package org.apache.ambari.server.security; -import com.google.inject.AbstractModule; -import com.google.inject.Guice; -import com.google.inject.Injector; -import junit.framework.Assert; +import java.io.File; +import java.util.Map; +import java.util.Properties; + +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.state.stack.OsFamily; import org.easymock.EasyMockSupport; @@ -31,13 +36,11 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.File; -import java.util.Map; -import java.util.Properties; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; + +import junit.framework.Assert; import static org.easymock.EasyMock.expectLastCall; @@ -56,6 +59,8 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { this.defatulPropertyValueMap = defatulPropertyValueMap; } + protected abstract void expectHttpServletRequestMock(HttpServletRequest request); + @Before public void setUp() throws Exception { temporaryFolder.create(); @@ -83,6 +88,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); servletResponse.setHeader(AbstractSecurityHeaderFilter.X_FRAME_OPTIONS_HEADER, defatulPropertyValueMap.get(AbstractSecurityHeaderFilter.X_FRAME_OPTIONS_HEADER)); @@ -126,6 +132,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); servletResponse.setHeader(AbstractSecurityHeaderFilter.STRICT_TRANSPORT_HEADER, defatulPropertyValueMap.get(AbstractSecurityHeaderFilter.STRICT_TRANSPORT_HEADER)); @@ -173,6 +180,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); servletResponse.setHeader(AbstractSecurityHeaderFilter.X_FRAME_OPTIONS_HEADER, "custom2"); @@ -219,6 +227,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); servletResponse.setHeader(AbstractSecurityHeaderFilter.STRICT_TRANSPORT_HEADER, "custom1"); @@ -266,6 +275,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); @@ -308,6 +318,7 @@ public abstract class AbstractSecurityHeaderFilterTest extends EasyMockSupport { FilterConfig filterConfig = createNiceMock(FilterConfig.class); HttpServletRequest servletRequest = createStrictMock(HttpServletRequest.class); + expectHttpServletRequestMock(servletRequest); HttpServletResponse servletResponse = createStrictMock(HttpServletResponse.class); http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilterTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilterTest.java index 0e3313e..e6a8eff 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilterTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariServerSecurityHeaderFilterTest.java @@ -18,12 +18,16 @@ package org.apache.ambari.server.security; -import org.apache.ambari.server.configuration.Configuration; - import java.util.Collections; import java.util.HashMap; import java.util.Map; +import javax.servlet.http.HttpServletRequest; + +import org.apache.ambari.server.configuration.Configuration; + +import static org.easymock.EasyMock.expect; + public class AmbariServerSecurityHeaderFilterTest extends AbstractSecurityHeaderFilterTest { private static final Map<String, String> PROPERTY_NAME_MAP; @@ -48,4 +52,9 @@ public class AmbariServerSecurityHeaderFilterTest extends AbstractSecurityHeader public AmbariServerSecurityHeaderFilterTest() { super(AmbariServerSecurityHeaderFilter.class, PROPERTY_NAME_MAP, DEFAULT_PROPERTY_VALUE_MAP); } + + @Override + protected void expectHttpServletRequestMock(HttpServletRequest request) { + expect(request.getAttribute(AbstractSecurityHeaderFilter.DENY_HEADER_OVERRIDES_FLAG)).andReturn(null); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/05c8eb5f/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilterTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilterTest.java index d760962..a2882ae 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilterTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/AmbariViewsSecurityHeaderFilterTest.java @@ -18,12 +18,16 @@ package org.apache.ambari.server.security; -import org.apache.ambari.server.configuration.Configuration; - import java.util.Collections; import java.util.HashMap; import java.util.Map; +import javax.servlet.http.HttpServletRequest; + +import org.apache.ambari.server.configuration.Configuration; + +import static org.easymock.EasyMock.expectLastCall; + public class AmbariViewsSecurityHeaderFilterTest extends AbstractSecurityHeaderFilterTest { private static final Map<String, String> PROPERTY_NAME_MAP; @@ -49,4 +53,11 @@ public class AmbariViewsSecurityHeaderFilterTest extends AbstractSecurityHeaderF public AmbariViewsSecurityHeaderFilterTest() { super(AmbariViewsSecurityHeaderFilter.class, PROPERTY_NAME_MAP, DEFAULT_PROPERTY_VALUE_MAP); } + + @Override + protected void expectHttpServletRequestMock(HttpServletRequest request) { + request.setAttribute(AbstractSecurityHeaderFilter.DENY_HEADER_OVERRIDES_FLAG, "true"); + expectLastCall(); + + } } \ No newline at end of file