From 578f970d35983ef807f526203fe4cfea35857742 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 21 Jun 2024 17:46:24 +0200 Subject: [PATCH] feat!: use OpenSSL instead of node-forge for DER-to-PEM conversion This commit removes node-forge as a dependency and the conversion logic for X.509 certificates from DER to PEM format that we used it for, and replaces that logic with direct calls to OpenSSL functions (which are always available in Node.js). The main downside of this approach is that it removes the ABI stability guarantees we get from Node-API and instead makes us dependent on the OpenSSL ABI as well. Currently, the functions used in this code are compatible with OpenSSL 1.1.x and 3.x, which should be good enough in the present and for at least a good number of years into the future. The main upside of this approach is that it is significantly faster, and should enable applications that use this package to get the system certificate list to do so by default, rather than as an opt-in feature. --- README.md | 9 ++++++++- binding.cc | 41 ++++++++++++++++++++++++++++++++++++++++- index.js | 16 +++------------- package.json | 3 +-- test.js | 1 + 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index a41f476..0295396 100644 --- a/README.md +++ b/README.md @@ -41,10 +41,17 @@ Valid options are: to try. Default: `['CERT_SYSTEM_STORE_LOCAL_MACHINE', 'CERT_SYSTEM_STORE_CURRENT_USER']`. This functions returns an array of PEM-formatted certificates. -(Note that this can be a fairly slow operation.) ## Testing You need to import `testkeys\certificate.pfx` manually into your local CA store in order for the tests to pass. Make sure to import that certificate with the "exportable private key" option. The password for the file is `pass`. + +## Compatibility + +Current versions of this package use OpenSSL to perform a format conversion +from DER to PEM. This means that, unlike other Node-API addons, this addon +has a stronger dependency on the ABI exposed by Node.js, and in particular +may need to be updated once Node.js starts using OpenSSL 4.x (which is not +planned at the time of writing). diff --git a/binding.cc b/binding.cc index c82a9b5..16de4f3 100644 --- a/binding.cc +++ b/binding.cc @@ -1,6 +1,11 @@ +#define NOMINMAX // otherwise windows.h #defines min and max as macros +#include #include #include #include +#include +#include +#include using namespace Napi; @@ -54,6 +59,13 @@ void ThrowWindowsError(Env env, const char* call) { throw Error::New(env, buf); } +void ThrowOpenSSLError(Env env, const char* call) { + std::string error = call; + error += " failed with: "; + error += ERR_error_string(ERR_get_error(), nullptr); + throw Error::New(env, error); +} + // Create a temporary certificate store, add 'cert' to it, and then // export it (using 'password' for encryption). Buffer CertToBuffer(Env env, PCCERT_CONTEXT cert, LPCWSTR password, DWORD export_flags) { @@ -79,6 +91,33 @@ Buffer CertToBuffer(Env env, PCCERT_CONTEXT cert, LPCWSTR password, DWORD return outbuf; } +// Export a X.509 certificate (which does not include a key) as a PEM string. +String CertToPEMBuffer(Env env, PCCERT_CONTEXT cert) { + const unsigned char* pbCertEncoded = reinterpret_cast(cert->pbCertEncoded); + X509* x509cert = d2i_X509(nullptr, &pbCertEncoded, cert->cbCertEncoded); + if (!x509cert) { + ThrowOpenSSLError(env, "d2i_X509()"); + } + Cleanup cleanup_cert([&]() { X509_free(x509cert); }); + + BIO* bio = BIO_new(BIO_s_mem()); + if (!bio) { + ThrowOpenSSLError(env, "BIO_new()"); + } + Cleanup cleanup_bio([&]() { BIO_free(bio); }); + + if (!PEM_write_bio_X509(bio, x509cert)) { + ThrowOpenSSLError(env, "PEM_write_bio_X509()"); + } + + char* string_data = nullptr; + long string_length = BIO_get_mem_data(bio, &string_data); + if (!string_data || static_cast(string_length) > std::numeric_limits::max()) { + ThrowOpenSSLError(env, "BIO_get_mem_data()"); + } + return String::New(env, string_data, string_length); +} + class CertStoreHandle { public: CertStoreHandle(HCERTSTORE store) : store_(store) {} @@ -136,7 +175,7 @@ Value ExportAllCertificates(const CallbackInfo& args) { Array result = Array::New(args.Env()); size_t index = 0; while (cert = sys_cs.next()) { - Buffer buf = CertToBuffer(args.Env(), cert, L"", 0); + String buf = CertToPEMBuffer(args.Env(), cert); result[index++] = buf; } return result; diff --git a/index.js b/index.js index 5dd24ad..f9671a8 100644 --- a/index.js +++ b/index.js @@ -1,10 +1,10 @@ +'use strict'; const { exportCertificateAndKey, exportAllCertificates, storeTypes } = require('bindings')('win_export_cert'); const { randomBytes } = require('crypto'); -const forge = require('node-forge'); const util = require('util'); const DEFAULT_STORE_TYPE_LIST = ['CERT_SYSTEM_STORE_LOCAL_MACHINE', 'CERT_SYSTEM_STORE_CURRENT_USER']; @@ -28,18 +28,8 @@ function exportSystemCertificates(opts = {}) { const result = new Set(); for (const storeType of storeTypeList) { - const certs = exportAllCertificates(store || 'ROOT', storeType); - for (const cert of certs) { - // Convert PKCS#12 (aka .pfx) to PEM - const asn1 = forge.asn1.fromDer(cert.toString('latin1')); - const pkcs12 = forge.pkcs12.pkcs12FromAsn1(asn1, ''); - const certBags = pkcs12.getBags({ bagType: forge.pki.oids.certBag })[forge.pki.oids.certBag]; - for (const bag of certBags) { - if (bag.cert) { - const pem = forge.pki.certificateToPem(bag.cert); - result.add(pem); - } - } + for (const cert of exportAllCertificates(store || 'ROOT', storeType)) { + result.add(cert); } } diff --git a/package.json b/package.json index 9017719..a766a43 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,7 @@ "gypfile": true, "dependencies": { "bindings": "^1.5.0", - "node-addon-api": "^3.1.0", - "node-forge": "^1.2.1" + "node-addon-api": "^3.1.0" }, "license": "Apache-2.0", "exports": { diff --git a/test.js b/test.js index 40ba7bb..28e720f 100644 --- a/test.js +++ b/test.js @@ -1,3 +1,4 @@ +'use strict'; const tls = require('tls'); const fs = require('fs'); const assert = require('assert');