From 2248ba760b1f97eb7f3f03859a64fe765ec3e809 Mon Sep 17 00:00:00 2001 From: Eric Bickle Date: Tue, 3 Mar 2020 13:35:28 -0800 Subject: [PATCH] src: fix missing extra ca in tls.rootCertificates Fixes tls.rootCertificates missing certificates loaded from NODE_EXTRA_CA_CERTS. Fixes: https://github.com/nodejs/node/issues/32074 PR-URL: https://github.com/nodejs/node/pull/32075 Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- src/node_crypto.cc | 63 +++++++++++++------ test/parallel/test-tls-root-certificates.js | 69 +++++++++++++-------- 2 files changed, 89 insertions(+), 43 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index fdaf91acdc0a1c..34534662312605 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -986,24 +986,6 @@ static X509_STORE* NewRootCertStore() { } -void GetRootCertificates(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Local result[arraysize(root_certs)]; - - for (size_t i = 0; i < arraysize(root_certs); i++) { - if (!String::NewFromOneByte( - env->isolate(), - reinterpret_cast(root_certs[i]), - NewStringType::kNormal).ToLocal(&result[i])) { - return; - } - } - - args.GetReturnValue().Set( - Array::New(env->isolate(), result, arraysize(root_certs))); -} - - void SecureContext::AddCACert(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2680,6 +2662,21 @@ static inline Local BIOToStringOrBuffer(Environment* env, } } +static MaybeLocal X509ToPEM(Environment* env, X509* cert) { + BIOPointer bio(BIO_new(BIO_s_mem())); + if (!bio) { + ThrowCryptoError(env, ERR_get_error(), "BIO_new"); + return MaybeLocal(); + } + + if (PEM_write_bio_X509(bio.get(), cert) == 0) { + ThrowCryptoError(env, ERR_get_error(), "PEM_write_bio_X509"); + return MaybeLocal(); + } + + return BIOToStringOrBuffer(env, bio.get(), kKeyFormatPEM); +} + static bool WritePublicKeyInner(EVP_PKEY* pkey, const BIOPointer& bio, const PublicKeyEncodingConfig& config) { @@ -6660,6 +6657,36 @@ void ExportChallenge(const FunctionCallbackInfo& args) { } +void GetRootCertificates(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + + if (root_cert_store == nullptr) + root_cert_store = NewRootCertStore(); + + stack_st_X509_OBJECT* objs = X509_STORE_get0_objects(root_cert_store); + int num_objs = sk_X509_OBJECT_num(objs); + + std::vector> result; + result.reserve(num_objs); + + for (int i = 0; i < num_objs; i++) { + X509_OBJECT* obj = sk_X509_OBJECT_value(objs, i); + if (X509_OBJECT_get_type(obj) == X509_LU_X509) { + X509* cert = X509_OBJECT_get0_X509(obj); + + Local value; + if (!X509ToPEM(env, cert).ToLocal(&value)) + return; + + result.push_back(value); + } + } + + args.GetReturnValue().Set( + Array::New(env->isolate(), result.data(), result.size())); +} + + // Convert the input public key to compressed, uncompressed, or hybrid formats. void ConvertKey(const FunctionCallbackInfo& args) { MarkPopErrorOnReturn mark_pop_error_on_return; diff --git a/test/parallel/test-tls-root-certificates.js b/test/parallel/test-tls-root-certificates.js index 5f7aa418ac05a3..f200231fa301a5 100644 --- a/test/parallel/test-tls-root-certificates.js +++ b/test/parallel/test-tls-root-certificates.js @@ -2,30 +2,49 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const fixtures = require('../common/fixtures'); const assert = require('assert'); const tls = require('tls'); - -assert(Array.isArray(tls.rootCertificates)); -assert(tls.rootCertificates.length > 0); - -// Getter should return the same object. -assert.strictEqual(tls.rootCertificates, tls.rootCertificates); - -// Array is immutable... -assert.throws(() => tls.rootCertificates[0] = 0, /TypeError/); -assert.throws(() => tls.rootCertificates.sort(), /TypeError/); - -// ...and so is the property. -assert.throws(() => tls.rootCertificates = 0, /TypeError/); - -// Does not contain duplicates. -assert.strictEqual(tls.rootCertificates.length, - new Set(tls.rootCertificates).size); - -assert(tls.rootCertificates.every((s) => { - return s.startsWith('-----BEGIN CERTIFICATE-----\n'); -})); - -assert(tls.rootCertificates.every((s) => { - return s.endsWith('\n-----END CERTIFICATE-----'); -})); +const { fork } = require('child_process'); + +if (process.argv[2] !== 'child') { + // Parent + const NODE_EXTRA_CA_CERTS = fixtures.path('keys', 'ca1-cert.pem'); + + fork( + __filename, + ['child'], + { env: { ...process.env, NODE_EXTRA_CA_CERTS } } + ).on('exit', common.mustCall(function(status) { + assert.strictEqual(status, 0); + })); +} else { + // Child + assert(Array.isArray(tls.rootCertificates)); + assert(tls.rootCertificates.length > 0); + + // Getter should return the same object. + assert.strictEqual(tls.rootCertificates, tls.rootCertificates); + + // Array is immutable... + assert.throws(() => tls.rootCertificates[0] = 0, /TypeError/); + assert.throws(() => tls.rootCertificates.sort(), /TypeError/); + + // ...and so is the property. + assert.throws(() => tls.rootCertificates = 0, /TypeError/); + + // Does not contain duplicates. + assert.strictEqual(tls.rootCertificates.length, + new Set(tls.rootCertificates).size); + + assert(tls.rootCertificates.every((s) => { + return s.startsWith('-----BEGIN CERTIFICATE-----\n'); + })); + + assert(tls.rootCertificates.every((s) => { + return s.endsWith('\n-----END CERTIFICATE-----\n'); + })); + + const extraCert = fixtures.readKey('ca1-cert.pem', 'utf8'); + assert(tls.rootCertificates.includes(extraCert)); +}