Introduce the ability to require CT for specific hosts

net/http/transport_security_state.h

 // Copyright (c) 2012 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 NET_HTTP_TRANSPORT_SECURITY_STATE_H_
 #define NET_HTTP_TRANSPORT_SECURITY_STATE_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 35 matching lines @@
   class NET_EXPORT Delegate {
    public:
     // This function may not block and may be called with internal locks held.
     // Thus it must not reenter the TransportSecurityState object.
     virtual void StateIsDirty(TransportSecurityState* state) = 0;
 
    protected:
     virtual ~Delegate() {}
   };
 
+  class NET_EXPORT RequireCTDelegate {
+   public:
+    // Provides a capability for altering the default handling of Certificate
+    // Transparency information, allowing it to be always required for some
+    // hosts, for some hosts to be opted out of the default policy, or
+    // allowing the TransportSecurityState to apply the default security
+    // policies.
+    enum class CTRequirementLevel {
+      // The host is required to always supply Certificate Transparency
+      // information that complies with the CT policy.
+      REQUIRED,
+
+      // The host is explicitly not required to supply Certificate
+      // Transparency information that complies with the CT policy.
+      NOT_REQUIRED,
+
+      // The delegate makes no statements, positive or negative, about
+      // requiring the host to supply Certificate Transparency information,
+      // allowing the default behaviour to happen.
+      DEFAULT,

[davidben, 2016/06/22 21:44:59]

To confirm, the reason DEFAULT is a notion is because TSS will have internal
logic? Is the intent that Expect-CT and Symantec bits be implemented in TSS and
the admin policy bits be in the delegate?

[Ryan Sleevi, 2016/06/22 22:07:39]

Yes (or eventually the CertVerifier)

+    };
+
+    // Called by the TransportSecurityState, allows the Delegate to override
+    // the default handling of Certificate Transparency requirements, if
+    // desired.
+    virtual CTRequirementLevel IsCTRequiredForHost(

[davidben, 2016/06/22 21:44:59]

Nit: A little weird to have an IsFoo function return something other than a
boolean. Although GetRequirementLevelForHost() is kinda wordy, so maybe this is
better?

[Ryan Sleevi, 2016/06/22 22:07:39]

It's just a tri-state bool ;)

( http://thedailywtf.com/articles/tri-state-boolean )

It's "Yes", "No", "#Yolo"

+        const std::string& hostname) = 0;
+
+   protected:
+    virtual ~RequireCTDelegate() = default;
+  };
+
   // A STSState describes the strict transport security state (required
   // upgrade to HTTPS).
   class NET_EXPORT STSState {
    public:
     enum UpgradeMode {
       // These numbers must match those in hsts_view.js, function modeToString.
       MODE_FORCE_HTTPS = 0,
       MODE_DEFAULT = 1,
     };
 
@@ skipping 187 matching lines @@
   bool ShouldUpgradeToSSL(const std::string& host);
   bool CheckPublicKeyPins(const HostPortPair& host_port_pair,
                           bool is_issued_by_known_root,
                           const HashValueVector& hashes,
                           const X509Certificate* served_certificate_chain,
                           const X509Certificate* validated_certificate_chain,
                           const PublicKeyPinReportStatus report_status,
                           std::string* failure_log);
   bool HasPublicKeyPins(const std::string& host);
 
+  // Returns true if connections to |host|, using the validated certificate
+  // |validated_certificate_chain|, are expected to be accompanied with
+  // valid Certificate Transparency information that complies with the
+  // connection's CTPolicyEnforcer.
+  //
+  // The behavior may be further be altered by setting a RequireCTDelegate
+  // via |SetRequireCTDelegate()|.
+  bool ShouldRequireCT(const std::string& host,
+                       const X509Certificate* validated_certificate_chain,
+                       const HashValueVector& hashes);
+
   // Assign a |Delegate| for persisting the transport security state. If
   // |NULL|, state will not be persisted. The caller retains
   // ownership of |delegate|.
   // Note: This is only used for serializing/deserializing the
   // TransportSecurityState.
   void SetDelegate(Delegate* delegate);
 
   void SetReportSender(ReportSenderInterface* report_sender);
 
   void SetExpectCTReporter(ExpectCTReporter* expect_ct_reporter);
 
+  // Assigns a delegate responsible for determining whether or not a
+  // connection to a given host should require Certificate Transparency
+  // information that complies with the CT policy provided by a
+  // CTPolicyEnforcer.
+  // If nullptr, no delegate will be consulted.
+  // The caller retains ownership of the |delegate|, and must persist for
+  // the lifetime of this object or until called with nullptr, whichever
+  // occurs first.
+  void SetRequireCTDelegate(RequireCTDelegate* delegate);
+
   // Clears all dynamic data (e.g. HSTS and HPKP data).
   //
   // Does NOT persist changes using the Delegate, as this function is only
   // used to clear any dynamic data prior to re-loading it from a file.
   // Note: This is only used for serializing/deserializing the
   // TransportSecurityState.
   void ClearDynamicData();
 
   // Inserts |state| into |enabled_sts_hosts_| under the key |hashed_host|.
   // |hashed_host| is already in the internal representation.
@@ skipping 184 matching lines @@
       ExpectStapleState* expect_staple_result) const;
 
   // The sets of hosts that have enabled TransportSecurity. |domain| will always
   // be empty for a STSState or PKPState in these maps; the domain
   // comes from the map keys instead. In addition, |upgrade_mode| in the
   // STSState is never MODE_DEFAULT and |HasPublicKeyPins| in the PKPState
   // always returns true.
   STSStateMap enabled_sts_hosts_;
   PKPStateMap enabled_pkp_hosts_;
 
-  Delegate* delegate_;
+  Delegate* delegate_ = nullptr;
 
-  ReportSenderInterface* report_sender_;
+  ReportSenderInterface* report_sender_ = nullptr;
 
   // True if static pins should be used.
   bool enable_static_pins_;

[davidben, 2016/06/22 21:44:59]

Why make the pointers = nullptr while the bools are set in the constructor? (I'm
unclear on when we prefer to do which.)

[Ryan Sleevi, 2016/06/22 22:07:39]

I haven't seen any clear guidance in any of the style guides, but the style I
was adopting was trying to be reflective of the policies regarding forward
declarations or sticking implementation details in unnamed namespaces:

What's in the header is, effectively, a contract - and a reason for your
dependencies to change. In this case, I didn't think that dependents should need
to be recompiled if we change how enable_static_* behaves (and, to be fair, it
changes in the ctor depending on compile flags that I don't think we want
leaking into the .h), but the assumption is we'd never "not" null-initialize the
pointers, so we won't end up churning these.

 
   // True if static expect-CT state should be used.
   bool enable_static_expect_ct_;
 
   // True if static expect-staple state should be used.
   bool enable_static_expect_staple_;
 
-  ExpectCTReporter* expect_ct_reporter_;
+  ExpectCTReporter* expect_ct_reporter_ = nullptr;
+
+  RequireCTDelegate* require_ct_delegate_ = nullptr;
 
   // Keeps track of reports that have been sent recently for
   // rate-limiting.
   ExpiringCache<std::string, bool, base::TimeTicks, std::less<base::TimeTicks>>
       sent_reports_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(TransportSecurityState);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_TRANSPORT_SECURITY_STATE_H_