From 4de702f4d2053b586d4b9b12181ef953e2773da8 Mon Sep 17 00:00:00 2001
From: "wtc@chromium.org"
<wtc@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 18 Sep 2009 17:46:10 +0000
Subject: [PATCH] We should pass the service principal name (SPN) of the format
"HTTP/host:port" as the third argument (pszTargetName) to
InitializeSecurityContext. This requires adding a host_and_port parameter to
some methods.
Remove obsolete (and incorrect) logging code in
HttpNetworkTransaction::PrepareForAuthRestart().
R=eroman
BUG=18009
TEST=none
Review URL: http://codereview.chromium.org/206022
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26588 0039d316-1c4b-4281-b951-d872f2087c98
---
net/http/http_auth.cc | 8 +--
net/http/http_auth.h | 5 ++
net/http/http_auth_handler.cc | 4 +-
net/http/http_auth_handler.h | 9 +++-
net/http/http_auth_handler_basic_unittest.cc | 3 +-
net/http/http_auth_handler_digest_unittest.cc | 3 +-
net/http/http_auth_handler_ntlm_win.cc | 11 ++--
net/http/http_auth_unittest.cc | 10 ++++
net/http/http_network_transaction.cc | 50 +------------------
9 files changed, 44 insertions(+), 59 deletions(-)
diff --git a/net/http/http_auth.cc b/net/http/http_auth.cc
index ce3e11024dfc8..ba9b1eaa818d5 100644
--- a/net/http/http_auth.cc
+++ b/net/http/http_auth.cc
@@ -19,6 +19,7 @@ namespace net {
// static
void HttpAuth::ChooseBestChallenge(const HttpResponseHeaders* headers,
Target target,
+ const GURL& origin,
scoped_refptr<HttpAuthHandler>* handler) {
// A connection-based authentication scheme must continue to use the
// existing handler object in |*handler|.
@@ -30,7 +31,7 @@ void HttpAuth::ChooseBestChallenge(const HttpResponseHeaders* headers,
ChallengeTokenizer props(challenge.begin(), challenge.end());
if (LowerCaseEqualsASCII(props.scheme(), (*handler)->scheme().c_str()) &&
(*handler)->InitFromChallenge(challenge.begin(), challenge.end(),
- target))
+ target, origin))
return;
}
}
@@ -42,7 +43,7 @@ void HttpAuth::ChooseBestChallenge(const HttpResponseHeaders* headers,
void* iter = NULL;
while (headers->EnumerateHeader(&iter, header_name, &cur_challenge)) {
scoped_refptr<HttpAuthHandler> cur;
- CreateAuthHandler(cur_challenge, target, &cur);
+ CreateAuthHandler(cur_challenge, target, origin, &cur);
if (cur && (!best || best->score() < cur->score()))
best.swap(cur);
}
@@ -52,6 +53,7 @@ void HttpAuth::ChooseBestChallenge(const HttpResponseHeaders* headers,
// static
void HttpAuth::CreateAuthHandler(const std::string& challenge,
Target target,
+ const GURL& origin,
scoped_refptr<HttpAuthHandler>* handler) {
// Find the right auth handler for the challenge's scheme.
ChallengeTokenizer props(challenge.begin(), challenge.end());
@@ -70,7 +72,7 @@ void HttpAuth::CreateAuthHandler(const std::string& challenge,
}
if (tmp_handler) {
if (!tmp_handler->InitFromChallenge(challenge.begin(), challenge.end(),
- target)) {
+ target, origin)) {
// Invalid/unsupported challenge.
tmp_handler = NULL;
}
diff --git a/net/http/http_auth.h b/net/http/http_auth.h
index 555769463a88b..38c99187a6cb2 100644
--- a/net/http/http_auth.h
+++ b/net/http/http_auth.h
@@ -75,6 +75,7 @@ class HttpAuth {
// |*handler| is set to NULL.
static void CreateAuthHandler(const std::string& challenge,
Target target,
+ const GURL& origin,
scoped_refptr<HttpAuthHandler>* handler);
// Iterate through the challenge headers, and pick the best one that
@@ -84,11 +85,15 @@ class HttpAuth {
// |*handler| is unchanged. If no supported challenge was found, |*handler|
// is set to NULL.
//
+ // |origin| is used by the NTLM authentication scheme to construct the
+ // service principal name. It is ignored by other schemes.
+ //
// TODO(wtc): Continuing to use the existing handler in |*handler| (for
// NTLM) is new behavior. Rename ChooseBestChallenge to fully encompass
// what it does now.
static void ChooseBestChallenge(const HttpResponseHeaders* headers,
Target target,
+ const GURL& origin,
scoped_refptr<HttpAuthHandler>* handler);
// ChallengeTokenizer breaks up a challenge string into the the auth scheme
diff --git a/net/http/http_auth_handler.cc b/net/http/http_auth_handler.cc
index 6dac1f0eecf22..9abdd9af24349 100644
--- a/net/http/http_auth_handler.cc
+++ b/net/http/http_auth_handler.cc
@@ -9,7 +9,9 @@ namespace net {
bool HttpAuthHandler::InitFromChallenge(std::string::const_iterator begin,
std::string::const_iterator end,
- HttpAuth::Target target) {
+ HttpAuth::Target target,
+ const GURL& origin) {
+ origin_ = origin;
target_ = target;
score_ = -1;
properties_ = -1;
diff --git a/net/http/http_auth_handler.h b/net/http/http_auth_handler.h
index 5ad96d2fbc2d7..1caf1a4c90ae1 100644
--- a/net/http/http_auth_handler.h
+++ b/net/http/http_auth_handler.h
@@ -26,7 +26,8 @@ class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> {
// Initialize the handler by parsing a challenge string.
bool InitFromChallenge(std::string::const_iterator begin,
std::string::const_iterator end,
- HttpAuth::Target target);
+ HttpAuth::Target target,
+ const GURL& origin);
// Lowercase name of the auth scheme
const std::string& scheme() const {
@@ -94,9 +95,13 @@ class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> {
// The lowercase auth-scheme {"basic", "digest", "ntlm", ...}
std::string scheme_;
- // The realm.
+ // The realm. Used by "basic" and "digest".
std::string realm_;
+ // The {scheme, host, port} for the authentication target. Used by "ntlm"
+ // to construct the service principal name.
+ GURL origin_;
+
// The score for this challenge. Higher numbers are better.
int score_;
diff --git a/net/http/http_auth_handler_basic_unittest.cc b/net/http/http_auth_handler_basic_unittest.cc
index a6e4e3d3d96fe..ca2004c8b4041 100644
--- a/net/http/http_auth_handler_basic_unittest.cc
+++ b/net/http/http_auth_handler_basic_unittest.cc
@@ -23,11 +23,12 @@ TEST(HttpAuthHandlerBasicTest, GenerateCredentials) {
// Empty username and empty password.
{ L"", L"", "Basic Og==" },
};
+ GURL origin("http://www.example.com");
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
std::string challenge = "Basic realm=\"Atlantis\"";
scoped_refptr<HttpAuthHandlerBasic> basic = new HttpAuthHandlerBasic;
basic->InitFromChallenge(challenge.begin(), challenge.end(),
- HttpAuth::AUTH_SERVER);
+ HttpAuth::AUTH_SERVER, origin);
std::string credentials = basic->GenerateCredentials(tests[i].username,
tests[i].password,
NULL, NULL);
diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc
index c4abe2605ebbd..5a272925f0031 100644
--- a/net/http/http_auth_handler_digest_unittest.cc
+++ b/net/http/http_auth_handler_digest_unittest.cc
@@ -235,11 +235,12 @@ TEST(HttpAuthHandlerDigestTest, AssembleCredentials) {
"qop=auth, nc=00000001, cnonce=\"15c07961ed8575c4\""
}
};
+ GURL origin("http://www.example.com");
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest;
std::string challenge = tests[i].challenge;
EXPECT_TRUE(digest->InitFromChallenge(
- challenge.begin(), challenge.end(), HttpAuth::AUTH_SERVER));
+ challenge.begin(), challenge.end(), HttpAuth::AUTH_SERVER, origin));
std::string creds = digest->AssembleCredentials(tests[i].req_method,
tests[i].req_path, tests[i].username, tests[i].password,
diff --git a/net/http/http_auth_handler_ntlm_win.cc b/net/http/http_auth_handler_ntlm_win.cc
index d638e8ac76479..79707405e8b40 100644
--- a/net/http/http_auth_handler_ntlm_win.cc
+++ b/net/http/http_auth_handler_ntlm_win.cc
@@ -10,7 +10,9 @@
#include "net/http/http_auth_handler_ntlm.h"
#include "base/logging.h"
+#include "base/string_util.h"
#include "net/base/net_errors.h"
+#include "net/base/net_util.h"
#pragma comment(lib, "secur32.lib")
@@ -134,13 +136,16 @@ int HttpAuthHandlerNTLM::GetNextToken(const void* in_token,
if (!out_buffer.pvBuffer)
return ERR_OUT_OF_MEMORY;
- // Name of the destination server. NULL for NTLM.
- SEC_WCHAR* target = NULL;
+ // The service principal name of the destination server. See
+ // http://msdn.microsoft.com/en-us/library/ms677949%28VS.85%29.aspx
+ std::wstring target(L"HTTP/");
+ target.append(ASCIIToWide(GetHostAndPort(origin_)));
+ wchar_t* target_name = const_cast<wchar_t*>(target.c_str());
// This returns a token that is passed to the remote server.
status = InitializeSecurityContext(&cred_, // phCredential
ctxt_ptr, // phContext
- target, // pszTargetName
+ target_name, // pszTargetName
0, // fContextReq
0, // Reserved1 (must be 0)
SECURITY_NATIVE_DREP, // TargetDataRep
diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc
index 9bd750e76714c..bcd7472bad21c 100644
--- a/net/http/http_auth_unittest.cc
+++ b/net/http/http_auth_unittest.cc
@@ -48,6 +48,7 @@ TEST(HttpAuthTest, ChooseBestChallenge) {
"",
}
};
+ GURL origin("http://www.example.com");
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
// Make a HttpResponseHeaders object.
@@ -62,6 +63,7 @@ TEST(HttpAuthTest, ChooseBestChallenge) {
scoped_refptr<HttpAuthHandler> handler;
HttpAuth::ChooseBestChallenge(headers.get(),
HttpAuth::AUTH_SERVER,
+ origin,
&handler);
if (handler) {
@@ -99,6 +101,7 @@ TEST(HttpAuthTest, ChooseBestChallengeConnectionBased) {
"",
}
};
+ GURL origin("http://www.example.com");
scoped_refptr<HttpAuthHandler> handler;
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
@@ -114,6 +117,7 @@ TEST(HttpAuthTest, ChooseBestChallengeConnectionBased) {
scoped_refptr<HttpAuthHandler> old_handler = handler;
HttpAuth::ChooseBestChallenge(headers.get(),
HttpAuth::AUTH_SERVER,
+ origin,
&handler);
EXPECT_TRUE(handler != NULL);
@@ -229,10 +233,13 @@ TEST(HttpAuthTest, GetAuthorizationHeaderName) {
}
TEST(HttpAuthTest, CreateAuthHandler) {
+ GURL server_origin("http://www.example.com");
+ GURL proxy_origin("http://cache.example.com:3128");
{
scoped_refptr<HttpAuthHandler> handler;
HttpAuth::CreateAuthHandler("Basic realm=\"FooBar\"",
HttpAuth::AUTH_SERVER,
+ server_origin,
&handler);
EXPECT_FALSE(handler.get() == NULL);
EXPECT_STREQ("basic", handler->scheme().c_str());
@@ -245,6 +252,7 @@ TEST(HttpAuthTest, CreateAuthHandler) {
scoped_refptr<HttpAuthHandler> handler;
HttpAuth::CreateAuthHandler("UNSUPPORTED realm=\"FooBar\"",
HttpAuth::AUTH_SERVER,
+ server_origin,
&handler);
EXPECT_TRUE(handler.get() == NULL);
}
@@ -252,6 +260,7 @@ TEST(HttpAuthTest, CreateAuthHandler) {
scoped_refptr<HttpAuthHandler> handler;
HttpAuth::CreateAuthHandler("Digest realm=\"FooBar\", nonce=\"xyz\"",
HttpAuth::AUTH_PROXY,
+ proxy_origin,
&handler);
EXPECT_FALSE(handler.get() == NULL);
EXPECT_STREQ("digest", handler->scheme().c_str());
@@ -264,6 +273,7 @@ TEST(HttpAuthTest, CreateAuthHandler) {
scoped_refptr<HttpAuthHandler> handler;
HttpAuth::CreateAuthHandler("NTLM",
HttpAuth::AUTH_SERVER,
+ server_origin,
&handler);
EXPECT_FALSE(handler.get() == NULL);
EXPECT_STREQ("ntlm", handler->scheme().c_str());
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 8fd0700870cdd..ad9de6aa65626 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -283,45 +283,6 @@ void HttpNetworkTransaction::PrepareForAuthRestart(HttpAuth::Target target) {
// connection even though the server says it's keep-alive.
}
- // If the auth scheme is connection-based but the proxy/server mistakenly
- // marks the connection as non-keep-alive, the auth is going to fail, so log
- // an error message.
- //
- // TODO(wtc): has_auth_identity is not the right condition. We should
- // be testing for "not round 1" here. See http://crbug.com/21015.
- if (!keep_alive && auth_handler_[target]->is_connection_based() &&
- has_auth_identity) {
- LOG(ERROR) << "Can't perform " << auth_handler_[target]->scheme()
- << " auth to the " << AuthTargetString(target) << " "
- << AuthOrigin(target) << " over a non-keep-alive connection";
-
- HttpVersion http_version = response_.headers->GetHttpVersion();
- LOG(ERROR) << " HTTP version is " << http_version.major_value() << "."
- << http_version.minor_value();
-
- std::string header_val;
- void* iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "connection",
- &header_val)) {
- LOG(ERROR) << " Has header Connection: " << header_val;
- }
-
- iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "proxy-connection",
- &header_val)) {
- LOG(ERROR) << " Has header Proxy-Connection: " << header_val;
- }
-
- // RFC 4559 requires that a proxy indicate its support of NTLM/Negotiate
- // authentication with a "Proxy-Support: Session-Based-Authentication"
- // response header.
- iter = NULL;
- while (response_.headers->EnumerateHeader(&iter, "proxy-support",
- &header_val)) {
- LOG(ERROR) << " Has header Proxy-Support: " << header_val;
- }
- }
-
// We don't need to drain the response body, so we act as if we had drained
// the response body.
DidDrainBodyForAuthRestart(keep_alive);
@@ -1914,6 +1875,7 @@ int HttpNetworkTransaction::HandleAuthChallenge() {
// Find the best authentication challenge that we support.
HttpAuth::ChooseBestChallenge(response_.headers.get(),
target,
+ AuthOrigin(target),
&auth_handler_[target]);
}
@@ -1965,18 +1927,10 @@ void HttpNetworkTransaction::PopulateAuthChallenge(HttpAuth::Target target) {
AuthChallengeInfo* auth_info = new AuthChallengeInfo;
auth_info->is_proxy = target == HttpAuth::AUTH_PROXY;
+ auth_info->host_and_port = ASCIIToWide(GetHostAndPort(AuthOrigin(target)));
auth_info->scheme = ASCIIToWide(auth_handler_[target]->scheme());
// TODO(eroman): decode realm according to RFC 2047.
auth_info->realm = ASCIIToWide(auth_handler_[target]->realm());
-
- std::string host_and_port;
- if (target == HttpAuth::AUTH_PROXY) {
- host_and_port = proxy_info_.proxy_server().host_and_port();
- } else {
- DCHECK(target == HttpAuth::AUTH_SERVER);
- host_and_port = GetHostAndPort(request_->url);
- }
- auth_info->host_and_port = ASCIIToWide(host_and_port);
response_.auth_challenge = auth_info;
}
--
GitLab