Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors

net/cert/cert_verify_proc_win.cc

 // 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.
 
 #include "net/cert/cert_verify_proc_win.h"
 
 #include <memory>
 #include <string>
 #include <vector>
 
+#include "base/debug/alias.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/memory/free_deleter.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/sha1.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_local.h"
+#include "base/time/time.h"
 #include "crypto/capi_util.h"
 #include "crypto/scoped_capi_types.h"
 #include "crypto/sha2.h"
 #include "net/base/net_errors.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/cert_verify_result.h"
 #include "net/cert/crl_set.h"
 #include "net/cert/ev_root_ca_metadata.h"
@@ skipping 861 matching lines @@
 }
 
 class ScopedThreadLocalCRLSet {
  public:
   explicit ScopedThreadLocalCRLSet(CRLSet* crl_set) {
     g_revocation_injector.Get().SetCRLSet(crl_set);
   }
   ~ScopedThreadLocalCRLSet() { g_revocation_injector.Get().SetCRLSet(nullptr); }
 };
 
+// Sends a crash dump (without actually crashing) when the system time
+// falls within the validity period of every certificate in
+// |verified_cert|'s chain. This is to investigate reports of odd
+// certificate errors that report ERR_CERT_DATE_INVALID when the
+// certificate chain's dates appear to be valid.
+//
+// TODO(estark): remove this after obtaining diagnostic data from
+// Canary. https://crbug.com/672906
+void MaybeDumpCertificateDateError(
+    const scoped_refptr<X509Certificate>& verified_cert,
+    DWORD error_status) {
+  const base::Time now = base::Time::NowFromSystemTime();
+  DWORD temp_error_status = error_status;
+  base::debug::Alias(&temp_error_status);

[Will Harris, 2016/12/09 21:33:05]

base::debug::Alias requires you to open up the crash dump in windbg

you could consider using a base::debug::ScopedCrashKey to set a crash key then
it would display in the crash UI directly.

[Ryan Sleevi, 2016/12/09 21:35:53]

Seems like 913-914 should be moved to 932, since that's where it's first used.

[estark, 2016/12/09 22:57:38]

Done.

[estark, 2016/12/09 22:57:38]

Done.

+  // If the leaf certificate is expired or not yet valid, nothing is odd.
+  if (now >= verified_cert->valid_expiry() ||
+      now <= verified_cert->valid_start()) {
+    return;
+  }
+  // Repeat the check for the rest of the certificates in the chain; if
+  // any of them is expired or not yet valid, nothing is odd.
+  X509Certificate::OSCertHandles intermediates =
+      verified_cert->GetIntermediateCertificates();
+  for (const auto& intermediate : intermediates) {
+    base::Time valid_start =
+        base::Time::FromFileTime(intermediate->pCertInfo->NotBefore);
+    base::Time valid_expiry =
+        base::Time::FromFileTime(intermediate->pCertInfo->NotAfter);
+    if (now >= valid_expiry || now <= valid_start)
+      return;
+  }
+  // None of the certificates in the chain appear to be expired or
+  // not-yet-valid, so send a crash dump for diagnostics.
+  base::debug::DumpWithoutCrashing();

[Will Harris, 2016/12/09 21:33:05]

can you make this canary only?

[estark, 2016/12/09 22:57:38]

Oh, sorry, I forgot to mention that -- I couldn't figure out a way to do that
outside of //chrome (i.e. without using chrome::GetChannel). Am I missing
something obvious?

[Will Harris, 2016/12/10 00:35:07]

ah yes that's right you can't depend on chrome/ from net/

in which case, just try and revert this as soon as you have the data.

+}
+
 }  // namespace
 
 CertVerifyProcWin::CertVerifyProcWin() {}
 
 CertVerifyProcWin::~CertVerifyProcWin() {}
 
 bool CertVerifyProcWin::SupportsAdditionalTrustAnchors() const {
   return false;
 }
 
@@ skipping 255 matching lines @@
         CERT_TRUST_IS_OFFLINE_REVOCATION) {
       verify_result->cert_status |= CERT_STATUS_REVOKED;
     }
   }
 
   ScopedPCCERT_CHAIN_CONTEXT scoped_chain_context(chain_context);
 
   verify_result->cert_status |= MapCertChainErrorStatusToCertStatus(
       chain_context->TrustStatus.dwErrorStatus);
 
+  // Send some diagnostic data in the event of certificate date errors
+  // that occur on chains with validity periods that are valid according
+  // to the system clock.
+  // TODO(estark): remove this after obtaining diagnostic data from
+  // Canary. https://crbug.com/672906
+  if (verify_result->cert_status & CERT_STATUS_DATE_INVALID) {
+    MaybeDumpCertificateDateError(verify_result->verified_cert,
+                                  chain_context->TrustStatus.dwErrorStatus);

[Ryan Sleevi, 2016/12/09 21:35:53]

I'd recommend you grab both dwErrorStatus and dwInfoStatus

[estark, 2016/12/09 22:57:38]

Done.

+  }
+
   // Flag certificates that have a Subject common name with a NULL character.
   if (CertSubjectCommonNameHasNull(cert_handle))
     verify_result->cert_status |= CERT_STATUS_INVALID;
 
   base::string16 hostname16 = base::ASCIIToUTF16(hostname);
 
   SSL_EXTRA_CERT_CHAIN_POLICY_PARA extra_policy_para;
   memset(&extra_policy_para, 0, sizeof(extra_policy_para));
   extra_policy_para.cbSize = sizeof(extra_policy_para);
   extra_policy_para.dwAuthType = AUTHTYPE_SERVER;
@@ skipping 53 matching lines @@
     return MapCertStatusToNetError(verify_result->cert_status);
 
   if (ev_policy_oid &&
       CheckEV(chain_context, rev_checking_enabled, ev_policy_oid)) {
     verify_result->cert_status |= CERT_STATUS_IS_EV;
   }
   return OK;
 }
 
 }  // namespace net