crypto: addressed PR comments

nodejs · Nov 2, 2022 · 758b5a6 · 758b5a6
1 parent e08a295
commit 758b5a6
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 36 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -1534,8 +1534,10 @@ 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`
 

diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc
@@ -206,54 +206,57 @@ void ReadSystemStoreCertificates(
   auto cleanup =
       OnScopeLeave([hStore]() { CHECK_EQ(CertCloseStore(hStore, 0), TRUE); });
 
-  PCCERT_CONTEXT pCtx = nullptr;
+  PCCERT_CONTEXT certificate_context_ptr = nullptr;
 
-  while ((pCtx = CertEnumCertificatesInStore(hStore, pCtx)) != nullptr) {
-    const DWORD cbSize = CertGetNameStringW(
-        pCtx, CERT_NAME_SIMPLE_DISPLAY_TYPE, 0, nullptr, nullptr, 0);
+  std::vector<X509*> system_root_certificates_X509;
 
-    CHECK_GT(cbSize, 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);
 
-    std::vector<wchar_t> pszName(cbSize);
+    CHECK_GT(certificate_buffer_size, 0);
 
-    CHECK_GT(CertGetNameStringW(pCtx,
+    std::vector<wchar_t> certificate_name(certificate_buffer_size);
+
+    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
 }

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