-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
tls: implement tls.getCACertificates() #57107
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,11 +42,13 @@ using ncrypto::MarkPopErrorOnReturn; | |
using ncrypto::SSLPointer; | ||
using ncrypto::StackOfX509; | ||
using ncrypto::X509Pointer; | ||
using ncrypto::X509View; | ||
using v8::Array; | ||
using v8::ArrayBufferView; | ||
using v8::Boolean; | ||
using v8::Context; | ||
using v8::DontDelete; | ||
using v8::EscapableHandleScope; | ||
using v8::Exception; | ||
using v8::External; | ||
using v8::FunctionCallbackInfo; | ||
|
@@ -57,7 +59,9 @@ using v8::Integer; | |
using v8::Isolate; | ||
using v8::JustVoid; | ||
using v8::Local; | ||
using v8::LocalVector; | ||
using v8::Maybe; | ||
using v8::MaybeLocal; | ||
using v8::Nothing; | ||
using v8::Object; | ||
using v8::PropertyAttribute; | ||
|
@@ -657,9 +661,6 @@ static void LoadCertsFromDir(std::vector<X509*>* certs, | |
return; | ||
} | ||
|
||
uv_fs_t stats_req; | ||
auto cleanup_stats = | ||
OnScopeLeave([&stats_req]() { uv_fs_req_cleanup(&stats_req); }); | ||
for (;;) { | ||
uv_dirent_t ent; | ||
|
||
|
@@ -676,12 +677,14 @@ static void LoadCertsFromDir(std::vector<X509*>* certs, | |
return; | ||
} | ||
|
||
uv_fs_t stats_req; | ||
std::string file_path = std::string(cert_dir) + "/" + ent.name; | ||
int stats_r = uv_fs_stat(nullptr, &stats_req, file_path.c_str(), nullptr); | ||
if (stats_r == 0 && | ||
(static_cast<uv_stat_t*>(stats_req.ptr)->st_mode & S_IFREG)) { | ||
LoadCertsFromFile(certs, file_path.c_str()); | ||
} | ||
uv_fs_req_cleanup(&stats_req); | ||
} | ||
} | ||
|
||
|
@@ -760,7 +763,7 @@ static std::vector<X509*> InitializeSystemStoreCertificates() { | |
return system_store_certs; | ||
} | ||
|
||
static std::vector<X509*>& GetSystemStoreRootCertificates() { | ||
static std::vector<X509*>& GetSystemStoreCACertificates() { | ||
// Use function-local static to guarantee thread safety. | ||
static std::vector<X509*> system_store_certs = | ||
InitializeSystemStoreCertificates(); | ||
|
@@ -832,7 +835,7 @@ X509_STORE* NewRootCertStore() { | |
CHECK_EQ(1, X509_STORE_add_cert(store, cert)); | ||
} | ||
if (per_process::cli_options->use_system_ca) { | ||
for (X509* cert : GetSystemStoreRootCertificates()) { | ||
for (X509* cert : GetSystemStoreCACertificates()) { | ||
CHECK_EQ(1, X509_STORE_add_cert(store, cert)); | ||
} | ||
} | ||
|
@@ -854,7 +857,7 @@ void CleanupCachedRootCertificates() { | |
} | ||
} | ||
if (has_cached_system_root_certs.load()) { | ||
for (X509* cert : GetSystemStoreRootCertificates()) { | ||
for (X509* cert : GetSystemStoreCACertificates()) { | ||
X509_free(cert); | ||
} | ||
} | ||
|
@@ -866,7 +869,7 @@ void CleanupCachedRootCertificates() { | |
} | ||
} | ||
|
||
void GetRootCertificates(const FunctionCallbackInfo<Value>& args) { | ||
void GetBundledRootCertificates(const FunctionCallbackInfo<Value>& args) { | ||
Environment* env = Environment::GetCurrent(args); | ||
Local<Value> result[arraysize(root_certs)]; | ||
|
||
|
@@ -883,6 +886,58 @@ void GetRootCertificates(const FunctionCallbackInfo<Value>& args) { | |
Array::New(env->isolate(), result, arraysize(root_certs))); | ||
} | ||
|
||
MaybeLocal<Array> X509sToArrayOfStrings(Environment* env, | ||
const std::vector<X509*>& certs) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you're using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've considered this but it seems a bit too complicated for this PR, because we get the raw pointers from the function-local static storage for thread safety. Passing in |
||
ClearErrorOnReturn clear_error_on_return; | ||
EscapableHandleScope scope(env->isolate()); | ||
|
||
LocalVector<Value> result(env->isolate(), certs.size()); | ||
for (size_t i = 0; i < certs.size(); ++i) { | ||
X509View view(certs[i]); | ||
auto pem_bio = view.toPEM(); | ||
if (!pem_bio) { | ||
ThrowCryptoError(env, ERR_get_error(), "X509 to PEM conversion"); | ||
return MaybeLocal<Array>(); | ||
} | ||
|
||
char* pem_data = nullptr; | ||
auto pem_size = BIO_get_mem_data(pem_bio.get(), &pem_data); | ||
if (pem_size <= 0 || !pem_data) { | ||
ThrowCryptoError(env, ERR_get_error(), "Reading PEM data"); | ||
return MaybeLocal<Array>(); | ||
} | ||
// PEM is base64-encoded, so it must be one-byte. | ||
if (!String::NewFromOneByte(env->isolate(), | ||
reinterpret_cast<uint8_t*>(pem_data), | ||
v8::NewStringType::kNormal, | ||
pem_size) | ||
.ToLocal(&result[i])) { | ||
return MaybeLocal<Array>(); | ||
} | ||
} | ||
return scope.Escape(Array::New(env->isolate(), result.data(), result.size())); | ||
} | ||
|
||
void GetSystemCACertificates(const FunctionCallbackInfo<Value>& args) { | ||
Environment* env = Environment::GetCurrent(args); | ||
Local<Array> results; | ||
if (X509sToArrayOfStrings(env, GetSystemStoreCACertificates()) | ||
.ToLocal(&results)) { | ||
args.GetReturnValue().Set(results); | ||
} | ||
} | ||
|
||
void GetExtraCACertificates(const FunctionCallbackInfo<Value>& args) { | ||
Environment* env = Environment::GetCurrent(args); | ||
if (extra_root_certs_file.empty()) { | ||
return args.GetReturnValue().Set(Array::New(env->isolate())); | ||
} | ||
Local<Array> results; | ||
if (X509sToArrayOfStrings(env, GetExtraCACertificates()).ToLocal(&results)) { | ||
args.GetReturnValue().Set(results); | ||
} | ||
} | ||
|
||
bool SecureContext::HasInstance(Environment* env, const Local<Value>& value) { | ||
return GetConstructorTemplate(env)->HasInstance(value); | ||
} | ||
|
@@ -966,8 +1021,14 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) { | |
GetConstructorTemplate(env), | ||
SetConstructorFunctionFlag::NONE); | ||
|
||
SetMethodNoSideEffect(context, | ||
target, | ||
"getBundledRootCertificates", | ||
GetBundledRootCertificates); | ||
SetMethodNoSideEffect( | ||
context, target, "getSystemCACertificates", GetSystemCACertificates); | ||
SetMethodNoSideEffect( | ||
context, target, "getRootCertificates", GetRootCertificates); | ||
context, target, "getExtraCACertificates", GetExtraCACertificates); | ||
} | ||
|
||
void SecureContext::RegisterExternalReferences( | ||
|
@@ -1007,7 +1068,9 @@ void SecureContext::RegisterExternalReferences( | |
|
||
registry->Register(CtxGetter); | ||
|
||
registry->Register(GetRootCertificates); | ||
registry->Register(GetBundledRootCertificates); | ||
registry->Register(GetSystemCACertificates); | ||
registry->Register(GetExtraCACertificates); | ||
} | ||
|
||
SecureContext* SecureContext::Create(Environment* env) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just a nit so feel free to ignore... it would be nice to be able to allocate the array at once rather than using multiple individual array.push(...) calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is different in JS - in JS if the array is preallocated, this becomes a holey array. using array.push keeps it compact, which can have perf implications. See https://v8.dev/blog/elements-kinds