Skip to content

Commit baffe59

Browse files
panvapanva
committed
crypto: cache valid ECDH key pairs
Avoid repeating EC key-pair validation after a pair has already been established or validated. Invalidate the positive-only cache whenever public-key mutation can make the pair inconsistent. PR-URL: #65615 Reviewed-By: James M Snell <jasnell@gmail.com> Assisted-by: Codex Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 6399470 commit baffe59

4 files changed

Lines changed: 175 additions & 1 deletion

File tree

‎benchmark/crypto/ecdh-compute-secret.js‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const assert = require('node:assert');
5+
const crypto = require('node:crypto');
6+
7+
const kCurve = 'prime256v1';
8+
const kPeerPoolSize = 32;
9+
const scenarios = [
10+
'first-after-generate',
11+
'full-lifecycle',
12+
'reused-local-same-peer',
13+
'reused-local-peer-pool',
14+
];
15+
16+
const bench = common.createBenchmark(main, {
17+
scenario: scenarios,
18+
n: [5_000],
19+
}, {
20+
test: { scenario: 'first-after-generate', n: 1 },
21+
});
22+
23+
function generateContext() {
24+
const context = crypto.createECDH(kCurve);
25+
context.generateKeys();
26+
return context;
27+
}
28+
29+
function verifySecret(secret, local, peer) {
30+
assert.deepStrictEqual(secret, peer.computeSecret(local.getPublicKey()));
31+
}
32+
33+
function firstAfterGenerate(n) {
34+
const peer = generateContext();
35+
const peerPublicKey = peer.getPublicKey();
36+
const warmup = generateContext();
37+
warmup.computeSecret(peerPublicKey);
38+
39+
const locals = Array.from({ length: n }, generateContext);
40+
const secrets = new Array(n);
41+
42+
bench.start();
43+
for (let i = 0; i < n; i++)
44+
secrets[i] = locals[i].computeSecret(peerPublicKey);
45+
bench.end(n);
46+
47+
verifySecret(secrets[n - 1], locals[n - 1], peer);
48+
}
49+
50+
function fullLifecycle(n) {
51+
const peer = generateContext();
52+
const peerPublicKey = peer.getPublicKey();
53+
const warmup = generateContext();
54+
warmup.computeSecret(peerPublicKey);
55+
56+
const locals = new Array(n);
57+
const secrets = new Array(n);
58+
59+
bench.start();
60+
for (let i = 0; i < n; i++) {
61+
const local = locals[i] = generateContext();
62+
secrets[i] = local.computeSecret(peerPublicKey);
63+
}
64+
bench.end(n);
65+
66+
verifySecret(secrets[n - 1], locals[n - 1], peer);
67+
}
68+
69+
function reusedLocalSamePeer(n) {
70+
const local = generateContext();
71+
const peer = generateContext();
72+
const peerPublicKey = peer.getPublicKey();
73+
local.computeSecret(peerPublicKey);
74+
75+
const secrets = new Array(n);
76+
77+
bench.start();
78+
for (let i = 0; i < n; i++)
79+
secrets[i] = local.computeSecret(peerPublicKey);
80+
bench.end(n);
81+
82+
verifySecret(secrets[n - 1], local, peer);
83+
}
84+
85+
function reusedLocalPeerPool(n) {
86+
const local = generateContext();
87+
const peers = Array.from(
88+
{ length: Math.min(n, kPeerPoolSize) },
89+
generateContext);
90+
const peerPublicKeys = peers.map((peer) => peer.getPublicKey());
91+
local.computeSecret(peerPublicKeys[0]);
92+
93+
const secrets = new Array(n);
94+
95+
bench.start();
96+
for (let i = 0; i < n; i++)
97+
secrets[i] = local.computeSecret(peerPublicKeys[i % peers.length]);
98+
bench.end(n);
99+
100+
const lastPeer = peers[(n - 1) % peers.length];
101+
verifySecret(secrets[n - 1], local, lastPeer);
102+
}
103+
104+
function main({ scenario, n }) {
105+
switch (scenario) {
106+
case 'first-after-generate':
107+
return firstAfterGenerate(n);
108+
case 'full-lifecycle':
109+
return fullLifecycle(n);
110+
case 'reused-local-same-peer':
111+
return reusedLocalSamePeer(n);
112+
case 'reused-local-peer-pool':
113+
return reusedLocalPeerPool(n);
114+
default:
115+
throw new Error(`Unsupported scenario: ${scenario}`);
116+
}
117+
}

‎src/crypto/crypto_ec.cc‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,12 @@ void ECDH::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
136136
ECDH* ecdh;
137137
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.This());
138138

139+
const uint64_t generation = ncrypto::getFipsStateGeneration();
140+
ecdh->has_valid_key_pair_ = false;
139141
if (!ecdh->key_.generate()) {
140142
return THROW_ERR_CRYPTO_OPERATION_FAILED(env, "Failed to generate key");
141143
}
144+
ecdh->MaybeCacheValidKeyPair(generation);
142145
}
143146

144147
ECPointPointer ECDH::BufferToPoint(Environment* env,
@@ -309,6 +312,7 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
309312

310313
ecdh->key_ = std::move(new_key);
311314
ecdh->group_ = ecdh->key_.getGroup();
315+
ecdh->has_valid_key_pair_ = false;
312316
}
313317

