Implement LoadTemporaryRoot for Windows

net/base/x509_certificate_mac.cc

 // Copyright (c) 2006-2008 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/base/x509_certificate.h"
 
 #include <CommonCrypto/CommonDigest.h>
 #include <Security/Security.h>
 #include <time.h>
 
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/sys_string_conversions.h"
 #include "net/base/cert_status_flags.h"
 #include "net/base/cert_verify_result.h"
 #include "net/base/net_errors.h"
+#include "net/base/test_root_certs.h"
 
 using base::mac::ScopedCFTypeRef;
 using base::Time;
 
 namespace net {
 
-class MacTrustedCertificates {
- public:
-  // Sets the trusted root certificate used by tests. Call with |cert| set
-  // to NULL to clear the test certificate.
-  void SetTestCertificate(X509Certificate* cert) {
-    AutoLock lock(lock_);
-    test_certificate_ = cert;
-  }
-
-  // Returns an array containing the trusted certificates for use with
-  // SecTrustSetAnchorCertificates(). Returns NULL if the system-supplied
-  // list of trust anchors is acceptable (that is, there is not test
-  // certificate available). Ownership follows the Create Rule (caller
-  // is responsible for calling CFRelease on the non-NULL result).
-  CFArrayRef CopyTrustedCertificateArray() {
-    AutoLock lock(lock_);
-
-    if (!test_certificate_)
-      return NULL;
-
-    // Failure to copy the anchor certificates or add the test certificate
-    // is non-fatal; SecTrustEvaluate() will use the system anchors instead.
-    CFArrayRef anchor_array;
-    OSStatus status = SecTrustCopyAnchorCertificates(&anchor_array);
-    if (status)
-      return NULL;
-    ScopedCFTypeRef<CFArrayRef> scoped_anchor_array(anchor_array);
-    CFMutableArrayRef merged_array = CFArrayCreateMutableCopy(
-        kCFAllocatorDefault, 0, anchor_array);
-    if (!merged_array)
-      return NULL;
-    CFArrayAppendValue(merged_array, test_certificate_->os_cert_handle());
-
-    return merged_array;
-  }
- private:
-  friend struct DefaultSingletonTraits<MacTrustedCertificates>;
-
-  // Obtain an instance of MacTrustedCertificates via the singleton
-  // interface.
-  MacTrustedCertificates() : test_certificate_(NULL) { }
-
-  // An X509Certificate object that may be appended to the list of
-  // system trusted anchors.
-  scoped_refptr<X509Certificate> test_certificate_;
-
-  // The trusted cache may be accessed from multiple threads.
-  mutable Lock lock_;
-
-  DISALLOW_COPY_AND_ASSIGN(MacTrustedCertificates);
-};
-
-void SetMacTestCertificate(X509Certificate* cert) {
-  Singleton<MacTrustedCertificates>::get()->SetTestCertificate(cert);
-}
-
 namespace {
 
 typedef OSStatus (*SecTrustCopyExtendedResultFuncPtr)(SecTrustRef,
                                                       CFDictionaryRef*);
 
 int NetErrorFromOSStatus(OSStatus status) {
   switch (status) {
     case noErr:
       return OK;
     case errSecNotAvailable:
@@ skipping 445 matching lines @@
   // Apple's cert code, which we suspect are caused by a thread-safety issue.
   // So as a speculative fix allow only one thread to use SecTrust on this cert.
   AutoLock lock(verification_lock_);
 
   SecTrustRef trust_ref = NULL;
   status = SecTrustCreateWithCertificates(cert_array, ssl_policy, &trust_ref);
   if (status)
     return NetErrorFromOSStatus(status);
   ScopedCFTypeRef<SecTrustRef> scoped_trust_ref(trust_ref);
 
-  // Set the trusted anchor certificates for the SecTrustRef by merging the
-  // system trust anchors and the test root certificate.
-  CFArrayRef anchor_array =
-      Singleton<MacTrustedCertificates>::get()->CopyTrustedCertificateArray();
-  ScopedCFTypeRef<CFArrayRef> scoped_anchor_array(anchor_array);
-  if (anchor_array) {
-    status = SecTrustSetAnchorCertificates(trust_ref, anchor_array);
-    if (status)
-      return NetErrorFromOSStatus(status);
-  }
+  TestRootCerts* root_certs = TestRootCerts::GetInstance();
+  status = root_certs->SetAnchorCertificates(trust_ref);

[bulach, 2010/11/17 17:17:30]

I know SetAnchorCertificates already checks, but I think at this point it'd be
clearer to comment that this is only for test code.

TestRootCerts* root_certs = TestRootCerts::GetInstance();
if (!root_certs->IsEmpty()) {
  // We're running tests, set the anchor certificates.
  status = root_certs->SetAnchorCertificates(trust_ref);
  if (status)
    return NetErrorFromOSStatus(status);
}

it may be even clearer to have this under a 
if (TestRootCerts::HasInstance()) {
  ...
}

and require that tests that needs this would initialize the singleton
beforehand. thoughts?

[wtc, 2010/11/18 02:12:49]

Nit: I find this harder to understand because this looks
like an operation on root_certs, but this actually acts
on trust_ref.  The original code shows that more clearly.

Two possible solutions:
1. Rename the method:
  status = root_certs->SetAnchorCertificatesOnSecTrust(trust_ref);
2. Go back to the style of the original code:
  CFArrayRef anchor_array = root_certs->CopyRootCertArray();
  status = SecTrustSetAnchorCertificates(trust_ref, anchor_array);

[Ryan Sleevi, 2010/11/18 05:31:58]

re: IsEmpty(), it seems unnecessary to put the check in two places. I've updated
the documentation of SetAnchorCertificates to better describe the behaviour,
especially with respect to IsEmpty(). Does that work for you?

re: HasInstance: How do you see something like that working? I don't see
anything that can check-without-create on Singleton<> or LazyInstance<>.

[Ryan Sleevi, 2010/11/18 05:31:58]

I went with 3, which renamed it to FixupSecTrustRef to match FixupChainContext
on Windows. Does that sufficiently disambiguate?

[bulach, 2010/11/18 12:42:12]

nit: I'd rename root_certs to test_root_certs to make it even more explicit this
is only used for tests.

this works for me, but just to clarify re:singleton:
since the tests would have to initialize the singleton on startup before the
actual test hits this, a I think a simple static bool on TestRootCerts
initialized to false and toggled on the singleton ctor would be sufficient..

your current code is clear to me now, but perhaps we'd like to avoid
instantiating the TestRootCerts object in production code and only initialize it
in test code?

+  if (status)
+    return NetErrorFromOSStatus(status);
 
   if (flags & VERIFY_REV_CHECKING_ENABLED) {
     // When called with VERIFY_REV_CHECKING_ENABLED, we ask SecTrustEvaluate()
     // to apply OCSP and CRL checking, but we're still subject to the global
     // settings, which are configured in the Keychain Access application (in
     // the Certificates tab of the Preferences dialog). If the user has
     // revocation disabled (which is the default), then we will get
     // kSecTrustResultRecoverableTrustFailure back from SecTrustEvaluate()
     // with one of a number of sub error codes indicating that revocation
     // checking did not occur. In that case, we'll set our own result to include
@@ skipping 439 matching lines @@
     }
     CFRelease(cert_chain);
   }
 exit:
   if (result)
     LOG(ERROR) << "CreateIdentityCertificateChain error " << result;
   return chain.release();
 }
 
 }  // namespace net