Make GetHostNameProc return a std::string.

Add the ScopedProcSetter helper class so that unit tests
restore the original GenerateRandom and GetHostName
functions on completion.

Merge NTLMAuthModule into HttpAuthHandlerNTLM.  The data
members domain_, username_, and password_ are moved.  The
Init method is inlined at its only call site.  The
GetNextToken method is moved.

Make generate_random_proc_ and get_host_name_proc_ static
members of the HttpAuthHandlerNTLM class.

R=eroman
BUG=6567,6824
Review URL: http://codereview.chromium.org/43113

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@12290 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 113 insertions, 123 deletions

diff --git a/net/http/http_auth_handler_ntlm.cc b/net/http/http_auth_handler_ntlm.cc
index 533b484..b409b63 100644
--- a/net/http/http_auth_handler_ntlm.cc
+++ b/net/http/http_auth_handler_ntlm.cc
@@ -18,6 +18,7 @@
 #include "base/sys_string_conversions.h"
 #include "net/base/base64.h"
 #include "net/base/net_errors.h"
+#include "net/base/net_util.h"
 #include "net/http/des.h"
 #include "net/http/md4.h"
 
@@ -439,28 +440,16 @@ static void GenerateRandom(uint8* output, size_t n) {
     output[i] = base::RandInt(0, 255);
 }
 
