Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1333dcc

Browse filesBrowse files
tniessenrvagg
tniessen
authored and
rvagg
committed
crypto: fix unencrypted DER PKCS8 parsing
The previously used OpenSSL call only supports encrypted PKCS8, this commit adds support for unencrypted PKCS8. PR-URL: #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>
1 parent 70e463c commit 1333dcc
Copy full SHA for 1333dcc

File tree

Expand file treeCollapse file tree

3 files changed

+134
-39
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+134
-39
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+66-39Lines changed: 66 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2821,6 +2821,59 @@ static MaybeLocal<Value> WritePublicKey(Environment* env,
28212821
return BIOToStringOrBuffer(env, bio.get(), config.format_);
28222822
}
28232823

2824+
static bool IsASN1Sequence(const unsigned char* data, size_t size,
2825+
size_t* data_offset, size_t* data_size) {
2826+
if (size < 2 || data[0] != 0x30)
2827+
return false;
2828+
2829+
if (data[1] & 0x80) {
2830+
// Long form.
2831+
size_t n_bytes = data[1] & ~0x80;
2832+
if (n_bytes + 2 > size || n_bytes > sizeof(size_t))
2833+
return false;
2834+
size_t length = 0;
2835+
for (size_t i = 0; i < n_bytes; i++)
2836+
length = (length << 8) | data[i + 2];
2837+
*data_offset = 2 + n_bytes;
2838+
*data_size = std::min(size - 2 - n_bytes, length);
2839+
} else {
2840+
// Short form.
2841+
*data_offset = 2;
2842+
*data_size = std::min<size_t>(size - 2, data[1]);
2843+
}
2844+
2845+
return true;
2846+
}
2847+
2848+
static bool IsRSAPrivateKey(const unsigned char* data, size_t size) {
2849+
// Both RSAPrivateKey and RSAPublicKey structures start with a SEQUENCE.
2850+
size_t offset, len;
2851+
if (!IsASN1Sequence(data, size, &offset, &len))
2852+
return false;
2853+
2854+
// An RSAPrivateKey sequence always starts with a single-byte integer whose
2855+
// value is either 0 or 1, whereas an RSAPublicKey starts with the modulus
2856+
// (which is the product of two primes and therefore at least 4), so we can
2857+
// decide the type of the structure based on the first three bytes of the
2858+
// sequence.
2859+
return len >= 3 &&
2860+
data[offset] == 2 &&
2861+
data[offset + 1] == 1 &&
2862+
!(data[offset + 2] & 0xfe);
2863+
}
2864+
2865+
static bool IsEncryptedPrivateKeyInfo(const unsigned char* data, size_t size) {
2866+
// Both PrivateKeyInfo and EncryptedPrivateKeyInfo start with a SEQUENCE.
2867+
size_t offset, len;
2868+
if (!IsASN1Sequence(data, size, &offset, &len))
2869+
return false;
2870+
2871+
// A PrivateKeyInfo sequence always starts with an integer whereas an
2872+
// EncryptedPrivateKeyInfo starts with an AlgorithmIdentifier.
2873+
return len >= 1 &&
2874+
data[offset] != 2;
2875+
}
2876+
28242877
static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config,
28252878
const char* key,
28262879
size_t key_len) {
@@ -2846,11 +2899,19 @@ static EVPKeyPointer ParsePrivateKey(const PrivateKeyEncodingConfig& config,
28462899
BIOPointer bio(BIO_new_mem_buf(key, key_len));
28472900
if (!bio)
28482901
return pkey;
2849-
char* pass = const_cast<char*>(config.passphrase_.get());
2850-
pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(),
2851-
nullptr,
2852-
PasswordCallback,
2853-
pass));
2902+
2903+
if (IsEncryptedPrivateKeyInfo(
2904+
reinterpret_cast<const unsigned char*>(key), key_len)) {
2905+
char* pass = const_cast<char*>(config.passphrase_.get());
2906+
pkey.reset(d2i_PKCS8PrivateKey_bio(bio.get(),
2907+
nullptr,
2908+
PasswordCallback,
2909+
pass));
2910+
} else {
2911+
PKCS8Pointer p8inf(d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr));
2912+
if (p8inf)
2913+
pkey.reset(EVP_PKCS82PKEY(p8inf.get()));
2914+
}
28542915
} else {
28552916
CHECK_EQ(config.type_.ToChecked(), kKeyEncodingSEC1);
28562917
const unsigned char* p = reinterpret_cast<const unsigned char*>(key);
@@ -3066,40 +3127,6 @@ static ManagedEVPPKey GetPrivateKeyFromJs(
30663127
}
30673128
}
30683129

