Skip to content

Commit f25edd3

Browse files
committed
fix: address code review findings from EC_KEY migration
- Replace CryptoKeyInternal duck type with CryptoKey import in hkdf.ts, removing the any escape hatch (#2) - Use EVP_PKEY_bits() instead of allocating EC_GROUP just for field_size in exportJwk; hardcode field sizes for known curves in initJwk (#3) - Move createEcEvpPkey from inline header to QuickCryptoUtils.cpp to avoid code duplication across translation units (#4) - Remove unnecessary 'as any' casts in JWK round-trip tests since KeyInputObject already accepts JWK (#6)
1 parent 6ef1915 commit f25edd3

6 files changed

Lines changed: 59 additions & 64 deletions

File tree

example/src/tests/keys/create_keys.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,8 +431,7 @@ for (const { namedCurve, crv, hash } of EC_CURVES) {
431431
expect(jwk.y).to.match(/^[A-Za-z0-9_-]+$/);
432432
expect(jwk.d).to.match(/^[A-Za-z0-9_-]+$/);
433433

434-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
435-
const reimported = createPrivateKey({ key: jwk as any, format: 'jwk' });
434+
const reimported = createPrivateKey({ key: jwk, format: 'jwk' });
436435
expect(reimported.type).to.equal('private');
437436
expect(reimported.asymmetricKeyType).to.equal('ec');
438437

@@ -478,8 +477,7 @@ for (const { namedCurve, crv, hash } of EC_CURVES) {
478477
expect(jwk.y).to.match(/^[A-Za-z0-9_-]+$/);
479478
expect(jwk.d).to.equal(undefined);
480479

481-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
482-
const reimported = createPublicKey({ key: jwk as any, format: 'jwk' });
480+
const reimported = createPublicKey({ key: jwk, format: 'jwk' });
483481
expect(reimported.type).to.equal('public');
484482
expect(reimported.asymmetricKeyType).to.equal('ec');
485483
},

packages/react-native-quick-crypto/android/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ add_library(
5858
../cpp/sign/HybridSignHandle.cpp
5959
../cpp/sign/HybridVerifyHandle.cpp
6060
../cpp/utils/HybridUtils.cpp
61+
../cpp/utils/QuickCryptoUtils.cpp
6162
${BLAKE3_SOURCES}
6263
../deps/fastpbkdf2/fastpbkdf2.c
6364
../deps/ncrypto/src/aead.cpp

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

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,10 @@ JWK HybridKeyObjectHandle::exportJwk(const JWK& key, bool handleRsaPss) {
258258

259259
std::string curve_name(curve_name_buf, name_len);
260260

261-
int nid = OBJ_txt2nid(curve_name.c_str());
262-
EC_GROUP* group = EC_GROUP_new_by_curve_name(nid);
263-
if (!group)
264-
throw std::runtime_error("Unknown curve");
265-
266-
// Get the field size in bytes for proper padding
267-
size_t field_size = (EC_GROUP_get_degree(group) + 7) / 8;
268-
EC_GROUP_free(group);
261+
int bits = EVP_PKEY_bits(pkey.get());
262+
if (bits <= 0)
263+
throw std::runtime_error("Failed to get EC key size");
264+
size_t field_size = (static_cast<size_t>(bits) + 7) / 8;
269265

270266
result.kty = JWKkty::EC;
271267

@@ -567,26 +563,22 @@ std::optional<KeyType> HybridKeyObjectHandle::initJwk(const JWK& keyData, std::o
567563

568564
std::string crv = keyData.crv.value();
569565

570-
// Map JWK curve names to OpenSSL group names
566+
// Map JWK curve names to OpenSSL group names and field sizes
571567
const char* group_name;
568+
size_t field_size;
572569
if (crv == "P-256") {
573570
group_name = "prime256v1";
571+
field_size = 32;
574572
} else if (crv == "P-384") {
575573
group_name = "secp384r1";
574+
field_size = 48;
576575
} else if (crv == "P-521") {
577576
group_name = "secp521r1";
577+
field_size = 66;
578578
} else {
579579
throw std::runtime_error("Unsupported EC curve: " + crv);
580580
}
581581

582-
// Get field size from curve to properly pad coordinates
583-
int nid = OBJ_txt2nid(group_name);
584-
EC_GROUP* group = EC_GROUP_new_by_curve_name(nid);
585-
if (!group)
586-
throw std::runtime_error("Failed to create EC group");
587-
size_t field_size = (EC_GROUP_get_degree(group) + 7) / 8;
588-
EC_GROUP_free(group);
589-
590582
// Decode public key coordinates
591583
BIGNUM* x_bn = base64url_to_bn(keyData.x.value());
592584
BIGNUM* y_bn = base64url_to_bn(keyData.y.value());
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#include "QuickCryptoUtils.hpp"
2+
#include <openssl/bn.h>
3+
#include <openssl/core_names.h>
4+
#include <openssl/evp.h>
5+
#include <openssl/param_build.h>
6+
#include <stdexcept>
7+
8+
namespace margelo::nitro::crypto {
9+
10+
EVP_PKEY* createEcEvpPkey(const char* group_name, const uint8_t* pub_oct, size_t pub_len, const BIGNUM* priv_bn) {
11+
OSSL_PARAM_BLD* bld = OSSL_PARAM_BLD_new();
12+
if (!bld)
13+
throw std::runtime_error("Failed to create OSSL_PARAM_BLD");
14+
15+
OSSL_PARAM_BLD_push_utf8_string(bld, OSSL_PKEY_PARAM_GROUP_NAME, group_name, 0);
16+
OSSL_PARAM_BLD_push_octet_string(bld, OSSL_PKEY_PARAM_PUB_KEY, pub_oct, pub_len);
17+
if (priv_bn)
18+
OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY, priv_bn);
19+
20+
OSSL_PARAM* params = OSSL_PARAM_BLD_to_param(bld);
21+
OSSL_PARAM_BLD_free(bld);
22+
if (!params)
23+
throw std::runtime_error("Failed to build EC parameters");
24+
25+
EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new_from_name(nullptr, "EC", nullptr);
26+
if (!ctx) {
27+
OSSL_PARAM_free(params);
28+
throw std::runtime_error("Failed to create EVP_PKEY_CTX for EC");
29+
}
30+
31+
int selection = priv_bn ? EVP_PKEY_KEYPAIR : EVP_PKEY_PUBLIC_KEY;
32+
EVP_PKEY* pkey = nullptr;
33+
if (EVP_PKEY_fromdata_init(ctx) <= 0 || EVP_PKEY_fromdata(ctx, &pkey, selection, params) <= 0) {
34+
EVP_PKEY_CTX_free(ctx);
35+
OSSL_PARAM_free(params);
36+
throw std::runtime_error("Failed to create EVP_PKEY from EC parameters");
37+
}
38+
39+
EVP_PKEY_CTX_free(ctx);
40+
OSSL_PARAM_free(params);
41+
return pkey;
42+
}
43+
44+
} // namespace margelo::nitro::crypto

packages/react-native-quick-crypto/cpp/utils/QuickCryptoUtils.hpp

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44
#include <cctype>
55
#include <limits>
66
#include <openssl/bn.h>
7-
#include <openssl/core_names.h>
87
#include <openssl/err.h>
98
#include <openssl/evp.h>
10-
#include <openssl/param_build.h>
119
#include <string>
1210
#include <vector>
1311

@@ -111,38 +109,6 @@ inline const EVP_MD* getDigestByName(const std::string& algorithm) {
111109
// Build an EVP_PKEY from EC curve name + public key octets + optional private key BIGNUM.
112110
// Uses OSSL_PARAM_BLD + EVP_PKEY_fromdata (OpenSSL 3.x, no deprecated EC_KEY APIs).
113111
// Caller owns the returned EVP_PKEY*.
114-
inline EVP_PKEY* createEcEvpPkey(const char* group_name, const uint8_t* pub_oct, size_t pub_len, const BIGNUM* priv_bn = nullptr) {
115-
OSSL_PARAM_BLD* bld = OSSL_PARAM_BLD_new();
116-
if (!bld)
117-
throw std::runtime_error("Failed to create OSSL_PARAM_BLD");
118-
119-
OSSL_PARAM_BLD_push_utf8_string(bld, OSSL_PKEY_PARAM_GROUP_NAME, group_name, 0);
120-
OSSL_PARAM_BLD_push_octet_string(bld, OSSL_PKEY_PARAM_PUB_KEY, pub_oct, pub_len);
121-
if (priv_bn)
122-
OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY, priv_bn);
123-
124-
OSSL_PARAM* params = OSSL_PARAM_BLD_to_param(bld);
125-
OSSL_PARAM_BLD_free(bld);
126-
if (!params)
127-
throw std::runtime_error("Failed to build EC parameters");
128-
129-
EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new_from_name(nullptr, "EC", nullptr);
130-
if (!ctx) {
131-
OSSL_PARAM_free(params);
132-
throw std::runtime_error("Failed to create EVP_PKEY_CTX for EC");
133-
}
134-
135-
int selection = priv_bn ? EVP_PKEY_KEYPAIR : EVP_PKEY_PUBLIC_KEY;
136-
EVP_PKEY* pkey = nullptr;
137-
if (EVP_PKEY_fromdata_init(ctx) <= 0 || EVP_PKEY_fromdata(ctx, &pkey, selection, params) <= 0) {
138-
EVP_PKEY_CTX_free(ctx);
139-
OSSL_PARAM_free(params);
140-
throw std::runtime_error("Failed to create EVP_PKEY from EC parameters");
141-
}
142-
143-
EVP_PKEY_CTX_free(ctx);
144-
OSSL_PARAM_free(params);
145-
return pkey;
146-
}
112+
EVP_PKEY* createEcEvpPkey(const char* group_name, const uint8_t* pub_oct, size_t pub_len, const BIGNUM* priv_bn = nullptr);
147113

148114
} // namespace margelo::nitro::crypto

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { NitroModules } from 'react-native-nitro-modules';
33
import type { Hkdf as HkdfNative } from './specs/hkdf.nitro';
44
import { binaryLikeToArrayBuffer, normalizeHashName } from './utils';
55
import type { BinaryLike } from './utils';
6+
import type { CryptoKey } from './keys';
67

78
type KeyMaterial = BinaryLike;
89
type Salt = BinaryLike;
@@ -15,13 +16,6 @@ export interface HkdfAlgorithm {
1516
info: BinaryLike;
1617
}
1718

18-
export interface CryptoKeyInternal {
19-
keyObject: {
20-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
21-
export: (...args: any[]) => any;
22-
};
23-
}
24-
2519
export interface HkdfCallback {
2620
(err: Error | null, derivedKey?: Buffer): void;
2721
}
@@ -123,7 +117,7 @@ export function hkdfSync(
123117

124118
export function hkdfDeriveBits(
125119
algorithm: HkdfAlgorithm,
126-
baseKey: CryptoKeyInternal,
120+
baseKey: CryptoKey,
127121
length: number,
128122
): ArrayBuffer {
129123
const hash = algorithm.hash;

0 commit comments

Comments
 (0)