Skip to content

Commit

Permalink
net: make HSTS hosts use the normal SSL interstitials
Browse files Browse the repository at this point in the history
(Reland of r102947, which was reverted in r102950.)

SSL interstitials have better translations for the error messages and this
returns us to the point where we have only a single UI for SSL errors, which
will make some future changes easier.

First, this change changes the SSL error callbacks to take an SSLInfo& rather
than a X509Certificate* (which was already a TODO(wtc) in the code). Most of
this change is the resulting plumbing.

It also adds a |is_hsts_host| flag to the callbacks to denote an HSTS host.

Finally, in ssl_policy.cc the |is_hsts_host| flag causes any error to be
fatal.

BUG=93527

http://codereview.chromium.org/7976036/

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102994 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
agl@chromium.org committed Sep 27, 2011
1 parent faf6cc7 commit e5624f0
Show file tree
Hide file tree
Showing 21 changed files with 97 additions and 95 deletions.
6 changes: 3 additions & 3 deletions content/browser/renderer_host/resource_dispatcher_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1306,10 +1306,10 @@ void ResourceDispatcherHost::OnCertificateRequested(

void ResourceDispatcherHost::OnSSLCertificateError(
net::URLRequest* request,
int cert_error,
net::X509Certificate* cert) {
const net::SSLInfo& ssl_info,
bool is_hsts_host) {
DCHECK(request);
SSLManager::OnSSLCertificateError(this, request, cert_error, cert);
SSLManager::OnSSLCertificateError(this, request, ssl_info, is_hsts_host);
}

bool ResourceDispatcherHost::CanGetCookies(
Expand Down
4 changes: 2 additions & 2 deletions content/browser/renderer_host/resource_dispatcher_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ class CONTENT_EXPORT ResourceDispatcherHost : public net::URLRequest::Delegate {
net::URLRequest* request,
net::SSLCertRequestInfo* cert_request_info) OVERRIDE;
virtual void OnSSLCertificateError(net::URLRequest* request,
int cert_error,
net::X509Certificate* cert) OVERRIDE;
const net::SSLInfo& ssl_info,
bool is_hsts_host) OVERRIDE;
virtual bool CanGetCookies(const net::URLRequest* request,
const net::CookieList& cookie_list) const OVERRIDE;
virtual bool CanSetCookie(const net::URLRequest* request,
Expand Down
16 changes: 7 additions & 9 deletions content/browser/ssl/ssl_cert_error_handler.cc
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
// Copyright (c) 2009 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.

#include "content/browser/ssl/ssl_cert_error_handler.h"

#include "content/browser/renderer_host/resource_dispatcher_host.h"
#include "content/browser/ssl/ssl_policy.h"
#include "net/base/cert_status_flags.h"
#include "net/base/x509_certificate.h"

SSLCertErrorHandler::SSLCertErrorHandler(
ResourceDispatcherHost* rdh,
net::URLRequest* request,
ResourceType::Type resource_type,
int cert_error,
net::X509Certificate* cert)
const net::SSLInfo& ssl_info,
bool is_hsts_host)
: SSLErrorHandler(rdh, request, resource_type),
cert_error_(cert_error) {
ssl_info_(ssl_info),
cert_error_(net::MapCertStatusToNetError(ssl_info.cert_status)),
is_hsts_host_(is_hsts_host) {
DCHECK(request == resource_dispatcher_host_->GetURLRequest(request_id_));

// We cannot use the request->ssl_info(), it's not been initialized yet, so
// we have to set the fields manually.
ssl_info_.cert = cert;
ssl_info_.SetCertError(cert_error);
}

SSLCertErrorHandler* SSLCertErrorHandler::AsSSLCertErrorHandler() {
Expand Down
10 changes: 6 additions & 4 deletions content/browser/ssl/ssl_cert_error_handler.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2009 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.

Expand All @@ -24,14 +24,15 @@ class SSLCertErrorHandler : public SSLErrorHandler {
SSLCertErrorHandler(ResourceDispatcherHost* rdh,
net::URLRequest* request,
ResourceType::Type resource_type,
int cert_error,
net::X509Certificate* cert);
const net::SSLInfo& ssl_info,
bool is_hsts_host);

virtual SSLCertErrorHandler* AsSSLCertErrorHandler();

// These accessors are available on either thread
const net::SSLInfo& ssl_info() const { return ssl_info_; }
int cert_error() const { return cert_error_; }
bool is_hsts_host() const { return is_hsts_host_; }

protected:
// SSLErrorHandler methods
Expand All @@ -42,8 +43,9 @@ class SSLCertErrorHandler : public SSLErrorHandler {
virtual ~SSLCertErrorHandler();

// These read-only members may be accessed on any thread.
net::SSLInfo ssl_info_;
const net::SSLInfo ssl_info_;
const int cert_error_; // The error we represent.
const bool is_hsts_host_; // true if the error is from an HSTS host.

DISALLOW_COPY_AND_ASSIGN(SSLCertErrorHandler);
};
Expand Down
14 changes: 8 additions & 6 deletions content/browser/ssl/ssl_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@
// static
void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh,
net::URLRequest* request,
int cert_error,
net::X509Certificate* cert) {
DVLOG(1) << "OnSSLCertificateError() cert_error: " << cert_error
<< " url: " << request->url().spec();
const net::SSLInfo& ssl_info,
bool is_hsts_host) {
DVLOG(1) << "OnSSLCertificateError() cert_error: "
<< net::MapCertStatusToNetError(ssl_info.cert_status)
<< " url: " << request->url().spec()
<< " cert_status: " << std::hex << ssl_info.cert_status;

ResourceDispatcherHostRequestInfo* info =
ResourceDispatcherHost::InfoForRequest(request);
Expand All @@ -39,8 +41,8 @@ void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh,
NewRunnableMethod(new SSLCertErrorHandler(rdh,
request,
info->resource_type(),
cert_error,
cert),
ssl_info,
is_hsts_host),
&SSLCertErrorHandler::Dispatch));
}