314318
void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
@@ -327,6 +331,7 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
327331
"Failed to convert Buffer to EC_POINT");
328332
}
329333

334+
ecdh->has_valid_key_pair_ = false;
330335
if (!ecdh->key_.setPublicKey(pub)) {
331336
return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
332337
"Failed to set EC_POINT as the public key");
@@ -347,9 +352,22 @@ bool ECDH::IsKeyValidForCurve(const BignumPointer& private_key) {
347352
private_key < order;
348353
}
349354

355+
void ECDH::MaybeCacheValidKeyPair(uint64_t generation) {
356+
has_valid_key_pair_ = generation == ncrypto::getFipsStateGeneration();
357+
if (has_valid_key_pair_) valid_key_pair_generation_ = generation;
358+
}
359+
350360
bool ECDH::IsKeyPairValid() {
361+
const uint64_t generation = ncrypto::getFipsStateGeneration();
362+
if (has_valid_key_pair_ && valid_key_pair_generation_ == generation) {
363+
return true;
364+
}
365+
has_valid_key_pair_ = false;
366+
351367
MarkPopErrorOnReturn mark_pop_error_on_return;
352-
return key_.checkKey();
368+
const bool is_valid = key_.checkKey();
369+
if (is_valid) MaybeCacheValidKeyPair(generation);
370+
return is_valid;
353371
}
354372

355373
// Convert the input public key to compressed, uncompressed, or hybrid formats.

‎src/crypto/crypto_ec.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,14 @@ class ECDH final : public BaseObject {
4848
static void GetPublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);
4949
static void SetPublicKey(const v8::FunctionCallbackInfo<v8::Value>& args);
5050

51+
void MaybeCacheValidKeyPair(uint64_t generation);
5152
bool IsKeyPairValid();
5253
bool IsKeyValidForCurve(const ncrypto::BignumPointer& private_key);
5354

5455
ncrypto::ECKeyPointer key_;
5556
const EC_GROUP* group_;
57+
bool has_valid_key_pair_ = false;
58+
uint64_t valid_key_pair_generation_ = 0;
5659
};
5760

5861
struct EcKeyPairParams final : public MemoryRetainer {

‎test/parallel/test-crypto-dh-curves.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,15 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
139139
ecdh4.setPrivateKey(ecdh1.getPrivateKey());
140140
ecdh4.setPublicKey(ecdh1.getPublicKey());
141141

142+
const ecdh4Secret = ecdh4.computeSecret(ecdh2.getPublicKey());
143+
assert.deepStrictEqual(ecdh4.computeSecret(ecdh2.getPublicKey()),
144+
ecdh4Secret);
145+
142146
assert.throws(() => {
143147
ecdh4.setPublicKey(ecdh3.getPublicKey());
144148
}, { message: 'Failed to convert Buffer to EC_POINT' });
149+
assert.deepStrictEqual(ecdh4.computeSecret(ecdh2.getPublicKey()),
150+
ecdh4Secret);
145151

146152
// Verify that we can use ECDH without having to use newly generated keys.
147153
const ecdh5 = crypto.createECDH('secp256k1');
@@ -185,6 +191,8 @@ if (availableCurves.has('prime256v1') && availableCurves.has('secp256k1')) {
185191
sharedSecret);
186192
assert.strictEqual(ecdh5.computeSecret(peerPubPtUnComp, 'hex', 'hex'),
187193
sharedSecret);
194+
assert.strictEqual(ecdh5.computeSecret(peerPubPtComp, 'hex', 'hex'),
195+
sharedSecret);
188196

189197
// Verify that we still have the same key pair as before the computation.
190198
assert.strictEqual(ecdh5.getPrivateKey('hex'), cafebabeKey);
@@ -254,3 +262,31 @@ if (availableCurves.has('prime256v1') && availableHashes.has('sha256')) {
254262
'-----END EC PRIVATE KEY-----';
255263
crypto.createSign('SHA256').sign(ecPrivateKey);
256264
}
265+
266+
if (crypto.getFips() && hasOpenSSL(3) && availableCurves.has('secp256k1')) {
267+
const originalFips = crypto.getFips();
268+
269+
try {
270+
crypto.setFips(0);
271+
const local = crypto.createECDH('secp256k1');
272+
const peer = crypto.createECDH('secp256k1');
273+
local.generateKeys();
274+
const peerPublicKey = peer.generateKeys();
275+
276+
local.computeSecret(peerPublicKey);
277+
crypto.setFips(1);
278+
assert.throws(() => local.computeSecret(peerPublicKey), {
279+
code: 'ERR_CRYPTO_INVALID_KEYPAIR',
280+
name: 'RangeError',
281+
});
282+
283+
const installed = crypto.createECDH('secp256k1');
284+
installed.setPrivateKey(Buffer.from('cafebabe'.repeat(8), 'hex'));
285+
assert.throws(() => installed.computeSecret(peerPublicKey), {
286+
code: 'ERR_CRYPTO_INVALID_KEYPAIR',
287+
name: 'RangeError',
288+
});
289+
} finally {
290+
crypto.setFips(originalFips);
291+
}
292+
}

0 commit comments

Comments
 (0)