# HG changeset patch # User prappo # Date 1510578791 -10800 # Node ID 371ed971281e155111ee3e079fa2b1d3404ab6a6 # Parent b36de693e838acfb6195c6c804ed7e645778d548 http-client-branch: common URLPermission paths diff -r b36de693e838 -r 371ed971281e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Mon Nov 13 14:43:09 2017 +0300 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java Mon Nov 13 16:13:11 2017 +0300 @@ -35,13 +35,14 @@ import java.security.AccessControlContext; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import jdk.incubator.http.internal.common.MinimalFuture; import jdk.incubator.http.internal.common.Utils; import jdk.incubator.http.internal.common.Log; +import static jdk.incubator.http.internal.common.Utils.permissionForProxy; + /** * One request/response exchange (handles 100/101 intermediate response also). * depth field used to track number of times a new request is being sent @@ -414,55 +415,13 @@ * If method is CONNECT, then uri must be of form "scheme://host:port" */ private static URLPermission permissionForServer(URI uri, - String method, - Map> headers) { - StringBuilder sb = new StringBuilder(); - - String urlstring, actionstring; - + String method, + Map> headers) { if (method.equals("CONNECT")) { - urlstring = uri.toString(); - actionstring = "CONNECT"; + return new URLPermission(uri.toString(), "CONNECT"); } else { - sb.append(uri.getScheme()).append("://") - .append(uri.getAuthority()) - .append(uri.getPath()); - urlstring = sb.toString(); - - sb = new StringBuilder(); - sb.append(method); - if (headers != null && !headers.isEmpty()) { - sb.append(':'); - Set keys = headers.keySet(); - boolean first = true; - for (String key : keys) { - if (!first) { - sb.append(','); - } - sb.append(key); - first = false; - } - } - actionstring = sb.toString(); + return Utils.permissionForServer(uri, method, headers.keySet().stream()); } - return new URLPermission(urlstring, actionstring); - } - - /** - * Returns the security permissions required to connect to the proxy, or - * null if none is required or applicable. - */ - static URLPermission permissionForProxy(HttpRequestImpl request) { - InetSocketAddress proxyAddress = request.proxy(); - if (proxyAddress == null) - return null; - - StringBuilder sb = new StringBuilder(); - sb.append("socket://") - .append(proxyAddress.getHostString()).append(":") - .append(proxyAddress.getPort()); - String urlString = sb.toString(); - return new URLPermission(urlString, "CONNECT"); } /** @@ -493,7 +452,7 @@ if (ps != null) { if (!method.equals("CONNECT")) { // a non-tunneling HTTP proxy. Need to check access - URLPermission proxyPerm = permissionForProxy(request); + URLPermission proxyPerm = permissionForProxy(request.proxy()); if (proxyPerm != null) { try { sm.checkPermission(proxyPerm, acc); diff -r b36de693e838 -r 371ed971281e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java Mon Nov 13 14:43:09 2017 +0300 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java Mon Nov 13 16:13:11 2017 +0300 @@ -25,6 +25,7 @@ package jdk.incubator.http.internal.common; +import jdk.incubator.http.HttpHeaders; import sun.net.NetProperties; import sun.net.util.IPAddressUtil; @@ -32,12 +33,14 @@ import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.IOException; +import java.io.PrintStream; import java.io.UncheckedIOException; -import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.lang.System.Logger; import java.lang.System.Logger.Level; import java.net.InetSocketAddress; +import java.net.URI; +import java.net.URLPermission; import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -45,14 +48,14 @@ import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.List; -import java.util.LinkedList; import java.util.Set; import java.util.concurrent.CompletionException; import java.util.function.Predicate; import java.util.function.Supplier; -import jdk.incubator.http.HttpHeaders; +import java.util.stream.Stream; + +import static java.util.stream.Collectors.joining; /** * Miscellaneous utilities @@ -119,6 +122,42 @@ private Utils() { } + /** + * Returns the security permissions required to connect to the proxy, or + * {@code null} if none is required or applicable. + */ + public static URLPermission permissionForProxy(InetSocketAddress proxyAddress) { + if (proxyAddress == null) + return null; + + StringBuilder sb = new StringBuilder(); + sb.append("socket://") + .append(proxyAddress.getHostString()).append(":") + .append(proxyAddress.getPort()); + String urlString = sb.toString(); + return new URLPermission(urlString, "CONNECT"); + } + + /** + * Returns the security permission required for the given details. + */ + public static URLPermission permissionForServer(URI uri, + String method, + Stream headers) { + String urlString = new StringBuilder() + .append(uri.getScheme()).append("://") + .append(uri.getAuthority()) + .append(uri.getPath()).toString(); + + StringBuilder actionStringBuilder = new StringBuilder(method); + String collected = headers.collect(joining(",")); + if (!collected.isEmpty()) { + actionStringBuilder.append(":").append(collected); + } + return new URLPermission(urlString, actionStringBuilder.toString()); + } + + // ABNF primitives defined in RFC 7230 private static final boolean[] tchar = new boolean[256]; private static final boolean[] fieldvchar = new boolean[256]; diff -r b36de693e838 -r 371ed971281e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/OpeningHandshake.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/OpeningHandshake.java Mon Nov 13 14:43:09 2017 +0300 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/OpeningHandshake.java Mon Nov 13 16:13:11 2017 +0300 @@ -175,7 +175,7 @@ null); // No fragment } catch (URISyntaxException e) { // Shouldn't happen: URI invariant - throw new InternalError(e); + throw new InternalError(e); // TODO: should actually report on this instead of throwing in builder (rev. 47704:34d7cc00f87a4b18b6c30c122fc3d55456833ae0) } } @@ -251,6 +251,7 @@ String expected = Base64.getEncoder().encodeToString(this.sha1.digest()); String actual = requireSingle(headers, HEADER_ACCEPT); if (!actual.trim().equals(expected)) { + // TODO: why do we need the value here? throw checkFailed("Bad " + HEADER_ACCEPT + ", expected:[" + expected + "] ,got:[" + actual.trim() + "]"); } diff -r b36de693e838 -r 371ed971281e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java Mon Nov 13 14:43:09 2017 +0300 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java Mon Nov 13 16:13:11 2017 +0300 @@ -65,6 +65,7 @@ import static java.util.concurrent.CompletableFuture.failedFuture; import static java.util.stream.Collectors.joining; import static jdk.incubator.http.internal.common.Pair.pair; +import static jdk.incubator.http.internal.common.Utils.permissionForProxy; import static jdk.incubator.http.internal.websocket.StatusCodes.CLOSED_ABNORMALLY; import static jdk.incubator.http.internal.websocket.StatusCodes.NO_STATUS_CODE; import static jdk.incubator.http.internal.websocket.StatusCodes.isLegalToSendFromClient; @@ -117,58 +118,24 @@ private final CompletableFuture closeReceived = new MinimalFuture<>(); private final CompletableFuture closeSent = new MinimalFuture<>(); - /** Returns the security permission required for the given details. */ - static URLPermission permissionForServer(URI uri, - Collection> headers) { - StringBuilder sb = new StringBuilder(); - sb.append(uri.getScheme()).append("://") - .append(uri.getAuthority()) - .append(uri.getPath()); - String urlstring = sb.toString(); - - String actionstring = headers.stream() - .map(p -> p.first) - .distinct() - .collect(joining(",")); - if (actionstring != null && !actionstring.equals("")) - actionstring = ":" + actionstring; // Note: no method in the action string - - return new URLPermission(urlstring, actionstring); - } - - /** - * Returns the security permissions required to connect to the proxy, or - * null if none is required or applicable. - */ - static URLPermission permissionForProxy(Proxy proxy) { - InetSocketAddress proxyAddress = (InetSocketAddress)proxy.address(); - - StringBuilder sb = new StringBuilder(); - sb.append("socket://") - .append(proxyAddress.getHostString()).append(":") - .append(proxyAddress.getPort()); - String urlString = sb.toString(); - return new URLPermission(urlString, "CONNECT"); - } - /** * Returns the proxy for the given URI when sent through the given client, - * or null if none is required or applicable. + * or {@code null} if none is required or applicable. */ - static Proxy proxyFor(HttpClient client, URI uri) { + private static Proxy proxyFor(HttpClient client, URI uri) { Optional optional = client.proxy(); - if (!optional.isPresent()) + if (!optional.isPresent()) { return null; - - uri = OpeningHandshake.createRequestURI(uri); // based on the HTTP scheme - List pl = optional.get().select(uri); - if (pl.size() < 1) + } + URI requestURI = OpeningHandshake.createRequestURI(uri); // based on the HTTP scheme + List pl = optional.get().select(requestURI); + if (pl.isEmpty()) { return null; - + } Proxy proxy = pl.get(0); - if (!proxy.type().equals(Proxy.Type.HTTP)) + if (proxy.type() != Proxy.Type.HTTP) { return null; - + } return proxy; } @@ -180,17 +147,23 @@ */ static void checkPermissions(BuilderImpl b, Proxy proxy) { SecurityManager sm = System.getSecurityManager(); - if (sm != null) { - sm.checkPermission(permissionForServer(b.getUri(), b.getHeaders())); - if (proxy != null) { - URLPermission perm = permissionForProxy(proxy); - if (perm != null) - sm.checkPermission(perm); - } + if (sm == null) { + return; + } + URLPermission perm1 = Utils.permissionForServer( + b.getUri(), "", b.getHeaders().stream().map(p -> p.first).distinct()); + sm.checkPermission(perm1); + if (proxy == null) { + return; + } + URLPermission perm2 = permissionForProxy((InetSocketAddress) proxy.address()); + if (perm2 != null) { + sm.checkPermission(perm2); } } static CompletableFuture newInstanceAsync(BuilderImpl b) { + // TODO: a security issue? TOCTOU: two accesses to b.getURI Proxy proxy = proxyFor(b.getClient(), b.getUri()); try { checkPermissions(b, proxy);