Adds domain names for all qualified CT logs

net/cert/ct_log_verifier.h

 // Copyright 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_CT_LOG_VERIFIER_H_
 #define NET_CERT_CT_LOG_VERIFIER_H_
 
 #include <string>
 
 #include "base/gtest_prod_util.h"
@@ skipping 22 matching lines @@
 // Tree Head (STH) signatures.
 // Immutable: Does not hold any state beyond the log information it was
 // initialized with.
 class NET_EXPORT CTLogVerifier
     : public base::RefCountedThreadSafe<CTLogVerifier> {
  public:
   // Creates a new CTLogVerifier that will verify SignedCertificateTimestamps
   // using |public_key|, which is a DER-encoded SubjectPublicKeyInfo.
   // If |public_key| refers to an unsupported public key, returns NULL.
   // |description| is a textual description of the log.
+  // |url| is the URL of the log's HTTPS API endpoint.
+  // |dns_domain| is the DNS name of the log's DNS API endpoint, if one exists.

[Ryan Sleevi, 2016/07/18 23:14:47]

Why should this be part of the CTLogVerifier API?

The description says this is a class for verifying signatures of a single CT log
- but this isn't a necessary parameter for that.

My guess is because CTLogVerifier has been overloaded to also, as a convenience,
hold status information about the log (it's description and URL are examples of
similar issues).

Are we just smuggling information through the layers via this class? Is that the
right design? I'm not sure, and it seems like now that we keep adding fields to
it, it's reasonable to ask if this is right - either we should change what
CTLogVerifier means, or we should change how we associate this data.

[Rob Percival, 2016/07/19 00:04:20]

The reason I implemented it this way was simply because this is how all of the
other log information is transferred, and it'd have been odd to put it somewhere
else. However, I think you're right to question why this class has all of these
properties. The only one it actually needs is |public_key|.

[Eran Messeri, 2016/07/21 15:03:51]

Ryan has a good point - the CTLogVerifier has been indeed overloaded to hold
additional (static) information about the log.

My proposal for this change is to clearly document it. Then, after this change
lands, revisit and see if there's usage of the CTLogVerifier that needs the URL
/ description / dns_domain but does not use any of the verification functions
and, if that's the case, think about how to properly separate (one way to do it
would be to have a CTLogInfo struct/class that would be passed around and
CTLogVerifiers would be created on-demand from the CT log key in CTLogInfo).

   static scoped_refptr<const CTLogVerifier> Create(
       const base::StringPiece& public_key,
       const base::StringPiece& description,
-      const base::StringPiece& url);
+      const base::StringPiece& url,
+      const base::StringPiece& dns_domain);
 
   // Returns the log's key ID (RFC6962, Section 3.2)
   const std::string& key_id() const { return key_id_; }
   // Returns the log's human-readable description.
   const std::string& description() const { return description_; }
   // Returns the log's URL
   const GURL& url() const { return url_; }
 
+  // Returns the log's DNS domain for CT over DNS queries, as described in
+  // https://github.com/google/certificate-transparency-rfcs/blob/master/dns/draft-ct-over-dns.md.
+  // This will be empty if the log has no DNS API endpoint.
+  const std::string& dns_domain() const { return dns_domain_; }
+
   // Verifies that |sct| is valid for |entry| and was signed by this log.
   bool Verify(const ct::LogEntry& entry,
               const ct::SignedCertificateTimestamp& sct) const;
 
   // Verifies that |signed_tree_head| is a valid Signed Tree Head (RFC 6962,
   // Section 3.5) for this log.
   bool VerifySignedTreeHead(const ct::SignedTreeHead& signed_tree_head) const;
 
   // Verifies that |proof| is a valid consistency proof (RFC 6962, Section
   // 2.1.2) for this log, and which proves that |old_tree_hash| has
   // been fully incorporated into the Merkle tree represented by
   // |new_tree_hash|.
   bool VerifyConsistencyProof(const ct::MerkleConsistencyProof& proof,
                               const std::string& old_tree_hash,
                               const std::string& new_tree_hash) const;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(CTLogVerifierTest, VerifySignature);
   friend class base::RefCountedThreadSafe<CTLogVerifier>;
 
-  CTLogVerifier(const base::StringPiece& description, const GURL& url);
+  CTLogVerifier(const base::StringPiece& description,
+                const GURL& url,
+                const base::StringPiece& dns_domain);
   ~CTLogVerifier();
 
   // Performs crypto-library specific initialization.
   bool Init(const base::StringPiece& public_key);
 
   // Performs the underlying verification using the selected public key. Note
   // that |signature| contains the raw signature data (eg: without any
   // DigitallySigned struct encoding).
   bool VerifySignature(const base::StringPiece& data_to_sign,
                        const base::StringPiece& signature) const;
 
   // Returns true if the signature and hash algorithms in |signature|
   // match those of the log
   bool SignatureParametersMatch(const ct::DigitallySigned& signature) const;
 
   std::string key_id_;
   std::string description_;
   GURL url_;
+  std::string dns_domain_;
   ct::DigitallySigned::HashAlgorithm hash_algorithm_;
   ct::DigitallySigned::SignatureAlgorithm signature_algorithm_;
 
   EVP_PKEY* public_key_;
 };
 
 }  // namespace net
 
 #endif  // NET_CERT_CT_LOG_VERIFIER_H_