[Backport] Security bug 1033260

Manual backport of patch:
macOS 10.13+ switched to using trustd over XPC to verify
certificates, but can (for unknown reasons) occasionally
fail, affecting the legacy/non-built-in verifier
(CertVerifyProcMac).

When this happens, don't re-validate the same certificate
multiple times, since trustd will keep complaining.

TBR=mattm@chromium.org
(cherry picked from commit a7bdc8bfd0fdf679e8667e16c489c592ca973f37)

Bug: 1033260
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#724437}
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3945@{#1006}
Cr-Branched-From: e4635fff7defbae0f9c29e798349f6fc0cce4b1b-refs/heads/master@{#706915}
Change-Id: I57ad4ebeac4059f232624b6af4345021ea3fbcfb
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

1 files changed, 21 insertions, 4 deletions

diff --git a/chromium/net/cert/cert_verify_proc_mac.cc b/chromium/net/cert/cert_verify_proc_mac.cc
index ea7407985cc..fd6902075d3 100644
--- a/chromium/net/cert/cert_verify_proc_mac.cc
+++ b/chromium/net/cert/cert_verify_proc_mac.cc
@@ -144,6 +144,7 @@ CertStatus CertStatusFromOSStatus(OSStatus status) {
       // TODO(wtc): Should we add CERT_STATUS_WRONG_USAGE?
       return CERT_STATUS_INVALID;
 
+    case errSecInternalError:
     case CSSMERR_APPLETP_CRL_BAD_URI:
     case CSSMERR_APPLETP_IDP_FAIL:
       return CERT_STATUS_INVALID;
@@ -513,6 +514,10 @@ CRLSetResult CheckRevocationWithCRLSet(CFArrayRef chain, CRLSet* crl_set) {
 // |verified_chain|, and |chain_info| with the verification results. On
 // failure, no output parameters are modified.
 //
+// WARNING: Beginning with macOS 10.13, if |trust_result| is equal to
+// kSecTrustResultInvalid, any further accesses via SecTrust APIs to |trust_ref|
+// may invalidate |chain_info|.
+//
 // Note: An OK return does not mean that |cert_array| is trusted, merely that
 // verification was performed successfully.
 //
@@ -614,6 +619,8 @@ int BuildAndEvaluateSecTrustRef(CFArrayRef cert_array,
     return NetErrorFromOSStatus(status);
   CFArrayRef tmp_verified_chain = NULL;
   CSSM_TP_APPLE_EVIDENCE_INFO* tmp_chain_info;
+  // WARNING: Beginning with OS X 10.13, |tmp_chain_info| may be freed by any
+  // other accesses via SecTrust APIs to |tmp_trust|.
   status = SecTrustGetResult(tmp_trust, &tmp_trust_result, &tmp_verified_chain,
                              &tmp_chain_info);
   if (status)
@@ -905,8 +912,21 @@ int VerifyWithGivenFlags(X509Certificate* cert,
   bool policy_fail_already_mapped = false;
   bool weak_key_or_signature_algorithm = false;
 
+  // As of macOS 10.13, if |trust_result| (from SecTrustGetResult) returns
+  // kSecTrustResultInvalid, subsequent invocations of SecTrust APIs may
+  // result in revalidating the SecTrust, invalidating the pointers such
+  // as |chain_info|. In releases earlier than 10.13, this call would have
+  // additional information, except that information is unused and
+  // irrelevant if the result was invalid, so the placeholder
+  // errSecInternalError is fine.
+  OSStatus cssm_result = errSecInternalError;
+  if (trust_result != kSecTrustResultInvalid) {
+    status = SecTrustGetCssmResultCode(trust_ref, &cssm_result);
+    if (status)
+      return NetErrorFromOSStatus(status);
+  }
+
   // Evaluate the results
-  OSStatus cssm_result;
   switch (trust_result) {
     case kSecTrustResultUnspecified:
     case kSecTrustResultProceed:
@@ -923,9 +943,6 @@ int VerifyWithGivenFlags(X509Certificate* cert,
 
     case kSecTrustResultRecoverableTrustFailure:
       // Certificate chain has a failure that can be overridden by the user.
-      status = SecTrustGetCssmResultCode(trust_ref, &cssm_result);
-      if (status)
-        return NetErrorFromOSStatus(status);
       if (cssm_result == CSSMERR_TP_VERIFY_ACTION_FAILED) {
         policy_failed = true;
       } else {