Skip to content

Commit 3f46289

Browse files
committed
crypto: remove verify setup key locking
Verify configuration only copies signature bytes and reads DSA/EC order width for P1363 conversion. Conversion constructs independent DER and BIGNUM values. Signing and verification already run without the mutex using separate operation contexts. Cover cold shared public keys and worker clones with checked P1363 verification while signatures, derivation, exports, metadata queries, and equality checks overlap. Compare source PKCS8 encoding after workers finish. OpenSSL's ordinary EC PKCS8 encoder temporarily changes source flags and races even on the unmodified base. Keep WebCrypto PKCS8 exports concurrent through their independent clones. Refs: #36825 Refs: #37816 Refs: https://docs.openssl.org/3.5/man7/openssl-threads/ Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex
1 parent 32b22fd commit 3f46289

2 files changed

Lines changed: 242 additions & 1 deletion

File tree

‎src/crypto/crypto_sig.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,6 @@ Maybe<void> SignTraits::AdditionalConfig(
712712
}
713713
// If this is an EC key (assuming ECDSA) we need to convert the
714714
// the signature from WebCrypto format into DER format...
715-
Mutex::ScopedLock lock(params->key.mutex());
716715
const auto& akey = params->key.GetAsymmetricKey();
717716
if (UseP1363Encoding(akey, params->dsa_encoding)) {
718717
params->signature = ConvertSignatureToDER(akey, signature.ToByteSource());
Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const assert = require('assert');
8+
const {
9+
createPrivateKey,
10+
createPublicKey,
11+
diffieHellman,
12+
generateKeyPairSync,
13+
KeyObject,
14+
sign,
15+
verify,
16+
} = require('crypto');
17+
const { once } = require('events');
18+
const { promisify } = require('util');
19+
const { Worker, parentPort, workerData } = require('worker_threads');
20+
const fixtures = require('../common/fixtures');
21+
const { subtle } = globalThis.crypto;
22+
const signAsync = promisify(sign);
23+
const verifyAsync = promisify(verify);
24+
const ecdsa = { name: 'ECDSA', hash: 'SHA-256' };
25+
const pkcs8 = { type: 'pkcs8', format: 'der' };
26+
const spki = { type: 'spki', format: 'der' };
27+
const iterations = 16;
28+
29+
async function exercise({ keys, peer, expected }) {
30+
// Check generated signatures and exports with independent imports that do
31+
// not warm the shared key's provider caches before its first operation.
32+
const referencePrivate = createPrivateKey({
33+
key: expected.originalPrivate, ...pkcs8,
34+
});
35+
const referencePublic = createPublicKey({ key: expected.public, ...spki });
36+
37+
for (let i = 0; i < iterations; i++) {
38+
const data = Buffer.from(`shared key operation ${i}`);
39+
const referenceSignature = Buffer.from(expected.signatures[i]);
40+
const pending = [];
41+
for (let j = 0; j < 4; j++) {
42+
pending.push(
43+
verifyAsync('sha256', data, {
44+
key: keys.publicKey,
45+
dsaEncoding: 'ieee-p1363',
46+
}, referenceSignature).then((valid) => {
47+
assert.strictEqual(valid, true);
48+
}),
49+
signAsync('sha256', data, keys.privateKey).then((signature) => {
50+
assert.strictEqual(
51+
verify('sha256', data, referencePublic, signature), true);
52+
}),
53+
);
54+
if (keys.ecdsaPrivate === undefined)
55+
continue;
56+
pending.push(
57+
subtle.verify(ecdsa, keys.ecdsaPublic, referenceSignature, data)
58+
.then((valid) => {
59+
assert.strictEqual(valid, true);
60+
}),
61+
subtle.sign(ecdsa, keys.ecdsaPrivate, data).then((signature) => {
62+
assert.strictEqual(verify('sha256', data, {
63+
key: referencePublic,
64+
dsaEncoding: 'ieee-p1363',
65+
}, Buffer.from(signature)), true);
66+
}),
67+
subtle.deriveBits({ name: 'ECDH', public: peer }, keys.ecdhPrivate, 256)
68+
.then((bits) => {
69+
assert.deepStrictEqual(
70+
Buffer.from(bits), Buffer.from(expected.bits));
71+
}),
72+
);
73+
}
74+
75+
// Export, compare, and query metadata while jobs use this same native key
76+
// in the threadpool and other Workers. Fresh wrappers also exercise the
77+
// native metadata getters on every iteration rather than their JS caches.
78+
const privateKey = keys.ecdsaPrivate === undefined ?
79+
structuredClone(keys.privateKey) : KeyObject.from(keys.ecdsaPrivate);
80+
const publicKey = keys.ecdsaPublic === undefined ?
81+
structuredClone(keys.publicKey) : KeyObject.from(keys.ecdsaPublic);
82+
assert.strictEqual(privateKey.equals(referencePrivate), true);
83+
assert.strictEqual(publicKey.equals(referencePublic), true);
84+
assert.strictEqual(privateKey.equals(keys.privateKey), true);
85+
assert.strictEqual(publicKey.equals(keys.publicKey), true);
86+
assert.deepStrictEqual(privateKey.asymmetricKeyDetails, {
87+
namedCurve: 'prime256v1',
88+
});
89+
assert.deepStrictEqual(publicKey.asymmetricKeyDetails, {
90+
namedCurve: 'prime256v1',
91+
});
92+
assert.deepStrictEqual(
93+
privateKey.export({ format: 'jwk' }), expected.privateJwk);
94+
assert.deepStrictEqual(
95+
publicKey.export({ format: 'jwk' }), expected.publicJwk);
96+
assert.deepStrictEqual(
97+
privateKey.export({ format: 'raw-private' }),
98+
Buffer.from(expected.rawPrivate));
99+
assert.deepStrictEqual(
100+
publicKey.export({ format: 'raw-public' }), Buffer.from(expected.rawPublic));
101+
assert.deepStrictEqual(publicKey.export({
102+
format: 'raw-public', type: 'compressed',
103+
}), Buffer.from(expected.compressedPublic));
104+
assert.deepStrictEqual(
105+
publicKey.export(spki), Buffer.from(expected.public));
106+
107+
if (keys.ecdsaPrivate === undefined) {
108+
assert.deepStrictEqual(diffieHellman({
109+
privateKey, publicKey: peer,
110+
}), Buffer.from(expected.bits));
111+
await Promise.all(pending);
112+
continue;
113+
}
114+
115+
for (const key of [keys.ecdsaPrivate, keys.ecdhPrivate]) {
116+
pending.push(subtle.exportKey('pkcs8', key).then((encoded) => {
117+
assert.deepStrictEqual(
118+
Buffer.from(encoded), Buffer.from(expected.private));
119+
}));
120+
}
121+
pending.push(
122+
subtle.exportKey('raw', keys.ecdsaPublic).then((encoded) => {
123+
assert.deepStrictEqual(
124+
Buffer.from(encoded), Buffer.from(expected.rawPublic));
125+
}),
126+
subtle.exportKey('spki', keys.ecdsaPublic).then((encoded) => {
127+
assert.deepStrictEqual(
128+
Buffer.from(encoded), Buffer.from(expected.public));
129+
}),
130+
subtle.exportKey('jwk', keys.ecdsaPrivate).then((jwk) => {
131+
assert.deepStrictEqual(jwk, {
132+
...expected.privateJwk, key_ops: ['sign'], ext: true,
133+
});
134+
}),
135+
);
136+
await Promise.all(pending);
137+
}
138+
}
139+
140+
if (workerData?.sharedKeyTest) {
141+
parentPort.postMessage('ready');
142+
Atomics.wait(workerData.barrier, 0, 0);
143+
exercise(workerData).then(common.mustCall());
144+
} else {
145+
function der(tag, ...parts) {
146+
const body = Buffer.concat(parts);
147+
assert(body.length < 128);
148+
return Buffer.concat([Buffer.from([tag, body.length]), body]);
149+
}
150+
151+
async function run(input, peer, expected, webcrypto = true) {
152+
// The KeyObject-only case performs no operations before the barrier,
153+
// exercising concurrent first use of the shared key. CryptoKey conversion
154+
// validates the key before its first concurrent sign/derive/export calls.
155+
const privateKey = createPrivateKey({ key: input, ...pkcs8 });
156+
const publicKey = createPublicKey(privateKey);
157+
const keys = { privateKey, publicKey };
158+
if (webcrypto) {
159+
keys.ecdsaPrivate = privateKey.toCryptoKey({
160+
name: 'ECDSA', namedCurve: 'P-256',
161+
}, true, ['sign']);
162+
keys.ecdsaPublic = publicKey.toCryptoKey({
163+
name: 'ECDSA', namedCurve: 'P-256',
164+
}, true, ['verify']);
165+
keys.ecdhPrivate = privateKey.toCryptoKey({
166+
name: 'ECDH', namedCurve: 'P-256',
167+
}, true, ['deriveBits']);
168+
}
169+
const barrier = new Int32Array(new SharedArrayBuffer(4));
170+
const data = { sharedKeyTest: true, keys, peer, expected, barrier };
171+
const ready = [];
172+
const exited = [];
173+
for (let i = 0; i < 3; i++) {
174+
const worker = new Worker(__filename, { workerData: data });
175+
ready.push(once(worker, 'message').then(([message]) => {
176+
assert.strictEqual(message, 'ready');
177+
}));
178+
exited.push(once(worker, 'exit').then(common.mustCall(([code]) => {
179+
assert.strictEqual(code, 0);
180+
})));
181+
}
182+
await Promise.all(ready);
183+
Atomics.store(barrier, 0, 1);
184+
Atomics.notify(barrier, 0);
185+
await Promise.all([exercise(data), ...exited]);
186+
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.
191+
assert.deepStrictEqual(
192+
privateKey.export(pkcs8), Buffer.from(expected.originalPrivate));
193+
}
194+
195+
(async () => {
196+
const reference = createPrivateKey(fixtures.readKey('ec_p256_private.pem'));
197+
const publicKey = createPublicKey(reference);
198+
const { publicKey: peer } = generateKeyPairSync('ec', {
199+
namedCurve: 'prime256v1',
200+
});
201+
const privateJwk = reference.export({ format: 'jwk' });
202+
const expected = {
203+
private: reference.export(pkcs8),
204+
public: publicKey.export(spki),
205+
privateJwk,
206+
publicJwk: publicKey.export({ format: 'jwk' }),
207+
rawPrivate: reference.export({ format: 'raw-private' }),
208+
rawPublic: publicKey.export({ format: 'raw-public' }),
209+
compressedPublic: publicKey.export({
210+
format: 'raw-public', type: 'compressed',
211+
}),
212+
bits: diffieHellman({ privateKey: reference, publicKey: peer }),
213+
signatures: Array.from({ length: iterations }, (_, i) => {
214+
return sign('sha256', Buffer.from(`shared key operation ${i}`), {
215+
key: reference,
216+
dsaEncoding: 'ieee-p1363',
217+
});
218+
}),
219+
};
220+
const cryptoPeer = peer.toCryptoKey({
221+
name: 'ECDH', namedCurve: 'P-256',
222+
}, true, []);
223+
await run(expected.private, peer, {
224+
...expected, originalPrivate: expected.private,
225+
}, false);
226+
await run(expected.private, cryptoPeer, {
227+
...expected, originalPrivate: expected.private,
228+
});
229+
230+
const algorithmIdentifier = der(0x30, Buffer.from(
231+
'06072a8648ce3d020106082a8648ce3d030107', 'hex'));
232+
const ecPrivateKey = der(
233+
0x30, Buffer.from('020101', 'hex'),
234+
der(0x04, Buffer.from(privateJwk.d, 'base64url')));
235+
const privateOnly = der(
236+
0x30, Buffer.from('020100', 'hex'), algorithmIdentifier,
237+
der(0x04, ecPrivateKey));
238+
const originalPrivate = createPrivateKey({ key: privateOnly, ...pkcs8 })
239+
.export(pkcs8);
240+
await run(privateOnly, cryptoPeer, { ...expected, originalPrivate });
241+
})().then(common.mustCall());
242+
}

0 commit comments

Comments
 (0)