3069-
static bool IsRSAPrivateKey(const unsigned char* data, size_t size) {
3070-
// Both RSAPrivateKey and RSAPublicKey structures start with a SEQUENCE.
3071-
if (size >= 2 && data[0] == 0x30) {
3072-
size_t offset;
3073-
if (data[1] & 0x80) {
3074-
// Long form.
3075-
size_t n_bytes = data[1] & ~0x80;
3076-
if (n_bytes + 2 > size || n_bytes > sizeof(size_t))
3077-
return false;
3078-
size_t i, length = 0;
3079-
for (i = 0; i < n_bytes; i++)
3080-
length = (length << 8) | data[i + 2];
3081-
offset = 2 + n_bytes;
3082-
size = std::min(size, length + 2);
3083-
} else {
3084-
// Short form.
3085-
offset = 2;
3086-
size = std::min<size_t>(size, data[1] + 2);
3087-
}
3088-
3089-
// An RSAPrivateKey sequence always starts with a single-byte integer whose
3090-
// value is either 0 or 1, whereas an RSAPublicKey starts with the modulus
3091-
// (which is the product of two primes and therefore at least 4), so we can
3092-
// decide the type of the structure based on the first three bytes of the
3093-
// sequence.
3094-
return size - offset >= 3 &&
3095-
data[offset] == 2 &&
3096-
data[offset + 1] == 1 &&
3097-
!(data[offset + 2] & 0xfe);
3098-
}
3099-
3100-
return false;
3101-
}
3102-
31033130
static ManagedEVPPKey GetPublicOrPrivateKeyFromJs(
31043131
const FunctionCallbackInfo<Value>& args,
31053132
unsigned int* offset,
Collapse file

‎src/node_crypto.h‎

Copy file name to clipboardExpand all lines: src/node_crypto.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ using BIOPointer = DeleteFnPtr<BIO, BIO_free_all>;
7777
using SSLCtxPointer = DeleteFnPtr<SSL_CTX, SSL_CTX_free>;
7878
using SSLSessionPointer = DeleteFnPtr<SSL_SESSION, SSL_SESSION_free>;
7979
using SSLPointer = DeleteFnPtr<SSL, SSL_free>;
80+
using PKCS8Pointer = DeleteFnPtr<PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO_free>;
8081
using EVPKeyPointer = DeleteFnPtr<EVP_PKEY, EVP_PKEY_free>;
8182
using EVPKeyCtxPointer = DeleteFnPtr<EVP_PKEY_CTX, EVP_PKEY_CTX_free>;
8283
using EVPMDPointer = DeleteFnPtr<EVP_MD_CTX, EVP_MD_CTX_free>;
Collapse file

‎test/parallel/test-crypto-keygen.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-keygen.js
+67Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,73 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
174174
testEncryptDecrypt(publicKey, key);
175175
testSignVerify(publicKey, key);
176176
}));
177+
178+
// Now do the same with an encrypted private key, but encoded as DER.
179+
generateKeyPair('rsa', {
180+
publicExponent: 0x10001,
181+
modulusLength: 512,
182+
publicKeyEncoding,
183+
privateKeyEncoding: {
184+
type: 'pkcs8',
185+
format: 'der',
186+
cipher: 'aes-256-cbc',
187+
passphrase: 'secret'
188+
}
189+
}, common.mustCall((err, publicKeyDER, privateKeyDER) => {
190+
assert.ifError(err);
191+
192+
assert(Buffer.isBuffer(publicKeyDER));
193+
assertApproximateSize(publicKeyDER, 74);
194+
195+
assert(Buffer.isBuffer(privateKeyDER));
196+
197+
// Since the private key is encrypted, signing shouldn't work anymore.
198+
const publicKey = { key: publicKeyDER, ...publicKeyEncoding };
199+
assert.throws(() => {
200+
testSignVerify(publicKey, {
201+
key: privateKeyDER,
202+
format: 'der',
203+
type: 'pkcs8'
204+
});
205+
}, /bad decrypt|asn1 encoding routines/);
206+
207+
const privateKey = {
208+
key: privateKeyDER,
209+
format: 'der',
210+
type: 'pkcs8',
211+
passphrase: 'secret'
212+
};
213+
testEncryptDecrypt(publicKey, privateKey);
214+
testSignVerify(publicKey, privateKey);
215+
}));
216+
217+
// Now do the same with an encrypted private key, but encoded as DER.
218+
generateKeyPair('rsa', {
219+
publicExponent: 0x10001,
220+
modulusLength: 512,
221+
publicKeyEncoding,
222+
privateKeyEncoding: {
223+
type: 'pkcs8',
224+
format: 'der'
225+
}
226+
}, common.mustCall((err, publicKeyDER, privateKeyDER) => {
227+
assert.ifError(err);
228+
229+
assert(Buffer.isBuffer(publicKeyDER));
230+
assertApproximateSize(publicKeyDER, 74);
231+
232+
assert(Buffer.isBuffer(privateKeyDER));
233+
234+
const publicKey = { key: publicKeyDER, ...publicKeyEncoding };
235+
const privateKey = {
236+
key: privateKeyDER,
237+
format: 'der',
238+
type: 'pkcs8',
239+
passphrase: 'secret'
240+
};
241+
testEncryptDecrypt(publicKey, privateKey);
242+
testSignVerify(publicKey, privateKey);
243+
}));
177244
}
178245

179246
{

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.