net: make HSTS hosts use the normal SSL interstitials

net/url_request/url_request_http_job.cc

 // 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.
 
 #include "net/url_request/url_request_http_job.h"
 
 #include "base/bind.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 671 matching lines @@
       }
     }
   }
 #endif
 
   if (result == OK) {
     SaveCookiesAndNotifyHeadersComplete();
   } else if (ShouldTreatAsCertificateError(result)) {
     // We encountered an SSL certificate error.  Ask our delegate to decide
     // what we should do.
-    // TODO(wtc): also pass ssl_info.cert_status, or just pass the whole
-    // ssl_info.

[wtc, 2011/09/23 00:04:51]

The ssl_info structure has become much bigger than it was
when I wrote this comment.  So I was wondering if it's better
to pass only ssl_info.cert_status.

After I reviewed the entire CL, I believe it is better to
pass ssl_info.

+
+    TransportSecurityState::DomainState domain_state;
+    const bool r = context_->transport_security_state()->IsEnabledForHost(

[wtc, 2011/09/23 00:04:51]

Please pick a better variable name than |r|.

+        &domain_state, request_info_.url.host(),
+        SSLConfigService::IsSNIAvailable(context_->ssl_config_service()));
+    // ERR_CERT_UNABLE_TO_CHECK_REVOCATION isn't forced to fatal, even for HSTS
+    // sites, because it happens due to transient network issues.
+    bool must_be_fatal = r && result != ERR_CERT_UNABLE_TO_CHECK_REVOCATION;
+
     NotifySSLCertificateError(
-        result, transaction_->GetResponseInfo()->ssl_info.cert);
+        result, transaction_->GetResponseInfo()->ssl_info, must_be_fatal);

[wtc, 2011/09/23 00:04:51]

In our current design of SSL error handling, I believe it is
better to simply pass the boolean info "is HSTS host" to
NotifySSLCertificateError, and let the SSL policy decide
whether the certificate error should be fatal.

   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     NotifyCertificateRequested(
         transaction_->GetResponseInfo()->cert_request_info);
   } else {
     NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, result));
   }
 }
 
 void URLRequestHttpJob::OnReadCompleted(int result) {
   read_in_progress_ = false;
 
   if (ShouldFixMismatchedContentLength(result))
     result = 0;
 
   if (result == 0) {
     NotifyDone(URLRequestStatus());
   } else if (result < 0) {
     NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, result));
   } else {
     // Clear the IO_PENDING status
     SetStatus(URLRequestStatus());
   }
 
   NotifyReadComplete(result);
 }
 
 bool URLRequestHttpJob::ShouldTreatAsCertificateError(int result) {

[wtc, 2011/09/23 00:04:51]

We should remove the ShouldTreatAsCertificateError method.

-  if (!IsCertificateError(result))
-    return false;
-
-  // Revocation check failures are always certificate errors, even if the host
-  // is using Strict-Transport-Security.
-  if (result == ERR_CERT_UNABLE_TO_CHECK_REVOCATION)
-    return true;
-
-  // Check whether our context is using Strict-Transport-Security.
-  if (!context_->transport_security_state())
-    return true;
-
-  TransportSecurityState::DomainState domain_state;
-  const bool r = context_->transport_security_state()->IsEnabledForHost(
-      &domain_state, request_info_.url.host(),
-      SSLConfigService::IsSNIAvailable(context_->ssl_config_service()));
-
-  return !r;
+  return IsCertificateError(result);
 }
 
 void URLRequestHttpJob::RestartTransactionWithAuth(
     const string16& username,
     const string16& password) {
   username_ = username;
   password_ = password;
 
   // These will be reset in OnStartCompleted.
   response_info_ = NULL;
@@ skipping 637 matching lines @@
   if (done_)
     return;
   done_ = true;
 
   RecordPerfHistograms(reason);
   if (reason == FINISHED)
     RecordCompressionHistograms();
 }
 
 }  // namespace net