-static void GetHostName(char* name, size_t namelen) {
-  if (gethostname(name, namelen) != 0)
-    name[0] = '\0';
-}
-
-// TODO(wtc): these two function pointers should become static members of
-// HttpAuthHandlerNTLM.  They are file-scope static variables now so that
-// GenerateType3Msg can use them without being a friend function.  We should
-// have HttpAuthHandlerNTLM absorb NTLMAuthModule and pass the host name and
-// random bytes as input arguments to GenerateType3Msg.
-static HttpAuthHandlerNTLM::GenerateRandomProc generate_random_proc_ =
-    GenerateRandom;
-static HttpAuthHandlerNTLM::HostNameProc get_host_name_proc_ = GetHostName;
-
 // Returns OK or a network error code.
 static int GenerateType3Msg(const string16& domain,
                             const string16& username,
                             const string16& password,
-                            const void*     in_buf,
-                            uint32          in_len,
-                            void**          out_buf,
-                            uint32*         out_len) {
+                            const std::string& hostname,
+                            const void* rand_8_bytes,
+                            const void* in_buf,
+                            uint32 in_len,
+                            void** out_buf,
+                            uint32* out_len) {
   // in_buf contains Type-2 msg (the challenge) from server.
 
   int rv;
@@ -478,7 +467,7 @@ static int GenerateType3Msg(const string16& domain,
 #endif
   string16 ucs_host_buf;
   // Temporary buffers for oem strings
-  std::string oem_domain_buf, oem_user_buf, oem_host_buf;
+  std::string oem_domain_buf, oem_user_buf;
   // Pointers and lengths for the string buffers; encoding is unicode if
   // the "negotiate unicode" flag was set in the Type-2 message.
   const void* domain_ptr;
@@ -529,14 +518,9 @@ static int GenerateType3Msg(const string16& domain,
   //
   // Get workstation name (use local machine's hostname).
   //
-  char host_buf[256];  // Host names are limited to 255 bytes.
-  get_host_name_proc_(host_buf, sizeof(host_buf));
-  host_len = strlen(host_buf);
-  if (host_len == 0)
-    return ERR_UNEXPECTED;
   if (unicode) {
     // hostname is ASCII, so we can do a simple zero-pad expansion:
-    ucs_host_buf.assign(host_buf, host_buf + host_len);
+    ucs_host_buf.assign(hostname.begin(), hostname.end());
     host_ptr = ucs_host_buf.data();
     host_len = ucs_host_buf.length() * 2;
 #ifdef IS_BIG_ENDIAN
@@ -544,7 +528,8 @@ static int GenerateType3Msg(const string16& domain,
                    ucs_host_buf.length());
 #endif
   } else {
-    host_ptr = host_buf;
+    host_ptr = hostname.data();
+    host_len = hostname.length();
   }
 
   //
@@ -567,7 +552,7 @@ static int GenerateType3Msg(const string16& domain,
     MD5Digest session_hash;
     uint8 temp[16];
 
-    generate_random_proc_(lm_resp, 8);
+    memcpy(lm_resp, rand_8_bytes, 8);
     memset(lm_resp + 8, 0, LM_RESP_LEN - 8);
 
     memcpy(temp, msg.challenge, 8);
@@ -638,71 +623,24 @@ static int GenerateType3Msg(const string16& domain,
   return OK;
 }
 
-//-----------------------------------------------------------------------------
-
-class NTLMAuthModule {
- public:
-  NTLMAuthModule() {}
-
-  ~NTLMAuthModule();
-
-  int Init(const string16& domain,
-           const string16& username,
-           const string16& password);
-
-  int GetNextToken(const void* in_token,
-                   uint32 in_token_len,
-                   void** out_token,
-                   uint32* out_token_len);
-
- private:
-  string16 domain_;
-  string16 username_;
-  string16 password_;
-};
-
-NTLMAuthModule::~NTLMAuthModule() {
-  ZapString(&password_);
-}
-
-int NTLMAuthModule::Init(const string16& domain,
-                         const string16& username,
-                         const string16& password) {
-  domain_ = domain;
-  username_ = username;
-  password_ = password;
-  return OK;
-}
-
-int NTLMAuthModule::GetNextToken(const void* in_token,
-                                 uint32 in_token_len,
-                                 void** out_token,
-                                 uint32* out_token_len) {
-  int rv;
-
-  // If in_token is non-null, then assume it contains a type 2 message...
-  if (in_token) {
-    LogToken("in-token", in_token, in_token_len);
-    rv = GenerateType3Msg(domain_, username_, password_, in_token,
-                          in_token_len, out_token, out_token_len);
-  } else {
-    rv = GenerateType1Msg(out_token, out_token_len);
-  }
-
-  if (rv == OK)
-    LogToken("out-token", *out_token, *out_token_len);
-
-  return rv;
-}
-
 // NTLM authentication is specified in "NTLM Over HTTP Protocol Specification"
 // [MS-NTHT].
 
-HttpAuthHandlerNTLM::HttpAuthHandlerNTLM()
-    : ntlm_module_(new NTLMAuthModule) {
+// static
+HttpAuthHandlerNTLM::GenerateRandomProc
+HttpAuthHandlerNTLM::generate_random_proc_ = GenerateRandom;
+
+// static
+HttpAuthHandlerNTLM::HostNameProc
+HttpAuthHandlerNTLM::get_host_name_proc_ = GetMyHostName;
+
+HttpAuthHandlerNTLM::HttpAuthHandlerNTLM() {
 }
 
 HttpAuthHandlerNTLM::~HttpAuthHandlerNTLM() {
+  // Wipe our copy of the password from memory, to reduce the chance of being
+  // written to the paging file on disk.
+  ZapString(&password_);
 }
 
 bool HttpAuthHandlerNTLM::NeedsIdentity() {
@@ -714,7 +652,6 @@ std::string HttpAuthHandlerNTLM::GenerateCredentials(
     const std::wstring& password,
     const HttpRequestInfo* request,
     const ProxyInfo* proxy) {
-  int rv;
   // TODO(wtc): See if we can use char* instead of void* for in_buf and
   // out_buf.  This change will need to propagate to GetNextToken,
   // GenerateType1Msg, and GenerateType3Msg, and perhaps further.
@@ -734,8 +671,9 @@ std::string HttpAuthHandlerNTLM::GenerateCredentials(
     domain = username.substr(0, backslash_idx);
     user = username.substr(backslash_idx + 1);
   }
-  rv = ntlm_module_->Init(WideToUTF16(domain), WideToUTF16(user),
-                          WideToUTF16(password));
+  domain_ = WideToUTF16(domain);
+  username_ = WideToUTF16(user);
+  password_ = WideToUTF16(password);
 
   // Initial challenge.
   if (auth_data_.empty()) {
@@ -759,7 +697,7 @@ std::string HttpAuthHandlerNTLM::GenerateCredentials(
     in_buf = decoded_auth_data.data();
   }
 
-  rv = ntlm_module_->GetNextToken(in_buf, in_buf_len, &out_buf, &out_buf_len);
+  int rv = GetNextToken(in_buf, in_buf_len, &out_buf, &out_buf_len);
   if (rv != OK)
     return std::string();
 
@@ -775,13 +713,20 @@ std::string HttpAuthHandlerNTLM::GenerateCredentials(
 }
 
 // static
-void HttpAuthHandlerNTLM::SetGenerateRandomProc(GenerateRandomProc proc) {
+HttpAuthHandlerNTLM::GenerateRandomProc
+HttpAuthHandlerNTLM::SetGenerateRandomProc(
+    GenerateRandomProc proc) {
+  GenerateRandomProc old_proc = generate_random_proc_;
   generate_random_proc_ = proc;
+  return old_proc;
 }
 
 // static
-void HttpAuthHandlerNTLM::SetHostNameProc(HostNameProc proc) {
+HttpAuthHandlerNTLM::HostNameProc HttpAuthHandlerNTLM::SetHostNameProc(
+    HostNameProc proc) {
+  HostNameProc old_proc = get_host_name_proc_;
   get_host_name_proc_ = proc;
+  return old_proc;
 }
 
 // The NTLM challenge header looks like:
@@ -811,4 +756,30 @@ bool HttpAuthHandlerNTLM::ParseChallenge(
   return true;
 }
 
+int HttpAuthHandlerNTLM::GetNextToken(const void* in_token,
+                                      uint32 in_token_len,
+                                      void** out_token,
+                                      uint32* out_token_len) {
+  int rv;
+
+  // If in_token is non-null, then assume it contains a type 2 message...
+  if (in_token) {
+    LogToken("in-token", in_token, in_token_len);
+    std::string hostname = get_host_name_proc_();
+    if (hostname.empty())
+      return ERR_UNEXPECTED;
+    uint8 rand_buf[8];
+    generate_random_proc_(rand_buf, 8);
+    rv = GenerateType3Msg(domain_, username_, password_, hostname, rand_buf,
+                          in_token, in_token_len, out_token, out_token_len);
+  } else {
+    rv = GenerateType1Msg(out_token, out_token_len);
+  }
+
+  if (rv == OK)
+    LogToken("out-token", *out_token, *out_token_len);
+
+  return rv;
+}
+
 }  // namespace net
diff --git a/net/http/http_auth_handler_ntlm.h b/net/http/http_auth_handler_ntlm.h
index 00c246a..9b8a3b1 100644
--- a/net/http/http_auth_handler_ntlm.h
+++ b/net/http/http_auth_handler_ntlm.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
+#include "base/string16.h"
 #include "net/http/http_auth_handler.h"
 
 namespace net {
@@ -21,11 +22,29 @@ class HttpAuthHandlerNTLM : public HttpAuthHandler {
   // A function that generates n random bytes in the output buffer.
   typedef void (*GenerateRandomProc)(uint8* output, size_t n);
 
-  // A function that returns the local host name as a null-terminated string
-  // in the output buffer. Returns an empty string if the local host name is
-  // not available.
-  // TODO(wtc): return a std::string instead.
-  typedef void (*HostNameProc)(char* name, size_t namelen);
+  // A function that returns the local host name. Returns an empty string if
+  // the local host name is not available.
+  typedef std::string (*HostNameProc)();
+
+  // For unit tests to override and restore the GenerateRandom and
+  // GetHostName functions.
+  class ScopedProcSetter {
+   public:
+    ScopedProcSetter(GenerateRandomProc random_proc,
+                     HostNameProc host_name_proc) {
+      old_random_proc_ = SetGenerateRandomProc(random_proc);
+      old_host_name_proc_ = SetHostNameProc(host_name_proc);
+    }
+
+    ~ScopedProcSetter() {
+      SetGenerateRandomProc(old_random_proc_);
+      SetHostNameProc(old_host_name_proc_);
+    }
+
+   private:
+    GenerateRandomProc old_random_proc_;
+    HostNameProc old_host_name_proc_;
+  };
 
   HttpAuthHandlerNTLM();
 
@@ -38,10 +57,6 @@ class HttpAuthHandlerNTLM : public HttpAuthHandler {
                                           const HttpRequestInfo* request,
                                           const ProxyInfo* proxy);
 
-  // For unit tests to override the GenerateRandom and GetHostName functions.
-  static void SetGenerateRandomProc(GenerateRandomProc proc);
-  static void SetHostNameProc(HostNameProc proc);
-
  protected:
   virtual bool Init(std::string::const_iterator challenge_begin,
                     std::string::const_iterator challenge_end) {
@@ -49,20 +64,29 @@ class HttpAuthHandlerNTLM : public HttpAuthHandler {
   }
 
  private:
+  // For unit tests to override the GenerateRandom and GetHostName functions.
+  // Returns the old function.
+  static GenerateRandomProc SetGenerateRandomProc(GenerateRandomProc proc);
+  static HostNameProc SetHostNameProc(HostNameProc proc);
+
   // Parse the challenge, saving the results into this instance.
   // Returns true on success.
   bool ParseChallenge(std::string::const_iterator challenge_begin,
                       std::string::const_iterator challenge_end);
 
-  // The actual implementation of NTLM.
-  //
-  // TODO(wtc): This artificial separation of the NTLM auth module from the
-  // NTLM auth handler comes from the Mozilla code.  It is due to an
-  // architecture constraint of Mozilla's (all crypto code must reside in the
-  // "PSM" component), so that the NTLM code, which does crypto, must be
-  // separated from the "netwerk" component.  Our source tree doesn't have
-  // this constraint, so we may want to merge NTLMAuthModule into this class.
-  scoped_ptr<NTLMAuthModule> ntlm_module_;
+  // Given an input token received from the server, generate the next output
+  // token to be sent to the server.
+  int GetNextToken(const void* in_token,
+                   uint32 in_token_len,
+                   void** out_token,
+                   uint32* out_token_len);
+
+  static GenerateRandomProc generate_random_proc_;
+  static HostNameProc get_host_name_proc_;
+
+  string16 domain_;
+  string16 username_;
+  string16 password_;
 
   // The base64-encoded string following "NTLM" in the "WWW-Authenticate" or
   // "Proxy-Authenticate" response header.
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 340dfe4..20e845e 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -302,7 +302,7 @@ void FillLargeHeadersString(std::string* str, int size) {
 
 // Alternative functions that eliminate randomness and dependency on the local
 // host name so that the generated NTLM messages are reproducible.
-void MyGenerateRandom1(uint8* output, size_t n) {
+void MockGenerateRandom1(uint8* output, size_t n) {
   static const uint8 bytes[] = {
     0x55, 0x29, 0x66, 0x26, 0x6b, 0x9c, 0x73, 0x54
   };
@@ -313,7 +313,7 @@ void MyGenerateRandom1(uint8* output, size_t n) {
   }
 }
 
-void MyGenerateRandom2(uint8* output, size_t n) {
+void MockGenerateRandom2(uint8* output, size_t n) {
   static const uint8 bytes[] = {
     0x96, 0x79, 0x85, 0xe7, 0x49, 0x93, 0x70, 0xa1,
     0x4e, 0xe7, 0x87, 0x45, 0x31, 0x5b, 0xd3, 0x1f
@@ -325,13 +325,8 @@ void MyGenerateRandom2(uint8* output, size_t n) {
   }
 }
 
-void MyGetHostName(char* name, size_t namelen) {
-  static const char hostname[] = "WTC-WIN7";
-  if (namelen >= arraysize(hostname)) {
-    memcpy(name, hostname, arraysize(hostname));
-  } else {
-    name[0] = '\0';
-  }
+std::string MockGetHostName() {
+  return "WTC-WIN7";
 }
 
 //-----------------------------------------------------------------------------
@@ -1580,8 +1575,8 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) {
 
 // Enter the correct password and authenticate successfully.
 TEST_F(HttpNetworkTransactionTest, NTLMAuth1) {
-  net::HttpAuthHandlerNTLM::SetGenerateRandomProc(MyGenerateRandom1);
-  net::HttpAuthHandlerNTLM::SetHostNameProc(MyGetHostName);
+  net::HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockGenerateRandom1,
+                                                         MockGetHostName);
 
   scoped_ptr<net::ProxyService> proxy_service(CreateNullProxyService());
   scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
@@ -1702,8 +1697,8 @@ TEST_F(HttpNetworkTransactionTest, NTLMAuth1) {
 
 // Enter a wrong password, and then the correct one.
 TEST_F(HttpNetworkTransactionTest, NTLMAuth2) {
-  net::HttpAuthHandlerNTLM::SetGenerateRandomProc(MyGenerateRandom2);
-  net::HttpAuthHandlerNTLM::SetHostNameProc(MyGetHostName);
+  net::HttpAuthHandlerNTLM::ScopedProcSetter proc_setter(MockGenerateRandom2,
+                                                         MockGetHostName);
 
   scoped_ptr<net::ProxyService> proxy_service(CreateNullProxyService());
   scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(