Skip to content

Commit 1b7eafe

Browse files
committed
refactor: address self-review feedback on PQC PKCS#8 validation
- Export PQC_SEEDLESS_PKCS8_LENGTHS and consume it from tests instead of redeclaring the table in the test file. - Replace startsWith fallback with explicit PQC_FAMILY lookup so the import-rejection error message can't mislabel an unknown algorithm whose seedless length happens to collide. - Move configurePqcOutputFormats() call into HybridKeyObjectHandle ctor (still guarded by std::call_once) so providers are configured on first handle construction rather than per-export. - Add ML-DSA-65 PKCS#8 round-trip + sign/verify test mirroring the existing ML-KEM-768 round-trip. - Replace absolute-path comment with repo-relative reference.
1 parent b30fc08 commit 1b7eafe

4 files changed

Lines changed: 66 additions & 37 deletions

File tree

example/src/tests/subtle/import_export.ts

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
// createPublicKey, // TODO: for 'bad usages' test
2929
// createPrivateKey, // TODO: for 'bad usages' test
3030
getRandomValues,
31+
PQC_SEEDLESS_PKCS8_LENGTHS,
3132
subtle,
3233
} from 'react-native-quick-crypto';
3334
import { MLKEM_VARIANTS } from './mlkem_constants';
@@ -2283,17 +2284,11 @@ test(SUITE, 'ML-DSA-44 importKey rejects jwk alg mismatch', async () => {
22832284
);
22842285
});
22852286

2286-
// --- ML-DSA seedless PKCS#8 import rejection (issue #997) ---
2287-
2288-
const MLDSA_SEEDLESS_PKCS8_LENGTHS: Record<MlDsaVariant, number> = {
2289-
'ML-DSA-44': 2588,
2290-
'ML-DSA-65': 4060,
2291-
'ML-DSA-87': 4924,
2292-
};
2287+
// --- ML-DSA seedless PKCS#8 import rejection (PR #997) ---
22932288

