diff options
author | Tobias Nießen <tniessen@tnie.de> | 2019-02-21 09:28:16 +0100 |
---|---|---|
committer | Tobias Nießen <tniessen@tnie.de> | 2019-02-23 13:53:58 +0100 |
commit | 8d69fdde1955e0b08bdbe6949090f459995784a7 (patch) | |
tree | fd6fc27eaf7053ea8f6d8158d5b52940712b0326 | |
parent | 10c3db3da682b85e7b44b2671f227449713cd4d8 (diff) | |
download | android-node-v8-8d69fdde1955e0b08bdbe6949090f459995784a7.tar.gz android-node-v8-8d69fdde1955e0b08bdbe6949090f459995784a7.tar.bz2 android-node-v8-8d69fdde1955e0b08bdbe6949090f459995784a7.zip |
crypto: fix unencrypted DER PKCS8 parsing
The previously used OpenSSL call only supports encrypted PKCS8,
this commit adds support for unencrypted PKCS8.
PR-URL: https://github.com/nodejs/node/pull/26236
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
-rw-r--r-- | src/node_crypto.cc | 105 | ||||
-rw-r--r-- | src/node_crypto.h | 1 | ||||
-rw-r--r-- | test/parallel/test-crypto-keygen.js | 67 |
3 files changed, 134 insertions, 39 deletions
diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 8b6587b6ea..98ae216348 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2848,6 +2848,59 @@ static MaybeLocal<Value> WritePublicKey(Environment* env, return BIOToStringOrBuffer(env, bio.get(), config.format_); } +static bool IsASN1Sequence(const unsigned char* data, size_t size, + size_t* data_offset, size_t* data_size) { + if (size < 2 || data[0] != 0x30) + return false; + + if (data[1] & 0x80) { + // Long form. + size_t n_bytes = data[1] & ~0x80; + if (n_bytes + 2 > size || n_bytes > sizeof(size_t)) + return false; + size_t length = 0; + for (size_t i = 0; i < n_bytes; i++) + length = (length << 8) | data[i + 2]; + *data_offset = 2 + n_bytes; + *data_size = std::min(size - 2 - n_bytes, length); + } else { + // Short form. + *data_offset = 2; + *data_size = std::min<size_t>(size - 2, data[1]); + } + + return true; +} + +static bool IsRSAPrivateKey(const unsigned char* data, size_t size) { + // Both RSAPrivateKey and RSAPublicKey structures start with a SEQUENCE. + size_t offset, len; + if (!IsASN1Sequence(data, size, &offset, &len)) + return false; + + // An RSAPrivateKey sequence always starts with a single-byte integer whose + // value is either 0 or 1, whereas an RSAPublicKey starts with the modulus + // (which is the product of two primes and therefore at least 4), so we can + // decide the type of the structure based on the first three bytes of the + // sequence. + return len >= 3 && + data[offset] == 2 && + data[offset + 1] == 1 && + !(data[offset + 2] & 0xfe); +} + +static bool IsEncryptedPrivateKeyInfo(const unsigned char* data, size_t size) { + // Both PrivateKeyInfo and EncryptedPrivateKeyInfo start with a SEQUENCE. + size_t offset, len; + if (!IsASN1Sequence(data, size, &offset, &len)) + return false; + + // A PrivateKeyInfo sequence always starts with an integer whereas an + // EncryptedPrivateKeyInfo starts with an AlgorithmIdentifier. + return len >= 1 && + data[offset] != 2; +} + static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config, const char* key, size_t key_len) { @@ -2873,11 +2926,19 @@ static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config, BIOPointer bio(BIO_new_mem_buf(key, key_len)); if (!bio) return pkey; - char* pass = const_cast<char*>(config.passphrase_.get()); - pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(), - nullptr, - PasswordCallback, - pass)); + + if (IsEncryptedPrivateKeyInfo( + reinterpret_cast<const unsigned char*>(key), key_len)) { + char* pass = const_cast<char*>(config.passphrase_.get()); + pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(), + nullptr, + PasswordCallback, + pass)); + } else { + PKCS8Pointer p8inf(d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr)); + if (p8inf) + pkey.reset(EVP_PKCS82PKEY(p8inf.get())); + } } else { CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSEC1); const unsigned char* p = reinterpret_cast<const unsigned char*>(key); @@ -3093,40 +3154,6 @@ static ManagedEVPPKey GetPrivateKeyFromJs( } } -static bool IsRSAPrivateKey(const unsigned char* data, size_t size) { - // Both RSAPrivateKey and RSAPublicKey structures start with a SEQUENCE. - if (size >= 2 && data[0] == 0x30) { - size_t offset; - if (data[1] & 0x80) { - // Long form. - size_t n_bytes = data[1] & ~0x80; - if (n_bytes + 2 > size || n_bytes > sizeof(size_t)) - return false; - size_t i, length = 0; - for (i = 0; i < n_bytes; i++) - length = (length << 8) | data[i + 2]; - offset = 2 + n_bytes; - size = std::min(size, length + 2); - } else { - // Short form. - offset = 2; - size = std::min<size_t>(size, data[1] + 2); - } - - // An RSAPrivateKey sequence always starts with a single-byte integer whose - // value is either 0 or 1, whereas an RSAPublicKey starts with the modulus - // (which is the product of two primes and therefore at least 4), so we can - // decide the type of the structure based on the first three bytes of the - // sequence. - return size - offset >= 3 && - data[offset] == 2 && - data[offset + 1] == 1 && - !(data[offset + 2] & 0xfe); - } - - return false; -} - static ManagedEVPPKey GetPublicOrPrivateKeyFromJs( const FunctionCallbackInfo<Value>& args, unsigned int* offset, diff --git a/src/node_crypto.h b/src/node_crypto.h index 7c346a6c14..e9862ff1bc 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -77,6 +77,7 @@ using BIOPointer = DeleteFnPtr<BIO, BIO_free_all>; using SSLCtxPointer = DeleteFnPtr<SSL_CTX, SSL_CTX_free>; using SSLSessionPointer = DeleteFnPtr<SSL_SESSION, SSL_SESSION_free>; using SSLPointer = DeleteFnPtr<SSL, SSL_free>; +using PKCS8Pointer = DeleteFnPtr<PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO_free>; using EVPKeyPointer = DeleteFnPtr<EVP_PKEY, EVP_PKEY_free>; using EVPKeyCtxPointer = DeleteFnPtr<EVP_PKEY_CTX, EVP_PKEY_CTX_free>; using EVPMDPointer = DeleteFnPtr<EVP_MD_CTX, EVP_MD_CTX_free>; diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js index ebbac7606f..7b3eee570d 100644 --- a/test/parallel/test-crypto-keygen.js +++ b/test/parallel/test-crypto-keygen.js @@ -174,6 +174,73 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher); testEncryptDecrypt(publicKey, key); testSignVerify(publicKey, key); })); + + // Now do the same with an encrypted private key, but encoded as DER. + generateKeyPair('rsa', { + publicExponent: 0x10001, + modulusLength: 512, + publicKeyEncoding, + privateKeyEncoding: { + type: 'pkcs8', + format: 'der', + cipher: 'aes-256-cbc', + passphrase: 'secret' + } + }, common.mustCall((err, publicKeyDER, privateKeyDER) => { + assert.ifError(err); + + assert(Buffer.isBuffer(publicKeyDER)); + assertApproximateSize(publicKeyDER, 74); + + assert(Buffer.isBuffer(privateKeyDER)); + + // Since the private key is encrypted, signing shouldn't work anymore. + const publicKey = { key: publicKeyDER, ...publicKeyEncoding }; + assert.throws(() => { + testSignVerify(publicKey, { + key: privateKeyDER, + format: 'der', + type: 'pkcs8' + }); + }, /bad decrypt|asn1 encoding routines/); + + const privateKey = { + key: privateKeyDER, + format: 'der', + type: 'pkcs8', + passphrase: 'secret' + }; + testEncryptDecrypt(publicKey, privateKey); + testSignVerify(publicKey, privateKey); + })); + + // Now do the same with an encrypted private key, but encoded as DER. + generateKeyPair('rsa', { + publicExponent: 0x10001, + modulusLength: 512, + publicKeyEncoding, + privateKeyEncoding: { + type: 'pkcs8', + format: 'der' + } + }, common.mustCall((err, publicKeyDER, privateKeyDER) => { + assert.ifError(err); + + assert(Buffer.isBuffer(publicKeyDER)); + assertApproximateSize(publicKeyDER, 74); + + assert(Buffer.isBuffer(privateKeyDER)); + + const publicKey = { key: publicKeyDER, ...publicKeyEncoding }; + const privateKey = { + key: privateKeyDER, + format: 'der', + type: 'pkcs8', + passphrase: 'secret' + }; + testEncryptDecrypt(publicKey, privateKey); + testSignVerify(publicKey, privateKey); + })); } { |