Support host-based deletion for SSLHostStateDelegate

chrome/browser/ssl/chrome_ssl_host_state_delegate.cc

Index: chrome/browser/ssl/chrome_ssl_host_state_delegate.cc
diff --git a/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc b/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc
index af2c261d3819aebaab88bf25e072def6ecafd8d6..a9849a55646a926550ee72be0955bfa7c13a1a08 100644
--- a/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc
+++ b/chrome/browser/ssl/chrome_ssl_host_state_delegate.cc
@@ -10,6 +10,7 @@
 
 #include "base/base64.h"
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/guid.h"
 #include "base/logging.h"
@@ -139,6 +140,17 @@ void MigrateOldSettings(HostContentSettingsMap* map) {
   }
 }
 
+bool HostFilterToPatternFilter(
+    const base::Callback<bool(const std::string&)>& host_filter,
+    const ContentSettingsPattern& primary_pattern,
+    const ContentSettingsPattern& secondary_pattern) {
+  // We only store data for "https://<host>" primary patterns, so we can convert
+  // them directly to a URL. The secondary pattern is unused.
+  GURL url(primary_pattern.ToString());
+  DCHECK(url.is_valid());

[estark, 2016/08/30 01:52:16]

Just checking that this DCHECK really is a DCHECK -- I can't figure out where
exactly this |primary_pattern| comes from. It's not e.g. a string typed by the
user, is it?

[msramek, 2016/08/30 14:39:41]

Oh, the shame! The DCHECK is wrong, thanks for catching this :)

The user can't type it, but an administrator can, because it's managed by
policy:
https://www.chromium.org/administrators/policy-list-3#AutoSelectCertificateFo...

If we want to delete data for <host>, we must delete all patterns for
"*://<host>:*" or stricter (i.e. with specified scheme or port). I.e. there are
patterns with invalid URLs that we still care about.

We need to compare two blackboxes - ContentSettingsPattern and |host_filter|,
and I don't think there's a way around that without making one of them
transparent. So I added a GetHost() method to ContentSettingsPattern.

I also added a TODO for myself to investigate whether broadly scoped exceptions
are in practice recognized by ChromeSSLHostStateDelegate. We can talk about that
with Joel when he returns.

[raymes, 2016/08/30 23:32:00]

Hmm, this file is dealing with CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS though,
not CONTENT_SETTINGS_TYPE_AUTO_SELECT_CERTIFICATE. I don't think
SSL_CERT_DECISIONS are managed by policy -- I think they are only set via the
chrome UI. Given that, can we avoid adding the GetHost for now?

The other content_settings changes look good to me though.

[msramek, 2016/08/31 13:43:11]

...correct. I panicked and searched for the wrong one :)

So! The only SetWebsiteSetting call for SSL_CERT_DECISIONS is in this file, and
it uses the default pattern scoping for the URL generated by
GetSecureGURLForHost(), which is GURL("https://<host>"). The default scoping
will create a "https://<host>:443" pattern, which is also a valid URL.

Even before the introduction of default scopes, SSL_CERT_DECISIONS used
FromURLNoWildcard, so there cannot be any legacy exceptions with wildcards
either.

This is not manageable by policy or extensions.

Which means that yes, the DCHECK is valid after all :-)

+  return host_filter.Run(url.host());
+}
+
 }  // namespace
 
 // This helper function gets the dictionary of certificate fingerprints to
@@ -313,9 +325,22 @@ void ChromeSSLHostStateDelegate::AllowCert(const std::string& host,
                                      std::string(), std::move(value));
 }
 
-void ChromeSSLHostStateDelegate::Clear() {
+void ChromeSSLHostStateDelegate::Clear(
+    const base::Callback<bool(const std::string&)>& host_filter) {
+  // Convert host matching to content settings pattern matching. Content
+  // settings deletion is done synchronously on the UI thread, so we can use
+  // |host_filter| by reference.
+  base::Callback<bool(const ContentSettingsPattern& primary_pattern,
+                      const ContentSettingsPattern& secondary_pattern)>
+      pattern_filter;
+  if (!host_filter.is_null()) {
+    pattern_filter =
+        base::Bind(&HostFilterToPatternFilter, base::ConstRef(host_filter));
+  }
+
   HostContentSettingsMapFactory::GetForProfile(profile_)
-      ->ClearSettingsForOneType(CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS);
+      ->ClearSettingsForOneTypeWithPredicate(
+          CONTENT_SETTINGS_TYPE_SSL_CERT_DECISIONS, pattern_filter);
 }
 
 content::SSLHostStateDelegate::CertJudgment