22942289
for (const variant of MLDSA_VARIANTS) {
22952290
test(SUITE, `${variant} pkcs8 import rejects seedless key`, async () => {
2296-
const seedless = new Uint8Array(MLDSA_SEEDLESS_PKCS8_LENGTHS[variant]);
2291+
const seedless = new Uint8Array(PQC_SEEDLESS_PKCS8_LENGTHS[variant] ?? 0);
22972292
await assertThrowsAsync(
22982293
async () =>
22992294
await subtle.importKey('pkcs8', seedless, { name: variant }, true, [
@@ -2320,6 +2315,37 @@ for (const variant of MLDSA_VARIANTS) {
23202315
);
23212316
}
23222317

2318+
test(SUITE, 'ML-DSA-65 pkcs8 round-trip + sign/verify', async () => {
2319+
const keyPair = (await subtle.generateKey({ name: 'ML-DSA-65' }, true, [
2320+
'sign',
2321+
'verify',
2322+
])) as CryptoKeyPair;
2323+
2324+
const exported = (await subtle.exportKey(
2325+
'pkcs8',
2326+
keyPair.privateKey as CryptoKey,
2327+
)) as ArrayBuffer;
2328+
expect(exported.byteLength).to.equal(54);
2329+
2330+
const imported = await subtle.importKey(
2331+
'pkcs8',
2332+
exported,
2333+
{ name: 'ML-DSA-65' },
2334+
true,
2335+
['sign'],
2336+
);
2337+
2338+
const message = new TextEncoder().encode('round-trip test');
2339+
const signature = await subtle.sign({ name: 'ML-DSA-65' }, imported, message);
2340+
const verified = await subtle.verify(
2341+
{ name: 'ML-DSA-65' },
2342+
keyPair.publicKey as CryptoKey,
2343+
signature,
2344+
message,
2345+
);
2346+
expect(verified).to.equal(true);
2347+
});
2348+
23232349
// --- ML-DSA raw-public and raw-seed export/import tests ---
23242350
// 'raw-public' is normalized to 'raw' internally (public key bytes)
23252351
// 'raw-seed' passes through directly (64-byte private seed)
@@ -2721,20 +2747,11 @@ test(SUITE, 'ML-KEM-768 importKey raw rejects bad usages', async () => {
27212747
);
27222748
});
27232749

2724-
// --- ML-KEM seedless PKCS#8 import rejection + export length (issue #997) ---
2725-
2726-
const MLKEM_SEEDLESS_PKCS8_LENGTHS: Record<
2727-
(typeof MLKEM_VARIANTS)[number],
2728-
number
2729-
> = {
2730-
'ML-KEM-512': 1660,
2731-
'ML-KEM-768': 2428,
2732-
'ML-KEM-1024': 3196,
2733-
};
2750+
// --- ML-KEM seedless PKCS#8 import rejection + export length (PR #997) ---
27342751

27352752
for (const variant of MLKEM_VARIANTS) {
27362753
test(SUITE, `${variant} pkcs8 import rejects seedless key`, async () => {
2737-
const seedless = new Uint8Array(MLKEM_SEEDLESS_PKCS8_LENGTHS[variant]);
2754+
const seedless = new Uint8Array(PQC_SEEDLESS_PKCS8_LENGTHS[variant] ?? 0);
27382755
await assertThrowsAsync(
27392756
async () =>
27402757
await subtle.importKey('pkcs8', seedless, { name: variant }, true, [

packages/react-native-quick-crypto/cpp/keys/HybridKeyObjectHandle.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace margelo::nitro::crypto {
2020
// Configure loaded providers to prefer seed-only PKCS#8 output for ML-DSA /
2121
// ML-KEM, falling back to priv-only when no seed is available. Without this,
2222
// OpenSSL defaults to "seed-priv" — a longer encoding that bundles both —
23-
// which breaks interop with Node and PR #997's exact-length export check.
23+
// which breaks interop with Node and the exact-length export check in subtle.ts.
2424
// Mirrors src/crypto/crypto_util.cc in Node.
2525
static void configurePqcOutputFormats() {
2626
static std::once_flag once;
@@ -37,6 +37,15 @@ static void configurePqcOutputFormats() {
3737
}
3838
#endif
3939

40+
HybridKeyObjectHandle::HybridKeyObjectHandle() : HybridObject(TAG) {
41+
#if OPENSSL_VERSION_NUMBER >= 0x30600000L
42+
// Configure once on first handle construction. Providers are guaranteed
43+
// loaded by this point (any prior crypto op routed through ncrypto), and
44+
// the call_once flag makes subsequent constructions cheap.
45+
configurePqcOutputFormats();
46+
#endif
47+
}
48+
4049
// Helper functions for base64url encoding/decoding with BIGNUMs
4150
static std::string bn_to_base64url(const BIGNUM* bn, size_t expected_size = 0) {
4251
if (!bn)
@@ -193,18 +202,6 @@ std::shared_ptr<ArrayBuffer> HybridKeyObjectHandle::exportKey(std::optional<KFor
193202
// This allows extracting the public key from a private key
194203
bool exportAsPublic = (exportType == KeyEncoding::SPKI) || (keyType == KeyType::PUBLIC);
195204

196-
#if OPENSSL_VERSION_NUMBER >= 0x30600000L
197-
if (!exportAsPublic && exportType == KeyEncoding::PKCS8) {
198-
const char* typeName = EVP_PKEY_get0_type_name(pkey.get());
199-
if (typeName != nullptr) {
200-
std::string name(typeName);
201-
if (name.starts_with("ML-KEM-") || name.starts_with("ML-DSA-")) {
202-
configurePqcOutputFormats();
203-
}
204-
}
205-
}
206-
#endif
207-
208205
// Create encoding config
209206
if (exportAsPublic) {
210207
ncrypto::EVPKeyPointer::PublicKeyEncodingConfig config(false, static_cast<ncrypto::EVPKeyPointer::PKFormatType>(exportFormat),

packages/react-native-quick-crypto/cpp/keys/HybridKeyObjectHandle.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace margelo::nitro::crypto {
1515

1616
class HybridKeyObjectHandle : public HybridKeyObjectHandleSpec {
1717
public:
18-
HybridKeyObjectHandle() : HybridObject(TAG) {}
18+
HybridKeyObjectHandle();
1919

2020
public:
2121
std::shared_ptr<ArrayBuffer> exportKey(std::optional<KFormatType> format, std::optional<KeyEncoding> type,

packages/react-native-quick-crypto/src/subtle.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,8 +1281,9 @@ function edImportKey(
12811281
// Lengths (in bytes) of seedless ML-DSA / ML-KEM PKCS#8 encodings. A PKCS#8
12821282
// blob of exactly this length contains only the expanded private key with no
12831283
// seed; Node rejects these to keep cross-implementation interop intact.
1284-
// Refs: ~/dev/node/lib/internal/crypto/ml_dsa.js:158-174, ml_kem.js:157-173.
1285-
const PQC_SEEDLESS_PKCS8_LENGTHS: Readonly<Record<string, number>> = {
1284+
// Refs: node lib/internal/crypto/ml_dsa.js (mlDsaImportKey, pkcs8 case)
1285+
// node lib/internal/crypto/ml_kem.js (mlKemImportKey, pkcs8 case)
1286+
export const PQC_SEEDLESS_PKCS8_LENGTHS: Readonly<Record<string, number>> = {
12861287
'ML-DSA-44': 2588,
12871288
'ML-DSA-65': 4060,
12881289
'ML-DSA-87': 4924,
@@ -1291,6 +1292,17 @@ const PQC_SEEDLESS_PKCS8_LENGTHS: Readonly<Record<string, number>> = {
12911292
'ML-KEM-1024': 3196,
12921293
};
12931294

1295+
// Map from PQC algorithm name to display family. Used to render the
1296+
// import-rejection error message in the same form Node emits.
1297+
const PQC_FAMILY: Readonly<Record<string, 'ML-DSA' | 'ML-KEM'>> = {
1298+
'ML-DSA-44': 'ML-DSA',
1299+
'ML-DSA-65': 'ML-DSA',
1300+
'ML-DSA-87': 'ML-DSA',
1301+
'ML-KEM-512': 'ML-KEM',
1302+
'ML-KEM-768': 'ML-KEM',
1303+
'ML-KEM-1024': 'ML-KEM',
1304+
};
1305+
12941306
function pqcImportKeyObject(
12951307
format: ImportFormat,
12961308
data: BufferLike | JWK,
@@ -1308,8 +1320,11 @@ function pqcImportKeyObject(
13081320
};
13091321
} else if (format === 'pkcs8') {
13101322
const ab = bufferLikeToArrayBuffer(data as BufferLike);
1311-
if (ab.byteLength === PQC_SEEDLESS_PKCS8_LENGTHS[name]) {
1312-
const family = name.startsWith('ML-DSA') ? 'ML-DSA' : 'ML-KEM';
1323+
const family = PQC_FAMILY[name];
1324+
if (
1325+
family !== undefined &&
1326+
ab.byteLength === PQC_SEEDLESS_PKCS8_LENGTHS[name]
1327+
) {
13131328
throw lazyDOMException(
13141329
`Importing an ${family} PKCS#8 key without a seed is not supported`,
13151330
'NotSupportedError',

0 commit comments

Comments
 (0)