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

@@ -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;
     }

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

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;

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_;
 

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);

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,

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

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());

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;
 }