From 03a840de533b1f1c201aa4e99f95f42f9a8269a4 Mon Sep 17 00:00:00 2001 From: Dave Roberge Date: Thu, 7 Apr 2016 12:27:18 -0400 Subject: [PATCH] Accommodate tunneling proxies that close the connection after an auth challenge. --- .../src/test/java/okhttp3/CallTest.java | 29 ++++++- .../okhttp3/internal/io/RealConnection.java | 86 ++++++++++++++----- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/okhttp-tests/src/test/java/okhttp3/CallTest.java b/okhttp-tests/src/test/java/okhttp3/CallTest.java index e46e9747a4a8..d17ddfeb93c8 100644 --- a/okhttp-tests/src/test/java/okhttp3/CallTest.java +++ b/okhttp-tests/src/test/java/okhttp3/CallTest.java @@ -24,6 +24,7 @@ import java.net.HttpURLConnection; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.ProtocolException; import java.net.Proxy; import java.net.ServerSocket; import java.net.UnknownHostException; @@ -2229,7 +2230,6 @@ private InetSocketAddress startNullServer() throws IOException { * OkHttp has a bug where a `Connection: close` response header is not honored when establishing * a TLS tunnel. https://github.com/square/okhttp/issues/2426 */ - @Ignore("currently broken") @Test public void proxyAuthenticateOnConnectWithConnectionClose() throws Exception { server.useHttps(sslContext.getSocketFactory(), true); server.setProtocols(Collections.singletonList(Protocol.HTTP_1_1)); @@ -2266,6 +2266,33 @@ private InetSocketAddress startNullServer() throws IOException { assertEquals(1, server.takeRequest().getSequenceNumber()); } + @Test public void tooManyProxyAuthFailuresWithConnectionClose() throws IOException { + server.useHttps(sslContext.getSocketFactory(), true); + server.setProtocols(Collections.singletonList(Protocol.HTTP_1_1)); + for (int i = 0; i < 21; i++) { + server.enqueue(new MockResponse() + .setResponseCode(407) + .addHeader("Proxy-Authenticate: Basic realm=\"localhost\"") + .addHeader("Connection: close")); + } + + client = client.newBuilder() + .sslSocketFactory(sslContext.getSocketFactory()) + .proxy(server.toProxyAddress()) + .proxyAuthenticator(new RecordingOkAuthenticator("password")) + .hostnameVerifier(new RecordingHostnameVerifier()) + .build(); + + Request request = new Request.Builder() + .url("https://android.com/foo") + .build(); + try { + client.newCall(request).execute(); + fail(); + } catch (ProtocolException expected) { + } + } + /** * Confirm that we don't send the Proxy-Authorization header from the request to the proxy server. * We used to have that behavior but it is problematic because unrelated requests end up sharing diff --git a/okhttp/src/main/java/okhttp3/internal/io/RealConnection.java b/okhttp/src/main/java/okhttp3/internal/io/RealConnection.java index ed8019e2f61b..160ef40043f4 100644 --- a/okhttp/src/main/java/okhttp3/internal/io/RealConnection.java +++ b/okhttp/src/main/java/okhttp3/internal/io/RealConnection.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.lang.ref.Reference; import java.net.ConnectException; +import java.net.ProtocolException; import java.net.Proxy; import java.net.Socket; import java.net.SocketTimeoutException; @@ -94,8 +95,6 @@ public void connect(int connectTimeout, int readTimeout, int writeTimeout, RouteException routeException = null; ConnectionSpecSelector connectionSpecSelector = new ConnectionSpecSelector(connectionSpecs); - Proxy proxy = route.proxy(); - Address address = route.address(); if (route.address().sslSocketFactory() == null && !connectionSpecs.contains(ConnectionSpec.CLEARTEXT)) { @@ -105,10 +104,12 @@ public void connect(int connectTimeout, int readTimeout, int writeTimeout, while (protocol == null) { try { - rawSocket = proxy.type() == Proxy.Type.DIRECT || proxy.type() == Proxy.Type.HTTP - ? address.socketFactory().createSocket() - : new Socket(proxy); - connectSocket(connectTimeout, readTimeout, writeTimeout, connectionSpecSelector); + if (route.requiresTunnel()) { + buildTunneledConnection(connectTimeout, readTimeout, writeTimeout, + connectionSpecSelector); + } else { + buildConnection(connectTimeout, readTimeout, writeTimeout, connectionSpecSelector); + } } catch (IOException e) { closeQuietly(socket); closeQuietly(rawSocket); @@ -132,9 +133,53 @@ public void connect(int connectTimeout, int readTimeout, int writeTimeout, } } + /** + * Does all the work to build an HTTPS connection over a proxy tunnel. The catch here is that a + * proxy server can issue an auth challenge and then close the connection. + */ + private void buildTunneledConnection(int connectTimeout, int readTimeout, int writeTimeout, + ConnectionSpecSelector connectionSpecSelector) throws IOException { + Request tunnelRequest = createTunnelRequest(); + HttpUrl url = tunnelRequest.url(); + int attemptedConnections = 0; + int maxAttempts = 21; + while (true) { + if (++attemptedConnections > maxAttempts) { + throw new ProtocolException("Too many tunnel connections attempted: " + maxAttempts); + } + + connectSocket(connectTimeout, readTimeout, writeTimeout, connectionSpecSelector); + tunnelRequest = createTunnel(readTimeout, writeTimeout, tunnelRequest, url); + + if (tunnelRequest == null) break; // Tunnel successfully created. + + // The proxy decided to close the connection after an auth challenge. We need to create a new + // connection, but this time with the auth credentials. + closeQuietly(rawSocket); + rawSocket = null; + sink = null; + source = null; + } + + establishProtocol(readTimeout, writeTimeout, connectionSpecSelector); + } + /** Does all the work necessary to build a full HTTP or HTTPS connection on a raw socket. */ + private void buildConnection(int connectTimeout, int readTimeout, int writeTimeout, + ConnectionSpecSelector connectionSpecSelector) throws IOException { + connectSocket(connectTimeout, readTimeout, writeTimeout, connectionSpecSelector); + establishProtocol(readTimeout, writeTimeout, connectionSpecSelector); + } + private void connectSocket(int connectTimeout, int readTimeout, int writeTimeout, ConnectionSpecSelector connectionSpecSelector) throws IOException { + Proxy proxy = route.proxy(); + Address address = route.address(); + + rawSocket = proxy.type() == Proxy.Type.DIRECT || proxy.type() == Proxy.Type.HTTP + ? address.socketFactory().createSocket() + : new Socket(proxy); + rawSocket.setSoTimeout(readTimeout); try { Platform.get().connectSocket(rawSocket, route.socketAddress(), connectTimeout); @@ -143,7 +188,10 @@ private void connectSocket(int connectTimeout, int readTimeout, int writeTimeout } source = Okio.buffer(Okio.source(rawSocket)); sink = Okio.buffer(Okio.sink(rawSocket)); + } + private void establishProtocol(int readTimeout, int writeTimeout, + ConnectionSpecSelector connectionSpecSelector) throws IOException { if (route.address().sslSocketFactory() != null) { connectTls(readTimeout, writeTimeout, connectionSpecSelector); } else { @@ -171,10 +219,6 @@ private void connectSocket(int connectTimeout, int readTimeout, int writeTimeout private void connectTls(int readTimeout, int writeTimeout, ConnectionSpecSelector connectionSpecSelector) throws IOException { - if (route.requiresTunnel()) { - createTunnel(readTimeout, writeTimeout); - } - Address address = route.address(); SSLSocketFactory sslSocketFactory = address.sslSocketFactory(); boolean success = false; @@ -237,10 +281,9 @@ private void connectTls(int readTimeout, int writeTimeout, * To make an HTTPS connection over an HTTP proxy, send an unencrypted CONNECT request to create * the proxy connection. This may need to be retried if the proxy requires authorization. */ - private void createTunnel(int readTimeout, int writeTimeout) throws IOException { + private Request createTunnel(int readTimeout, int writeTimeout, Request tunnelRequest, + HttpUrl url) throws IOException { // Make an SSL Tunnel on the first message pair of each SSL + proxy connection. - Request tunnelRequest = createTunnelRequest(); - HttpUrl url = tunnelRequest.url(); String requestLine = "CONNECT " + Util.hostHeader(url, true) + " HTTP/1.1"; while (true) { Http1xStream tunnelConnection = new Http1xStream(null, source, sink); @@ -268,12 +311,16 @@ private void createTunnel(int readTimeout, int writeTimeout) throws IOException if (!source.buffer().exhausted() || !sink.buffer().exhausted()) { throw new IOException("TLS tunnel buffered too many bytes!"); } - return; + return null; case HTTP_PROXY_AUTH: tunnelRequest = route.address().proxyAuthenticator().authenticate(route, response); - if (tunnelRequest != null) continue; - throw new IOException("Failed to authenticate with proxy"); + if (tunnelRequest == null) throw new IOException("Failed to authenticate with proxy"); + + if ("close".equalsIgnoreCase(response.header("Connection"))) { + return tunnelRequest; + } + break; default: throw new IOException( @@ -283,10 +330,9 @@ private void createTunnel(int readTimeout, int writeTimeout) throws IOException } /** - * Returns a request that creates a TLS tunnel via an HTTP proxy, or null if no tunnel is - * necessary. Everything in the tunnel request is sent unencrypted to the proxy server, so tunnels - * include only the minimum set of headers. This avoids sending potentially sensitive data like - * HTTP cookies to the proxy unencrypted. + * Returns a request that creates a TLS tunnel via an HTTP proxy. Everything in the tunnel request + * is sent unencrypted to the proxy server, so tunnels include only the minimum set of headers. + * This avoids sending potentially sensitive data like HTTP cookies to the proxy unencrypted. */ private Request createTunnelRequest() throws IOException { return new Request.Builder()