YARN-2528. Relaxed http response split vulnerability protection for the origins header and made it accept multiple origins in CrossOriginFilter. Contributed by Jonathan Eagles.

This commit is contained in:
Zhijie Shen 2014-09-12 21:33:01 -07:00
parent 3c9181722b
commit 98588cf044
3 changed files with 61 additions and 31 deletions

View File

@ -351,6 +351,10 @@ Release 2.6.0 - UNRELEASED
YARN-2542. Fixed NPE when retrieving ApplicationReport from TimeLineServer. YARN-2542. Fixed NPE when retrieving ApplicationReport from TimeLineServer.
(Zhijie Shen via jianhe) (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 Release 2.5.1 - 2014-09-05
INCOMPATIBLE CHANGES INCOMPATIBLE CHANGES

View File

@ -19,8 +19,6 @@
package org.apache.hadoop.yarn.server.timeline.webapp; package org.apache.hadoop.yarn.server.timeline.webapp;
import java.io.IOException; import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
@ -106,12 +104,12 @@ public void destroy() {
private void doCrossFilter(HttpServletRequest req, HttpServletResponse res) { private void doCrossFilter(HttpServletRequest req, HttpServletResponse res) {
String origin = encodeHeader(req.getHeader(ORIGIN)); String originsList = encodeHeader(req.getHeader(ORIGIN));
if (!isCrossOrigin(origin)) { if (!isCrossOrigin(originsList)) {
return; return;
} }
if (!isOriginAllowed(origin)) { if (!areOriginsAllowed(originsList)) {
return; return;
} }
@ -127,7 +125,7 @@ private void doCrossFilter(HttpServletRequest req, HttpServletResponse res) {
return; 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_CREDENTIALS, Boolean.TRUE.toString());
res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, getAllowedMethodsHeader()); res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, getAllowedMethodsHeader());
res.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, getAllowedHeadersHeader()); res.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, getAllowedHeadersHeader());
@ -191,35 +189,36 @@ static String encodeHeader(final String header) {
if (header == null) { if (header == null) {
return null; return null;
} }
try { // Protect against HTTP response splitting vulnerability
// Protect against HTTP response splitting vulnerability // since value is written as part of the response header
// since value is written as part of the response header // Ensure this header only has one header by removing
return URLEncoder.encode(header, "ASCII"); // CRs and LFs
} catch (UnsupportedEncodingException e) { return header.split("\n|\r")[0].trim();
return null;
}
} }
static boolean isCrossOrigin(String origin) { static boolean isCrossOrigin(String originsList) {
return origin != null; return originsList != null;
} }
@VisibleForTesting @VisibleForTesting
boolean isOriginAllowed(String origin) { boolean areOriginsAllowed(String originsList) {
if (allowAllOrigins) { if (allowAllOrigins) {
return true; return true;
} }
for (String allowedOrigin : allowedOrigins) { String[] origins = originsList.trim().split("\\s+");
if (allowedOrigin.contains("*")) { for (String origin : origins) {
String regex = allowedOrigin.replace(".", "\\.").replace("*", ".*"); for (String allowedOrigin : allowedOrigins) {
Pattern p = Pattern.compile(regex); if (allowedOrigin.contains("*")) {
Matcher m = p.matcher(origin); String regex = allowedOrigin.replace(".", "\\.").replace("*", ".*");
if (m.matches()) { Pattern p = Pattern.compile(regex);
Matcher m = p.matcher(origin);
if (m.matches()) {
return true;
}
} else if (allowedOrigin.equals(origin)) {
return true; return true;
} }
} else if (allowedOrigin.equals(origin)) {
return true;
} }
} }
return false; return false;

View File

@ -77,7 +77,27 @@ public void testAllowAllOrigins() throws ServletException, IOException {
// Object under test // Object under test
CrossOriginFilter filter = new CrossOriginFilter(); CrossOriginFilter filter = new CrossOriginFilter();
filter.init(filterConfig); 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 @Test
@ -93,10 +113,17 @@ public void testPatternMatchingOrigins() throws ServletException, IOException {
filter.init(filterConfig); filter.init(filterConfig);
// match multiple sub-domains // match multiple sub-domains
Assert.assertFalse(filter.isOriginAllowed("example.com")); Assert.assertFalse(filter.areOriginsAllowed("example.com"));
Assert.assertFalse(filter.isOriginAllowed("foo:example.com")); Assert.assertFalse(filter.areOriginsAllowed("foo:example.com"));
Assert.assertTrue(filter.isOriginAllowed("foo.example.com")); Assert.assertTrue(filter.areOriginsAllowed("foo.example.com"));
Assert.assertTrue(filter.isOriginAllowed("foo.bar.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 @Test
@ -238,7 +265,7 @@ public void testCrossOriginFilterAfterRestart() throws ServletException {
Assert.assertTrue("Allowed methods do not match", Assert.assertTrue("Allowed methods do not match",
filter.getAllowedMethodsHeader() filter.getAllowedMethodsHeader()
.compareTo("GET,POST") == 0); .compareTo("GET,POST") == 0);
Assert.assertTrue(filter.isOriginAllowed("example.com")); Assert.assertTrue(filter.areOriginsAllowed("example.com"));
//destroy filter values and clear conf //destroy filter values and clear conf
filter.destroy(); filter.destroy();
@ -260,7 +287,7 @@ public void testCrossOriginFilterAfterRestart() throws ServletException {
Assert.assertTrue("Allowed methods do not match", Assert.assertTrue("Allowed methods do not match",
filter.getAllowedMethodsHeader() filter.getAllowedMethodsHeader()
.compareTo("GET,HEAD") == 0); .compareTo("GET,HEAD") == 0);
Assert.assertTrue(filter.isOriginAllowed("newexample.com")); Assert.assertTrue(filter.areOriginsAllowed("newexample.com"));
//destroy filter values //destroy filter values
filter.destroy(); filter.destroy();