Remove x509_certificate_win.

Windows has been on use_byte_certs since M61.

Bug: 671420
Change-Id: I78c7f7f2a2e94ebd18d4d0c76909a9549231ec85
Reviewed-on: https://chromium-review.googlesource.com/661885Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501374}

net/BUILD.gn

@@ -601,10 +601,8 @@ component("net") {
       "cert/test_root_certs_mac.cc",
       "cert/test_root_certs_nss.cc",
       "cert/test_root_certs_win.cc",
-      "cert/x509_cert_types_win.cc",
       "cert/x509_certificate_known_roots_win.h",
       "cert/x509_certificate_nss.cc",
-      "cert/x509_certificate_win.cc",
       "cert/x509_util_android.cc",
       "cert/x509_util_ios.cc",
       "cert/x509_util_ios.h",
@@ -1824,12 +1822,6 @@ component("net") {
       if (use_nss_certs) {
         sources -= [ "cert/x509_certificate_nss.cc" ]
       }
-      if (is_win) {
-        sources -= [
-          "cert/x509_cert_types_win.cc",
-          "cert/x509_certificate_win.cc",
-        ]
-      }
     } else {
       sources -= [ "cert/x509_certificate_bytes.cc" ]
     }

net/cert/x509_cert_types.h

@@ -37,7 +37,7 @@ struct NET_EXPORT CertPrincipal {
   explicit CertPrincipal(const std::string& name);
   ~CertPrincipal();
 
-#if BUILDFLAG(USE_BYTE_CERTS) || defined(OS_WIN)
+#if BUILDFLAG(USE_BYTE_CERTS)
   // Parses a BER-format DistinguishedName.
   // TODO(mattm): change this to take a der::Input.
   bool ParseDistinguishedName(const void* ber_name_data, size_t length);

net/cert/x509_cert_types_unittest.cc

@@ -14,7 +14,7 @@ namespace net {
 
 namespace {
 
-#if BUILDFLAG(USE_BYTE_CERTS) || defined(OS_WIN)
+#if BUILDFLAG(USE_BYTE_CERTS)
 TEST(X509TypesTest, ParseDNVerisign) {
   CertPrincipal verisign;
   EXPECT_TRUE(verisign.ParseDistinguishedName(VerisignDN, sizeof(VerisignDN)));

net/cert/x509_cert_types_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/x509_cert_types.h"
-
-#include <windows.h>
-
-#include <memory>
-
-#include "base/logging.h"
-#include "base/memory/free_deleter.h"
-#include "base/strings/string_util.h"
-#include "base/strings/utf_string_conversions.h"
-#include "crypto/capi_util.h"
-#include "crypto/wincrypt_shim.h"
-
-namespace net {
-
-namespace {
-
-// A list of OIDs to decode. Any OID not on this list will be ignored for
-// purposes of parsing.
-const char* const kOIDs[] = {
-  szOID_COMMON_NAME,
-  szOID_LOCALITY_NAME,
-  szOID_STATE_OR_PROVINCE_NAME,
-  szOID_COUNTRY_NAME,
-  szOID_STREET_ADDRESS,
-  szOID_ORGANIZATION_NAME,
-  szOID_ORGANIZATIONAL_UNIT_NAME,
-  szOID_DOMAIN_COMPONENT
-};
-
-// Converts the value for |attribute| to an UTF-8 string, storing the result
-// in |value|. Returns false if the string cannot be converted.
-bool GetAttributeValue(PCERT_RDN_ATTR attribute,
-                       std::string* value) {
-  DWORD chars_needed = CertRDNValueToStrW(attribute->dwValueType,
-                                          &attribute->Value, NULL, 0);
-  if (chars_needed == 0)
-    return false;
-  if (chars_needed == 1) {
-    // The value is actually an empty string (chars_needed includes a single
-    // char for a NULL value). Don't bother converting - just clear the
-    // string.
-    value->clear();
-    return true;
-  }
-  std::wstring wide_name;
-  DWORD chars_written = CertRDNValueToStrW(
-      attribute->dwValueType, &attribute->Value,
-      base::WriteInto(&wide_name, chars_needed), chars_needed);
-  if (chars_written <= 1)
-    return false;
-  wide_name.resize(chars_written - 1);
-  *value = base::WideToUTF8(wide_name);
-  return true;
-}
-
-// Adds a type+value pair to the appropriate vector from a C array.
-// The array is keyed by the matching OIDs from kOIDS[].
-bool AddTypeValuePair(PCERT_RDN_ATTR attribute,
-                      std::vector<std::string>* values[]) {
-  for (size_t oid = 0; oid < arraysize(kOIDs); ++oid) {
-    if (strcmp(attribute->pszObjId, kOIDs[oid]) == 0) {
-      std::string value;
-      if (!GetAttributeValue(attribute, &value))
-        return false;
-      values[oid]->push_back(value);
-      break;
-    }
-  }
-  return true;
-}
-
-// Stores the first string of the vector, if any, to *single_value.
-void SetSingle(const std::vector<std::string>& values,
-               std::string* single_value) {
-  // We don't expect to have more than one CN, L, S, and C.
-  LOG_IF(WARNING, values.size() > 1) << "Didn't expect multiple values";
-  if (!values.empty())
-    *single_value = values[0];
-}
-
-}  // namespace
-
-bool CertPrincipal::ParseDistinguishedName(const void* ber_name_data,
-                                           size_t length) {
-  DCHECK(ber_name_data);
-
-  CRYPT_DECODE_PARA decode_para;
-  decode_para.cbSize = sizeof(decode_para);
-  decode_para.pfnAlloc = crypto::CryptAlloc;
-  decode_para.pfnFree = crypto::CryptFree;
-  CERT_NAME_INFO* name_info = NULL;
-  DWORD name_info_size = 0;
-  BOOL rv;
-  rv = CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-                           WINCRYPT_X509_NAME,
-                           reinterpret_cast<const BYTE*>(ber_name_data),
-                           length,
-                           CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG,
-                           &decode_para,
-                           &name_info, &name_info_size);
-  if (!rv)
-    return false;
-  std::unique_ptr<CERT_NAME_INFO, base::FreeDeleter> scoped_name_info(
-      name_info);
-
-  std::vector<std::string> common_names, locality_names, state_names,
-      country_names;
-
-  std::vector<std::string>* values[] = {
-      &common_names, &locality_names,
-      &state_names, &country_names,
-      &this->street_addresses,
-      &this->organization_names,
-      &this->organization_unit_names,
-      &this->domain_components
-  };
-  DCHECK(arraysize(kOIDs) == arraysize(values));
-
-  for (DWORD cur_rdn = 0; cur_rdn < name_info->cRDN; ++cur_rdn) {
-    PCERT_RDN rdn = &name_info->rgRDN[cur_rdn];
-    for (DWORD cur_ava = 0; cur_ava < rdn->cRDNAttr; ++cur_ava) {
-      PCERT_RDN_ATTR ava = &rdn->rgRDNAttr[cur_ava];
-      if (!AddTypeValuePair(ava, values))
-        return false;
-    }
-  }
-
-  SetSingle(common_names, &this->common_name);
-  SetSingle(locality_names, &this->locality_name);
-  SetSingle(state_names, &this->state_or_province_name);
-  SetSingle(country_names, &this->country_name);
-  return true;
-}
-
-}  // namespace net

net/cert/x509_certificate.h

@@ -22,9 +22,6 @@
 
 #if BUILDFLAG(USE_BYTE_CERTS)
 #include "third_party/boringssl/src/include/openssl/base.h"
-#elif defined(OS_WIN)
-#include <windows.h>
-#include "crypto/wincrypt_shim.h"
 #elif defined(USE_NSS_CERTS)
 // Forward declaration; real one in <cert.h>
 struct CERTCertificateStr;
@@ -55,8 +52,6 @@ class NET_EXPORT X509Certificate
   // TODO(mattm): Remove OSCertHandle type and clean up the interfaces once all
   // platforms use the CRYPTO_BUFFER version.
   typedef CRYPTO_BUFFER* OSCertHandle;
-#elif defined(OS_WIN)
-  typedef PCCERT_CONTEXT OSCertHandle;
 #elif defined(USE_NSS_CERTS)
   typedef struct CERTCertificateStr* OSCertHandle;
 #else

net/cert/x509_util_win.cc

@@ -25,7 +25,6 @@ using ScopedHCERTSTORE = crypto::ScopedCAPIHandle<
 scoped_refptr<X509Certificate> CreateX509CertificateFromCertContexts(
     PCCERT_CONTEXT os_cert,
     const std::vector<PCCERT_CONTEXT>& os_chain) {
-#if BUILDFLAG(USE_BYTE_CERTS)
   if (!os_cert || !os_cert->pbCertEncoded || !os_cert->cbCertEncoded)
     return nullptr;
   bssl::UniquePtr<CRYPTO_BUFFER> cert_handle(
@@ -52,9 +51,6 @@ scoped_refptr<X509Certificate> CreateX509CertificateFromCertContexts(
   scoped_refptr<X509Certificate> result(
       X509Certificate::CreateFromHandle(cert_handle.get(), intermediates_raw));
   return result;
-#else
-  return X509Certificate::CreateFromHandle(os_cert, os_chain);
-#endif
 }
 
 ScopedPCCERT_CONTEXT CreateCertContextWithChain(const X509Certificate* cert) {
@@ -75,7 +71,6 @@ ScopedPCCERT_CONTEXT CreateCertContextWithChain(
 
   PCCERT_CONTEXT primary_cert = nullptr;
 
-#if BUILDFLAG(USE_BYTE_CERTS)
   BOOL ok = CertAddEncodedCertificateToStore(
       store.get(), X509_ASN_ENCODING,
       CRYPTO_BUFFER_data(cert->os_cert_handle()),
@@ -97,27 +92,6 @@ ScopedPCCERT_CONTEXT CreateCertContextWithChain(
       LOG(WARNING) << "error parsing intermediate";
     }
   }
-#else
-  PCCERT_CONTEXT os_cert_handle = cert->os_cert_handle();
-  const std::vector<PCCERT_CONTEXT>& intermediate_ca_certs =
-      cert->GetIntermediateCertificates();
-
-  // NOTE: This preserves all of the properties of |os_cert_handle| except
-  // for CERT_KEY_PROV_HANDLE_PROP_ID and CERT_KEY_CONTEXT_PROP_ID - the two
-  // properties that hold access to already-opened private keys. If a handle
-  // has already been unlocked (eg: PIN prompt), then the first time that the
-  // identity is used for client auth, it may prompt the user again.
-  BOOL ok = CertAddCertificateContextToStore(
-      store.get(), os_cert_handle, CERT_STORE_ADD_ALWAYS, &primary_cert);
-  if (!ok || !primary_cert)
-    return nullptr;
-  ScopedPCCERT_CONTEXT scoped_primary_cert(primary_cert);
-
-  for (PCCERT_CONTEXT intermediate : intermediate_ca_certs) {
-    CertAddCertificateContextToStore(store.get(), intermediate,
-                                     CERT_STORE_ADD_ALWAYS, NULL);
-  }
-#endif
 
   // Note: |primary_cert| retains a reference to |store|, so the store will
   // actually be freed when |primary_cert| is freed.

net/cert/x509_util_win.h

@@ -47,35 +47,6 @@ NET_EXPORT scoped_refptr<X509Certificate> CreateX509CertificateFromCertContexts(
 // multiple threads if no further modifications happen, it is generally
 // preferable for each thread that needs such a context to obtain its own,
 // rather than risk thread-safety issues by sharing.
-//
-// ------------------------------------------------------------------------
-// The following remarks only apply when USE_BYTE_CERTS=false (e.g., when
-// using x509_certificate_win).
-// TODO(mattm): remove references to USE_BYTE_CERTS and clean up the rest of
-// the comment when x509_certificate_win is deleted.
-//
-// The returned PCCERT_CONTEXT *MUST NOT* be stored in an X509Certificate, as
-// this will cause os_cert_handle() to return incorrect results.
-//
-// Depending on the CryptoAPI function, Windows may need to access the
-// HCERTSTORE that the passed-in PCCERT_CONTEXT belongs to, such as to
-// locate additional intermediates. However, all X509Certificate handles are
-// added to a NULL HCERTSTORE, allowing the system to manage the resources.  As
-// a result, intermediates for |cert->os_cert_handle()| cannot be located
-// simply via |cert->os_cert_handle()->hCertStore|, as it refers to a magic
-// value indicating "only this certificate".
-//
-// To avoid this problems, a new in-memory HCERTSTORE is created containing
-// just this certificate and its intermediates. The handle to the version of
-// the current certificate in the new HCERTSTORE is then returned, with the
-// PCCERT_CONTEXT's HCERTSTORE set to be automatically freed when the returned
-// certificate handle is freed.
-//
-// Because of how X509Certificate caching is implemented, attempting to
-// create an X509Certificate from the returned PCCERT_CONTEXT may result in
-// the original handle (and thus the originall HCERTSTORE) being returned by
-// os_cert_handle(). For this reason, the returned PCCERT_CONTEXT *MUST NOT*
-// be stored in an X509Certificate.
 NET_EXPORT ScopedPCCERT_CONTEXT
 CreateCertContextWithChain(const X509Certificate* cert);
 

net/ssl/client_cert_store_win.cc

@@ -187,13 +187,6 @@ ClientCertIdentityList GetClientCertsImpl(HCERTSTORE cert_store,
       intermediates.pop_back();
     }
 
-    // TODO(mattm): The following comment is only true when not using
-    // USE_BYTE_CERTS. Remove it once the non-byte-certs code is also removed.
-    // TODO(svaldez): cert currently wraps cert_context2 which may be backed
-    // by a smartcard with threading difficulties. Instead, create a fresh
-    // X509Certificate with CreateFromBytes and route cert_context2 into the
-    // SSLPrivateKey. Probably changing CertificateList to be a
-    // pair<X509Certificate, SSLPrivateKeyCallback>.
     scoped_refptr<X509Certificate> cert =
         x509_util::CreateX509CertificateFromCertContexts(cert_context2,
                                                          intermediates);
@@ -237,7 +230,6 @@ void ClientCertStoreWin::GetClientCerts(
     return;
   }
 
-#if BUILDFLAG(USE_BYTE_CERTS)
   if (base::PostTaskAndReplyWithResult(
           GetSSLPlatformKeyTaskRunner().get(), FROM_HERE,
           // Caller is responsible for keeping the |request| alive
@@ -250,11 +242,6 @@ void ClientCertStoreWin::GetClientCerts(
 
   // If the task could not be posted, behave as if there were no certificates.
   callback.Run(ClientCertIdentityList());
-#else
-  // When using PCERT_CONTEXT based X509Certificate, must do this on the same
-  // thread.
-  callback.Run(GetClientCertsWithMyCertStore(request));
-#endif
 }
 
 // static