diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 9c264ec8040..56c8b45daf8 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -323,6 +323,10 @@ Release 2.6.0 - UNRELEASED YARN-2542. Fixed NPE when retrieving ApplicationReport from TimeLineServer. (Zhijie Shen via jianhe) + YARN-2528. Relaxed http response split vulnerability protection for the origins + header and made it accept multiple origins in CrossOriginFilter. (Jonathan + Eagles via zjshen) + Release 2.5.1 - 2014-09-05 INCOMPATIBLE CHANGES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java b/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 cceee5422db..d5fab7ac64c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/webapp/CrossOriginFilter.java +++ b/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 class CrossOriginFilter implements Filter { 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 @@ public class CrossOriginFilter implements Filter { 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 @@ public class CrossOriginFilter implements Filter { 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 a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java b/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 e0990f97c2b..5e093418d75 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestCrossOriginFilter.java +++ b/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 class TestCrossOriginFilter { // 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 class TestCrossOriginFilter { 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 class TestCrossOriginFilter { 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 class TestCrossOriginFilter { 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();