Expand Down
5 changes: 3 additions & 2 deletions content/browser/ssl/ssl_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class ResourceRequestDetails;
class SSLPolicy;

namespace net {
class SSLInfo;
class URLRequest;
} // namespace net

Expand All @@ -49,8 +50,8 @@ class SSLManager : public NotificationObserver {
// Called on the IO thread.
static void OnSSLCertificateError(ResourceDispatcherHost* resource_dispatcher,
net::URLRequest* request,
int cert_error,
net::X509Certificate* cert);
const net::SSLInfo& ssl_info,
bool is_hsts_host);

// Called when SSL state for a host or tab changes. Broadcasts the
// SSL_INTERNAL_STATE_CHANGED notification.
Expand Down
2 changes: 1 addition & 1 deletion content/browser/ssl/ssl_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void SSLPolicy::OnCertError(SSLCertErrorHandler* handler) {
case net::ERR_CERT_DATE_INVALID:
case net::ERR_CERT_AUTHORITY_INVALID:
case net::ERR_CERT_WEAK_SIGNATURE_ALGORITHM:
OnCertErrorInternal(handler, true);
OnCertErrorInternal(handler, !handler->is_hsts_host());
break;
case net::ERR_CERT_NO_REVOCATION_MECHANISM:
// Ignore this error.
Expand Down
3 changes: 2 additions & 1 deletion net/base/cert_status_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#pragma once

#include "base/basictypes.h"
#include "net/base/net_export.h"

namespace net {

Expand Down Expand Up @@ -48,7 +49,7 @@ CertStatus MapNetErrorToCertStatus(int error);

// Maps the most serious certificate error in the certificate status flags
// to the equivalent network error code.
int MapCertStatusToNetError(CertStatus cert_status);
NET_EXPORT int MapCertStatusToNetError(CertStatus cert_status);

} // namespace net

Expand Down
7 changes: 4 additions & 3 deletions net/proxy/proxy_script_fetcher_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/logging.h"
#include "base/message_loop.h"
#include "base/string_util.h"
#include "net/base/cert_status_flags.h"
#include "net/base/data_url.h"
#include "net/base/io_buffer.h"
#include "net/base/load_flags.h"
Expand Down Expand Up @@ -190,12 +191,12 @@ void ProxyScriptFetcherImpl::OnAuthRequired(URLRequest* request,
}

void ProxyScriptFetcherImpl::OnSSLCertificateError(URLRequest* request,
int cert_error,
X509Certificate* cert) {
const SSLInfo& ssl_info,
bool is_hsts_host) {
DCHECK_EQ(request, cur_request_.get());
LOG(WARNING) << "SSL certificate error when fetching PAC script, aborting.";
// Certificate errors are in same space as net errors.
result_code_ = cert_error;
result_code_ = MapCertStatusToNetError(ssl_info.cert_status);
request->Cancel();
}

Expand Down
5 changes: 3 additions & 2 deletions net/proxy/proxy_script_fetcher_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ class NET_EXPORT ProxyScriptFetcherImpl : public ProxyScriptFetcher,
// URLRequest::Delegate methods:
virtual void OnAuthRequired(URLRequest* request,
AuthChallengeInfo* auth_info) OVERRIDE;
virtual void OnSSLCertificateError(URLRequest* request, int cert_error,
X509Certificate* cert) OVERRIDE;
virtual void OnSSLCertificateError(URLRequest* request,
const SSLInfo& ssl_info,
bool is_hsts_ok) OVERRIDE;
virtual void OnResponseStarted(URLRequest* request) OVERRIDE;
virtual void OnReadCompleted(URLRequest* request, int num_bytes) OVERRIDE;

Expand Down
10 changes: 5 additions & 5 deletions net/url_request/url_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ void URLRequest::Delegate::OnCertificateRequested(
}

void URLRequest::Delegate::OnSSLCertificateError(URLRequest* request,
int cert_error,
X509Certificate* cert) {
const SSLInfo& ssl_info,
bool is_hsts_ok) {
request->Cancel();
}

Expand Down Expand Up @@ -783,10 +783,10 @@ void URLRequest::NotifyCertificateRequested(
delegate_->OnCertificateRequested(this, cert_request_info);
}

void URLRequest::NotifySSLCertificateError(int cert_error,
X509Certificate* cert) {
void URLRequest::NotifySSLCertificateError(const SSLInfo& ssl_info,
bool is_hsts_host) {
if (delegate_)
delegate_->OnSSLCertificateError(this, cert_error, cert);
delegate_->OnSSLCertificateError(this, ssl_info, is_hsts_host);
}

bool URLRequest::CanGetCookies(const CookieList& cookie_list) const {
Expand Down
11 changes: 8 additions & 3 deletions net/url_request/url_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class CookieOptions;
class HostPortPair;
class IOBuffer;
class SSLCertRequestInfo;
class SSLInfo;
class UploadData;
class URLRequestContext;
class URLRequestJob;
Expand Down Expand Up @@ -266,9 +267,12 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
// safe thing and Cancel() the request or decide to proceed by calling
// ContinueDespiteLastError(). cert_error is a ERR_* error code
// indicating what's wrong with the certificate.
// If |is_hsts_host| is true then the host in question is an HSTS host
// which demands a higher level of security. In this case, errors must not
// be bypassable by the user.
virtual void OnSSLCertificateError(URLRequest* request,
int cert_error,
X509Certificate* cert);
const SSLInfo& ssl_info,
bool is_hsts_host);

// Called when reading cookies to allow the delegate to block access to the
// cookie. This method will never be invoked when LOAD_DO_NOT_SEND_COOKIES
Expand Down Expand Up @@ -713,7 +717,8 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
// of these functions.
void NotifyAuthRequired(AuthChallengeInfo* auth_info);
void NotifyCertificateRequested(SSLCertRequestInfo* cert_request_info);
void NotifySSLCertificateError(int cert_error, X509Certificate* cert);
void NotifySSLCertificateError(const SSLInfo& ssl_info,
bool is_hsts_host);
bool CanGetCookies(const CookieList& cookie_list) const;
bool CanSetCookie(const std::string& cookie_line,
CookieOptions* options) const;
Expand Down
36 changes: 10 additions & 26 deletions net/url_request/url_request_http_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -686,13 +686,18 @@ void URLRequestHttpJob::OnStartCompleted(int result) {

if (result == OK) {
SaveCookiesAndNotifyHeadersComplete();
} else if (ShouldTreatAsCertificateError(result)) {
} else if (IsCertificateError(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.
NotifySSLCertificateError(
result, transaction_->GetResponseInfo()->ssl_info.cert);

TransportSecurityState::DomainState domain_state;
const bool is_hsts_host =
context_->transport_security_state() &&
context_->transport_security_state()->IsEnabledForHost(
&domain_state, request_info_.url.host(),
SSLConfigService::IsSNIAvailable(context_->ssl_config_service()));
NotifySSLCertificateError(transaction_->GetResponseInfo()->ssl_info,
is_hsts_host);
} else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
NotifyCertificateRequested(
transaction_->GetResponseInfo()->cert_request_info);
Expand All @@ -719,27 +724,6 @@ void URLRequestHttpJob::OnReadCompleted(int result) {
NotifyReadComplete(result);
}

bool URLRequestHttpJob::ShouldTreatAsCertificateError(int result) {
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;
}

void URLRequestHttpJob::RestartTransactionWithAuth(
const string16& username,
const string16& password) {
Expand Down
2 changes: 0 additions & 2 deletions net/url_request/url_request_http_job.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ class URLRequestHttpJob : public URLRequestJob {
void OnReadCompleted(int result);
void NotifyBeforeSendHeadersCallback(int result);

bool ShouldTreatAsCertificateError(int result);

void RestartTransactionWithAuth(const string16& username,
const string16& password);

Expand Down
6 changes: 3 additions & 3 deletions net/url_request/url_request_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,12 @@ void URLRequestJob::NotifyCertificateRequested(
request_->NotifyCertificateRequested(cert_request_info);
}

void URLRequestJob::NotifySSLCertificateError(int cert_error,
X509Certificate* cert) {
void URLRequestJob::NotifySSLCertificateError(const SSLInfo& ssl_info,
bool is_hsts_host) {
if (!request_)
return; // The request was destroyed, so there is no more work to do.

request_->NotifySSLCertificateError(cert_error, cert);
request_->NotifySSLCertificateError(ssl_info, is_hsts_host);
}

bool URLRequestJob::CanGetCookies(const CookieList& cookie_list) const {
Expand Down
4 changes: 3 additions & 1 deletion net/url_request/url_request_job.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class HttpRequestHeaders;
class HttpResponseInfo;
class IOBuffer;
class SSLCertRequestInfo;
class SSLInfo;
class URLRequest;
class UploadData;
class URLRequestStatus;
Expand Down Expand Up @@ -197,7 +198,8 @@ class NET_EXPORT URLRequestJob : public base::RefCounted<URLRequestJob>,
void NotifyCertificateRequested(SSLCertRequestInfo* cert_request_info);

// Notifies the job about an SSL certificate error.
void NotifySSLCertificateError(int cert_error, X509Certificate* cert);
void NotifySSLCertificateError(const SSLInfo& ssl_info,
bool is_hsts_host);

// Delegates to URLRequest::Delegate.
bool CanGetCookies(const CookieList& cookie_list) const;
Expand Down
4 changes: 2 additions & 2 deletions net/url_request/url_request_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ void TestDelegate::OnAuthRequired(net::URLRequest* request,
}

void TestDelegate::OnSSLCertificateError(net::URLRequest* request,
int cert_error,
net::X509Certificate* cert) {
const net::SSLInfo& ssl_info,
bool is_hsts_host) {
// The caller can control whether it needs all SSL requests to go through,
// independent of any possible errors, or whether it wants SSL errors to
// cancel the request.
Expand Down
4 changes: 2 additions & 2 deletions net/url_request/url_request_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ class TestDelegate : public net::URLRequest::Delegate {
virtual void OnAuthRequired(net::URLRequest* request,
net::AuthChallengeInfo* auth_info) OVERRIDE;
virtual void OnSSLCertificateError(net::URLRequest* request,
int cert_error,
net::X509Certificate* cert) OVERRIDE;
const net::SSLInfo& ssl_info,
bool is_hsts_host) OVERRIDE;
virtual bool CanGetCookies(const net::URLRequest* request,
const net::CookieList& cookie_list) const OVERRIDE;
virtual bool CanSetCookie(const net::URLRequest* request,
Expand Down
Loading

0 comments on commit e5624f0

Please sign in to comment.