From 29d89c98559df117bdd2d7d943a12344c72ac9fc Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 3 May 2017 12:37:13 +0200 Subject: [PATCH] src: split CryptoPemCallback into two functions Currently the function CryptoPemCallback is used for two things: 1. As a passphrase callback. 2. To avoid the default OpenSSL passphrase routine. The default OpenSSL passphase routine would apply if both the callback and the passphrase are null pointers and the typical behaviour is to prompt for the passphase which is not appropriate in node. This commit suggests that the PasswordCallback function only handle passphrases, and that an additional function named NoPasswordCallback used for the second case to avoid OpenSSL's passphase routine. PR-URL: https://github.com/nodejs/node/pull/12827 Reviewed-By: Sam Roberts Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/node_crypto.cc | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index dac4a2e76bfe76..5ef7d96007b4c6 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -229,8 +229,6 @@ static void crypto_lock_cb(int mode, int n, const char* file, int line) { } -// This callback is used by OpenSSL when it needs to query for the passphrase -// which may be used for encrypted PEM structures. static int PasswordCallback(char *buf, int size, int rwflag, void *u) { if (u) { size_t buflen = static_cast(size); @@ -244,6 +242,16 @@ static int PasswordCallback(char *buf, int size, int rwflag, void *u) { } +// This callback is used to avoid the default passphrase callback in OpenSSL +// which will typically prompt for the passphrase. The prompting is designed +// for the OpenSSL CLI, but works poorly for Node.js because it involves +// synchronous interaction with the controlling terminal, something we never +// want, and use this function to avoid it. +static int NoPasswordCallback(char *buf, int size, int rwflag, void *u) { + return 0; +} + + void ThrowCryptoError(Environment* env, unsigned long err, // NOLINT(runtime/int) const char* default_message = nullptr) { @@ -613,7 +621,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, // that we are interested in ERR_clear_error(); - x = PEM_read_bio_X509_AUX(in, nullptr, PasswordCallback, nullptr); + x = PEM_read_bio_X509_AUX(in, nullptr, NoPasswordCallback, nullptr); if (x == nullptr) { SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_PEM_LIB); @@ -631,7 +639,10 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, goto done; } - while ((extra = PEM_read_bio_X509(in, nullptr, PasswordCallback, nullptr))) { + while ((extra = PEM_read_bio_X509(in, + nullptr, + NoPasswordCallback, + nullptr))) { if (sk_X509_push(extra_certs, extra)) continue; @@ -728,7 +739,7 @@ static X509_STORE* NewRootCertStore() { if (root_certs_vector.empty()) { for (size_t i = 0; i < arraysize(root_certs); i++) { BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i])); - X509 *x509 = PEM_read_bio_X509(bp, nullptr, PasswordCallback, nullptr); + X509 *x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); BIO_free(bp); // Parse errors from the built-in roots are fatal. @@ -771,7 +782,7 @@ void SecureContext::AddCACert(const FunctionCallbackInfo& args) { X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_); while (X509* x509 = - PEM_read_bio_X509(bio, nullptr, PasswordCallback, nullptr)) { + PEM_read_bio_X509(bio, nullptr, NoPasswordCallback, nullptr)) { if (cert_store == root_cert_store) { cert_store = NewRootCertStore(); SSL_CTX_set_cert_store(sc->ctx_, cert_store); @@ -803,7 +814,7 @@ void SecureContext::AddCRL(const FunctionCallbackInfo& args) { return; X509_CRL* crl = - PEM_read_bio_X509_CRL(bio, nullptr, PasswordCallback, nullptr); + PEM_read_bio_X509_CRL(bio, nullptr, NoPasswordCallback, nullptr); if (crl == nullptr) { BIO_free_all(bio); @@ -842,7 +853,7 @@ static unsigned long AddCertsFromFile( // NOLINT(runtime/int) } while (X509* x509 = - PEM_read_bio_X509(bio, nullptr, PasswordCallback, nullptr)) { + PEM_read_bio_X509(bio, nullptr, NoPasswordCallback, nullptr)) { X509_STORE_add_cert(store, x509); X509_free(x509); } @@ -4387,7 +4398,7 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem, // Split this out into a separate function once we have more than one // consumer of public keys. if (strncmp(key_pem, PUBLIC_KEY_PFX, PUBLIC_KEY_PFX_LEN) == 0) { - pkey = PEM_read_bio_PUBKEY(bp, nullptr, PasswordCallback, nullptr); + pkey = PEM_read_bio_PUBKEY(bp, nullptr, NoPasswordCallback, nullptr); if (pkey == nullptr) goto exit; } else if (strncmp(key_pem, PUBRSA_KEY_PFX, PUBRSA_KEY_PFX_LEN) == 0) { @@ -4403,7 +4414,7 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem, goto exit; } else { // X.509 fallback - x509 = PEM_read_bio_X509(bp, nullptr, PasswordCallback, nullptr); + x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); if (x509 == nullptr) goto exit; @@ -4530,7 +4541,7 @@ bool PublicKeyCipher::Cipher(const char* key_pem, goto exit; } else if (operation == kPublic && strncmp(key_pem, CERTIFICATE_PFX, CERTIFICATE_PFX_LEN) == 0) { - x509 = PEM_read_bio_X509(bp, nullptr, PasswordCallback, nullptr); + x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); if (x509 == nullptr) goto exit;