Remember user decisions on invalid certificates behind a flag

chrome/browser/ssl/chrome_ssl_host_state_decisions.h

+// Copyright (c) 2014 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.
+
+#ifndef CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DECISIONS_H_
+#define CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DECISIONS_H_
+
+#include "base/scoped_ptr.h"
+#include "base/time/clock.h"
+#include "base/time/time.h"
+#include "content/public/browser/ssl_host_state_decisions.h"
+
+class GURL;
+class Profile;
+
+namespace base {
+
+class DictionaryValue;
+
+}  //  namespace base
+
+// Implementation of the tracking of user decisions on SSL errors for sites.
+// Tracks in the user has allowed, denied, or not seen an exception for the

[Ryan Sleevi, 2014/07/17 23:19:35]

typo, s/in/if/

[jww, 2014/07/21 23:39:33]

Done.

+// specified cite, SSL fingerprint, and error. If the user makes a decision,

[Ryan Sleevi, 2014/07/17 23:19:36]

typo, s/cite/site/

[jww, 2014/07/21 23:39:33]

Done.

+// stores the decision until either the session ends or for a length of time
+// (across session restarts), based on Chrome flags.

[Ryan Sleevi, 2014/07/17 23:19:35]

s/Chrome/command line/ ?
Or perhaps
s/Chrome/preference/, if you're implemented in terms of prefs, rather than
directly flags?

[jww, 2014/07/21 23:39:33]

Done.

+class ChromeSSLHostStateDecisions : public content::SSLHostStateDecisions {

[Ryan Sleevi, 2014/07/17 23:19:35]

Does this belong in namespace chrome? Do we do that? I thought we did that?
Maybe we don't.

[jww, 2014/07/21 23:39:33]

It appears there's a variety of different ways its done. I was basing this on
src/chrome/browser/download/download_service.h and
src/chrome/browser/download/download_service_factory.h which don't used the
chrome:: namespace. Let me know if you feel strongly about using the chrome::
namespace, though.

+ public:
+  explicit ChromeSSLHostStateDecisions(Profile* profile);
+  virtual ~ChromeSSLHostStateDecisions();
+
+  // SSLHostStateDecisions:
+  virtual void DenyCert(const GURL& url,
+                        net::X509Certificate* cert,
+                        net::CertStatus error) OVERRIDE;
+  virtual void AllowCert(const GURL& url,
+                         net::X509Certificate* cert,
+                         net::CertStatus error) OVERRIDE;
+  virtual void Clear() OVERRIDE;
+  virtual net::CertPolicy::Judgment QueryPolicy(const GURL& url,
+                                                net::X509Certificate* cert,
+                                                net::CertStatus error) OVERRIDE;
+  virtual void RevokeAllowAndDenyPreferences(const GURL& url) OVERRIDE;
+  virtual bool HasAllowedOrDeniedCert(const GURL& url) OVERRIDE;
+
+  // Called on the UI thread when the profile is about to be destroyed.
+  void ShutdownOnUIThread() {}
+
+ protected:
+  // SetClock takes ownership of the passed in clock.
+  void SetClock(scoped_ptr<base::Clock> clock) {
+    clock_.reset(clock.release());
+  }
+
+ private:
+  friend class ForgetInstantlySSLHostStateDecisionsTest;
+  friend class RememberSSLHostStateDecisionsTest;

[Ryan Sleevi, 2014/07/17 23:19:35]

Prefer one or the other. Feels weird to friend the fixture, since 58-60 cover
the specific tests, and friending the fixture won't affect derived tests (the
TEST_F), since each of those is a new derived class that inherits from your
fixture (hence the FRIEND_TEST_ALL_PREFIXES)

[jww, 2014/07/21 23:39:33]

Whoops, my misunderstanding (I thought both were needed).

+  FRIEND_TEST_ALL_PREFIXES(ForgetInstantlySSLHostStateDecisionsTest,
+                           MakeAndForgetException);
+  FRIEND_TEST_ALL_PREFIXES(RememberSSLHostStateDecisionsTest, AfterRestart);
+
+  // Used to specify whether new content setting entries should be created if
+  // they don't already exist when querying the user's settings.
+  enum CreateDictionaryEntriesDisposition {
+    CreateDictionaryEntries,
+    DoNotCreateDictionaryEntries
+  };
+  // Specifies whether user SSL error decisions should be forgetten at the end

[Ryan Sleevi, 2014/07/17 23:19:35]

linebreak

[jww, 2014/07/21 23:39:33]

Done.

+  // of this current session (the "old style" of remembering decisions), or

[Ryan Sleevi, 2014/07/17 23:19:36]

s/"old style"/"old" style

Or just drop the quotes, it's cleaner that way.

[jww, 2014/07/21 23:39:33]

Done.

+  // whether they should be remembered across session restarts for a specified
+  // length of time, deteremined by default_ssl_cert_decision_expiration_delta_.

[Ryan Sleevi, 2014/07/17 23:19:36]

add |foo_| around the variable.

[jww, 2014/07/21 23:39:33]

Done.

+  enum RememberSSLExceptionDecisionsDisposition {
+    ForgetSSLExceptionDecisionsAtSessionEnd,
+    RememberSSLExceptionDecisionsForDelta
+  };
+
+  // Modify the user's content settings to specify a judgement made for a
+  // specific site and certificate, where |url| is the site in question, |cert|
+  // is the certificate with an error, |error| is the error in the certificate,
+  // and |judgement| is the user decision to be recorded.
+  void ChangeCertPolicy(const GURL& url,
+                        net::X509Certificate* cert,
+                        net::CertStatus error,
+                        net::CertPolicy::Judgment judgment);
+
+  // Query the content settings to retrieve a dictionary of certificate
+  // fingerprints and errors of certificates to user decisions, as set by
+  // ChangeCertPolicy. Returns NULL on a failure.
+  //
+  // |dict| specifies the user's full exceptions dictionary for a specific site
+  // in their content settings. Must be retrieved directly from a website
+  // setting in the the profile's HostContentSettingsMap.
+  //
+  // If |create_entries| specifies CreateDictionaryEntries, then
+  // 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.
+  base::DictionaryValue* GetValidCertDecisionsDict(
+      base::DictionaryValue* dict,
+      CreateDictionaryEntriesDisposition create_entries);
+
+  scoped_ptr<base::Clock> clock_;
+  RememberSSLExceptionDecisionsDisposition should_remember_ssl_decisions_;
+  base::TimeDelta default_ssl_cert_decision_expiration_delta_;
+  Profile* profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeSSLHostStateDecisions);
+};
+
+#endif  // CHROME_BROWSER_SSL_CHROME_SSL_HOST_STATE_DECISIONS_H_