Skip to content

Commit

Permalink
fix: skip direct OpenSSL API interaction for certificate conversion
Browse files Browse the repository at this point in the history
Electron does not expose its bundled SSL library to users, unlike
Node.js (which is probably reasonable, considering that it uses
BoringSSL, which will generally have different ABI properties than
OpenSSL). As a consequence, building this addon for Electron fails
currently.

However, there is an easier solution available now that I overlooked;
the X509Certificate class available since Node.js 15.6.0 supports
direct conversion from DER to PEM as well, so no native interactions
with the OpenSSL API are actually necessary these days.
  • Loading branch information
addaleax committed Jul 11, 2024
1 parent 3c589f8 commit 118b1f1
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 48 deletions.
8 changes: 0 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,3 @@ This functions returns an array of PEM-formatted certificates.
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).
39 changes: 1 addition & 38 deletions binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
#include <napi.h>
#include <windows.h>
#include <wincrypt.h>
#include <openssl/err.h>
#include <openssl/x509.h>
#include <openssl/pem.h>

using namespace Napi;

Expand Down Expand Up @@ -59,13 +56,6 @@ 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<BYTE> CertToBuffer(Env env, PCCERT_CONTEXT cert, LPCWSTR password, DWORD export_flags) {
Expand All @@ -91,33 +81,6 @@ Buffer<BYTE> 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<const unsigned char*>(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<uintmax_t>(string_length) > std::numeric_limits<size_t>::max()) {
ThrowOpenSSLError(env, "BIO_get_mem_data()");
}
return String::New(env, string_data, string_length);
}

class CertStoreHandle {
public:
CertStoreHandle(HCERTSTORE store) : store_(store) {}
Expand Down Expand Up @@ -175,7 +138,7 @@ Value ExportAllCertificates(const CallbackInfo& args) {
Array result = Array::New(args.Env());
size_t index = 0;
while (cert = sys_cs.next()) {
String buf = CertToPEMBuffer(args.Env(), cert);
Buffer<BYTE> buf = Buffer<BYTE>::Copy(args.Env(), cert->pbCertEncoded, cert->cbCertEncoded);
result[index++] = buf;
}
return result;
Expand Down
5 changes: 3 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const {
exportAllCertificates,
storeTypes
} = require('bindings')('win_export_cert');
const { randomBytes } = require('crypto');
const { randomBytes, X509Certificate } = require('crypto');
const util = require('util');

const DEFAULT_STORE_TYPE_LIST = ['CERT_SYSTEM_STORE_LOCAL_MACHINE', 'CERT_SYSTEM_STORE_CURRENT_USER'];
Expand All @@ -29,7 +29,8 @@ function exportSystemCertificates(opts = {}) {
const result = new Set();
for (const storeType of storeTypeList) {
for (const cert of exportAllCertificates(store || 'ROOT', storeType)) {
result.add(cert);
// X509Certificate was added in Node.js 15 and accepts DER as input, but .toString() returns PEM
result.add(new X509Certificate(cert).toString());
}
}

Expand Down

0 comments on commit 118b1f1

Please sign in to comment.