-
Notifications
You must be signed in to change notification settings - Fork 7.3k
tls: Use OpenSSL default trusted CA list and allow configuration of the trusted CA path #25363
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 |
---|---|---|
|
@@ -699,6 +699,32 @@ void SecureContext::AddRootCerts(const FunctionCallbackInfo<Value>& args) { | |
|
||
assert(sc->ca_store_ == NULL); | ||
|
||
if (args.Length() == 2) { | ||
char* caFile = NULL; | ||
char* caPath = NULL; | ||
if (args[0]->IsString()) { | ||
String::Utf8Value file(args[0]); | ||
caFile = *file; | ||
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. This is unsafe; you're assigning a pointer to memory that goes out of scope immediately afterwards. Happens a few lines below as well. |
||
} else if (!args[0]->IsUndefined() && !args[0]->IsNull()) { | ||
return sc->env()->ThrowTypeError("Bad parameter"); | ||
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'd move the type checking to JS land and just |
||
} | ||
if (args[1]->IsString()) { | ||
String::Utf8Value path(args[1]); | ||
caPath = *path; | ||
} else if (!args[1]->IsUndefined() && !args[1]->IsNull()) { | ||
return sc->env()->ThrowTypeError("Bad parameter"); | ||
} | ||
if (!SSL_CTX_load_verify_locations(sc->ctx_, caFile, caPath)) | ||
return sc->env()->ThrowTypeError("Error loading CA certificates from caFile or caPath"); | ||
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. Long line. |
||
return; | ||
} | ||
|
||
if (args.Length() != 0) | ||
return sc->env()->ThrowTypeError("Bad parameter"); | ||
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'd drop this and put a |
||
|
||
if (SSL_CTX_set_default_verify_paths(sc->ctx_)) | ||
return; | ||
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 understand how this works but it's very implicit. Can you add a comment explaining it and maybe move it into the |
||
|
||
if (!root_cert_store) { | ||
root_cert_store = X509_STORE_new(); | ||
|
||
|
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.
Can you use
nullptr
here and snake_case the variable names?