8213189: Make restricted headers in HTTP Client configurable and remove Date by default

Reviewed-by: dfuchs
This commit is contained in:
Michael McMahon 2018-11-14 14:23:21 +00:00
parent eafb5eb05e
commit 028f2e14b3
14 changed files with 376 additions and 28 deletions

View File

@ -99,3 +99,21 @@ ftp.nonProxyHosts=localhost|127.*|[::1]
#jdk.http.auth.proxying.disabledSchemes=
jdk.http.auth.tunneling.disabledSchemes=Basic
#
# Allow restricted HTTP request headers
#
# By default, the following request headers are not allowed to be set by user code
# in HttpRequests: "connection", "content-length", "expect", "host" and "upgrade".
# The 'jdk.httpclient.allowRestrictedHeaders' property allows one or more of these
# headers to be specified as a comma separated list to override the default restriction.
# The names are case-insensitive and white-space is ignored (removed before processing
# the list). Note, this capability is mostly intended for testing and isn't expected
# to be used in real deployments. Protocol errors or other undefined behavior is likely
# to occur when using them. The property is not set by default.
# Note also, that there may be other headers that are restricted from being set
# depending on the context. This includes the "Authorization" header when the
# relevant HttpClient has an authenticator set. These restrictions cannot be
# overridden by this property.
#
# jdk.httpclient.allowRestrictedHeaders=host
#

View File

