Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
SanitizeProxyAuth: Whitelist all hop-by-hop headers
Browse files Browse the repository at this point in the history 
We were discarding "Proxy-Authenticate: keep-alive" headers in HTTP/1.0 responses, which broke multi-step connection-level authentication methods for proxies that sent HTTP/1.0 responses.

BUG=463937

Review URL: https://codereview.chromium.org/982733002

Cr-Commit-Position: refs/heads/master@{#319219}
  • Loading branch information
ttuttle authored and Commit bot committed Mar 5, 2015
1 parent 46b403c commit 34f63b5
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 7 deletions.
236 changes: 231 additions & 5 deletions net/http/http_network_transaction_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2372,7 +2372,125 @@ TEST_P(HttpNetworkTransactionTest, BasicAuthKeepAliveImpatientServer) {

// Test the request-challenge-retry sequence for basic auth, over a connection
// that requires a restart when setting up an SSL tunnel.
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAlive) {
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAliveHttp10) {
HttpRequestInfo request;
request.method = "GET";
request.url = GURL("https://www.google.com/");
// when the no authentication data flag is set.
request.load_flags = net::LOAD_DO_NOT_SEND_AUTH_DATA;

// Configure against proxy server "myproxy:70".
session_deps_.proxy_service.reset(
ProxyService::CreateFixedFromPacResult("PROXY myproxy:70"));
CapturingBoundNetLog log;
session_deps_.net_log = log.bound().net_log();
scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_));

// Since we have proxy, should try to establish tunnel.
MockWrite data_writes1[] = {
MockWrite(
"CONNECT www.google.com:443 HTTP/1.1\r\n"
"Host: www.google.com\r\n"
"Proxy-Connection: keep-alive\r\n\r\n"),

// After calling trans->RestartWithAuth(), this is the request we should
// be issuing -- the final header line contains the credentials.
MockWrite(
"CONNECT www.google.com:443 HTTP/1.1\r\n"
"Host: www.google.com\r\n"
"Proxy-Connection: keep-alive\r\n"
"Proxy-Authorization: Basic Zm9vOmJhcg==\r\n\r\n"),

MockWrite(
"GET / HTTP/1.1\r\n"
"Host: www.google.com\r\n"
"Connection: keep-alive\r\n\r\n"),
};

// The proxy responds to the connect with a 407, using a persistent
// connection.
MockRead data_reads1[] = {
// No credentials.
MockRead("HTTP/1.0 407 Proxy Authentication Required\r\n"),
MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n\r\n"),

MockRead("HTTP/1.0 200 Connection Established\r\n\r\n"),

MockRead("HTTP/1.1 200 OK\r\n"),
MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
MockRead("Content-Length: 5\r\n\r\n"),
MockRead(SYNCHRONOUS, "hello"),
};

StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
data_writes1, arraysize(data_writes1));
session_deps_.socket_factory->AddSocketDataProvider(&data1);
SSLSocketDataProvider ssl(ASYNC, OK);
session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl);

TestCompletionCallback callback1;

scoped_ptr<HttpTransaction> trans(
new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));

int rv = trans->Start(&request, callback1.callback(), log.bound());
EXPECT_EQ(ERR_IO_PENDING, rv);

rv = callback1.WaitForResult();
EXPECT_EQ(OK, rv);
net::CapturingNetLog::CapturedEntryList entries;
log.GetEntries(&entries);
size_t pos = ExpectLogContainsSomewhere(
entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS,
NetLog::PHASE_NONE);
ExpectLogContainsSomewhere(
entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS,
NetLog::PHASE_NONE);

const HttpResponseInfo* response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
EXPECT_FALSE(response->headers->IsKeepAlive());
ASSERT_FALSE(response->headers.get() == NULL);
EXPECT_EQ(407, response->headers->response_code());
EXPECT_TRUE(HttpVersion(1, 0) == response->headers->GetHttpVersion());
EXPECT_TRUE(CheckBasicProxyAuth(response->auth_challenge.get()));

LoadTimingInfo load_timing_info;
// CONNECT requests and responses are handled at the connect job level, so
// the transaction does not yet have a connection.
EXPECT_FALSE(trans->GetLoadTimingInfo(&load_timing_info));

TestCompletionCallback callback2;

