Skip to content

Commit 554dd55

Browse files
panvanodejs-github-bot
authored andcommitted
crypto: isolate provider EC PKCS8 encoding
OpenSSL's provider ec_pki_priv_to_der() temporarily changes EC_KEY encoding flags on the input key. Concurrent PKCS8 exports can emit inconsistent DER or affect a concurrent SEC1 export. The old Node key mutex never covered this path. Encode provider EC and SM2 keys through EVP_PKEY_dup(), preserving encoding flags, point conversion form, parameters, and provider. Legacy OpenSSL and BoringSSL already encode using local flags. Restore concurrent KeyObject PKCS8 DER assertions and cover PEM and SEC1 exports, including ECPrivateKey inputs without a public point. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #66413 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 9267ac8 commit 554dd55

2 files changed

Lines changed: 29 additions & 6 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4401,11 +4401,26 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
44014401
break;
44024402
}
44034403
case PKEncodingType::PKCS8: {
4404+
EVP_PKEY* export_key = get();
4405+
#if NCRYPTO_USE_OPENSSL3_PROVIDER
4406+
// OpenSSL's provider EC PKCS8 encoders temporarily change encoding flags.
4407+
// Use an independent key so concurrent exports do not change the source.
4408+
EVPKeyPointer key_copy;
4409+
if ((isA(KeyAlgorithm::EC) || isA(KeyAlgorithm::SM2)) &&
4410+
EVP_PKEY_get0_provider(get()) != nullptr) {
4411+
key_copy.reset(EVP_PKEY_dup(get()));
4412+
if (!key_copy) {
4413+
return Result<BIOPointer, bool>(false,
4414+
mark_pop_error_on_return.peekError());
4415+
}
4416+
export_key = key_copy.get();
4417+
}
4418+
#endif
44044419
switch (config.format) {
44054420
case PKFormatType::PEM: {
44064421
// Encode PKCS#8 as PEM.
44074422
err = PEM_write_bio_PKCS8PrivateKey(bio.get(),
4408-
get(),
4423+
export_key,
44094424
config.cipher,
44104425
passphrase.data,
44114426
passphrase.len,
@@ -4415,7 +4430,7 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
44154430
}
44164431
case PKFormatType::DER: {
44174432
err = i2d_PKCS8PrivateKey_bio(bio.get(),
4418-
get(),
4433+
export_key,
44194434
config.cipher,
44204435
passphrase.data,
44214436
passphrase.len,

‎test/parallel/test-crypto-key-reuse-concurrent.js‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ const signAsync = promisify(sign);
2323
const verifyAsync = promisify(verify);
2424
const ecdsa = { name: 'ECDSA', hash: 'SHA-256' };
2525
const pkcs8 = { type: 'pkcs8', format: 'der' };
26+
const pkcs8Pem = { type: 'pkcs8', format: 'pem' };
27+
const sec1 = { type: 'sec1', format: 'der' };
2628
const spki = { type: 'spki', format: 'der' };
2729
const iterations = 16;
2830

@@ -33,6 +35,8 @@ async function exercise({ keys, peer, expected }) {
3335
key: expected.originalPrivate, ...pkcs8,
3436
});
3537
const referencePublic = createPublicKey({ key: expected.public, ...spki });
38+
const referencePem = referencePrivate.export(pkcs8Pem);
39+
const referenceSec1 = referencePrivate.export(sec1);
3640

3741
for (let i = 0; i < iterations; i++) {
3842
const data = Buffer.from(`shared key operation ${i}`);
@@ -103,6 +107,10 @@ async function exercise({ keys, peer, expected }) {
103107
}), Buffer.from(expected.compressedPublic));
104108
assert.deepStrictEqual(
105109
publicKey.export(spki), Buffer.from(expected.public));
110+
assert.deepStrictEqual(
111+
privateKey.export(pkcs8), Buffer.from(expected.originalPrivate));
112+
assert.strictEqual(privateKey.export(pkcs8Pem), referencePem);
113+
assert.deepStrictEqual(privateKey.export(sec1), referenceSec1);
106114

107115
if (keys.ecdsaPrivate === undefined) {
108116
assert.deepStrictEqual(diffieHellman({
@@ -184,12 +192,12 @@ if (workerData?.sharedKeyTest) {
184192
Atomics.notify(barrier, 0);
185193
await Promise.all([exercise(data), ...exited]);
186194

187-
// Ordinary PKCS8 encoding temporarily changes EC encoding flags in some
188-
// OpenSSL versions, so compare only after all shared-key users finish.
189-
// WebCrypto export must add the public point on a copy and leave the
190-
// original key's encoding flags unchanged.
195+
// PKCS8 export must leave the source encoding flags unchanged, including
196+
// whether SEC1 includes parameters and whether the public point is omitted.
191197
assert.deepStrictEqual(
192198
privateKey.export(pkcs8), Buffer.from(expected.originalPrivate));
199+
assert.deepStrictEqual(privateKey.export(sec1),
200+
createPrivateKey({ key: input, ...pkcs8 }).export(sec1));
193201
}
194202

195203
(async () => {

0 commit comments

Comments
 (0)