Commit ba0293aa authored by estark's avatar estark Committed by Commit bot

Revert of Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors (patchset...

Revert of Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors (patchset #5 id:80001 of https://codereview.chromium.org/2567643003/ )

Reason for revert:
Turns out this issue is trivial to reproduce locally (I probably should have tried that first...) so diagnostic data from crash dumps isn't needed.

Original issue's description:
> Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors
>
> Safe Browsing Extended Reporting reports show a number of certificate
> errors from Windows users where the certificate error is
> ERR_CERT_DATE_INVALID but the certificate chain appears to have
> perfectly valid dates.
>
> To get some diagnostic data from Canary, this CL calls
> DumpWithoutCrashing() when we observe such a chain. This CL is intended
> to be reverted after obtaining a few days of data from Canary.
>
> BUG=672906
>
> Committed: https://crrev.com/822fd5f5b3ab71603d34a376928bf05323abfd0b
> Cr-Commit-Position: refs/heads/master@{#437726}

TBR=rsleevi@chromium.org,wfh@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=672906

Review-Url: https://codereview.chromium.org/2572553002
Cr-Commit-Position: refs/heads/master@{#437916}
parent 03adaf93
...@@ -8,16 +8,12 @@ ...@@ -8,16 +8,12 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/debug/crash_logging.h"
#include "base/debug/dump_without_crashing.h"
#include "base/memory/free_deleter.h" #include "base/memory/free_deleter.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/sha1.h" #include "base/sha1.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_local.h" #include "base/threading/thread_local.h"
#include "base/time/time.h"
#include "crypto/capi_util.h" #include "crypto/capi_util.h"
#include "crypto/scoped_capi_types.h" #include "crypto/scoped_capi_types.h"
#include "crypto/sha2.h" #include "crypto/sha2.h"
...@@ -899,47 +895,6 @@ class ScopedThreadLocalCRLSet { ...@@ -899,47 +895,6 @@ class ScopedThreadLocalCRLSet {
~ScopedThreadLocalCRLSet() { g_revocation_injector.Get().SetCRLSet(nullptr); } ~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,
DWORD info_status) {
const base::Time now = base::Time::NowFromSystemTime();
// 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::ScopedCrashKey error_status_crash_key(
"cert_verify_proc_win_date_error_error_status",
base::IntToString(error_status));
base::debug::ScopedCrashKey info_status_crash_key(
"cert_verify_proc_win_date_error_info_status",
base::IntToString(info_status));
base::debug::DumpWithoutCrashing();
}
} // namespace } // namespace
CertVerifyProcWin::CertVerifyProcWin() {} CertVerifyProcWin::CertVerifyProcWin() {}
...@@ -1215,17 +1170,6 @@ int CertVerifyProcWin::VerifyInternal( ...@@ -1215,17 +1170,6 @@ int CertVerifyProcWin::VerifyInternal(
verify_result->cert_status |= MapCertChainErrorStatusToCertStatus( verify_result->cert_status |= MapCertChainErrorStatusToCertStatus(
chain_context->TrustStatus.dwErrorStatus); 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,
chain_context->TrustStatus.dwInfoStatus);
}
// Flag certificates that have a Subject common name with a NULL character. // Flag certificates that have a Subject common name with a NULL character.
if (CertSubjectCommonNameHasNull(cert_handle)) if (CertSubjectCommonNameHasNull(cert_handle))
verify_result->cert_status |= CERT_STATUS_INVALID; verify_result->cert_status |= CERT_STATUS_INVALID;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment