From 15e8061b7a7a314f0041b81e323987463661e57f Mon Sep 17 00:00:00 2001 From: ShenHongFei Date: Wed, 18 Dec 2024 15:15:24 +0800 Subject: [PATCH] src: modify SecureContext::SetCACert to not use root_certs --- benchmark/tls/create-secure-context.js | 23 ++++++++++ src/crypto/crypto_context.cc | 62 ++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 benchmark/tls/create-secure-context.js diff --git a/benchmark/tls/create-secure-context.js b/benchmark/tls/create-secure-context.js new file mode 100644 index 00000000000000..720d862a9ac876 --- /dev/null +++ b/benchmark/tls/create-secure-context.js @@ -0,0 +1,23 @@ +'use strict'; +const common = require('../common.js'); +const fixtures = require('../../test/common/fixtures'); +const tls = require('tls'); + +const bench = common.createBenchmark(main, { + n: [1, 5], + ca: [0, 1], +}); + +function main(conf) { + const n = conf.n; + + const options = { + key: fixtures.readKey('rsa_private.pem'), + cert: fixtures.readKey('rsa_cert.crt'), + ca: conf.ca === 1 ? [fixtures.readKey('rsa_ca.crt')] : undefined, + }; + + bench.start(); + tls.createSecureContext(options); + bench.end(n); +} diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index aa5fc61f19e435..1d4a716abb089d 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -60,6 +60,45 @@ X509_STORE* GetOrCreateRootCertStore() { return store; } +// copies an X509_STORE object, including the certificates and crls it contains. +X509_STORE* CopyX509Store(X509_STORE* src) { + // create a new X509_STORE + X509_STORE* dest = X509_STORE_new(); + + if (*system_cert_path != '\0') { + ERR_set_mark(); + X509_STORE_load_locations(dest, system_cert_path, nullptr); + ERR_pop_to_mark(); + } + + if (per_process::cli_options->ssl_openssl_cert_store) { + CHECK_EQ(1, X509_STORE_set_default_paths(dest)); + } + + // copy flags + X509_STORE_set1_param(dest, X509_STORE_get0_param(src)); + + // get the certificates and crls from the source store + STACK_OF(X509_OBJECT)* objs = X509_STORE_get0_objects(src); + for (int i = 0; i < sk_X509_OBJECT_num(objs); i++) { + X509_OBJECT* obj = sk_X509_OBJECT_value(objs, i); + int type = X509_OBJECT_get_type(obj); + if (type == X509_LU_X509) { + // copy certs + X509* x509 = X509_OBJECT_get0_X509(obj); + X509_up_ref(x509); + X509_STORE_add_cert(dest, x509); + } else if (type == X509_LU_CRL) { + // copy crls + X509_CRL* crl = X509_OBJECT_get0_X509_CRL(obj); + X509_CRL_up_ref(crl); + X509_STORE_add_crl(dest, crl); + } + } + + return dest; +} + // Takes a string or buffer and loads it into a BIO. // Caller responsible for BIO_free_all-ing the returned object. BIOPointer LoadBIO(Environment* env, Local v) { @@ -785,9 +824,26 @@ void SecureContext::SetCACert(const BIOPointer& bio) { if (!bio) return; while (X509Pointer x509 = X509Pointer(PEM_read_bio_X509_AUX( bio.get(), nullptr, NoPasswordCallback, nullptr))) { - CHECK_EQ(1, - X509_STORE_add_cert(GetCertStoreOwnedByThisSecureContext(), - x509.get())); + // Avoid calling GetCertStoreOwnedByThisSecureContext() in SetCACert method + // because it will create X509_STORE based on root_certs (more than 150) and + // is very slow + + if (!own_cert_store_cache_) { + // Is cert_store here a globally shared root cert store? There are two + // possibilities + // 1. AddRootCerts has been called, and cert_store is a globally shared + // root cert store + // 2. AddRootCerts has not been called, and cert_store is the default cert + // store of OpenSSL Directly creating a new store is not a correct + // implementation of copy on write + SSL_CTX_set_cert_store(ctx_.get(), + own_cert_store_cache_ = CopyX509Store( + SSL_CTX_get_cert_store(ctx_.get()))); + // No need to call X509_STORE_free manually, SSL_CTX_set_cert_store will + // take over the ownership of X509_STORE + } + + CHECK_EQ(1, X509_STORE_add_cert(own_cert_store_cache_, x509.get())); CHECK_EQ(1, SSL_CTX_add_client_CA(ctx_.get(), x509.get())); } }