Skip to content

Commit

Permalink
Add additional UMA stats for remembering certificate decisions.
Browse files Browse the repository at this point in the history
In the new experiment for remembering certificate decisions by users, decisions
expire after a certain amount of time. This CL adds measures to explicitly
measure how ofter users change their mind after an expiration of their prior
decision.

BUG=262615
R=felt@chromium.org
TBR=gunsch@chromium.org,torne@chromium.org

Review URL: https://codereview.chromium.org/450833002

Cr-Commit-Position: refs/heads/master@{#289787}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289787 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jww@chromium.org committed Aug 15, 2014
1 parent e58c5e3 commit 83c13fe
Show file tree
Hide file tree
Showing 19 changed files with 421 additions and 164 deletions.
1 change: 1 addition & 0 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ void AwContentBrowserClient::AllowCertificateError(
ResourceType resource_type,
bool overridable,
bool strict_enforcement,
bool expired_previous_decision,
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* result) {
AwContentsClientBridgeBase* client =
Expand Down
1 change: 1 addition & 0 deletions android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
content::ResourceType resource_type,
bool overridable,
bool strict_enforcement,
bool expired_previous_decision,
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* result) OVERRIDE;
virtual void SelectClientCertificate(
Expand Down
11 changes: 9 additions & 2 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,7 @@ void ChromeContentBrowserClient::AllowCertificateError(
ResourceType resource_type,
bool overridable,
bool strict_enforcement,
bool expired_previous_decision,
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* result) {
if (resource_type != content::RESOURCE_TYPE_MAIN_FRAME) {
Expand Down Expand Up @@ -1721,9 +1722,15 @@ void ChromeContentBrowserClient::AllowCertificateError(

// Otherwise, display an SSL blocking page. The interstitial page takes
// ownership of ssl_blocking_page.
int options_mask = 0;
if (overridable)
options_mask = SSLBlockingPage::OVERRIDABLE;
if (strict_enforcement)
options_mask = SSLBlockingPage::STRICT_ENFORCEMENT;
if (expired_previous_decision)
options_mask = SSLBlockingPage::EXPIRED_BUT_PREVIOUSLY_ALLOWED;
SSLBlockingPage* ssl_blocking_page = new SSLBlockingPage(
tab, cert_error, ssl_info, request_url, overridable,
strict_enforcement, callback);
tab, cert_error, ssl_info, request_url, options_mask, callback);
ssl_blocking_page->Show();
}

Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
content::ResourceType resource_type,
bool overridable,
bool strict_enforcement,
bool expired_previous_decision,
const base::Callback<void(bool)>& callback,
content::CertificateRequestResultType* request) OVERRIDE;
virtual void SelectClientCertificate(
Expand Down
31 changes: 23 additions & 8 deletions chrome/browser/ssl/chrome_ssl_host_state_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,12 @@ std::string GetKey(net::X509Certificate* cert, net::CertStatus error) {
// expired, a new dictionary will be created
base::DictionaryValue* ChromeSSLHostStateDelegate::GetValidCertDecisionsDict(
base::DictionaryValue* dict,
CreateDictionaryEntriesDisposition create_entries) {
CreateDictionaryEntriesDisposition create_entries,
bool* expired_previous_decision) {
// This needs to be done first in case the method is short circuited by an
// early failure.
*expired_previous_decision = false;

// Extract the version of the certificate decision structure from the content
// setting.
int version;
Expand Down Expand Up @@ -202,11 +207,12 @@ base::DictionaryValue* ChromeSSLHostStateDelegate::GetValidCertDecisionsDict(
if (should_remember_ssl_decisions_ !=
ForgetSSLExceptionDecisionsAtSessionEnd &&
decision_expiration.ToInternalValue() <= now.ToInternalValue()) {
*expired_previous_decision = true;

if (create_entries == DoNotCreateDictionaryEntries)
return NULL;

expired = true;

base::Time expiration_time =
now + default_ssl_cert_decision_expiration_delta_;
// Unfortunately, JSON (and thus content settings) doesn't support int64
Expand Down Expand Up @@ -276,12 +282,16 @@ void ChromeSSLHostStateDelegate::Clear() {
net::CertPolicy::Judgment ChromeSSLHostStateDelegate::QueryPolicy(
const std::string& host,
net::X509Certificate* cert,
net::CertStatus error) {
net::CertStatus error,
bool* expired_previous_decision) {
HostContentSettingsMap* map = profile_->GetHostContentSettingsMap();
GURL url = GetSecureGURLForHost(host);
scoped_ptr<base::Value> value(map->GetWebsiteSetting(
url, url, CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS, std::string(), NULL));

// Set a default value in case this method is short circuited and doesn't do a
// full query.
*expired_previous_decision = false;
if (!value.get() || !value->IsType(base::Value::TYPE_DICTIONARY))
return net::CertPolicy::UNKNOWN;

Expand All @@ -291,10 +301,14 @@ net::CertPolicy::Judgment ChromeSSLHostStateDelegate::QueryPolicy(
DCHECK(success);

base::DictionaryValue* cert_error_dict; // Owned by value
cert_error_dict =
GetValidCertDecisionsDict(dict, DoNotCreateDictionaryEntries);
if (!cert_error_dict)
cert_error_dict = GetValidCertDecisionsDict(
dict, DoNotCreateDictionaryEntries, expired_previous_decision);
if (!cert_error_dict) {
// This revoke is necessary to clear any old expired setting that may
// lingering in the case that an old decision expried.
RevokeUserDecisions(host);
return net::CertPolicy::UNKNOWN;
}

success = cert_error_dict->GetIntegerWithoutPathExpansion(GetKey(cert, error),
&policy_decision);
Expand Down Expand Up @@ -406,8 +420,9 @@ void ChromeSSLHostStateDelegate::ChangeCertPolicy(
bool success = value->GetAsDictionary(&dict);
DCHECK(success);

base::DictionaryValue* cert_dict =
GetValidCertDecisionsDict(dict, CreateDictionaryEntries);
bool expired_previous_decision; // unused value in this function
base::DictionaryValue* cert_dict = GetValidCertDecisionsDict(
dict, CreateDictionaryEntries, &expired_previous_decision);
// If a a valid certificate dictionary cannot be extracted from the content
// setting, that means it's in an unknown format. Unfortunately, there's
// nothing to be done in that case, so a silent fail is the only option.
Expand Down
16 changes: 12 additions & 4 deletions chrome/browser/ssl/chrome_ssl_host_state_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ class ChromeSSLHostStateDelegate : public content::SSLHostStateDelegate {
net::X509Certificate* cert,
net::CertStatus error) OVERRIDE;
virtual void Clear() OVERRIDE;
virtual net::CertPolicy::Judgment QueryPolicy(const std::string& host,
net::X509Certificate* cert,
net::CertStatus error) OVERRIDE;
virtual net::CertPolicy::Judgment QueryPolicy(
const std::string& host,
net::X509Certificate* cert,
net::CertStatus error,
bool* expired_previous_decision) OVERRIDE;
virtual void HostRanInsecureContent(const std::string& host,
int pid) OVERRIDE;
virtual bool DidHostRunInsecureContent(const std::string& host,
Expand Down Expand Up @@ -66,6 +68,8 @@ class ChromeSSLHostStateDelegate : public content::SSLHostStateDelegate {
FRIEND_TEST_ALL_PREFIXES(ForgetInstantlySSLHostStateDelegateTest,
MakeAndForgetException);
FRIEND_TEST_ALL_PREFIXES(RememberSSLHostStateDelegateTest, AfterRestart);
FRIEND_TEST_ALL_PREFIXES(RememberSSLHostStateDelegateTest,
QueryPolicyExpired);

// Used to specify whether new content setting entries should be created if
// they don't already exist when querying the user's settings.
Expand Down Expand Up @@ -105,9 +109,13 @@ class ChromeSSLHostStateDelegate : public content::SSLHostStateDelegate {
// GetValidCertDecisionsDict will create a new set of entries within the
// dictionary if they do not already exist. Otherwise will fail and return if
// NULL if they do not exist.
//
// |expired_previous_decision| is set to true if there had been a previous
// decision made by the user but it has expired. Otherwise it is set to false.
base::DictionaryValue* GetValidCertDecisionsDict(
base::DictionaryValue* dict,
CreateDictionaryEntriesDisposition create_entries);
CreateDictionaryEntriesDisposition create_entries,
bool* expired_previous_decision);

scoped_ptr<base::Clock> clock_;
RememberSSLExceptionDecisionsDisposition should_remember_ssl_decisions_;
Expand Down
Loading

0 comments on commit 83c13fe

Please sign in to comment.