diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java index cceee54..d5fab7a 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java @@ -19,8 +19,6 @@ package org.apache.hadoop.yarn.server.timeline.webapp; import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -106,12 +104,12 @@ public void destroy() { private void doCrossFilter(HttpServletRequest req, HttpServletResponse res) { - String origin = encodeHeader(req.getHeader(ORIGIN)); - if (!isCrossOrigin(origin)) { + String originsList = encodeHeader(req.getHeader(ORIGIN)); + if (!isCrossOrigin(originsList)) { return; } - if (!isOriginAllowed(origin)) { + if (!areOriginsAllowed(originsList)) { return; } @@ -127,7 +125,7 @@ private void doCrossFilter(HttpServletRequest req, HttpServletResponse res) { return; } - res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin); + res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, originsList); res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.TRUE.toString()); res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, getAllowedMethodsHeader()); res.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, getAllowedHeadersHeader()); @@ -191,35 +189,36 @@ static String encodeHeader(final String header) { if (header == null) { return null; } - try { - // Protect against HTTP response splitting vulnerability - // since value is written as part of the response header - return URLEncoder.encode(header, "ASCII"); - } catch (UnsupportedEncodingException e) { - return null; - } + // Protect against HTTP response splitting vulnerability + // since value is written as part of the response header + // Ensure this header only has one header by removing + // CRs and LFs + return header.split("\n|\r")[0].trim(); } - static boolean isCrossOrigin(String origin) { - return origin != null; + static boolean isCrossOrigin(String originsList) { + return originsList != null; } @VisibleForTesting - boolean isOriginAllowed(String origin) { + boolean areOriginsAllowed(String originsList) { if (allowAllOrigins) { return true; } - for (String allowedOrigin : allowedOrigins) { - if (allowedOrigin.contains("*")) { - String regex = allowedOrigin.replace(".", "\\.").replace("*", ".*"); - Pattern p = Pattern.compile(regex); - Matcher m = p.matcher(origin); - if (m.matches()) { + String[] origins = originsList.trim().split("\\s+"); + for (String origin : origins) { + for (String allowedOrigin : allowedOrigins) { + if (allowedOrigin.contains("*")) { + String regex = allowedOrigin.replace(".", "\\.").replace("*", ".*"); + Pattern p = Pattern.compile(regex); + Matcher m = p.matcher(origin); + if (m.matches()) { + return true; + } + } else if (allowedOrigin.equals(origin)) { return true; } - } else if (allowedOrigin.equals(origin)) { - return true; } } return false; diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java index e0990f9..5e09341 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java @@ -77,7 +77,27 @@ public void testAllowAllOrigins() throws ServletException, IOException { // Object under test CrossOriginFilter filter = new CrossOriginFilter(); filter.init(filterConfig); - Assert.assertTrue(filter.isOriginAllowed("example.com")); + Assert.assertTrue(filter.areOriginsAllowed("example.com")); + } + + @Test + public void testEncodeHeaders() { + String validOrigin = "http://localhost:12345"; + String encodedValidOrigin = CrossOriginFilter.encodeHeader(validOrigin); + Assert.assertEquals("Valid origin encoding should match exactly", + validOrigin, encodedValidOrigin); + + String httpResponseSplitOrigin = validOrigin + " \nSecondHeader: value"; + String encodedResponseSplitOrigin = + CrossOriginFilter.encodeHeader(httpResponseSplitOrigin); + Assert.assertEquals("Http response split origin should be protected against", + validOrigin, encodedResponseSplitOrigin); + + // Test Origin List + String validOriginList = "http://foo.example.com:12345 http://bar.example.com:12345"; + String encodedValidOriginList = CrossOriginFilter.encodeHeader(validOriginList); + Assert.assertEquals("Valid origin list encoding should match exactly", + validOriginList, encodedValidOriginList); } @Test @@ -93,10 +113,17 @@ public void testPatternMatchingOrigins() throws ServletException, IOException { filter.init(filterConfig); // match multiple sub-domains - Assert.assertFalse(filter.isOriginAllowed("example.com")); - Assert.assertFalse(filter.isOriginAllowed("foo:example.com")); - Assert.assertTrue(filter.isOriginAllowed("foo.example.com")); - Assert.assertTrue(filter.isOriginAllowed("foo.bar.example.com")); + Assert.assertFalse(filter.areOriginsAllowed("example.com")); + Assert.assertFalse(filter.areOriginsAllowed("foo:example.com")); + Assert.assertTrue(filter.areOriginsAllowed("foo.example.com")); + Assert.assertTrue(filter.areOriginsAllowed("foo.bar.example.com")); + + // First origin is allowed + Assert.assertTrue(filter.areOriginsAllowed("foo.example.com foo.nomatch.com")); + // Second origin is allowed + Assert.assertTrue(filter.areOriginsAllowed("foo.nomatch.com foo.example.com")); + // No origin in list is allowed + Assert.assertFalse(filter.areOriginsAllowed("foo.nomatch1.com foo.nomatch2.com")); } @Test @@ -238,7 +265,7 @@ public void testCrossOriginFilterAfterRestart() throws ServletException { Assert.assertTrue("Allowed methods do not match", filter.getAllowedMethodsHeader() .compareTo("GET,POST") == 0); - Assert.assertTrue(filter.isOriginAllowed("example.com")); + Assert.assertTrue(filter.areOriginsAllowed("example.com")); //destroy filter values and clear conf filter.destroy(); @@ -260,7 +287,7 @@ public void testCrossOriginFilterAfterRestart() throws ServletException { Assert.assertTrue("Allowed methods do not match", filter.getAllowedMethodsHeader() .compareTo("GET,HEAD") == 0); - Assert.assertTrue(filter.isOriginAllowed("newexample.com")); + Assert.assertTrue(filter.areOriginsAllowed("newexample.com")); //destroy filter values filter.destroy();