@ -585,6 +585,19 @@ final class Exchange<T> {
} catch (SecurityException e) {
return e;
}
String hostHeader = userHeaders.firstValue("Host").orElse(null);
if (hostHeader != null && !hostHeader.equalsIgnoreCase(u.getHost())) {
// user has set a Host header different to request URI
// must check that for URLPermission also
URI u1 = replaceHostInURI(u, hostHeader);
URLPermission p1 = permissionForServer(u1, method, userHeaders.map());
try {
assert acc != null;
sm.checkPermission(p1, acc);
} catch (SecurityException e) {
return e;
}
}
ProxySelector ps = client.proxySelector();
if (ps != null) {
if (!method.equals("CONNECT")) {
@ -602,6 +615,15 @@ final class Exchange<T> {
return null;
}
private static URI replaceHostInURI(URI u, String hostPort) {
StringBuilder sb = new StringBuilder();
sb.append(u.getScheme())
.append("://")
.append(hostPort)
.append(u.getRawPath());
return URI.create(sb.toString());
}
HttpClient.Version version() {
return multi.version();
}

View File

@ -29,6 +29,7 @@ import java.io.IOException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.function.Function;
import java.net.http.HttpClient;
import java.net.http.HttpResponse;
import jdk.internal.net.http.common.Logger;
import jdk.internal.net.http.common.MinimalFuture;
@ -64,6 +65,9 @@ abstract class ExchangeImpl<T> {
return exchange;
}
HttpClient client() {
return exchange.client();
}
/**
* Returns the {@link HttpConnection} instance to which this exchange is

View File

@ -27,6 +27,7 @@ package jdk.internal.net.http;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.http.HttpClient;
import java.net.http.HttpResponse.BodyHandler;
import java.net.http.HttpResponse.BodySubscriber;
import java.nio.ByteBuffer;
@ -706,6 +707,10 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
}
}
HttpClient client() {
return client;
}
String dbgString() {
return "Http1Exchange";
}

View File

@ -27,6 +27,7 @@ package jdk.internal.net.http;
import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
@ -59,7 +60,7 @@ class Http1Request {
private final Http1Exchange<?> http1Exchange;
private final HttpConnection connection;
private final HttpRequest.BodyPublisher requestPublisher;
private final HttpHeaders userHeaders;
private volatile HttpHeaders userHeaders;
private final HttpHeadersBuilder systemHeadersBuilder;
private volatile boolean streaming;
private volatile long contentLength;
@ -91,7 +92,7 @@ class Http1Request {
}
private void collectHeaders0(StringBuilder sb) {
public void collectHeaders0(StringBuilder sb) {
BiPredicate<String,String> filter =
connection.headerFilter(request);
@ -99,6 +100,15 @@ class Http1Request {
BiPredicate<String,String> nocookies = NOCOOKIES.and(filter);
HttpHeaders systemHeaders = systemHeadersBuilder.build();
HttpClient client = http1Exchange.client();
// Filter overridable headers from userHeaders
userHeaders = HttpHeaders.of(userHeaders.map(), Utils.CONTEXT_RESTRICTED(client));
final HttpHeaders uh = userHeaders;
// Filter any headers from systemHeaders that are set in userHeaders
systemHeaders = HttpHeaders.of(systemHeaders.map(), (k,v) -> uh.firstValue(k).isEmpty());
// If we're sending this request through a tunnel,
// then don't send any preemptive proxy-* headers that

View File

@ -608,8 +608,20 @@ class Stream<T> extends ExchangeImpl<T> {
if (contentLength > 0) {
h.setHeader("content-length", Long.toString(contentLength));
}
URI uri = request.uri();
if (uri != null) {
h.setHeader("host", Utils.hostString(request));
}
HttpHeaders sysh = filterHeaders(h.build());
HttpHeaders userh = filterHeaders(request.getUserHeaders());
// Filter context restricted from userHeaders
userh = HttpHeaders.of(userh.map(), Utils.CONTEXT_RESTRICTED(client()));
final HttpHeaders uh = userh;
// Filter any headers from systemHeaders that are set in userHeaders
sysh = HttpHeaders.of(sysh.map(), (k,v) -> uh.firstValue(k).isEmpty());
OutgoingHeaders<Stream<T>> f = new OutgoingHeaders<>(sysh, userh, this);
if (contentLength == 0) {
f.setFlag(HeadersFrame.END_STREAM);

View File

@ -45,6 +45,7 @@ import java.net.ConnectException;
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.URLPermission;
import java.net.http.HttpClient;
import java.net.http.HttpHeaders;
import java.net.http.HttpTimeoutException;
import java.nio.ByteBuffer;
@ -75,6 +76,7 @@ import java.util.stream.Stream;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import jdk.internal.net.http.HttpRequestImpl;
/**
* Miscellaneous utilities
@ -127,14 +129,23 @@ public final class Utils {
public static final BiPredicate<String,String> ACCEPT_ALL = (x,y) -> true;
private static final Set<String> DISALLOWED_HEADERS_SET;
private static final Set<String> DISALLOWED_HEADERS_SET = getDisallowedHeaders();
static {
// A case insensitive TreeSet of strings.
TreeSet<String> treeSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
treeSet.addAll(Set.of("connection", "content-length",
"date", "expect", "from", "host", "upgrade", "via", "warning"));
DISALLOWED_HEADERS_SET = Collections.unmodifiableSet(treeSet);
private static Set<String> getDisallowedHeaders() {
Set<String> headers = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
headers.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade"));
String v = getNetProperty("jdk.httpclient.allowRestrictedHeaders");
if (v != null) {
// any headers found are removed from set.
String[] tokens = v.trim().split(",");
for (String token : tokens) {
headers.remove(token);
}
return Collections.unmodifiableSet(headers);
} else {
return Collections.unmodifiableSet(headers);
}
}
public static final BiPredicate<String, String>
@ -156,6 +167,19 @@ public final class Utils {
return true;
};
// Headers that are not generally restricted, and can therefore be set by users,
// but can in some contexts be overridden by the implementation.
// Currently, only contains "Authorization" which will
// be overridden, when an Authenticator is set on the HttpClient.
// Needs to be BiPred<String,String> to fit with general form of predicates
// used by caller.
public static final BiPredicate<String, String> CONTEXT_RESTRICTED(HttpClient client) {
return (k, v) -> client.authenticator() == null ||
! (k.equalsIgnoreCase("Authorization")
&& k.equalsIgnoreCase("Proxy-Authorization"));
}
private static final Predicate<String> IS_PROXY_HEADER = (k) ->
k != null && k.length() > 6 && "proxy-".equalsIgnoreCase(k.substring(0,6));
private static final Predicate<String> NO_PROXY_HEADER =
@ -323,8 +347,8 @@ public final class Utils {
Stream<String> headers) {
String urlString = new StringBuilder()
.append(uri.getScheme()).append("://")
.append(uri.getAuthority())
.append(uri.getPath()).toString();
.append(uri.getRawAuthority())
.append(uri.getRawPath()).toString();
StringBuilder actionStringBuilder = new StringBuilder(method);
String collected = headers.collect(joining(","));
@ -794,6 +818,33 @@ public final class Utils {
return getDebugLogger(dbgTag, errLevel);
}
/**
* Return the host string from a HttpRequestImpl
*
* @param request
* @return
*/
public static String hostString(HttpRequestImpl request) {
URI uri = request.uri();
int port = uri.getPort();
String host = uri.getHost();
boolean defaultPort;
if (port == -1) {
defaultPort = true;
} else if (uri.getScheme().equalsIgnoreCase("https")) {
defaultPort = port == 443;
} else {
defaultPort = port == 80;
}
if (defaultPort) {
return host;
} else {
return host + ":" + Integer.toString(port);
}
}
/**
* Get a logger for debug HPACK traces.The logger should only be used
* with levels whose severity is {@code <= DEBUG}.

View File

@ -339,7 +339,7 @@ public class RequestBuilderTest {
// headers that are allowed now, but weren't before
private static final Set<String> FORMERLY_RESTRICTED = Set.of("referer", "origin",
"OriGin", "Referer");
"OriGin", "Referer", "Date", "via", "WarnIng");
@Test
public void testFormerlyRestricted() throws URISyntaxException {
@ -354,14 +354,9 @@ public class RequestBuilderTest {
}
private static final Set<String> RESTRICTED = Set.of("connection", "content-length",
"date", "expect", "from", "host",
"upgrade", "via", "warning",
"Connection", "Content-Length",
"DATE", "eXpect", "frOm", "hosT",
"upgradE", "vIa", "Warning",
"CONNection", "CONTENT-LENGTH",
"Date", "EXPECT", "From", "Host",
"Upgrade", "Via", "WARNING");
"expect", "host", "upgrade", "Connection", "Content-Length",
"eXpect", "hosT", "upgradE", "CONNection", "CONTENT-LENGTH",
"EXPECT", "Host", "Upgrade");
interface WithHeader {
HttpRequest.Builder withHeader(HttpRequest.Builder builder, String name, String value);

View File

@ -0,0 +1,97 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8178699
* @modules java.net.http
* @run main/othervm RestrictedHeadersTest
* @run main/othervm -Djdk.httpclient.allowRestrictedHeaders=content-length,connection RestrictedHeadersTest content-length connection
* @run main/othervm -Djdk.httpclient.allowRestrictedHeaders=host,upgrade RestrictedHeadersTest host upgrade
* @run main/othervm -Djdk.httpclient.allowRestrictedHeaders=via RestrictedHeadersTest via
*/
import java.net.URI;
import java.net.http.HttpRequest;
import java.util.Set;
public class RestrictedHeadersTest {
public static void main(String[] args) {
if (args.length == 0) {
runDefaultTest();
} else {
runTest(Set.of(args));
}
}
// This list must be same as impl
static Set<String> defaultRestrictedHeaders =
Set.of("connection", "content-length", "expect", "host", "upgrade");
private static void runDefaultTest() {
System.out.println("DEFAULT TEST: no property set");
for (String header : defaultRestrictedHeaders) {
checkHeader(header, "foo", false);
}
// miscellaneous others that should succeed
checkHeader("foobar", "barfoo", true);
checkHeader("date", "today", true);
}
private static void checkHeader(String name, String value, boolean succeed) {
try {
HttpRequest request = HttpRequest.newBuilder(URI.create("https://foo.com/"))
.header(name, value)
.GET()
.build();
if (!succeed) {
String s = name+"/"+value+" should have failed";
throw new RuntimeException(s);
}
System.out.printf("%s = %s succeeded as expected\n", name, value);
} catch (IllegalArgumentException iae) {
if (succeed) {
String s = name+"/"+value+" should have succeeded";
throw new RuntimeException(s);
}
System.out.printf("%s = %s failed as expected\n", name, value);
}
}
// args is the Set of allowed restricted headers
private static void runTest(Set<String> args) {
System.out.print("RUNTEST: allowed headers set in property: ");
for (String arg : args) System.out.printf("%s ", arg);
System.out.println("");
for (String header : args) {
checkHeader(header, "val", true);
}
for (String header : defaultRestrictedHeaders) {
if (!args.contains(header)) {
checkHeader(header, "foo", false);
}
}
}
}

View File

@ -38,6 +38,9 @@
* @run testng/othervm
* -Djdk.httpclient.HttpClient.log=requests,headers,errors
* SpecialHeadersTest
* @run testng/othervm -Djdk.httpclient.allowRestrictedHeaders=Host
* -Djdk.httpclient.HttpClient.log=requests,headers,errors
* SpecialHeadersTest
*/
import com.sun.net.httpserver.HttpServer;
@ -70,6 +73,7 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import static java.lang.System.err;
import static java.lang.System.out;
@ -104,21 +108,33 @@ public class SpecialHeadersTest implements HttpServerAdapters {
{"ORIGIN: upper"},
};
// Needs net.property enabled for this part of test
static final String[][] headerNamesAndValues1 = new String[][]{
{"Host: <DEFAULT>"},
{"Host: camel-cased"},
{"host: all-lower-case"},
{"hoSt: mixed"}
};
@DataProvider(name = "variants")
public Object[][] variants() {
String prop = System.getProperty("jdk.httpclient.allowRestrictedHeaders");
boolean hostTest = prop != null && prop.equalsIgnoreCase("host");
final String[][] testInput = hostTest ? headerNamesAndValues1 : headerNamesAndValues;
List<Object[]> list = new ArrayList<>();
for (boolean sameClient : new boolean[] { false, true }) {
Arrays.asList(headerNamesAndValues).stream()
Arrays.asList(testInput).stream()
.map(e -> new Object[] {httpURI, e[0], sameClient})
.forEach(list::add);
Arrays.asList(headerNamesAndValues).stream()
Arrays.asList(testInput).stream()
.map(e -> new Object[] {httpsURI, e[0], sameClient})
.forEach(list::add);
Arrays.asList(headerNamesAndValues).stream()
Arrays.asList(testInput).stream()
.map(e -> new Object[] {http2URI, e[0], sameClient})
.forEach(list::add);
Arrays.asList(headerNamesAndValues).stream()
Arrays.asList(testInput).stream()
.map(e -> new Object[] {https2URI, e[0], sameClient})
.forEach(list::add);
}
@ -131,7 +147,8 @@ public class SpecialHeadersTest implements HttpServerAdapters {
return "Java-http-client/" + System.getProperty("java.version");
}
static final Map<String, String> DEFAULTS = Map.of("USER-AGENT", userAgent());
static final Map<String, Function<URI,String>> DEFAULTS = Map.of(
"USER-AGENT", u -> userAgent(), "HOST", u -> u.getRawAuthority());
@Test(dataProvider = "variants")
void test(String uriString, String headerNameAndValue, boolean sameClient) throws Exception {
@ -142,9 +159,9 @@ public class SpecialHeadersTest implements HttpServerAdapters {
String v = headerNameAndValue.substring(index+1).trim();
String key = name.toUpperCase(Locale.ROOT);
boolean useDefault = "<DEFAULT>".equals(v);
String value = useDefault ? DEFAULTS.get(key) : v;
URI uri = URI.create(uriString+"?name="+key);
String value = useDefault ? DEFAULTS.get(key).apply(uri) : v;
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {
@ -210,7 +227,7 @@ public class SpecialHeadersTest implements HttpServerAdapters {
return Optional.empty();
}
@Override public HttpHeaders headers() {
Map<String, List<String>> map = Map.of("via", List.of("http://foo.com"));
Map<String, List<String>> map = Map.of("upgrade", List.of("http://foo.com"));
return HttpHeaders.of(map, (x, y) -> true);
}
};
@ -231,9 +248,9 @@ public class SpecialHeadersTest implements HttpServerAdapters {
String v = headerNameAndValue.substring(index+1).trim();
String key = name.toUpperCase(Locale.ROOT);
boolean useDefault = "<DEFAULT>".equals(v);
String value = useDefault ? DEFAULTS.get(key) : v;
URI uri = URI.create(uriString+"?name="+key);
String value = useDefault ? DEFAULTS.get(key).apply(uri) : v;
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {

View File

@ -0,0 +1,45 @@
//
// Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
// DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
//
// This code is free software; you can redistribute it and/or modify it
// under the terms of the GNU General Public License version 2 only, as
// published by the Free Software Foundation.
//
// This code is distributed in the hope that it will be useful, but WITHOUT
// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
// version 2 for more details (a copy is included in the LICENSE file that
// accompanied this code).
//
// You should have received a copy of the GNU General Public License version
// 2 along with this work; if not, write to the Free Software Foundation,
// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
//
// Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
// or visit www.oracle.com if you need additional information or have any
// questions.
//
// Policy 16: Test tries to set Host header to localhost:123 but there is no permission
grant {
// permissions common to all tests
permission java.util.PropertyPermission "*", "read";
permission java.io.FilePermission "${test.classes}${/}-", "read,write,delete";
permission java.lang.RuntimePermission "modifyThread";
permission java.util.logging.LoggingPermission "control", "";
permission java.net.SocketPermission "localhost:1024-", "accept,listen";
permission java.io.FilePermission "${test.src}${/}docs${/}-", "read";
permission java.lang.RuntimePermission "createClassLoader";
// permissions specific to this test
permission java.net.URLPermission "http://localhost:${port.number}/files/foo.txt", "GET:Host";
};
// For proxy only. Not being tested
grant codebase "file:${test.classes}/proxydir/-" {
permission java.net.SocketPermission "localhost:1024-", "accept,listen,connect";
permission java.net.SocketPermission "localhost:1024-", "connect,resolve";
};

View File

@ -0,0 +1,45 @@
//
// Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
// DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
//
// This code is free software; you can redistribute it and/or modify it
// under the terms of the GNU General Public License version 2 only, as
// published by the Free Software Foundation.
//
// This code is distributed in the hope that it will be useful, but WITHOUT
// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
// version 2 for more details (a copy is included in the LICENSE file that
// accompanied this code).
//
// You should have received a copy of the GNU General Public License version
// 2 along with this work; if not, write to the Free Software Foundation,
// Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
//
// Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
// or visit www.oracle.com if you need additional information or have any
// questions.
//
// Policy 17. Grant permission to port 123 (no connect attempt is actually made)
grant {
// permissions common to all tests
permission java.util.PropertyPermission "*", "read";
permission java.io.FilePermission "${test.classes}${/}-", "read,write,delete";
permission java.lang.RuntimePermission "modifyThread";
permission java.util.logging.LoggingPermission "control", "";
permission java.net.SocketPermission "localhost:1024-", "accept,listen";
permission java.io.FilePermission "${test.src}${/}docs${/}-", "read";
permission java.lang.RuntimePermission "createClassLoader";
// permissions specific to this test
permission java.net.URLPermission "http://localhost:${port.number}/files/foo.txt", "GET:Host";
permission java.net.URLPermission "http://foohost:123/files/foo.txt", "GET:Host";
};
// For proxy only. Not being tested
grant codebase "file:${test.classes}/proxydir/-" {
permission java.net.SocketPermission "localhost:1024-", "accept,listen,connect";
permission java.net.SocketPermission "localhost:1024-", "connect,resolve";
};

View File

@ -70,6 +70,8 @@ public class Driver {
runtest("10.policy", "10");
runtest("11.policy", "11");
runtest("12.policy", "12");
runtest("16.policy", "16", "-Djdk.httpclient.allowRestrictedHeaders=Host");
runtest("17.policy", "17", "-Djdk.httpclient.allowRestrictedHeaders=Host");
System.out.println("DONE");
}
@ -114,7 +116,11 @@ public class Driver {
}
public static void runtest(String policy, String testnum) throws Throwable {
runtest(policy, testnum, null);
}
public static void runtest(String policy, String testnum, String addProp) throws Throwable {
String testJdk = System.getProperty("test.jdk", "?");
String testSrc = System.getProperty("test.src", "?");
String testClassPath = System.getProperty("test.class.path", "?");
@ -136,6 +142,9 @@ public class Driver {
cmd.add("-Dport.number=" + Integer.toString(Utils.getFreePort()));
cmd.add("-Dport.number1=" + Integer.toString(Utils.getFreePort()));
cmd.add("-Djdk.httpclient.HttpClient.log=all,frames:all");
if (addProp != null) {
cmd.add(addProp);
}
cmd.add("-cp");
cmd.add(testClassPath);
cmd.add("Security");

View File

@ -377,6 +377,24 @@ public class Security {
else
throw new RuntimeException(t);
}
}),
// (16) allowed to set Host header but does not have permission
TestAndResult.of(true, () -> { //Policy 16
URI u = URI.create("http://localhost:" + port + "/files/foo.txt");
HttpRequest request = HttpRequest.newBuilder(u)
.header("Host", "foohost:123")
.GET().build();
HttpResponse<?> response = client.send(request, ofString());
System.out.println("Received response:" + response);
}),
// (17) allowed to set Host header and does have permission
TestAndResult.of(false, () -> { //Policy 17
URI u = URI.create("http://localhost:" + port + "/files/foo.txt");
HttpRequest request = HttpRequest.newBuilder(u)
.header("Host", "foohost:123")
.GET().build();
HttpResponse<?> response = client.send(request, ofString());
System.out.println("Received response:" + response);
})
};
}