mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 18:25:13 +08:00
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: https://github.com/nodejs/node/pull/66413 Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
1 parent
9267ac84c0
commit
554dd55eb8
2 files changed
+29
-6
No files matched your search
Vendored
+17
-2
@@ -4401,11 +4401,26 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
|
||||
break;
|
||||
}
|
||||
case PKEncodingType::PKCS8: {
|
||||
EVP_PKEY* export_key = get();
|
||||
#if NCRYPTO_USE_OPENSSL3_PROVIDER
|
||||
// OpenSSL's provider EC PKCS8 encoders temporarily change encoding flags.
|
||||
// Use an independent key so concurrent exports do not change the source.
|
||||
EVPKeyPointer key_copy;
|
||||
if ((isA(KeyAlgorithm::EC) || isA(KeyAlgorithm::SM2)) &&
|
||||
EVP_PKEY_get0_provider(get()) != nullptr) {
|
||||
key_copy.reset(EVP_PKEY_dup(get()));
|
||||
if (!key_copy) {
|
||||
return Result<BIOPointer, bool>(false,
|
||||
mark_pop_error_on_return.peekError());
|
||||
}
|
||||
export_key = key_copy.get();
|
||||
}
|
||||
#endif
|
||||
switch (config.format) {
|
||||
case PKFormatType::PEM: {
|
||||
// Encode PKCS#8 as PEM.
|
||||
err = PEM_write_bio_PKCS8PrivateKey(bio.get(),
|
||||
get(),
|
||||
export_key,
|
||||
config.cipher,
|
||||
passphrase.data,
|
||||
passphrase.len,
|
||||
@@ -4415,7 +4430,7 @@ Result<BIOPointer, bool> EVPKeyPointer::writePrivateKey(
|
||||
}
|
||||
case PKFormatType::DER: {
|
||||
err = i2d_PKCS8PrivateKey_bio(bio.get(),
|
||||
get(),
|
||||
export_key,
|
||||
config.cipher,
|
||||
passphrase.data,
|
||||
passphrase.len,
|
||||
|
||||
@@ -23,6 +23,8 @@ const signAsync = promisify(sign);
|
||||
const verifyAsync = promisify(verify);
|
||||
const ecdsa = { name: 'ECDSA', hash: 'SHA-256' };
|
||||
const pkcs8 = { type: 'pkcs8', format: 'der' };
|
||||
const pkcs8Pem = { type: 'pkcs8', format: 'pem' };
|
||||
const sec1 = { type: 'sec1', format: 'der' };
|
||||
const spki = { type: 'spki', format: 'der' };
|
||||
const iterations = 16;
|
||||
|
||||
@@ -33,6 +35,8 @@ async function exercise({ keys, peer, expected }) {
|
||||
key: expected.originalPrivate, ...pkcs8,
|
||||
});
|
||||
const referencePublic = createPublicKey({ key: expected.public, ...spki });
|
||||
const referencePem = referencePrivate.export(pkcs8Pem);
|
||||
const referenceSec1 = referencePrivate.export(sec1);
|
||||
|
||||
for (let i = 0; i < iterations; i++) {
|
||||
const data = Buffer.from(`shared key operation ${i}`);
|
||||
@@ -103,6 +107,10 @@ async function exercise({ keys, peer, expected }) {
|
||||
}), Buffer.from(expected.compressedPublic));
|
||||
assert.deepStrictEqual(
|
||||
publicKey.export(spki), Buffer.from(expected.public));
|
||||
assert.deepStrictEqual(
|
||||
privateKey.export(pkcs8), Buffer.from(expected.originalPrivate));
|
||||
assert.strictEqual(privateKey.export(pkcs8Pem), referencePem);
|
||||
assert.deepStrictEqual(privateKey.export(sec1), referenceSec1);
|
||||
|
||||
if (keys.ecdsaPrivate === undefined) {
|
||||
assert.deepStrictEqual(diffieHellman({
|
||||
@@ -184,12 +192,12 @@ if (workerData?.sharedKeyTest) {
|
||||
Atomics.notify(barrier, 0);
|
||||
await Promise.all([exercise(data), ...exited]);
|
||||
|
||||
// Ordinary PKCS8 encoding temporarily changes EC encoding flags in some
|
||||
// OpenSSL versions, so compare only after all shared-key users finish.
|
||||
// WebCrypto export must add the public point on a copy and leave the
|
||||
// original key's encoding flags unchanged.
|
||||
// PKCS8 export must leave the source encoding flags unchanged, including
|
||||
// whether SEC1 includes parameters and whether the public point is omitted.
|
||||
assert.deepStrictEqual(
|
||||
privateKey.export(pkcs8), Buffer.from(expected.originalPrivate));
|
||||
assert.deepStrictEqual(privateKey.export(sec1),
|
||||
createPrivateKey({ key: input, ...pkcs8 }).export(sec1));
|
||||
}
|
||||
|
||||
(async () => {
|
||||
|
||||
Reference in new issue
Block a user