Certificate Transparency: Add the high-level interface for verifying SCTs over multiple logs

net/cert/multi_log_ct_verifier.h

+// Copyright (c) 2013 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_CERT_MULTI_LOG_CT_VERIFIER_H_
+#define NET_CERT_MULTI_LOG_CT_VERIFIER_H_
+
+#include <map>
+#include <string>
+
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "net/base/net_export.h"
+#include "net/cert/ct_verifier.h"
+#include "net/cert/signed_certificate_timestamp.h"
+
+namespace net {
+
+namespace ct {
+struct LogEntry;
+}  // namespace ct
+
+class CTLogVerifier;
+
+// A Certificate Transparency verifier that can verifies Signed Certificate

[wtc, 2013/11/21 02:05:02]

Nit: can verifies => can verify

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+// Timestamps from multiple logs.
+// There should be a global instance of this class and for all known logs,
+// AddLog should be called with a CTLogVerifier (which is created from the
+// Log's public key).

[wtc, 2013/11/21 02:05:02]

Nit: "Log's" doesn't need to be capitalized

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+class NET_EXPORT MultiLogCTVerifier : public CTVerifier {
+ public:
+  explicit MultiLogCTVerifier();

[wtc, 2013/11/21 02:05:02]

Remove "explicit". It's only necessary for constructors that take one parameter.

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+  virtual ~MultiLogCTVerifier();
+
+  void AddLog(scoped_ptr<CTLogVerifier> log_verifier);
+
+  // CTVerifier implementation:
+  virtual int Verify(X509Certificate* verified_cert,

[wtc, 2013/11/21 02:05:02]

Nit: we should simply name this parameter |cert|.

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+                     const std::string& sct_list_from_ocsp,
+                     const std::string& sct_list_from_tls_handshake,

[wtc, 2013/11/21 02:05:02]

Nit: change "from_tls_handshake" to "from_tls_extension", if you agree.

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+                     ct::CTVerifyResult* result) OVERRIDE;
+
+ private:
+  // Verify a list of SCTs from |encoded_sct_list|, placing the verification
+  // results in |result|. Fills in the origin field of each SCT from
+  // |origin|.

[wtc, 2013/11/21 02:05:02]

1. Nit: "Fills in the origin field of each SCT from |origin|" is very confusing
without looking at the code in the .cc file. It would be better to describe at a
more abstract level how |origin| would be used by this function, for example,
  Treat each SCT as being obtained from |origin|.
Or
  The SCTs in the list come from |origin|.

2. Nit: explain what |expected_entry| is for.

[Eran M. (Google), 2013/11/21 20:06:02]

Done.

+  bool VerifySCTs(const std::string& encoded_sct_list,
+                  const ct::LogEntry& expected_entry,
+                  ct::SignedCertificateTimestamp::Origin origin,
+                  ct::CTVerifyResult* result);
+
+  // Verifies a single, parsed SCT against all logs.
+  bool VerifySingleSCT(
+      const ct::SignedCertificateTimestamp& sct,
+      const ct::LogEntry& expected_entry,
+      ct::CTVerifyResult* result);
+
+  typedef std::map<std::string, linked_ptr<CTLogVerifier> > IDToLogMap;

[wtc, 2013/11/21 02:05:02]

1. Nit: typedef should be listed at the beginning of the "private" section.

Recommended by the Style Guide:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

2. Nit: document what the std::string "ID" is. I guess it's the ID of the log's
public key?

[Eran M. (Google), 2013/11/21 20:06:02]

1. Done

2. You're right - the ID is a hash of the log's public key, which is defined in
section 3.2. of RFC6962 (CT). Added that information in the comment.

+  IDToLogMap logs_;
+
+  DISALLOW_COPY_AND_ASSIGN(MultiLogCTVerifier);
+};
+
+}  // namespace net
+
+#endif  // NET_CERT_MULTI_LOG_CT_VERIFIER_H_