rv =
trans->RestartWithAuth(AuthCredentials(kFoo, kBar), callback2.callback());
EXPECT_EQ(ERR_IO_PENDING, rv);

rv = callback2.WaitForResult();
EXPECT_EQ(OK, rv);

response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);

EXPECT_TRUE(response->headers->IsKeepAlive());
EXPECT_EQ(200, response->headers->response_code());
EXPECT_EQ(5, response->headers->GetContentLength());
EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());

// The password prompt info should not be set.
EXPECT_TRUE(response->auth_challenge.get() == NULL);

EXPECT_TRUE(trans->GetLoadTimingInfo(&load_timing_info));
TestLoadTimingNotReusedWithPac(load_timing_info,
CONNECT_TIMING_HAS_SSL_TIMES);

trans.reset();
session->CloseAllConnections();
}

// Test the request-challenge-retry sequence for basic auth, over a connection
// that requires a restart when setting up an SSL tunnel.
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAliveHttp11) {
HttpRequestInfo request;
request.method = "GET";
request.url = GURL("https://www.google.com/");
Expand Down Expand Up @@ -2448,6 +2566,7 @@ TEST_P(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAlive) {

const HttpResponseInfo* response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
EXPECT_FALSE(response->headers->IsKeepAlive());
ASSERT_FALSE(response->headers.get() == NULL);
EXPECT_EQ(407, response->headers->response_code());
EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());
Expand Down Expand Up @@ -2487,8 +2606,115 @@ TEST_P(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAlive) {
}

// Test the request-challenge-retry sequence for basic auth, over a keep-alive
// proxy connection, when setting up an SSL tunnel.
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) {
// proxy connection with HTTP/1.0 responses, when setting up an SSL tunnel.
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyKeepAliveHttp10) {
HttpRequestInfo request;
request.method = "GET";
request.url = GURL("https://www.google.com/");
// Ensure that proxy authentication is attempted even
// when the no authentication data flag is set.
request.load_flags = net::LOAD_DO_NOT_SEND_AUTH_DATA;

// Configure against proxy server "myproxy:70".
session_deps_.proxy_service.reset(ProxyService::CreateFixed("myproxy:70"));
CapturingBoundNetLog log;
session_deps_.net_log = log.bound().net_log();
scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps_));

scoped_ptr<HttpTransaction> trans(
new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));

// Since we have proxy, should try to establish tunnel.
MockWrite data_writes1[] = {
MockWrite(
"CONNECT www.google.com:443 HTTP/1.1\r\n"
"Host: www.google.com\r\n"
"Proxy-Connection: keep-alive\r\n\r\n"),

// After calling trans->RestartWithAuth(), this is the request we should
// be issuing -- the final header line contains the credentials.
MockWrite(
"CONNECT www.google.com:443 HTTP/1.1\r\n"
"Host: www.google.com\r\n"
"Proxy-Connection: keep-alive\r\n"
"Proxy-Authorization: Basic Zm9vOmJheg==\r\n\r\n"),
};

// The proxy responds to the connect with a 407, using a persistent
// connection. (Since it's HTTP/1.0, keep-alive has to be explicit.)
MockRead data_reads1[] = {
// No credentials.
MockRead("HTTP/1.0 407 Proxy Authentication Required\r\n"),
MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
MockRead("Proxy-Connection: keep-alive\r\n"),
MockRead("Content-Length: 10\r\n\r\n"),
MockRead("0123456789"),

// Wrong credentials (wrong password).
MockRead("HTTP/1.0 407 Proxy Authentication Required\r\n"),
MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
MockRead("Proxy-Connection: keep-alive\r\n"),
MockRead("Content-Length: 10\r\n\r\n"),
// No response body because the test stops reading here.
MockRead(SYNCHRONOUS, ERR_UNEXPECTED), // Should not be reached.
};

StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
data_writes1, arraysize(data_writes1));
session_deps_.socket_factory->AddSocketDataProvider(&data1);

TestCompletionCallback callback1;

int rv = trans->Start(&request, callback1.callback(), log.bound());
EXPECT_EQ(ERR_IO_PENDING, rv);

rv = callback1.WaitForResult();
EXPECT_EQ(OK, rv);
net::CapturingNetLog::CapturedEntryList entries;
log.GetEntries(&entries);
size_t pos = ExpectLogContainsSomewhere(
entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS,
NetLog::PHASE_NONE);
ExpectLogContainsSomewhere(
entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS,
NetLog::PHASE_NONE);

