Don't try to use explicit credentials with schemes that don't support it.

BUG=94617
TEST=net_unittests --gtest_filter=HttpAuthControllerTest.NoExplicitCredentialsAllowed.



Review URL: http://codereview.chromium.org/7748033

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

13 files changed, 249 insertions, 57 deletions

diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index f337c30..477478a 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -316,53 +316,68 @@ int HttpAuthController::HandleAuthChallenge(
 
   bool can_send_auth = (target_ != HttpAuth::AUTH_SERVER ||
                         !do_not_send_server_auth);
-  if (!handler_.get() && can_send_auth) {
-    // Find the best authentication challenge that we support.
-    HttpAuth::ChooseBestChallenge(http_auth_handler_factory_,
-                                  headers, target_, auth_origin_,
-                                  disabled_schemes_, net_log,
-                                  &handler_);
-    if (handler_.get())
-      HistogramAuthEvent(handler_.get(), AUTH_EVENT_START);
-  }
 
-  if (!handler_.get()) {
-    if (establishing_tunnel) {
-      LOG(ERROR) << "Can't perform auth to the "
-                 << HttpAuth::GetAuthTargetString(target_) << " "
-                 << auth_origin_ << " when establishing a tunnel"
-                 << AuthChallengeLogMessage(headers.get());
-
-      // We are establishing a tunnel, we can't show the error page because an
-      // active network attacker could control its contents.  Instead, we just
-      // fail to establish the tunnel.
-      DCHECK(target_ == HttpAuth::AUTH_PROXY);
-      return ERR_PROXY_AUTH_UNSUPPORTED;
+  do {
+    if (!handler_.get() && can_send_auth) {
+      // Find the best authentication challenge that we support.
+      HttpAuth::ChooseBestChallenge(http_auth_handler_factory_,
+                                    headers, target_, auth_origin_,
+                                    disabled_schemes_, net_log,
+                                    &handler_);
+      if (handler_.get())
+        HistogramAuthEvent(handler_.get(), AUTH_EVENT_START);
     }
-    // We found no supported challenge -- let the transaction continue
-    // so we end up displaying the error page.
-    return OK;
-  }
 
-  if (handler_->NeedsIdentity()) {
-    // Pick a new auth identity to try, by looking to the URL and auth cache.
-    // If an identity to try is found, it is saved to identity_.
-    SelectNextAuthIdentityToTry();
-  } else {
-    // Proceed with the existing identity or a null identity.
-    identity_.invalid = false;
-  }
+    if (!handler_.get()) {
+      if (establishing_tunnel) {
+        LOG(ERROR) << "Can't perform auth to the "
+                   << HttpAuth::GetAuthTargetString(target_) << " "
+                   << auth_origin_ << " when establishing a tunnel"
+                   << AuthChallengeLogMessage(headers.get());
+
+        // We are establishing a tunnel, we can't show the error page because an
+        // active network attacker could control its contents.  Instead, we just
+        // fail to establish the tunnel.
+        DCHECK(target_ == HttpAuth::AUTH_PROXY);
+        return ERR_PROXY_AUTH_UNSUPPORTED;
+      }
+      // We found no supported challenge -- let the transaction continue so we
+      // end up displaying the error page.
+      return OK;
+    }
 
-  // From this point on, we are restartable.
+    if (handler_->NeedsIdentity()) {
+      // Pick a new auth identity to try, by looking to the URL and auth cache.
+      // If an identity to try is found, it is saved to identity_.
+      SelectNextAuthIdentityToTry();
+    } else {
+      // Proceed with the existing identity or a null identity.
+      identity_.invalid = false;
+    }
 
-  if (identity_.invalid) {
-    // We have exhausted all identity possibilities, all we can do now is
-    // pass the challenge information back to the client.
-    PopulateAuthChallenge();
-  } else {
-    auth_info_ = NULL;
-  }
+    // From this point on, we are restartable.
+
+    if (identity_.invalid) {
+      // We have exhausted all identity possibilities.
+      if (!handler_->AllowsExplicitCredentials()) {
+        // If the handler doesn't accept explicit credentials, then we need to
+        // choose a different auth scheme.
+        HistogramAuthEvent(handler_.get(), AUTH_EVENT_REJECT);
+        InvalidateCurrentHandler(INVALIDATE_HANDLER_AND_DISABLE_SCHEME);
+      } else {
+        // Pass the challenge information back to the client.
+        PopulateAuthChallenge();
+      }
+    } else {
+      auth_info_ = NULL;
+    }
 
+    // If we get here and we don't have a handler_, that's because we
+    // invalidated it due to not having any viable identities to use with it. Go
+    // back and try again.
+    // TODO(asanka): Instead we should create a priority list of
+    //     <handler,identity> and iterate through that.
+  } while(!handler_.get());
   return OK;
 }
 
@@ -419,9 +434,12 @@ bool HttpAuthController::HaveAuth() const {
 void HttpAuthController::InvalidateCurrentHandler(
     InvalidateHandlerAction action) {
   DCHECK(CalledOnValidThread());
+  DCHECK(handler_.get());
 
   if (action == INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS)
     InvalidateRejectedAuthFromCache();
+  if (action == INVALIDATE_HANDLER_AND_DISABLE_SCHEME)
+    DisableAuthScheme(handler_->auth_scheme());
   handler_.reset();
   identity_ = HttpAuth::Identity();
 }
diff --git a/net/http/http_auth_controller.h b/net/http/http_auth_controller.h
index 4b1e828..8dc9886 100644
--- a/net/http/http_auth_controller.h
+++ b/net/http/http_auth_controller.h
@@ -78,6 +78,7 @@ class NET_EXPORT_PRIVATE HttpAuthController
   // Actions for InvalidateCurrentHandler()
   enum InvalidateHandlerAction {
     INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS,
+    INVALIDATE_HANDLER_AND_DISABLE_SCHEME,
     INVALIDATE_HANDLER
   };
 
diff --git a/net/http/http_auth_controller_unittest.cc b/net/http/http_auth_controller_unittest.cc
index fafc1e7..f93b698 100644
--- a/net/http/http_auth_controller_unittest.cc
+++ b/net/http/http_auth_controller_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "net/http/http_auth_controller.h"
 
+#include "base/utf_string_conversions.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/test_completion_callback.h"
@@ -28,6 +29,15 @@ enum SchemeState {
   SCHEME_IS_ENABLED
 };
 
+scoped_refptr<HttpResponseHeaders> HeadersFromString(const char* string) {
+  std::string raw_string(string);
+  std::string headers_string = HttpUtil::AssembleRawHeaders(
+      raw_string.c_str(), raw_string.length());
+  scoped_refptr<HttpResponseHeaders> headers(
+      new HttpResponseHeaders(headers_string));
+  return headers;
+}
+
 // Runs an HttpAuthController with a single round mock auth handler
 // that returns |handler_rv| on token generation.  The handler runs in
 // async if |run_mode| is RUN_HANDLER_ASYNC.  Upon completion, the
@@ -45,14 +55,10 @@ void RunSingleRoundAuthTest(HandlerRunMode run_mode,
   request.method = "GET";
   request.url = GURL("http://example.com");
 
-  const std::string headers_raw_string =
+  scoped_refptr<HttpResponseHeaders> headers(HeadersFromString(
       "HTTP/1.1 407\r\n"
       "Proxy-Authenticate: MOCK foo\r\n"
-      "\r\n";
-  std::string headers_string = HttpUtil::AssembleRawHeaders(
-      headers_raw_string.c_str(), headers_raw_string.length());
-  scoped_refptr<HttpResponseHeaders> headers(
-      new HttpResponseHeaders(headers_string));
+      "\r\n"));
 
   HttpAuthHandlerMock::Factory auth_handler_factory;
   HttpAuthHandlerMock* auth_handler = new HttpAuthHandlerMock();
@@ -67,7 +73,7 @@ void RunSingleRoundAuthTest(HandlerRunMode run_mode,
                              &dummy_auth_cache, &auth_handler_factory));
   ASSERT_EQ(OK,
             controller->HandleAuthChallenge(headers, false, false, dummy_log));
-  EXPECT_TRUE(controller->HaveAuthHandler());
+  ASSERT_TRUE(controller->HaveAuthHandler());
   controller->ResetAuth(string16(), string16());
   EXPECT_TRUE(controller->HaveAuth());
 
@@ -109,4 +115,121 @@ TEST(HttpAuthControllerTest, PermanentErrors) {
                          ERR_INVALID_AUTH_CREDENTIALS, SCHEME_IS_ENABLED);
 }
 
+// If an HttpAuthHandler indicates that it doesn't allow explicit
+// credentials, don't prompt for credentials.
+TEST(HttpAuthControllerTest, NoExplicitCredentialsAllowed) {
+  // Modified mock HttpAuthHandler for this test.
+  class MockHandler : public HttpAuthHandlerMock {
+   public:
+    MockHandler(int expected_rv, HttpAuth::Scheme scheme)
+        : expected_scheme_(scheme) {
+      SetGenerateExpectation(false, expected_rv);
+    }
+
+   protected:
+    virtual bool Init(HttpAuth::ChallengeTokenizer* challenge) OVERRIDE {
+      HttpAuthHandlerMock::Init(challenge);
+      set_allows_default_credentials(true);
+      set_allows_explicit_credentials(false);
+      set_connection_based(true);
+      // Pretend to be SCHEME_BASIC so we can test failover logic.
+      if (challenge->scheme() == "Basic") {
+        auth_scheme_ = HttpAuth::AUTH_SCHEME_BASIC;
+        --score_;  // Reduce score, so we rank below Mock.
+        set_allows_explicit_credentials(true);
+      }
+      EXPECT_EQ(expected_scheme_, auth_scheme_);
+      return true;
+    }
+
+    virtual int GenerateAuthTokenImpl(const string16* username,
+                                      const string16* password,
+                                      const HttpRequestInfo* request,
+                                      CompletionCallback* callback,
+                                      std::string* auth_token) OVERRIDE {
+      int result =
+          HttpAuthHandlerMock::GenerateAuthTokenImpl(username, password,
+                                                     request, callback,
+                                                     auth_token);
+      EXPECT_TRUE(result != OK ||
+                  !AllowsExplicitCredentials() || !username->empty());
+      return result;
+    }
+
+   private:
+    HttpAuth::Scheme expected_scheme_;
+  };
+
+  BoundNetLog dummy_log;
+  HttpAuthCache dummy_auth_cache;
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://example.com");
+
+  HttpRequestHeaders request_headers;
+  scoped_refptr<HttpResponseHeaders> headers(HeadersFromString(
+      "HTTP/1.1 401\r\n"
+      "WWW-Authenticate: Mock\r\n"
+      "WWW-Authenticate: Basic\r\n"
+      "\r\n"));
+
+  HttpAuthHandlerMock::Factory auth_handler_factory;
+
+  // Handlers for the first attempt at authentication.  AUTH_SCHEME_MOCK handler
+  // accepts the default identity and successfully constructs a token.
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(OK, HttpAuth::AUTH_SCHEME_MOCK), HttpAuth::AUTH_SERVER);
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(ERR_UNEXPECTED, HttpAuth::AUTH_SCHEME_BASIC),
+      HttpAuth::AUTH_SERVER);
+
+  // Handlers for the second attempt.  Neither should be used to generate a
+  // token.  Instead the controller should realize that there are no viable
+  // identities to use with the AUTH_SCHEME_MOCK handler and fail.
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(ERR_UNEXPECTED, HttpAuth::AUTH_SCHEME_MOCK),
+      HttpAuth::AUTH_SERVER);
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(ERR_UNEXPECTED, HttpAuth::AUTH_SCHEME_BASIC),
+      HttpAuth::AUTH_SERVER);
+
+  // Fallback handlers for the second attempt.  The AUTH_SCHEME_MOCK handler
+  // should be discarded due to the disabled scheme, and the AUTH_SCHEME_BASIC
+  // handler should successfully be used to generate a token.
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(ERR_UNEXPECTED, HttpAuth::AUTH_SCHEME_MOCK),
+      HttpAuth::AUTH_SERVER);
+  auth_handler_factory.AddMockHandler(
+      new MockHandler(OK, HttpAuth::AUTH_SCHEME_BASIC),
+      HttpAuth::AUTH_SERVER);
+  auth_handler_factory.set_do_init_from_challenge(true);
+
+  scoped_refptr<HttpAuthController> controller(
+      new HttpAuthController(HttpAuth::AUTH_SERVER,
+                             GURL("http://example.com"),
+                             &dummy_auth_cache, &auth_handler_factory));
+  ASSERT_EQ(OK,
+            controller->HandleAuthChallenge(headers, false, false, dummy_log));
+  ASSERT_TRUE(controller->HaveAuthHandler());
+  controller->ResetAuth(string16(), string16());
+  EXPECT_TRUE(controller->HaveAuth());
+
+  // Should only succeed if we are using the AUTH_SCHEME_MOCK MockHandler.
+  EXPECT_EQ(OK, controller->MaybeGenerateAuthToken(&request, NULL, dummy_log));
+  controller->AddAuthorizationHeader(&request_headers);
+
+  // Once a token is generated, simulate the receipt of a server response
+  // indicating that the authentication attempt was rejected.
+  ASSERT_EQ(OK,
+            controller->HandleAuthChallenge(headers, false, false, dummy_log));
+  ASSERT_TRUE(controller->HaveAuthHandler());
+  controller->ResetAuth(ASCIIToUTF16("Hello"), string16());
+  EXPECT_TRUE(controller->HaveAuth());
+  EXPECT_TRUE(controller->IsAuthSchemeDisabled(HttpAuth::AUTH_SCHEME_MOCK));
+  EXPECT_FALSE(controller->IsAuthSchemeDisabled(HttpAuth::AUTH_SCHEME_BASIC));
+
+  // Should only succeed if we are using the AUTH_SCHEME_BASIC MockHandler.
+  EXPECT_EQ(OK, controller->MaybeGenerateAuthToken(&request, NULL, dummy_log));
+}
+
 }  // namespace net
