Certificate Transparency: Start tracking logs' state

components/certificate_transparency/single_tree_tracker.h

+// Copyright 2016 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 COMPONENTS_CERTIFICATE_TRANSPARENCY_SINGLE_TREE_TRACKER_H_
+#define COMPONENTS_CERTIFICATE_TRANSPARENCY_SINGLE_TREE_TRACKER_H_
+
+#include <map>
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "base/time/time.h"
+#include "net/cert/ct_verifier.h"
+#include "net/cert/signed_tree_head.h"
+#include "net/cert/sth_observer.h"
+
+namespace net {
+class CTLogVerifier;
+class X509Certificate;
+
+namespace ct {
+struct SignedCertificateTimestamp;
+}  // namespace ct
+
+}  // namespace net
+
+namespace certificate_transparency {
+
+// Tracks the state of an individual CT log's Merkle Tree. By keeping the
+// latest Signed Tree Head for a log, this class can check inclusion for
+// observed SCTs.

[Ryan Sleevi, 2016/05/09 17:03:49]

Can  you expand this comment more? What is involved with "tracks the state".
What's the state for tracking? The second sentence seems to talk about
"potentially can be used for", rather than "is used for"

// Tracks the state of an individual Certificate Transparency Log's Merkle Tree.
A
// CT Log is made up of many Signed Tree Heads, for which every older STH must
// be incorporated into the current/newer STH. As new certificates are logged,
// new SCTs are produced, and eventually, those SCTs are incorporated into the
log
// and a new STH is produced, with there being a consistency proof between the
// SCTs and the new STH, and between the old STH and the new STH.
// This class tracks STHs that are observed, and ensures that as newer STHs are
// observed, they consistently incorporate older STHs, establishing that the log
is
// consistent. Further, as SCTs are observed, their status is checked against
the STH
// to ensure that they were properly logged. If an SCT is newer than the latest
STH,
// then this class observes that when an STH is observed that should have
incorporated
// those SCTs, the SCTs are present.
//
// To accomplish this, this class needs to be notified of when new SCTs are
observed
// (which it does by implementing net::CTVerifier::Observer) and when new STHs
are
// observed (which it does by implementing net::ct::STHObserver). Once connected
to
// sources providing that data, the status for a given SCT can be queried by
calling
// GetLogEntryInclusionCheck ...


That's a super-wordy explanation, but try to be explicit about the details here.
The goal is to explain how this class fits within the CT ecosystem, minimizing
shared/assumed knowledge when possible. I'm handwaving away how the consistency
proofs are fetched, or proven, but I'm establishing terms like SCT and STH,
since those are abbreviations being used here in this class and serve to
contextualize the interface and all the comments on the methods, and also tries
to set up the "Well, once you have all this, what's it good for?"

[Eran Messeri, 2016/05/10 20:01:10]

Done - used your text with some adaptations.

+class SingleTreeTracker : public net::CTVerifier::Observer,
+                          public net::ct::STHObserver {
+ public:
+  // TODO(eranm): This enum will expand to include check success/failure,
+  // see crbug.com/506227
+  enum SCTInclusionStatus {
+    // SCT was not observed by this class.
+    SCT_NOT_OBSERVED,
+
+    // SCT was observed but the STH known to this class is not old
+    // enough to check for inclusion.
+    SCT_PENDING_NEWER_STH,
+
+    // SCT is known and there's a new-enough STH to check inclusion against.
+    // Actual inclusion check has to be performed.
+    SCT_PENDING_INCLUSION_CHECK
+  };
+
+  explicit SingleTreeTracker(scoped_refptr<const net::CTLogVerifier> ct_log);
+  ~SingleTreeTracker() override;
+
+  // net::ct::CTVerifier::Observer implementation.

[Ryan Sleevi, 2016/05/09 17:03:49]

Now that we have net::ct::STHObserver, it does make me wonder whether we should
refactoring CTVerifier::Observer to instead be net::ct::SCTObserver, and have
net::ct::CTVerifier take it (that is, moving it from being an inner class to
being a standalone pure virtual interface)

[Eran Messeri, 2016/05/10 20:01:10]

Acknowledged - can this be done in a follow-up CL? Added a TODO.

+  // Performs an inclusion check for the given certificate if the latest
+  // STH known for this log is older than sct.timestamp + MMD, enqueues
+  // the SCT for future checking later on.
+  // TODO(eranm): Make sure not to perform any synchronous, blocking operation
+  // here as this callback is invoked during certificate validation.
+  void OnSCTVerified(net::X509Certificate* cert,
+                     const net::ct::SignedCertificateTimestamp* sct) override;
+
+  // net::ct::STHObserver implementation.
+  // After verification of the signature over the |sth|, uses this
+  // STH for future inclusion checks.
+  void NewSTHObserved(const net::ct::SignedTreeHead& sth) override;
+
+  // Returns the status of a given log entry (that is assembled from
+  // |cert| and |sct|).

[Ryan Sleevi, 2016/05/09 17:03:49]

You're implicitly dropping something here, which is that |cert| and |sct| must
have been previously observed via OnSCTVerified; this isn't a general querying
mechanism.

[Eran Messeri, 2016/05/10 20:01:10]

I don't quite grasp the reasoning behind this - right now if |cert| and |sct|
weren't previously observed, SCT_NOT_OBSERVED is returned.
I've updated the documentation to reflect that if |cert| and |sct| were not
observed, do not match each other or the |sct| is not for this log then
SCT_NOT_OBSERVED will be returned.

+  SCTInclusionStatus GetLogEntryInclusionStatus(
+      net::X509Certificate* cert,
+      const net::ct::SignedCertificateTimestamp* sct);
+
+ private:
+  // Logs an error and aborts if the provided log_id is not the same as the
+  // log id this instance knows of.
+  void CheckLogId(const std::string& log_id);
+
+  // Holds the latest STH fetched and verified for each log.
+  net::ct::SignedTreeHead verified_sth_;
+  // The log tracked.
+  scoped_refptr<const net::CTLogVerifier> ct_log_;
+  // List of log entries pending inclusion check.

[Ryan Sleevi, 2016/05/09 17:03:49]

Some newlines between each of these members would help readability.

// A explanation
A a_;

// B explanation
B b_;

// C explanation
C c_

[Eran Messeri, 2016/05/10 20:01:10]

Done.

+  // TODO(eranm): Rather than rely on the timestamp, extend to to use the
+  // whole MerkleTreeLeaf (RFC6962, section 3.4.) as a key. See crbug.com/506227
+  std::map<base::Time, SCTInclusionStatus> entries_status_;
+
+  DISALLOW_COPY_AND_ASSIGN(SingleTreeTracker);
+};
+
+}  // namespace certificate_transparency
+
+#endif  // COMPONENTS_CERTIFICATE_TRANSPARENCY_SINGLE_TREE_TRACKER_H_