Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,20 @@
import org.apache.hadoop.yarn.webapp.hamlet2.Hamlet;
import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
import org.apache.http.Header;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.NameValuePair;
import org.apache.http.ProtocolException;
import org.apache.http.client.HttpClient;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.client.utils.URLEncodedUtils;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.DefaultRedirectStrategy;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.protocol.HttpContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -265,6 +269,12 @@ private void proxyLink(final HttpServletRequest req,
// similar could cause issues otherwise.
InetAddress localAddress = InetAddress.getByName(proxyHost);
LOG.debug("local InetAddress for proxy host: {}", localAddress);
// The tracking URL is supplied by the application and is fetched
// server-side, so a malicious AM can return a redirect pointing at the
// cluster's internal services or the cloud metadata endpoint. Only follow
// redirects that stay on the application's own host.
httpClientBuilder.setRedirectStrategy(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The link is derived from ApplicationReport#getOriginalTrackingUrl(), which is controlled by the AM. SameHostRedirectStrategy is only invoked after the initial response is a redirect, so a directly supplied unsafe tracking URL reaches HttpGet/HttpPut without this check.

In addition, the allowed host is derived from the same untrusted URL, so it does not establish an independent trust boundary. Please validate the initial destination against an independently trusted policy or source before executing the request, and add an end-to-end regression test for a directly supplied target in addition to the redirect-strategy unit test.

new SameHostRedirectStrategy(link.getHost()));
httpClientBuilder.setDefaultRequestConfig(
connectionTimeoutEnabled ?
RequestConfig.custom()
Expand Down Expand Up @@ -335,7 +345,41 @@ private void proxyLink(final HttpServletRequest req,
base.releaseConnection();
}
}


/**
* Refuse to follow a proxied redirect whose target host differs from the
* host of the application's tracking URL. Same-host redirects (the common
* case for an application web UI) are still followed.
*/
@VisibleForTesting
static void checkSameHost(String allowedHost, URI target)
throws ProtocolException {
String targetHost = target == null ? null : target.getHost();
if (allowedHost != null && targetHost != null
&& !allowedHost.equalsIgnoreCase(targetHost)) {
throw new ProtocolException(
"Refusing to follow application redirect to a different host: "
+ targetHost);
}
}

@VisibleForTesting
static final class SameHostRedirectStrategy extends DefaultRedirectStrategy {
private final String allowedHost;

SameHostRedirectStrategy(String allowedHost) {
this.allowedHost = allowedHost;
}

@Override
public URI getLocationURI(HttpRequest request, HttpResponse response,
HttpContext context) throws ProtocolException {
URI target = super.getLocationURI(request, response, context);
checkSameHost(allowedHost, target);
return target;
}
}

private static String getCheckCookieName(ApplicationId id){
return "checked_"+id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.http.HttpHost;
import org.apache.http.HttpVersion;
import org.apache.http.ProtocolException;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.message.BasicStatusLine;
import org.apache.http.protocol.HttpCoreContext;

import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeys;
Expand Down Expand Up @@ -567,6 +576,30 @@ void testCheckHttpsStrictAndNotProvided() throws Exception {
Mockito.verify(resp, Mockito.times(1)).setContentType(MimeType.HTML);
}

@Test
void testProxyRedirectToDifferentHostIsRefused() throws Exception {
WebAppProxyServlet.SameHostRedirectStrategy strategy =
new WebAppProxyServlet.SameHostRedirectStrategy("amhost");

HttpGet request = new HttpGet("http://amhost:8042/app");
HttpClientContext context = HttpClientContext.create();
context.setAttribute(HttpCoreContext.HTTP_TARGET_HOST,
new HttpHost("amhost", 8042, "http"));
context.setAttribute(HttpCoreContext.HTTP_REQUEST, request);

BasicHttpResponse sameHost = new BasicHttpResponse(
new BasicStatusLine(HttpVersion.HTTP_1_1, 302, "Found"));
sameHost.setHeader("Location", "http://amhost:8042/app/next");
assertEquals("amhost",
strategy.getLocationURI(request, sameHost, context).getHost());

BasicHttpResponse crossHost = new BasicHttpResponse(
new BasicStatusLine(HttpVersion.HTTP_1_1, 302, "Found"));
crossHost.setHeader("Location", "http://169.254.169.254/latest/meta-data/");
assertThrows(ProtocolException.class,
() -> strategy.getLocationURI(request, crossHost, context));
}

private String readInputStream(InputStream input) throws Exception {
ByteArrayOutputStream data = new ByteArrayOutputStream();
byte[] buffer = new byte[512];
Expand Down
Loading