diff --git a/net/http/http_auth_gssapi_posix.cc b/net/http/http_auth_gssapi_posix.cc
index 37b28df..431cfc4 100644
--- a/net/http/http_auth_gssapi_posix.cc
+++ b/net/http/http_auth_gssapi_posix.cc
@@ -674,6 +674,10 @@ bool HttpAuthGSSAPI::NeedsIdentity() const {
   return decoded_server_auth_token_.empty();
 }
 
+bool HttpAuthGSSAPI::AllowsExplicitCredentials() const {
+  return false;
+}
+
 void HttpAuthGSSAPI::Delegate() {
   can_delegate_ = true;
 }
diff --git a/net/http/http_auth_gssapi_posix.h b/net/http/http_auth_gssapi_posix.h
index 8e8a114..3367713 100644
--- a/net/http/http_auth_gssapi_posix.h
+++ b/net/http/http_auth_gssapi_posix.h
@@ -234,6 +234,8 @@ class NET_EXPORT_PRIVATE HttpAuthGSSAPI {
 
   bool NeedsIdentity() const;
 
+  bool AllowsExplicitCredentials() const;
+
   HttpAuth::AuthorizationResult ParseChallenge(
       HttpAuth::ChallengeTokenizer* tok);
 
diff --git a/net/http/http_auth_handler.cc b/net/http/http_auth_handler.cc
index 51bd6aa..d123c21 100644
--- a/net/http/http_auth_handler.cc
+++ b/net/http/http_auth_handler.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -90,6 +90,10 @@ bool HttpAuthHandler::AllowsDefaultCredentials() {
   return false;
 }
 
+bool HttpAuthHandler::AllowsExplicitCredentials() {
+  return true;
+}
+
 void HttpAuthHandler::OnGenerateAuthTokenComplete(int rv) {
   CompletionCallback* callback = original_callback_;
   FinishGenerateAuthToken();
diff --git a/net/http/http_auth_handler.h b/net/http/http_auth_handler.h
index f4e35a4..8f237e8 100644
--- a/net/http/http_auth_handler.h
+++ b/net/http/http_auth_handler.h
@@ -135,6 +135,11 @@ class NET_EXPORT_PRIVATE HttpAuthHandler {
   // TODO(cbentzel): Add a pointer to Firefox documentation about risk.
   virtual bool AllowsDefaultCredentials();
 
+  // Returns whether explicit credentials can be used with this handler.  If
+  // true the user may be prompted for credentials if an implicit identity
+  // cannot be determined.
+  virtual bool AllowsExplicitCredentials();
+
  protected:
   enum Property {
     ENCRYPTS_IDENTITY = 1 << 0,
diff --git a/net/http/http_auth_handler_mock.cc b/net/http/http_auth_handler_mock.cc
index 9887937..9331ae7 100644
--- a/net/http/http_auth_handler_mock.cc
+++ b/net/http/http_auth_handler_mock.cc
@@ -18,7 +18,9 @@ HttpAuthHandlerMock::HttpAuthHandlerMock()
     generate_async_(false), generate_rv_(OK),
     auth_token_(NULL),
     first_round_(true),
-    connection_based_(false) {
+    connection_based_(false),
+    allows_default_credentials_(false),
+    allows_explicit_credentials_(true) {
 }
 
 HttpAuthHandlerMock::~HttpAuthHandlerMock() {
@@ -73,7 +75,9 @@ void HttpAuthHandlerMock::SetGenerateExpectation(bool async, int rv) {
 
 HttpAuth::AuthorizationResult HttpAuthHandlerMock::HandleAnotherChallenge(
     HttpAuth::ChallengeTokenizer* challenge) {
-  if (!is_connection_based())
+  // If we receive an empty challenge for a connection based scheme, or a second
+  // challenge for a non connection based scheme, assume it's a rejection.
+  if (!is_connection_based() || challenge->base64_param().empty())
     return HttpAuth::AUTHORIZATION_RESULT_REJECT;
   if (!LowerCaseEqualsASCII(challenge->scheme(), "mock"))
     return HttpAuth::AUTHORIZATION_RESULT_INVALID;
@@ -84,6 +88,14 @@ bool HttpAuthHandlerMock::NeedsIdentity() {
   return first_round_;
 }
 
+bool HttpAuthHandlerMock::AllowsDefaultCredentials() {
+  return allows_default_credentials_;
+}
+
+bool HttpAuthHandlerMock::AllowsExplicitCredentials() {
+  return allows_explicit_credentials_;
+}
+
 bool HttpAuthHandlerMock::Init(HttpAuth::ChallengeTokenizer* challenge) {
   auth_scheme_ = HttpAuth::AUTH_SCHEME_MOCK;
   score_ = 1;
diff --git a/net/http/http_auth_handler_mock.h b/net/http/http_auth_handler_mock.h
index e7d5997..1406239 100644
--- a/net/http/http_auth_handler_mock.h
+++ b/net/http/http_auth_handler_mock.h
@@ -75,14 +75,24 @@ class HttpAuthHandlerMock : public HttpAuthHandler {
     connection_based_ = connection_based;
   }
 
+  void set_allows_default_credentials(bool allows_default_credentials) {
+    allows_default_credentials_ = allows_default_credentials;
+  }
+
+  void set_allows_explicit_credentials(bool allows_explicit_credentials) {
+    allows_explicit_credentials_ = allows_explicit_credentials;
+  }
+
   const GURL& request_url() const {
     return request_url_;
   }
 
   // HttpAuthHandler:
   virtual HttpAuth::AuthorizationResult HandleAnotherChallenge(
-      HttpAuth::ChallengeTokenizer* challenge);
-  virtual bool NeedsIdentity();
+      HttpAuth::ChallengeTokenizer* challenge) OVERRIDE;
+  virtual bool NeedsIdentity() OVERRIDE;
+  virtual bool AllowsDefaultCredentials() OVERRIDE;
+  virtual bool AllowsExplicitCredentials() OVERRIDE;
 
  protected:
   virtual bool Init(HttpAuth::ChallengeTokenizer* challenge);
@@ -106,6 +116,8 @@ class HttpAuthHandlerMock : public HttpAuthHandler {
   std::string* auth_token_;
   bool first_round_;
   bool connection_based_;
+  bool allows_default_credentials_;
+  bool allows_explicit_credentials_;
   GURL request_url_;
 };
 
diff --git a/net/http/http_auth_handler_negotiate.cc b/net/http/http_auth_handler_negotiate.cc
index 52a2b1e..d5d0f80 100644
--- a/net/http/http_auth_handler_negotiate.cc
+++ b/net/http/http_auth_handler_negotiate.cc
@@ -182,6 +182,10 @@ bool HttpAuthHandlerNegotiate::AllowsDefaultCredentials() {
   return url_security_manager_->CanUseDefaultCredentials(origin_);
 }
 
+bool HttpAuthHandlerNegotiate::AllowsExplicitCredentials() {
+  return auth_system_.AllowsExplicitCredentials();
+}
+
 // The Negotiate challenge header looks like:
 //   WWW-Authenticate: NEGOTIATE auth-data
 bool HttpAuthHandlerNegotiate::Init(HttpAuth::ChallengeTokenizer* challenge) {
diff --git a/net/http/http_auth_handler_negotiate.h b/net/http/http_auth_handler_negotiate.h
index 1727ec8..2397bdb 100644
--- a/net/http/http_auth_handler_negotiate.h
+++ b/net/http/http_auth_handler_negotiate.h
@@ -108,9 +108,10 @@ class NET_EXPORT_PRIVATE HttpAuthHandlerNegotiate : public HttpAuthHandler {
 
   // HttpAuthHandler:
   virtual HttpAuth::AuthorizationResult HandleAnotherChallenge(
-      HttpAuth::ChallengeTokenizer* challenge);
-  virtual bool NeedsIdentity();
-  virtual bool AllowsDefaultCredentials();
+      HttpAuth::ChallengeTokenizer* challenge) OVERRIDE;
+  virtual bool NeedsIdentity() OVERRIDE;
+  virtual bool AllowsDefaultCredentials() OVERRIDE;
+  virtual bool AllowsExplicitCredentials() OVERRIDE;
 
  protected:
   virtual bool Init(HttpAuth::ChallengeTokenizer* challenge);
diff --git a/net/http/http_auth_sspi_win.cc b/net/http/http_auth_sspi_win.cc
index 1a629f1..d07ce4c 100644
--- a/net/http/http_auth_sspi_win.cc
+++ b/net/http/http_auth_sspi_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -210,6 +210,10 @@ bool HttpAuthSSPI::NeedsIdentity() const {
   return decoded_server_auth_token_.empty();
 }
 
+bool HttpAuthSSPI::AllowsExplicitCredentials() const {
+  return true;
+}
+
 void HttpAuthSSPI::Delegate() {
   can_delegate_ = true;
 }
diff --git a/net/http/http_auth_sspi_win.h b/net/http/http_auth_sspi_win.h
index 1a6e0af..b3ef3c9 100644
--- a/net/http/http_auth_sspi_win.h
+++ b/net/http/http_auth_sspi_win.h
@@ -132,6 +132,8 @@ class NET_EXPORT_PRIVATE HttpAuthSSPI {
 
   bool NeedsIdentity() const;
 
+  bool AllowsExplicitCredentials() const;
+
   HttpAuth::AuthorizationResult ParseChallenge(
       HttpAuth::ChallengeTokenizer* tok);