Skip to content

Commit

Permalink
Merge pull request square#2467 from square/dr-Apr072016-tunnel-proxy-…
Browse files Browse the repository at this point in the history
…close

Accommodate tunneling proxies that close the connection after an auth challenge.
  • Loading branch information
swankjesse committed Apr 8, 2016
2 parents 837dc04 + 03a840d commit e712faa
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 21 deletions.
29 changes: 28 additions & 1 deletion okhttp-tests/src/test/java/okhttp3/CallTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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
Expand Down
86 changes: 66 additions & 20 deletions okhttp/src/main/java/okhttp3/internal/io/RealConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -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()
Expand Down

0 comments on commit e712faa

Please sign in to comment.