crypto: addressed PR comments

nodejs · Nov 3, 2022 · 00f666a · 00f666a
1 parent 9d628a7
commit 00f666a
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 37 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -1534,8 +1534,8 @@ See `SSL_CERT_DIR` and `SSL_CERT_FILE`.
 Node.js uses the trusted CA certificates present in the system store along with
 the `--use-bundled-ca`, `--use-openssl-ca` options.
 
-Note, Only current user certificates are accessible using this method, not the
-local machine store. This option is available to Windows only.
+Only current user certificates are accessible using this method, not the local
+machine store. This option is available to Windows only.
 
 ### `--use-largepages=mode`
 
@@ -1880,6 +1880,7 @@ Node.js options that are allowed are:
 * `--no-global-search-paths`
 * `--no-warnings`
 * `--node-memory-debug`
+* `--node-use-system-ca`
 * `--openssl-config`
 * `--openssl-legacy-provider`
 * `--openssl-shared-config`

diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
@@ -199,61 +199,66 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
 
 void ReadSystemStoreCertificates(
     std::vector<std::string>* system_root_certificates) {
+#ifdef _WIN32
   const HCERTSTORE hStore = CertOpenSystemStoreW(0, L"ROOT");
-  CHECK_NE(hStore, NULLPTR);
+  CHECK_NE(hStore, nullptr);
 
   auto cleanup =
       OnScopeLeave([hStore]() { CHECK_EQ(CertCloseStore(hStore, 0), TRUE); });
 
-  PCCERT_CONTEXT pCtx = nullptr;
+  PCCERT_CONTEXT certificate_context_ptr = nullptr;
+
+  std::vector<X509*> system_root_certificates_X509;
 
-  while ((pCtx = CertEnumCertificatesInStore(hStore, pCtx)) != nullptr) {
-    const DWORD cbSize = CertGetNameStringW(
-        pCtx, CERT_NAME_SIMPLE_DISPLAY_TYPE, 0, nullptr, nullptr, 0);
+  while ((certificate_context_ptr = CertEnumCertificatesInStore(
+              hStore, certificate_context_ptr)) != nullptr) {
+    const DWORD certificate_buffer_size =
+        CertGetNameStringW(certificate_context_ptr,
+                           CERT_NAME_SIMPLE_DISPLAY_TYPE,
+                           0,
+                           nullptr,
+                           nullptr,
+                           0);
 
-    CHECK_GT(cbSize, 0);
+    CHECK_GT(certificate_buffer_size, 0);
 
-    std::vector<wchar_t> pszName(cbSize);
+    std::vector<wchar_t> certificate_name(certificate_buffer_size);
 
-    CHECK_GT(CertGetNameStringW(pCtx,
+    CHECK_GT(CertGetNameStringW(certificate_context_ptr,
                                 CERT_NAME_SIMPLE_DISPLAY_TYPE,
                                 0,
                                 nullptr,
-                                pszName.data(),
-                                cbSize),
+                                certificate_name.data(),
+                                certificate_buffer_size),
              0);
+    const unsigned char* certificate_src_ptr =
+        reinterpret_cast<const unsigned char*>(
+            certificate_context_ptr->pbCertEncoded);
+    const size_t certificate_src_length =
+        certificate_context_ptr->cbCertEncoded;
 
-    const char* certificate_src_ptr =
-        reinterpret_cast<const char*>(pCtx->pbCertEncoded);
-    const size_t slen = pCtx->cbCertEncoded;
-    const size_t dlen = base64_encoded_size(slen);
-
-    char* certificate_dst_ptr = UncheckedMalloc(dlen);
-
-    CHECK_NOT_NULL(certificate_dst_ptr);
-
-    auto cleanup =
-        OnScopeLeave([certificate_dst_ptr]() { free(certificate_dst_ptr); });
+    X509* cert =
+        d2i_X509(nullptr, &certificate_src_ptr, certificate_src_length);
 
-    const size_t written =
-        base64_encode(certificate_src_ptr, slen, certificate_dst_ptr, dlen);
-    CHECK_EQ(written, dlen);
+    system_root_certificates_X509.emplace_back(cert);
+  }
 
-    std::string base64_string_output(certificate_dst_ptr, dlen);
+  for (size_t i = 0; i < system_root_certificates_X509.size(); i++) {
+    int result = 0;
 
-    constexpr size_t distance = 72;
-    size_t pos = distance;
+    BIOPointer bio(BIO_new(BIO_s_mem()));
+    CHECK(bio);
 
-    while (pos < base64_string_output.size()) {
-      base64_string_output.insert(pos, "\n");
-      pos += distance + 1;
-    }
+    BUF_MEM* mem = nullptr;
+    result = PEM_write_bio_X509(bio.get(), system_root_certificates_X509[i]);
 
-    base64_string_output = "-----BEGIN CERTIFICATE-----\n" +
-                           base64_string_output + "\n-----END CERTIFICATE-----";
+    BIO_get_mem_ptr(bio.get(), &mem);
+    std::string certificate_string_pem(mem->data, mem->length);
+    system_root_certificates->emplace_back(certificate_string_pem);
 
-    system_root_certificates->emplace_back(std::move(base64_string_output));
+    bio.reset();
   }
+#endif
 }
 
 X509_STORE* NewRootCertStore() {

diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js
@@ -64,6 +64,9 @@ if (common.hasCrypto) {
   expectNoWorker('--use-bundled-ca', 'B\n');
   if (!common.hasOpenSSL3)
     expectNoWorker('--openssl-config=_ossl_cfg', 'B\n');
+  if (common.isWindows) {
+    expectNoWorker('--node-use-system-ca', 'B\n');
+  }
 }
 
 // V8 options

diff --git a/test/parallel/test-cli-node-print-help.js b/test/parallel/test-cli-node-print-help.js
@@ -27,7 +27,8 @@ function validateNodePrintHelp() {
     { compileConstant: HAVE_OPENSSL,
       flags: [ '--openssl-config=...', '--tls-cipher-list=...',
                '--use-bundled-ca', '--use-openssl-ca',
-               '--enable-fips', '--force-fips' ] },
+               '--enable-fips', '--force-fips',
+               '--node-use-system-ca' ] },
     { compileConstant: NODE_HAVE_I18N_SUPPORT,
       flags: [ '--icu-data-dir=...', 'NODE_ICU_DATA' ] },
     { compileConstant: HAVE_INSPECTOR,

diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js
@@ -59,6 +59,7 @@ const conditionalOpts = [
         common.hasOpenSSL3 ? '--openssl-shared-config' : '',
         '--tls-cipher-list',
         '--use-bundled-ca',
+        common.isWindows ? '--node-use-system-ca' : '',
         '--use-openssl-ca',
         '--secure-heap',
         '--secure-heap-min',