From 31279ae45eeb74d0955014ceffacb5c40d2f3ee5 Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Wed, 11 May 2016 10:58:32 -0700 Subject: [PATCH] HADOOP-13008. Add XFS Filter for UIs to Hadoop Common. Contributed by Larry McCay. (cherry picked from commit dee279b532e7286362518b531c9daea9ae8606f4) --- .../org/apache/hadoop/conf/Configuration.java | 21 +++ .../http/RestCsrfPreventionFilter.java | 12 +- .../security/http/XFrameOptionsFilter.java | 167 ++++++++++++++++++ .../hadoop/security/http/package-info.java | 22 +++ .../http/TestRestCsrfPreventionFilter.java | 40 +++-- .../http/TestXFrameOptionsFilter.java | 151 ++++++++++++++++ 6 files changed, 384 insertions(+), 29 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index eaaf6e82712..f016119554e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -2478,6 +2478,27 @@ public class Configuration implements Iterable>, return result.entrySet().iterator(); } + /** + * Constructs a mapping of configuration and includes all properties that + * start with the specified configuration prefix. Property names in the + * mapping are trimmed to remove the configuration prefix. + * + * @param confPrefix configuration prefix + * @return mapping of configuration properties with prefix stripped + */ + public Map getPropsWithPrefix(String confPrefix) { + Map configMap = new HashMap<>(); + for (Map.Entry entry : this) { + String name = entry.getKey(); + if (name.startsWith(confPrefix)) { + String value = this.get(name); + name = name.substring(confPrefix.length()); + configMap.put(name, value); + } + } + return configMap; + } + private Document parse(DocumentBuilder builder, URL url) throws IOException, SAXException { if (!quietmode) { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java index 33579b42d80..59cb0d65995 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/RestCsrfPreventionFilter.java @@ -18,7 +18,6 @@ package org.apache.hadoop.security.http; import java.io.IOException; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.regex.Matcher; @@ -228,16 +227,7 @@ public class RestCsrfPreventionFilter implements Filter { */ public static Map getFilterParams(Configuration conf, String confPrefix) { - Map filterConfigMap = new HashMap<>(); - for (Map.Entry entry : conf) { - String name = entry.getKey(); - if (name.startsWith(confPrefix)) { - String value = conf.get(name); - name = name.substring(confPrefix.length()); - filterConfigMap.put(name, value); - } - } - return filterConfigMap; + return conf.getPropsWithPrefix(confPrefix); } /** diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java new file mode 100644 index 00000000000..99f2ca843e0 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/XFrameOptionsFilter.java @@ -0,0 +1,167 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.security.http; + +import java.io.IOException; +import java.util.Map; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.conf.Configuration; + +/** + * This filter protects webapps from clickjacking attacks that + * are possible through use of Frames to embed the resources in another + * application and intercept clicks to accomplish nefarious things. + */ +@InterfaceAudience.Public +@InterfaceStability.Evolving +public class XFrameOptionsFilter implements Filter { + public static final String X_FRAME_OPTIONS = "X-Frame-Options"; + public static final String CUSTOM_HEADER_PARAM = "xframe-options"; + + private String option = "DENY"; + + @Override + public void destroy() { + } + + @Override + public void doFilter(ServletRequest req, ServletResponse res, + FilterChain chain) throws IOException, ServletException { + ((HttpServletResponse) res).setHeader(X_FRAME_OPTIONS, option); + chain.doFilter(req, + new XFrameOptionsResponseWrapper((HttpServletResponse) res)); + } + + @Override + public void init(FilterConfig config) throws ServletException { + String customOption = config.getInitParameter(CUSTOM_HEADER_PARAM); + if (customOption != null) { + option = customOption; + } + } + + /** + * Constructs a mapping of configuration properties to be used for filter + * initialization. The mapping includes all properties that start with the + * specified configuration prefix. Property names in the mapping are trimmed + * to remove the configuration prefix. + * + * @param conf configuration to read + * @param confPrefix configuration prefix + * @return mapping of configuration properties to be used for filter + * initialization + */ + public static Map getFilterParams(Configuration conf, + String confPrefix) { + return conf.getPropsWithPrefix(confPrefix); + } + + /** + * This wrapper allows the rest of the filter pipeline to + * see the configured value when interrogating the response. + * It also blocks other filters from setting the value to + * anything other than what is configured. + * + */ + public class XFrameOptionsResponseWrapper + extends HttpServletResponseWrapper { + /** + * Ctor to take wrap the provided response. + * @param response the response to wrap + */ + public XFrameOptionsResponseWrapper(HttpServletResponse response) { + super(response); + } + + @Override + public void addHeader(String name, String value) { + // don't allow additional values to be added along + // with the configured options value + if (!name.equals(X_FRAME_OPTIONS)) { + super.addHeader(name, value); + } + } + + @Override + public void setHeader(String name, String value) { + // don't allow overwriting of configured value + if (!name.equals(X_FRAME_OPTIONS)) { + super.setHeader(name, value); + } + } + + @Override + public void setDateHeader(String name, long date) { + // don't allow overwriting of configured value + if (!name.equals(X_FRAME_OPTIONS)) { + super.setDateHeader(name, date); + } + } + + @Override + public void addDateHeader(String name, long date) { + // don't allow additional values to be added along + // with the configured options value + if (!name.equals(X_FRAME_OPTIONS)) { + super.addDateHeader(name, date); + } + } + + @Override + public void setIntHeader(String name, int value) { + // don't allow overwriting of configured value + if (!name.equals(X_FRAME_OPTIONS)) { + super.setIntHeader(name, value); + } + } + + @Override + // don't allow additional values to be added along + // with the configured options value + public void addIntHeader(String name, int value) { + if (!name.equals(X_FRAME_OPTIONS)) { + super.addIntHeader(name, value); + } + } + + @Override + public boolean containsHeader(String name) { + boolean contains = false; + // allow the filterchain and subsequent + // filters to see that the header is set + if (name.equals(X_FRAME_OPTIONS)) { + return (option != null); + } else { + super.containsHeader(name); + } + return contains; + } + } +} + diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java new file mode 100644 index 00000000000..8e9398eb679 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/http/package-info.java @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +@InterfaceAudience.Public +@InterfaceStability.Evolving +package org.apache.hadoop.security.http; +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; \ No newline at end of file diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java index 29dccd3a200..6052ef059a7 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestRestCsrfPreventionFilter.java @@ -32,6 +32,10 @@ import javax.servlet.http.HttpServletResponse; import org.junit.Test; import org.mockito.Mockito; +/** + * This class tests the behavior of the RestCsrfPreventionFilter. + * + */ public class TestRestCsrfPreventionFilter { private static final String NON_BROWSER = "java"; @@ -43,7 +47,7 @@ public class TestRestCsrfPreventionFilter { private static final String X_CUSTOM_HEADER = "X-CUSTOM_HEADER"; @Test - public void testNoHeaderDefaultConfig_badRequest() + public void testNoHeaderDefaultConfigBadRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -56,7 +60,7 @@ public class TestRestCsrfPreventionFilter { // CSRF has not been sent HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). - thenReturn(null); + thenReturn(null); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). thenReturn(BROWSER_AGENT); @@ -75,7 +79,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testNoHeaderCustomAgentConfig_badRequest() + public void testNoHeaderCustomAgentConfigBadRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -91,7 +95,7 @@ public class TestRestCsrfPreventionFilter { // CSRF has not been sent HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). - thenReturn(null); + thenReturn(null); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). thenReturn("curl"); @@ -110,7 +114,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testNoHeaderDefaultConfigNonBrowser_goodRequest() + public void testNoHeaderDefaultConfigNonBrowserGoodRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -123,7 +127,7 @@ public class TestRestCsrfPreventionFilter { // CSRF has not been sent HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). - thenReturn(null); + thenReturn(null); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). thenReturn(NON_BROWSER); @@ -140,7 +144,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testHeaderPresentDefaultConfig_goodRequest() + public void testHeaderPresentDefaultConfigGoodRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -168,7 +172,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testHeaderPresentCustomHeaderConfig_goodRequest() + public void testHeaderPresentCustomHeaderConfigGoodRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -197,7 +201,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testMissingHeaderWithCustomHeaderConfig_badRequest() + public void testMissingHeaderWithCustomHeaderConfigBadRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -209,7 +213,7 @@ public class TestRestCsrfPreventionFilter { thenReturn(null); HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). - thenReturn(BROWSER_AGENT); + thenReturn(BROWSER_AGENT); // CSRF has not been sent Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). @@ -228,7 +232,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testMissingHeaderNoMethodsToIgnoreConfig_badRequest() + public void testMissingHeaderNoMethodsToIgnoreConfigBadRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -239,7 +243,7 @@ public class TestRestCsrfPreventionFilter { thenReturn(""); HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). - thenReturn(BROWSER_AGENT); + thenReturn(BROWSER_AGENT); // CSRF has not been sent Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). @@ -260,7 +264,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testMissingHeaderIgnoreGETMethodConfig_goodRequest() + public void testMissingHeaderIgnoreGETMethodConfigGoodRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -271,7 +275,7 @@ public class TestRestCsrfPreventionFilter { thenReturn("GET"); HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). - thenReturn(BROWSER_AGENT); + thenReturn(BROWSER_AGENT); // CSRF has not been sent Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). @@ -292,7 +296,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testMissingHeaderMultipleIgnoreMethodsConfig_goodRequest() + public void testMissingHeaderMultipleIgnoreMethodsConfigGoodRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -303,7 +307,7 @@ public class TestRestCsrfPreventionFilter { thenReturn("GET,OPTIONS"); HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). - thenReturn(BROWSER_AGENT); + thenReturn(BROWSER_AGENT); // CSRF has not been sent Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). @@ -324,7 +328,7 @@ public class TestRestCsrfPreventionFilter { } @Test - public void testMissingHeaderMultipleIgnoreMethodsConfig_badRequest() + public void testMissingHeaderMultipleIgnoreMethodsConfigBadRequest() throws ServletException, IOException { // Setup the configuration settings of the server FilterConfig filterConfig = Mockito.mock(FilterConfig.class); @@ -335,7 +339,7 @@ public class TestRestCsrfPreventionFilter { thenReturn("GET,OPTIONS"); HttpServletRequest mockReq = Mockito.mock(HttpServletRequest.class); Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_USER_AGENT)). - thenReturn(BROWSER_AGENT); + thenReturn(BROWSER_AGENT); // CSRF has not been sent Mockito.when(mockReq.getHeader(RestCsrfPreventionFilter.HEADER_DEFAULT)). diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java new file mode 100644 index 00000000000..d9e36c42e44 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/http/TestXFrameOptionsFilter.java @@ -0,0 +1,151 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.security.http; + +import java.util.Collection; +import java.util.ArrayList; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +/** + * Test the default and customized behaviors of XFrameOptionsFilter. + * + */ +public class TestXFrameOptionsFilter { + private static final String X_FRAME_OPTIONS = "X-Frame-Options"; + + @Test + public void testDefaultOptionsValue() throws Exception { + final Collection headers = new ArrayList(); + FilterConfig filterConfig = Mockito.mock(FilterConfig.class); + Mockito.when(filterConfig.getInitParameter( + XFrameOptionsFilter.CUSTOM_HEADER_PARAM)).thenReturn(null); + + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + HttpServletResponse response = Mockito.mock(HttpServletResponse.class); + FilterChain chain = Mockito.mock(FilterChain.class); + + Mockito.doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Assert.assertTrue( + "header should be visible inside chain and filters.", + ((HttpServletResponse)args[1]). + containsHeader(X_FRAME_OPTIONS)); + return null; + } + } + ).when(chain).doFilter(Mockito.anyObject(), + Mockito.anyObject()); + + Mockito.doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Assert.assertTrue( + "Options value incorrect should be DENY but is: " + + args[1], "DENY".equals(args[1])); + headers.add((String)args[1]); + return null; + } + } + ).when(response).setHeader(Mockito.anyObject(), + Mockito.anyObject()); + + XFrameOptionsFilter filter = new XFrameOptionsFilter(); + filter.init(filterConfig); + + filter.doFilter(request, response, chain); + + Assert.assertEquals("X-Frame-Options count not equal to 1.", + headers.size(), 1); + } + + @Test + public void testCustomOptionsValueAndNoOverrides() throws Exception { + final Collection headers = new ArrayList(); + FilterConfig filterConfig = Mockito.mock(FilterConfig.class); + Mockito.when(filterConfig.getInitParameter( + XFrameOptionsFilter.CUSTOM_HEADER_PARAM)).thenReturn("SAMEORIGIN"); + + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + final HttpServletResponse response = + Mockito.mock(HttpServletResponse.class); + FilterChain chain = Mockito.mock(FilterChain.class); + + Mockito.doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + HttpServletResponse resp = (HttpServletResponse) args[1]; + Assert.assertTrue( + "Header should be visible inside chain and filters.", + resp.containsHeader(X_FRAME_OPTIONS)); + // let's try and set another value for the header and make + // sure that it doesn't overwrite the configured value + Assert.assertTrue(resp instanceof + XFrameOptionsFilter.XFrameOptionsResponseWrapper); + resp.setHeader(X_FRAME_OPTIONS, "LJM"); + return null; + } + } + ).when(chain).doFilter(Mockito.anyObject(), + Mockito.anyObject()); + + Mockito.doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Assert.assertEquals( + "Options value incorrect should be SAMEORIGIN but is: " + + args[1], "SAMEORIGIN", args[1]); + headers.add((String)args[1]); + return null; + } + } + ).when(response).setHeader(Mockito.anyObject(), + Mockito.anyObject()); + + XFrameOptionsFilter filter = new XFrameOptionsFilter(); + filter.init(filterConfig); + + filter.doFilter(request, response, chain); + + Assert.assertEquals("X-Frame-Options count not equal to 1.", + headers.size(), 1); + + Assert.assertEquals("X-Frame-Options count not equal to 1.", + headers.toArray()[0], "SAMEORIGIN"); + } +}