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

net/cert/multi_log_ct_verifier.cc

+// 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.
+
+#include "net/cert/multi_log_ct_verifier.h"
+
+#include "net/base/net_errors.h"
+#include "net/cert/ct_log_verifier.h"
+#include "net/cert/ct_objects_extractor.h"
+#include "net/cert/ct_serialization.h"
+#include "net/cert/ct_verify_result.h"
+#include "net/cert/x509_certificate.h"
+
+namespace net {
+
+MultiLogCTVerifier::MultiLogCTVerifier() { }
+
+MultiLogCTVerifier::~MultiLogCTVerifier() { }
+
+void MultiLogCTVerifier::AddLog(scoped_ptr<CTLogVerifier> log_verifier) {
+  if (!log_verifier)
+    return;

[wtc, 2013/11/21 23:26:34]

Are you sure we need to allow a null |log_verifier|?

[Eran M. (Google), 2013/11/23 21:02:07]

No, but I wouldn't want to crash here in production code. Added DCHECK.

[wtc, 2013/11/25 23:15:50]

A crash may help expose a bug sooner. If we know none of the callers will pass
null, the null check is not necessary. Alternatively, the function can return
bool to report a failure.

+
+  std::string log_id(log_verifier->key_id());
+  logs_[log_id].reset(log_verifier.release());
+}
+
+int MultiLogCTVerifier::Verify(
+    X509Certificate* cert,
+    const std::string& sct_list_from_ocsp,
+    const std::string& sct_list_from_tls_extension,
+    ct::CTVerifyResult* result)  {
+  DCHECK(cert);
+  DCHECK(result);
+
+  result->verified_scts.clear();
+  result->unverified_scts.clear();
+  result->unknown_logs_scts.clear();
+
+  bool has_verified_scts = false;
+
+  std::string embedded_scts;
+  if (!cert->GetIntermediateCertificates().empty() &&
+      ct::ExtractEmbeddedSCTList(
+          cert->os_cert_handle(),
+          &embedded_scts)) {
+    ct::LogEntry precert_entry;
+
+    has_verified_scts =
+        ct::GetPrecertLogEntry(
+            cert->os_cert_handle(),
+            cert->GetIntermediateCertificates().front(),
+            &precert_entry) &&
+        VerifySCTs(
+            embedded_scts,
+            precert_entry,
+            ct::SignedCertificateTimestamp::SCT_EMBEDDED,
+            result);
+  }
+
+  ct::LogEntry x509_entry;
+  if (!ct::GetX509LogEntry(cert->os_cert_handle(), &x509_entry))
+    return has_verified_scts ? OK : ERR_CT_LOG_ENTRY_CREATION_FAILED;
+
+  has_verified_scts |= VerifySCTs(
+      sct_list_from_ocsp,
+      x509_entry,
+      ct::SignedCertificateTimestamp::SCT_FROM_OCSP_RESPONSE,
+      result);
+
+  has_verified_scts |= VerifySCTs(
+      sct_list_from_tls_extension,
+      x509_entry,
+      ct::SignedCertificateTimestamp::SCT_FROM_TLS_HANDSHAKE,
+      result);
+
+  if (has_verified_scts)
+    return OK;
+
+  if (!result->unverified_scts.empty())
+    return ERR_NO_SCTS_VERIFIED_OK;
+
+  if (!result->unknown_logs_scts.empty())
+    return ERR_NO_SCTS_FROM_KNOWN_LOGS;
+
+  return ERR_NO_SCTS_PRESENT;

[wtc, 2013/11/21 23:26:34]

It should be sufficient to just use one error code here because the caller can
inspect |result| to distinguish the three error conditions.

[Eran M. (Google), 2013/11/23 21:02:07]

Will be done in the next patch, when I introduce the new error code.

+}
+
+bool MultiLogCTVerifier::VerifySCTs(
+    const std::string& encoded_sct_list,
+    const ct::LogEntry& expected_entry,
+    ct::SignedCertificateTimestamp::Origin origin,
+    ct::CTVerifyResult* result) {
+  if (logs_.empty())
+    return false;
+
+  base::StringPiece temp(encoded_sct_list);
+  std::vector<base::StringPiece> sct_list;
+
+  if (!ct::DecodeSCTList(&temp, &sct_list))
+    return false;
+
+  bool verified = false;
+  for (std::vector<base::StringPiece>::const_iterator it = sct_list.begin();
+       it != sct_list.end(); ++it) {
+    base::StringPiece encoded_sct(*it);
+    ct::SignedCertificateTimestamp decoded_sct;
+    if (!DecodeSignedCertificateTimestamp(&encoded_sct, &decoded_sct)) {
+      // XXX(rsleevi): Should we really just skip over bad SCTs?
+      continue;
+    }
+    decoded_sct.origin = origin;
+
+    verified |= VerifySingleSCT(decoded_sct, expected_entry, result);
+  }
+
+  return verified;
+}
+
+bool MultiLogCTVerifier::VerifySingleSCT(
+    const ct::SignedCertificateTimestamp& sct,
+    const ct::LogEntry& expected_entry,
+    ct::CTVerifyResult* result) {
+
+  // Assume this SCT is untrusted until proven otherwise.
+
+  IDToLogMap::iterator it = logs_.find(sct.log_id);
+  if (it == logs_.end()) {
+    DVLOG(1) << "SCT does not match any known log.";
+    result->unknown_logs_scts.push_back(sct);
+    return false;
+  }
+
+  if (!it->second->Verify(expected_entry, sct)) {
+    DVLOG(1) << "Unable to verify SCT signature.";
+    result->unverified_scts.push_back(sct);
+    return false;
+  }
+
+  // SCT verified ok, just make sure the timestamp is legitimate.
+  // Add 1 second to allow some slack for accepting SCTs which have *Just*
+  // been issued.

[wtc, 2013/11/21 23:26:34]

1. The comment in the reference implementation says:
  / Allow a bit of slack, say 1 second into the future.

If we want to allow 1 second into the future, it seems that we should add 1
second to base::Time::Now():
  if (sct.timestamp > base::Time::Now() + base::TimeDelta::FromSeconds(1)) {

(The reference implementation uses util::TimeInMilliseconds() + 1000.)

Your test actually makes the check stricter rather than slacker.

2. To allow clock skew, a 1-second slack is too short.

[Eran M. (Google), 2013/11/23 21:02:07]

I've dropped this 1 second interval entirely. You're right, a 1-second slack is
too short.
On the next iteration we should measure how common is SCT failure due to this
cause and adjust slack accordingly.

+  if (sct.timestamp + base::TimeDelta::FromSeconds(1) > base::Time::Now()) {
+    DVLOG(1) << "SCT is from the future!";
+    result->unverified_scts.push_back(sct);
+    return false;
+  }
+
+  result->verified_scts.push_back(sct);
+  return true;
+}
+
+} // namespace net