Mac: Use SecPolicyCreateSSL instead of CreatePolicy(&CSSMOID_APPLE_TP_SSL, ...)

net/cert/x509_util_mac.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/x509_util_mac.h"
 
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/strings/sys_string_conversions.h"
 #include "third_party/apple_apsl/cssmapplePriv.h"
 
 namespace net {
 
 // CSSM functions are deprecated as of OSX 10.7, but have no replacement.
 // https://bugs.chromium.org/p/chromium/issues/detail?id=590914#c1
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 
 namespace x509_util {
@@ skipping 26 matching lines @@
       return err;
     }
   }
   return noErr;
 }
 
 }  // namespace
 
 
 OSStatus CreateSSLClientPolicy(SecPolicyRef* policy) {
-  CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
-  memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
-  tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
-  tp_ssl_options.Flags |= CSSM_APPLE_TP_SSL_CLIENT;
-
-  return CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
-                      sizeof(tp_ssl_options), policy);
+  *policy = SecPolicyCreateSSL(false /* server */, nullptr);
+  // XXX check *policy is not null?
+  return noErr;

[Ryan Sleevi, 2016/09/27 22:04:42]

ERR_NOT_IMPLEMENTED / errSecNoPolicyModule

[mattm, 2016/09/27 23:39:22]

Done.

 }
 
 OSStatus CreateSSLServerPolicy(const std::string& hostname,
                                SecPolicyRef* policy) {
+  base::ScopedCFTypeRef<CFStringRef> hostname_cfstring;

[Ryan Sleevi, 2016/09/27 22:04:42]

hostname_cfstring feels a little weird (naming), in that it uses hungarian type
notation.

scoped_hostname ? I suppose that feels as bad

[mattm, 2016/09/27 23:39:22]

I suppose an argument could be made about variables whose only purpose is to
convert something to a different type, maybe including the type in the name is
relevant then?

   if (!hostname.empty()) {
-    CSSM_APPLE_TP_SSL_OPTIONS tp_ssl_options;
-    memset(&tp_ssl_options, 0, sizeof(tp_ssl_options));
-    tp_ssl_options.Version = CSSM_APPLE_TP_SSL_OPTS_VERSION;
-    tp_ssl_options.ServerName = hostname.data();
-    tp_ssl_options.ServerNameLen = hostname.size();
-
-    return CreatePolicy(&CSSMOID_APPLE_TP_SSL, &tp_ssl_options,
-                        sizeof(tp_ssl_options), policy);
+    hostname_cfstring.reset(base::SysUTF8ToCFStringRef(hostname));
+    if (!hostname_cfstring) {
+      // XXX better error code?

[Ryan Sleevi, 2016/09/27 22:04:42]

ERR_NOT_IMPLEMENTED / errSecNoPolicyModule

[mattm, 2016/09/27 23:39:22]

Done.

+      return errSecParam;
+    }
   }
 
-  return CreatePolicy(&CSSMOID_APPLE_TP_SSL, nullptr, 0U, policy);
+  *policy = SecPolicyCreateSSL(true /* server */, hostname_cfstring.get());
+  // XXX check *policy is not null?
+  return noErr;
 }
 
 OSStatus CreateBasicX509Policy(SecPolicyRef* policy) {
   return CreatePolicy(&CSSMOID_APPLE_X509_BASIC, NULL, 0, policy);

[Ryan Sleevi, 2016/09/27 22:04:42]

For safety, we can probably replace this with
https://developer.apple.com/reference/security/1397202-secpolicycreatebasicx5...

[mattm, 2016/09/27 23:39:22]

Done.

 }
 
 OSStatus CreateRevocationPolicies(bool enable_revocation_checking,
                                   bool enable_ev_checking,
                                   CFMutableArrayRef policies) {
   OSStatus status = noErr;
 
   // In order to bypass the system revocation checking settings, the
   // SecTrustRef must have at least one revocation policy associated with it.
   // Since it is not known prior to verification whether the Apple TP will
@@ skipping 142 matching lines @@
   CSSM_CL_CertAbortQuery(cl_handle_, results_handle);
   field->Reset(cl_handle_, oid, field_ptr);
   return CSSM_OK;
 }
 
 }  // namespace x509_util
 
 #pragma clang diagnostic pop  // "-Wdeprecated-declarations"
 
 }  // namespace net