tls: Use OpenSSL default trusted CA list and allow configuration of the trusted CA path #25363
Conversation
node.js previously only used a hard-coded list of CA certificates defined in src/node_root_certs.h. This made it difficult for system administrators to update the CA list or add/remove CAs, as the CA list both required a recompile of node.js to make changes and required managing the node.js CA list independently from the CA list used by other OpenSSL applications on the system. With this change, node.js will instead use the OpenSSL default CA list (whose path is defined at OpenSSL compile-time but may be overridden using the SSL_CERT_FILE or SSL_CERT_DIR environment variables), which allows node.js to use the same system-wide CA list as other OpenSSL-based programs, and allows system administrators to easily update or customize the CA list in a central location without the need to recompile node.js. The hard-coded CA list is retained, but is only used if the OpenSSL default CA list is not available.
node.js currently accepts a 'ca' option to override the default trusted CA list, however trusted CAs must be manually loaded into a javascript array by the caller to use this option. This change adds additional 'caFile' and 'caPath' options which allow callers to specify the path to a file or path containing trusted CA certificates. This allows the trusted CA list to be overridden in the same manner as is allowed by other OpenSSL-based applications.
|
@indutny ... thoughts on this one? |
|
I'm -1 for using default CA list, and +1 for introducing new API ;) |
|
Why -1 for the default CA list? Would it be better if I kept the existing behavior as the default, and added some options to allow the user to switch to the OpenSSL CA list instead? (Perhaps a compile-time option to enable the OpenSSL CA list, and/or an environment variable which causes the OpenSSL CA list to be used at runtime?) I'd like to have some ability to change the CA list without changing code... |
|
I like the idea of having an option to use it, but using it by default doesn't seem a good idea to me. Many systems has old list of CAs, and we are trying to keep it up to date in node.js using Mozilla's list. cc @bnoordhuis |
|
Ok, thanks. I'll update the patch to make this an option rather than the default... |
| @@ -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; | |||
There was a problem hiding this comment.
Can you use nullptr here and snake_case the variable names?
|
One caveat with this change: openssl lazy-loads the certificate files using synchronous I/O. If the files are located on a slow file system (like NFS), it's going to block the node process for some time. |
Please see the commit messages for additional explanation. These are actually two separate changes - I would have split this into two separate pull requests except that the changes are adjacent in the source code, and the documentation for the changes is interdependent, so I couldn't split this into two pull requests without causing merge conflicts between them.