diff --git a/doc/api/errors.md b/doc/api/errors.md index 44ec5548aeab35..c23a90fcbcbe3d 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -807,6 +807,12 @@ instance, calling [`cipher.getAuthTag()`][] before calling `cipher.final()`. The PBKDF2 algorithm failed for unspecified reasons. OpenSSL does not provide more details and therefore neither does Node.js. + +### ERR_CRYPTO_READ_KEY + +An error occurred while parsing a cryptographic key, e.g., the key is encrypted +but no decryption passphrase was specified. + ### ERR_CRYPTO_SCRYPT_INVALID_PARAMETER diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9658c1d51a208b..99a7783283c9d1 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -153,13 +153,33 @@ template int SSLWrap::SelectALPNCallback( unsigned int inlen, void* arg); +class PasswordCallbackInfo { + public: + explicit PasswordCallbackInfo(const char* passphrase) + : passphrase_(passphrase) {} + + inline const char* GetPassword() { + needs_passphrase_ = true; + return passphrase_; + } + + inline bool CalledButEmpty() { + return needs_passphrase_ && passphrase_ == nullptr; + } + + private: + const char* passphrase_; + bool needs_passphrase_ = false; +}; static int PasswordCallback(char* buf, int size, int rwflag, void* u) { - if (u) { + PasswordCallbackInfo* info = static_cast(u); + const char* passphrase = info->GetPassword(); + if (passphrase != nullptr) { size_t buflen = static_cast(size); - size_t len = strlen(static_cast(u)); + size_t len = strlen(passphrase); len = len > buflen ? buflen : len; - memcpy(buf, u, len); + memcpy(buf, passphrase, len); return len; } @@ -698,11 +718,12 @@ void SecureContext::SetKey(const FunctionCallbackInfo& args) { node::Utf8Value passphrase(env->isolate(), args[1]); + PasswordCallbackInfo cb_info(len == 1 ? nullptr : *passphrase); EVPKeyPointer key( PEM_read_bio_PrivateKey(bio.get(), nullptr, PasswordCallback, - len == 1 ? nullptr : *passphrase)); + &cb_info)); if (!key) { unsigned long err = ERR_get_error(); // NOLINT(runtime/int) @@ -2899,13 +2920,14 @@ static bool IsSupportedAuthenticatedMode(const EVP_CIPHER_CTX* ctx) { return IsSupportedAuthenticatedMode(cipher); } -enum class ParsePublicKeyResult { - kParsePublicOk, - kParsePublicNotRecognized, - kParsePublicFailed +enum class ParseKeyResult { + kParseKeyOk, + kParseKeyNotRecognized, + kParseKeyNeedPassphrase, + kParseKeyFailed }; -static ParsePublicKeyResult TryParsePublicKey( +static ParseKeyResult TryParsePublicKey( EVPKeyPointer* pkey, const BIOPointer& bp, const char* name, @@ -2919,7 +2941,7 @@ static ParsePublicKeyResult TryParsePublicKey( MarkPopErrorOnReturn mark_pop_error_on_return; if (PEM_bytes_read_bio(&der_data, &der_len, nullptr, name, bp.get(), nullptr, nullptr) != 1) - return ParsePublicKeyResult::kParsePublicNotRecognized; + return ParseKeyResult::kParseKeyNotRecognized; } // OpenSSL might modify the pointer, so we need to make a copy before parsing. @@ -2927,25 +2949,25 @@ static ParsePublicKeyResult TryParsePublicKey( pkey->reset(parse(&p, der_len)); OPENSSL_clear_free(der_data, der_len); - return *pkey ? ParsePublicKeyResult::kParsePublicOk : - ParsePublicKeyResult::kParsePublicFailed; + return *pkey ? ParseKeyResult::kParseKeyOk : + ParseKeyResult::kParseKeyFailed; } -static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey, - const char* key_pem, - int key_pem_len) { +static ParseKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey, + const char* key_pem, + int key_pem_len) { BIOPointer bp(BIO_new_mem_buf(const_cast(key_pem), key_pem_len)); if (!bp) - return ParsePublicKeyResult::kParsePublicFailed; + return ParseKeyResult::kParseKeyFailed; - ParsePublicKeyResult ret; + ParseKeyResult ret; // Try parsing as a SubjectPublicKeyInfo first. ret = TryParsePublicKey(pkey, bp, "PUBLIC KEY", [](const unsigned char** p, long l) { // NOLINT(runtime/int) return d2i_PUBKEY(nullptr, p, l); }); - if (ret != ParsePublicKeyResult::kParsePublicNotRecognized) + if (ret != ParseKeyResult::kParseKeyNotRecognized) return ret; // Maybe it is PKCS#1. @@ -2954,7 +2976,7 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey, [](const unsigned char** p, long l) { // NOLINT(runtime/int) return d2i_PublicKey(EVP_PKEY_RSA, nullptr, p, l); }); - if (ret != ParsePublicKeyResult::kParsePublicNotRecognized) + if (ret != ParseKeyResult::kParseKeyNotRecognized) return ret; // X.509 fallback. @@ -2966,25 +2988,25 @@ static ParsePublicKeyResult ParsePublicKeyPEM(EVPKeyPointer* pkey, }); } -static bool ParsePublicKey(EVPKeyPointer* pkey, - const PublicKeyEncodingConfig& config, - const char* key, - size_t key_len) { +static ParseKeyResult ParsePublicKey(EVPKeyPointer* pkey, + const PublicKeyEncodingConfig& config, + const char* key, + size_t key_len) { if (config.format_ == kKeyFormatPEM) { - ParsePublicKeyResult r = - ParsePublicKeyPEM(pkey, key, key_len); - return r == ParsePublicKeyResult::kParsePublicOk; + return ParsePublicKeyPEM(pkey, key, key_len); } else { CHECK_EQ(config.format_, kKeyFormatDER); + const unsigned char* p = reinterpret_cast(key); if (config.type_.ToChecked() == kKeyEncodingPKCS1) { pkey->reset(d2i_PublicKey(EVP_PKEY_RSA, nullptr, &p, key_len)); - return pkey; } else { CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSPKI); pkey->reset(d2i_PUBKEY(nullptr, &p, key_len)); - return pkey; } + + return *pkey ? ParseKeyResult::kParseKeyOk : + ParseKeyResult::kParseKeyFailed; } } @@ -3099,56 +3121,59 @@ static bool IsEncryptedPrivateKeyInfo(const unsigned char* data, size_t size) { data[offset] != 2; } -static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config, - const char* key, - size_t key_len) { - EVPKeyPointer pkey; +static ParseKeyResult ParsePrivateKey(EVPKeyPointer* pkey, + const PrivateKeyEncodingConfig& config, + const char* key, + size_t key_len) { + PasswordCallbackInfo pc_info(config.passphrase_.get()); if (config.format_ == kKeyFormatPEM) { BIOPointer bio(BIO_new_mem_buf(key, key_len)); if (!bio) - return pkey; + return ParseKeyResult::kParseKeyFailed; - char* pass = const_cast(config.passphrase_.get()); - pkey.reset(PEM_read_bio_PrivateKey(bio.get(), - nullptr, - PasswordCallback, - pass)); + pkey->reset(PEM_read_bio_PrivateKey(bio.get(), + nullptr, + PasswordCallback, + &pc_info)); } else { CHECK_EQ(config.format_, kKeyFormatDER); if (config.type_.ToChecked() == kKeyEncodingPKCS1) { const unsigned char* p = reinterpret_cast(key); - pkey.reset(d2i_PrivateKey(EVP_PKEY_RSA, nullptr, &p, key_len)); + pkey->reset(d2i_PrivateKey(EVP_PKEY_RSA, nullptr, &p, key_len)); } else if (config.type_.ToChecked() == kKeyEncodingPKCS8) { BIOPointer bio(BIO_new_mem_buf(key, key_len)); if (!bio) - return pkey; + return ParseKeyResult::kParseKeyFailed; if (IsEncryptedPrivateKeyInfo( reinterpret_cast(key), key_len)) { - char* pass = const_cast(config.passphrase_.get()); - pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(), - nullptr, - PasswordCallback, - pass)); + pkey->reset(d2i_PKCS8PrivateKey_bio(bio.get(), + nullptr, + PasswordCallback, + &pc_info)); } else { PKCS8Pointer p8inf(d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr)); if (p8inf) - pkey.reset(EVP_PKCS82PKEY(p8inf.get())); + pkey->reset(EVP_PKCS82PKEY(p8inf.get())); } } else { CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSEC1); const unsigned char* p = reinterpret_cast(key); - pkey.reset(d2i_PrivateKey(EVP_PKEY_EC, nullptr, &p, key_len)); + pkey->reset(d2i_PrivateKey(EVP_PKEY_EC, nullptr, &p, key_len)); } } // OpenSSL can fail to parse the key but still return a non-null pointer. if (ERR_peek_error() != 0) - pkey.reset(); + pkey->reset(); - return pkey; + if (*pkey) + return ParseKeyResult::kParseKeyOk; + if (pc_info.CalledButEmpty()) + return ParseKeyResult::kParseKeyNeedPassphrase; + return ParseKeyResult::kParseKeyFailed; } ByteSource::ByteSource(ByteSource&& other) @@ -3284,6 +3309,26 @@ static PublicKeyEncodingConfig GetPublicKeyEncodingFromJs( return result; } +#define READ_FAILURE_MSG(type) ("Failed to read " type " key") + +static inline ManagedEVPPKey GetParsedKey(Environment* env, + EVPKeyPointer&& pkey, + ParseKeyResult ret, + const char* default_msg) { + switch (ret) { + case ParseKeyResult::kParseKeyOk: + CHECK(pkey); + break; + case ParseKeyResult::kParseKeyNeedPassphrase: + THROW_ERR_CRYPTO_READ_KEY(env, "Passphrase required for encrypted key"); + break; + default: + ThrowCryptoError(env, ERR_get_error(), default_msg); + } + + return ManagedEVPPKey(std::move(pkey)); +} + static NonCopyableMaybe GetPrivateKeyEncodingFromJs( const FunctionCallbackInfo& args, unsigned int* offset, @@ -3339,11 +3384,11 @@ static ManagedEVPPKey GetPrivateKeyFromJs( GetPrivateKeyEncodingFromJs(args, offset, kKeyContextInput); if (config.IsEmpty()) return ManagedEVPPKey(); - EVPKeyPointer pkey = - ParsePrivateKey(config.Release(), key.get(), key.size()); - if (!pkey) - ThrowCryptoError(env, ERR_get_error(), "Failed to read private key"); - return ManagedEVPPKey(std::move(pkey)); + + EVPKeyPointer pkey; + ParseKeyResult ret = + ParsePrivateKey(&pkey, config.Release(), key.get(), key.size()); + return GetParsedKey(env, std::move(pkey), ret, READ_FAILURE_MSG("private")); } else { CHECK(args[*offset]->IsObject() && allow_key_object); KeyObject* key; @@ -3364,15 +3409,16 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs( GetPrivateKeyEncodingFromJs(args, offset, kKeyContextInput); if (config_.IsEmpty()) return ManagedEVPPKey(); + + ParseKeyResult ret; PrivateKeyEncodingConfig config = config_.Release(); EVPKeyPointer pkey; if (config.format_ == kKeyFormatPEM) { // For PEM, we can easily determine whether it is a public or private key // by looking for the respective PEM tags. - ParsePublicKeyResult ret = ParsePublicKeyPEM(&pkey, data.get(), - data.size()); - if (ret == ParsePublicKeyResult::kParsePublicNotRecognized) { - pkey = ParsePrivateKey(config, data.get(), data.size()); + ret = ParsePublicKeyPEM(&pkey, data.get(), data.size()); + if (ret == ParseKeyResult::kParseKeyNotRecognized) { + ret = ParsePrivateKey(&pkey, config, data.get(), data.size()); } } else { // For DER, the type determines how to parse it. SPKI, PKCS#8 and SEC1 are @@ -3395,14 +3441,14 @@ static ManagedEVPPKey GetPublicOrPrivateKeyFromJs( } if (is_public) { - ParsePublicKey(&pkey, config, data.get(), data.size()); + ret = ParsePublicKey(&pkey, config, data.get(), data.size()); } else { - pkey = ParsePrivateKey(config, data.get(), data.size()); + ret = ParsePrivateKey(&pkey, config, data.get(), data.size()); } } - if (!pkey) - ThrowCryptoError(env, ERR_get_error(), "Failed to read asymmetric key"); - return ManagedEVPPKey(std::move(pkey)); + + return GetParsedKey(env, std::move(pkey), ret, + READ_FAILURE_MSG("asymmetric")); } else { CHECK(args[*offset]->IsObject()); KeyObject* key = Unwrap(args[*offset].As()); @@ -3585,6 +3631,7 @@ KeyType KeyObject::GetKeyType() const { void KeyObject::Init(const FunctionCallbackInfo& args) { KeyObject* key; ASSIGN_OR_RETURN_UNWRAP(&key, args.Holder()); + MarkPopErrorOnReturn mark_pop_error_on_return; unsigned int offset; ManagedEVPPKey pkey; @@ -4780,6 +4827,8 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { Sign* sign; ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder()); + ClearErrorOnReturn clear_error_on_return; + unsigned int offset = 0; ManagedEVPPKey key = GetPrivateKeyFromJs(args, &offset, true); if (!key) @@ -4791,8 +4840,6 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { CHECK(args[offset + 1]->IsInt32()); int salt_len = args[offset + 1].As()->Value(); - ClearErrorOnReturn clear_error_on_return; - SignResult ret = sign->SignFinal( key, padding, diff --git a/src/node_errors.h b/src/node_errors.h index 9d3f2ead71279d..b7387f1dca657e 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -43,6 +43,7 @@ void FatalException(v8::Isolate* isolate, V(ERR_BUFFER_TOO_LARGE, Error) \ V(ERR_CANNOT_TRANSFER_OBJECT, TypeError) \ V(ERR_CONSTRUCT_CALL_REQUIRED, Error) \ + V(ERR_CRYPTO_READ_KEY, TypeError) \ V(ERR_INVALID_ARG_VALUE, TypeError) \ V(ERR_INVALID_ARG_TYPE, TypeError) \ V(ERR_INVALID_MODULE_SPECIFIER, TypeError) \ diff --git a/test/fixtures/keys/Makefile b/test/fixtures/keys/Makefile index 964fb2f5f1451b..3be2cd6655f7e6 100644 --- a/test/fixtures/keys/Makefile +++ b/test/fixtures/keys/Makefile @@ -26,6 +26,7 @@ all: \ dh2048.pem \ dsa1025.pem \ dsa_private_1025.pem \ + dsa_private_encrypted_1025.pem \ dsa_public_1025.pem \ ec-cert.pem \ ec.pfx \ @@ -549,6 +550,9 @@ dsa1025.pem: dsa_private_1025.pem: openssl gendsa -out dsa_private_1025.pem dsa1025.pem +dsa_private_encrypted_1025.pem: + openssl pkcs8 -in dsa_private_1025.pem -topk8 -passout 'pass:secret' -out dsa_private_encrypted_1025.pem + dsa_public_1025.pem: openssl dsa -in dsa_private_1025.pem -pubout -out dsa_public_1025.pem diff --git a/test/fixtures/keys/dsa_private_encrypted_1025.pem b/test/fixtures/keys/dsa_private_encrypted_1025.pem new file mode 100644 index 00000000000000..b75ffaf90df5ea --- /dev/null +++ b/test/fixtures/keys/dsa_private_encrypted_1025.pem @@ -0,0 +1,12 @@ +-----BEGIN ENCRYPTED PRIVATE KEY----- +MIIBvTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQIqTW00yecdxMCAggA +MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBBKgO4UF0LfCkPyS+iCvSrtBIIB +YD3W6FyEZ97/crnoyRqjPUtr2Mm4KJMtaB5ZiGFzZEzd6AH7N/dbtAAMIibtsjmd +RYdIptpET6xTpUhM8TvpULyYaZnhZJKTpVUrTVdvFTS3DYDutu7aWRLTrle6LzcY +XpIppeP8ZmYFdRBQxhF+KoDsP4O0QA+vWl2W2VmRfr+sK9R+qV89w0YMjEWHsYY+ +VZsDbJBGKkj9gzIvxIsRyack/+RsbiSDrh6WTw+D0jrX/IMbgPjvYfBFhpxGC7zR +hDn9r3JaO2KdHh9kMtvQfshA1n636kb0X6ewY57BhEs3J4hpMg46c6YFry94to24 +jxl5KutM0CFea7mYGtNf6WJXBsm7JSW03kjlqYoZGK43KNgZhzKAsXaNkoRkA5cw +BzGfgmG6dHTpeAY9G4vM4inhCmGFA8Tx189g+xzRv16uFXRb8WFIllne1fEFaXRr +1Rz2G6SPJkA3fsrl8zUIB0Y= +-----END ENCRYPTED PRIVATE KEY----- diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index c9cb90d408500f..f7948b05859fd3 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -21,6 +21,10 @@ const fixtures = require('../common/fixtures'); const publicPem = fixtures.readSync('test_rsa_pubkey.pem', 'ascii'); const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii'); +const publicDsa = fixtures.readKey('dsa_public_1025.pem', 'ascii'); +const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem', + 'ascii'); + { // Attempting to create an empty key should throw. common.expectsError(() => { @@ -210,3 +214,26 @@ const privatePem = fixtures.readSync('test_rsa_privkey.pem', 'ascii'); }); } }); + +{ + // Reading an encrypted key without a passphrase should fail. + common.expectsError(() => createPrivateKey(privateDsa), { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); + + const publicKey = createPublicKey(publicDsa); + assert.strictEqual(publicKey.type, 'public'); + assert.strictEqual(publicKey.asymmetricKeyType, 'dsa'); + assert.strictEqual(publicKey.symmetricKeySize, undefined); + + const privateKey = createPrivateKey({ + key: privateDsa, + format: 'pem', + passphrase: 'secret' + }); + assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(privateKey.asymmetricKeyType, 'dsa'); + assert.strictEqual(privateKey.symmetricKeySize, undefined); +} diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index 64964cc7aedf7d..535c676e50f003 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -166,9 +166,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. const publicKey = { key: publicKeyDER, ...publicKeyEncoding }; - assert.throws(() => { - testSignVerify(publicKey, privateKey); - }, /bad decrypt|asn1 encoding routines/); + common.expectsError(() => testSignVerify(publicKey, privateKey), { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); const key = { key: privateKey, passphrase: 'secret' }; testEncryptDecrypt(publicKey, key); @@ -196,13 +198,19 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since the private key is encrypted, signing shouldn't work anymore. const publicKey = { key: publicKeyDER, ...publicKeyEncoding }; - assert.throws(() => { + common.expectsError(() => { testSignVerify(publicKey, { key: privateKeyDER, format: 'der', type: 'pkcs8' }); - }, /bad decrypt|asn1 encoding routines/); + }, { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); + + // Signing should work with the correct password. const privateKey = { key: privateKeyDER, @@ -274,12 +282,16 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); assertApproximateSize(privateKeyDER, 336); // Since the private key is encrypted, signing shouldn't work anymore. - assert.throws(() => { - testSignVerify(publicKey, { + common.expectsError(() => { + return testSignVerify(publicKey, { key: privateKeyDER, ...privateKeyEncoding }); - }, /bad decrypt|asn1 encoding routines/); + }, { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); // Signing should work with the correct password. testSignVerify(publicKey, { @@ -338,9 +350,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); assert(sec1EncExp('AES-128-CBC').test(privateKey)); // Since the private key is encrypted, signing shouldn't work anymore. - assert.throws(() => { - testSignVerify(publicKey, privateKey); - }, /bad decrypt|asn1 encoding routines/); + common.expectsError(() => testSignVerify(publicKey, privateKey), { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); testSignVerify(publicKey, { key: privateKey, passphrase: 'secret' }); })); @@ -371,9 +385,11 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); assert(pkcs8EncExp.test(privateKey)); // Since the private key is encrypted, signing shouldn't work anymore. - assert.throws(() => { - testSignVerify(publicKey, privateKey); - }, /bad decrypt|asn1 encoding routines/); + common.expectsError(() => testSignVerify(publicKey, privateKey), { + type: TypeError, + code: 'ERR_CRYPTO_READ_KEY', + message: 'Passphrase required for encrypted key' + }); testSignVerify(publicKey, { key: privateKey,