diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js index 40433e9abf04b4..b3025ee9d9caba 100644 --- a/lib/internal/crypto/keygen.js +++ b/lib/internal/crypto/keygen.js @@ -140,7 +140,9 @@ function parseKeyEncoding(keyType, options) { if (cipher != null) { if (typeof cipher !== 'string') throw new ERR_INVALID_OPT_VALUE('privateKeyEncoding.cipher', cipher); - if (privateType !== PK_ENCODING_PKCS8) { + if (privateFormat === PK_FORMAT_DER && + (privateType === PK_ENCODING_PKCS1 || + privateType === PK_ENCODING_SEC1)) { throw new ERR_CRYPTO_INCOMPATIBLE_KEY_OPTIONS( strPrivateType, 'does not support encryption'); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 153bfd2d018466..9fdb5fdfcc0ce6 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5060,20 +5060,24 @@ class GenerateKeyPairJob : public CryptoJob { // Now do the same for the private key (which is a bit more difficult). if (private_key_encoding_.type_ == PK_ENCODING_PKCS1) { - // PKCS#1 is only permitted for RSA keys and without encryption. + // PKCS#1 is only permitted for RSA keys. CHECK_EQ(EVP_PKEY_id(pkey), EVP_PKEY_RSA); - CHECK_NULL(private_key_encoding_.cipher_); RSAPointer rsa(EVP_PKEY_get1_RSA(pkey)); if (private_key_encoding_.format_ == PK_FORMAT_PEM) { // Encode PKCS#1 as PEM. - if (PEM_write_bio_RSAPrivateKey(bio.get(), rsa.get(), - nullptr, nullptr, 0, - nullptr, nullptr) != 1) + char* pass = private_key_encoding_.passphrase_.get(); + if (PEM_write_bio_RSAPrivateKey( + bio.get(), rsa.get(), + private_key_encoding_.cipher_, + reinterpret_cast(pass), + private_key_encoding_.passphrase_length_, + nullptr, nullptr) != 1) return false; } else { - // Encode PKCS#1 as DER. + // Encode PKCS#1 as DER. This does not permit encryption. CHECK_EQ(private_key_encoding_.format_, PK_FORMAT_DER); + CHECK_NULL(private_key_encoding_.cipher_); if (i2d_RSAPrivateKey_bio(bio.get(), rsa.get()) != 1) return false; } @@ -5101,20 +5105,24 @@ class GenerateKeyPairJob : public CryptoJob { } else { CHECK_EQ(private_key_encoding_.type_, PK_ENCODING_SEC1); - // SEC1 is only permitted for EC keys and without encryption. + // SEC1 is only permitted for EC keys. CHECK_EQ(EVP_PKEY_id(pkey), EVP_PKEY_EC); - CHECK_NULL(private_key_encoding_.cipher_); ECKeyPointer ec_key(EVP_PKEY_get1_EC_KEY(pkey)); if (private_key_encoding_.format_ == PK_FORMAT_PEM) { // Encode SEC1 as PEM. - if (PEM_write_bio_ECPrivateKey(bio.get(), ec_key.get(), - nullptr, nullptr, 0, - nullptr, nullptr) != 1) + char* pass = private_key_encoding_.passphrase_.get(); + if (PEM_write_bio_ECPrivateKey( + bio.get(), ec_key.get(), + private_key_encoding_.cipher_, + reinterpret_cast(pass), + private_key_encoding_.passphrase_length_, + nullptr, nullptr) != 1) return false; } else { - // Encode SEC1 as DER. + // Encode SEC1 as DER. This does not permit encryption. CHECK_EQ(private_key_encoding_.format_, PK_FORMAT_DER); + CHECK_NULL(private_key_encoding_.cipher_); if (i2d_ECPrivateKey_bio(bio.get(), ec_key.get()) != 1) return false; } diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index 08e37e2ac0d902..af60c662f9340d 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -47,19 +47,23 @@ function testSignVerify(publicKey, privateKey) { } // Constructs a regular expression for a PEM-encoded key with the given label. -function getRegExpForPEM(label) { +function getRegExpForPEM(label, cipher) { const head = `\\-\\-\\-\\-\\-BEGIN ${label}\\-\\-\\-\\-\\-`; + const rfc1421Header = cipher == null ? '' : + `\nProc-Type: 4,ENCRYPTED\nDEK-Info: ${cipher},[^\n]+\n`; const body = '([a-zA-Z0-9\\+/=]{64}\n)*[a-zA-Z0-9\\+/=]{1,64}'; const end = `\\-\\-\\-\\-\\-END ${label}\\-\\-\\-\\-\\-`; - return new RegExp(`^${head}\n${body}\n${end}\n$`); + return new RegExp(`^${head}${rfc1421Header}\n${body}\n${end}\n$`); } const pkcs1PubExp = getRegExpForPEM('RSA PUBLIC KEY'); const pkcs1PrivExp = getRegExpForPEM('RSA PRIVATE KEY'); +const pkcs1EncExp = (cipher) => getRegExpForPEM('RSA PRIVATE KEY', cipher); const spkiExp = getRegExpForPEM('PUBLIC KEY'); const pkcs8Exp = getRegExpForPEM('PRIVATE KEY'); const pkcs8EncExp = getRegExpForPEM('ENCRYPTED PRIVATE KEY'); const sec1Exp = getRegExpForPEM('EC PRIVATE KEY'); +const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); // Since our own APIs only accept PEM, not DER, we need to convert DER to PEM // for testing. @@ -137,6 +141,42 @@ function convertDERToPEM(label, der) { testEncryptDecrypt(publicKey, privateKey); testSignVerify(publicKey, privateKey); })); + + // Now do the same with an encrypted private key. + generateKeyPair('rsa', { + publicExponent: 0x10001, + modulusLength: 4096, + publicKeyEncoding: { + type: 'pkcs1', + format: 'der' + }, + privateKeyEncoding: { + type: 'pkcs1', + format: 'pem', + cipher: 'aes-256-cbc', + passphrase: 'secret' + } + }, common.mustCall((err, publicKeyDER, privateKey) => { + assert.ifError(err); + + // The public key is encoded as DER (which is binary) instead of PEM. We + // will still need to convert it to PEM for testing. + assert(Buffer.isBuffer(publicKeyDER)); + const publicKey = convertDERToPEM('RSA PUBLIC KEY', publicKeyDER); + assertApproximateSize(publicKey, 720); + + assert.strictEqual(typeof privateKey, 'string'); + assert(pkcs1EncExp('AES-256-CBC').test(privateKey)); + + // Since the private key is encrypted, signing shouldn't work anymore. + assert.throws(() => { + testSignVerify(publicKey, privateKey); + }, /bad decrypt|asn1 encoding routines/); + + const key = { key: privateKey, passphrase: 'secret' }; + testEncryptDecrypt(publicKey, key); + testSignVerify(publicKey, key); + })); } { @@ -203,6 +243,36 @@ function convertDERToPEM(label, der) { testSignVerify(publicKey, privateKey); })); + + // Do the same with an encrypted private key. + generateKeyPair('ec', { + namedCurve: 'prime256v1', + paramEncoding: 'named', + publicKeyEncoding: { + type: 'spki', + format: 'pem' + }, + privateKeyEncoding: { + type: 'sec1', + format: 'pem', + cipher: 'aes-128-cbc', + passphrase: 'secret' + } + }, common.mustCall((err, publicKey, privateKey) => { + assert.ifError(err); + + assert.strictEqual(typeof publicKey, 'string'); + assert(spkiExp.test(publicKey)); + assert.strictEqual(typeof privateKey, 'string'); + 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/); + + testSignVerify(publicKey, { key: privateKey, passphrase: 'secret' }); + })); } { @@ -640,7 +710,7 @@ function convertDERToPEM(label, der) { }); } - // Attempting to encrypt a non-PKCS#8 key. + // Attempting to encrypt a DER-encoded, non-PKCS#8 key. for (const type of ['pkcs1', 'sec1']) { common.expectsError(() => { generateKeyPairSync(type === 'pkcs1' ? 'rsa' : 'ec', { @@ -649,7 +719,7 @@ function convertDERToPEM(label, der) { publicKeyEncoding: { type: 'spki', format: 'pem' }, privateKeyEncoding: { type, - format: 'pem', + format: 'der', cipher: 'aes-128-cbc', passphrase: 'hello' }