const HttpResponseInfo* response = trans->GetResponseInfo();
ASSERT_TRUE(response);
ASSERT_TRUE(response->headers);
EXPECT_TRUE(response->headers->IsKeepAlive());
EXPECT_EQ(407, response->headers->response_code());
EXPECT_EQ(10, response->headers->GetContentLength());
EXPECT_TRUE(HttpVersion(1, 0) == response->headers->GetHttpVersion());
EXPECT_TRUE(CheckBasicProxyAuth(response->auth_challenge.get()));

TestCompletionCallback callback2;

// Wrong password (should be "bar").
rv =
trans->RestartWithAuth(AuthCredentials(kFoo, kBaz), callback2.callback());
EXPECT_EQ(ERR_IO_PENDING, rv);

rv = callback2.WaitForResult();
EXPECT_EQ(OK, rv);

response = trans->GetResponseInfo();
ASSERT_TRUE(response);
ASSERT_TRUE(response->headers);
EXPECT_TRUE(response->headers->IsKeepAlive());
EXPECT_EQ(407, response->headers->response_code());
EXPECT_EQ(10, response->headers->GetContentLength());
EXPECT_TRUE(HttpVersion(1, 0) == response->headers->GetHttpVersion());
EXPECT_TRUE(CheckBasicProxyAuth(response->auth_challenge.get()));

// Flush the idle socket before the NetLog and HttpNetworkTransaction go
// out of scope.
session->CloseAllConnections();
}

// Test the request-challenge-retry sequence for basic auth, over a keep-alive
// proxy connection with HTTP/1.1 responses, when setting up an SSL tunnel.
TEST_P(HttpNetworkTransactionTest, BasicAuthProxyKeepAliveHttp11) {
HttpRequestInfo request;
request.method = "GET";
request.url = GURL("https://www.google.com/");
Expand Down Expand Up @@ -2562,7 +2788,7 @@ TEST_P(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) {
ASSERT_TRUE(response->headers);
EXPECT_TRUE(response->headers->IsKeepAlive());
EXPECT_EQ(407, response->headers->response_code());
EXPECT_EQ(-1, response->headers->GetContentLength());
EXPECT_EQ(10, response->headers->GetContentLength());
EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());
EXPECT_TRUE(CheckBasicProxyAuth(response->auth_challenge.get()));

Expand All @@ -2581,7 +2807,7 @@ TEST_P(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) {
ASSERT_TRUE(response->headers);
EXPECT_TRUE(response->headers->IsKeepAlive());
EXPECT_EQ(407, response->headers->response_code());
EXPECT_EQ(-1, response->headers->GetContentLength());
EXPECT_EQ(10, response->headers->GetContentLength());
EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());
EXPECT_TRUE(CheckBasicProxyAuth(response->auth_challenge.get()));

Expand Down
14 changes: 12 additions & 2 deletions net/http/proxy_client_socket.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,19 @@ bool ProxyClientSocket::SanitizeProxyAuth(HttpResponseInfo* response) {
scoped_refptr<HttpResponseHeaders> new_headers = new HttpResponseHeaders(
HttpUtil::AssembleRawHeaders(kHeaders, arraysize(kHeaders)));

// Copy status line and all hop-by-hop headers to preserve keep-alive
// behavior.
new_headers->ReplaceStatusLine(old_headers->GetStatusLine());
CopyHeaderValues(old_headers, new_headers, "Connection");
CopyHeaderValues(old_headers, new_headers, "Proxy-Authenticate");
CopyHeaderValues(old_headers, new_headers, "connection");
CopyHeaderValues(old_headers, new_headers, "proxy-connection");
CopyHeaderValues(old_headers, new_headers, "keep-alive");
CopyHeaderValues(old_headers, new_headers, "trailer");
CopyHeaderValues(old_headers, new_headers, "transfer-encoding");
CopyHeaderValues(old_headers, new_headers, "upgrade");

CopyHeaderValues(old_headers, new_headers, "content-length");

CopyHeaderValues(old_headers, new_headers, "proxy-authenticate");

response->headers = new_headers;
return true;
Expand Down

0 comments on commit 34f63b5

Please sign in to comment.