Skip to content

Commit a0cc8cc

Browse files
committed
refactor: address code review issues for DSA/DH key generation
- Convert HybridDsaKeyPair from raw EVP_PKEY* to RAII unique_ptr - Remove dead setGroupName/groupName_ from DH Nitro spec and C++ - Replace duplicate DH test with primeLength coverage - Simplify always-true ternaries in DSA/DH format helpers - Delegate DSA modulusLength validation to OpenSSL
1 parent 8c2e828 commit a0cc8cc

10 files changed

Lines changed: 20 additions & 58 deletions

File tree

example/src/tests/keys/generate_keypair.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -783,15 +783,15 @@ test(SUITE, 'generateKeyPair DH with named group modp14', async () => {
783783
expect(publicKey).to.match(/^-----BEGIN PUBLIC KEY-----/);
784784
});
785785

786-
test(SUITE, 'generateKeyPair DH with PEM encoding', async () => {
786+
test(SUITE, 'generateKeyPair DH with primeLength', async () => {
787787
const { privateKey, publicKey } = await new Promise<{
788788
privateKey: string;
789789
publicKey: string;
790790
}>((resolve, reject) => {
791791
generateKeyPair(
792792
'dh',
793793
{
794-
groupName: 'modp14',
794+
primeLength: 2048,
795795
publicKeyEncoding: { type: 'spki', format: 'pem' },
796796
privateKeyEncoding: { type: 'pkcs8', format: 'pem' },
797797
},

packages/react-native-quick-crypto/cpp/dh/HybridDhKeyPair.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ void HybridDhKeyPair::setGenerator(double generator) {
3636
generator_ = static_cast<int>(generator);
3737
}
3838

39-
void HybridDhKeyPair::setGroupName(const std::string& groupName) {
40-
groupName_ = groupName;
41-
}
42-
4339
std::shared_ptr<Promise<void>> HybridDhKeyPair::generateKeyPair() {
4440
return Promise<void>::async([this]() { this->generateKeyPairSync(); });
4541
}
@@ -109,7 +105,7 @@ void HybridDhKeyPair::generateKeyPairSync() {
109105
throw std::runtime_error("DH: failed to generate parameters");
110106
}
111107
} else {
112-
throw std::runtime_error("DH: either prime, primeLength, or groupName must be set");
108+
throw std::runtime_error("DH: either prime or primeLength must be set");
113109
}
114110

115111
std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)> params_guard(params, EVP_PKEY_free);

packages/react-native-quick-crypto/cpp/dh/HybridDhKeyPair.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,13 @@ class HybridDhKeyPair : public HybridDhKeyPairSpec {
2222
void setPrimeLength(double primeLength) override;
2323
void setPrime(const std::shared_ptr<ArrayBuffer>& prime) override;
2424
void setGenerator(double generator) override;
25-
void setGroupName(const std::string& groupName) override;
2625
std::shared_ptr<ArrayBuffer> getPublicKey() override;
2726
std::shared_ptr<ArrayBuffer> getPrivateKey() override;
2827

2928
private:
3029
int primeLength_ = 0;
3130
std::vector<uint8_t> prime_;
3231
int generator_ = 2;
33-
std::string groupName_;
3432

3533
using EVP_PKEY_ptr = std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>;
3634
EVP_PKEY_ptr pkey_{nullptr, EVP_PKEY_free};

packages/react-native-quick-crypto/cpp/dsa/HybridDsaKeyPair.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ void HybridDsaKeyPair::generateKeyPairSync() {
3030
throw std::runtime_error("DSA modulusLength must be set before generating key pair");
3131
}
3232

33-
if (pkey != nullptr) {
34-
EVP_PKEY_free(pkey);
35-
pkey = nullptr;
36-
}
33+
pkey_.reset();
3734

3835
// Step 1: Generate DSA parameters
3936
std::unique_ptr<EVP_PKEY_CTX, decltype(&EVP_PKEY_CTX_free)> param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DSA, nullptr), EVP_PKEY_CTX_free);
@@ -79,11 +76,11 @@ void HybridDsaKeyPair::generateKeyPairSync() {
7976
throw std::runtime_error("DSA: failed to generate key pair");
8077
}
8178

82-
pkey = raw_pkey;
79+
pkey_.reset(raw_pkey);
8380
}
8481

8582
std::shared_ptr<ArrayBuffer> HybridDsaKeyPair::getPublicKey() {
86-
if (pkey == nullptr) {
83+
if (!pkey_) {
8784
throw std::runtime_error("DSA: no key pair generated");
8885
}
8986

@@ -92,7 +89,7 @@ std::shared_ptr<ArrayBuffer> HybridDsaKeyPair::getPublicKey() {
9289
throw std::runtime_error("DSA: failed to create BIO for public key export");
9390
}
9491

95-
if (i2d_PUBKEY_bio(bio, pkey) != 1) {
92+
if (i2d_PUBKEY_bio(bio, pkey_.get()) != 1) {
9693
BIO_free(bio);
9794
throw std::runtime_error("DSA: failed to export public key");
9895
}
@@ -106,7 +103,7 @@ std::shared_ptr<ArrayBuffer> HybridDsaKeyPair::getPublicKey() {
106103
}
107104

108105
std::shared_ptr<ArrayBuffer> HybridDsaKeyPair::getPrivateKey() {
109-
if (pkey == nullptr) {
106+
if (!pkey_) {
110107
throw std::runtime_error("DSA: no key pair generated");
111108
}
112109

@@ -115,7 +112,7 @@ std::shared_ptr<ArrayBuffer> HybridDsaKeyPair::getPrivateKey() {
115112
throw std::runtime_error("DSA: failed to create BIO for private key export");
116113
}
117114

118-
if (i2d_PKCS8PrivateKey_bio(bio, pkey, nullptr, nullptr, 0, nullptr, nullptr) != 1) {
115+
if (i2d_PKCS8PrivateKey_bio(bio, pkey_.get(), nullptr, nullptr, 0, nullptr, nullptr) != 1) {
119116
BIO_free(bio);
120117
throw std::runtime_error("DSA: failed to export private key");
121118
}

packages/react-native-quick-crypto/cpp/dsa/HybridDsaKeyPair.hpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,7 @@ namespace margelo::nitro::crypto {
1111
class HybridDsaKeyPair : public HybridDsaKeyPairSpec {
1212
public:
1313
HybridDsaKeyPair() : HybridObject(TAG) {}
14-
~HybridDsaKeyPair() override {
15-
if (pkey != nullptr) {
16-
EVP_PKEY_free(pkey);
17-
pkey = nullptr;
18-
}
19-
}
14+
~HybridDsaKeyPair() override = default;
2015

2116
public:
2217
std::shared_ptr<Promise<void>> generateKeyPair() override;
@@ -29,7 +24,9 @@ class HybridDsaKeyPair : public HybridDsaKeyPairSpec {
2924
private:
3025
int modulusLength_ = 0;
3126
int divisorLength_ = -1;
32-
EVP_PKEY* pkey = nullptr;
27+
28+
using EVP_PKEY_ptr = std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>;
29+
EVP_PKEY_ptr pkey_{nullptr, EVP_PKEY_free};
3330
};
3431

3532
} // namespace margelo::nitro::crypto

packages/react-native-quick-crypto/nitrogen/generated/shared/c++/HybridDhKeyPairSpec.cpp

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react-native-quick-crypto/nitrogen/generated/shared/c++/HybridDhKeyPairSpec.hpp

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,7 @@ function dh_formatKeyPairOutput(
7575
publicKey: PublicKeyObject | Buffer | string | ArrayBuffer;
7676
privateKey: PrivateKeyObject | Buffer | string | ArrayBuffer;
7777
} {
78-
const {
79-
publicFormat,
80-
publicType,
81-
privateFormat,
82-
privateType,
83-
cipher,
84-
passphrase,
85-
} = encoding;
78+
const { publicFormat, privateFormat, cipher, passphrase } = encoding;
8679

8780
const publicKeyData = dh.native.getPublicKey();
8881
const privateKeyData = dh.native.getPrivateKey();
@@ -109,9 +102,7 @@ function dh_formatKeyPairOutput(
109102
} else {
110103
const format =
111104
publicFormat === KFormatType.PEM ? KFormatType.PEM : KFormatType.DER;
112-
const keyEncoding =
113-
publicType === KeyEncoding.SPKI ? KeyEncoding.SPKI : KeyEncoding.SPKI;
114-
const exported = pub.handle.exportKey(format, keyEncoding);
105+
const exported = pub.handle.exportKey(format, KeyEncoding.SPKI);
115106
if (format === KFormatType.PEM) {
116107
publicKey = Buffer.from(new Uint8Array(exported)).toString('utf-8');
117108
} else {
@@ -124,11 +115,9 @@ function dh_formatKeyPairOutput(
124115
} else {
125116
const format =
126117
privateFormat === KFormatType.PEM ? KFormatType.PEM : KFormatType.DER;
127-
const keyEncoding =
128-
privateType === KeyEncoding.PKCS8 ? KeyEncoding.PKCS8 : KeyEncoding.PKCS8;
129118
const exported = priv.handle.exportKey(
130119
format,
131-
keyEncoding,
120+
KeyEncoding.PKCS8,
132121
cipher,
133122
passphrase,
134123
);

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ function dsa_prepareKeyGenParams(
3434

3535
const { modulusLength, divisorLength } = options;
3636

37-
if (!modulusLength || modulusLength < 1024) {
37+
if (!modulusLength || modulusLength <= 0) {
3838
throw new Error('Invalid or missing modulusLength for DSA key generation');
3939
}
4040

@@ -48,14 +48,7 @@ function dsa_formatKeyPairOutput(
4848
publicKey: PublicKeyObject | Buffer | string | ArrayBuffer;
4949
privateKey: PrivateKeyObject | Buffer | string | ArrayBuffer;
5050
} {
51-
const {
52-
publicFormat,
53-
publicType,
54-
privateFormat,
55-
privateType,
56-
cipher,
57-
passphrase,
58-
} = encoding;
51+
const { publicFormat, privateFormat, cipher, passphrase } = encoding;
5952

6053
const publicKeyData = dsa.native.getPublicKey();
6154
const privateKeyData = dsa.native.getPrivateKey();
@@ -82,9 +75,7 @@ function dsa_formatKeyPairOutput(
8275
} else {
8376
const format =
8477
publicFormat === KFormatType.PEM ? KFormatType.PEM : KFormatType.DER;
85-
const keyEncoding =
86-
publicType === KeyEncoding.SPKI ? KeyEncoding.SPKI : KeyEncoding.SPKI;
87-
const exported = pub.handle.exportKey(format, keyEncoding);
78+
const exported = pub.handle.exportKey(format, KeyEncoding.SPKI);
8879
if (format === KFormatType.PEM) {
8980
publicKey = Buffer.from(new Uint8Array(exported)).toString('utf-8');
9081
} else {
@@ -97,11 +88,9 @@ function dsa_formatKeyPairOutput(
9788
} else {
9889
const format =
9990
privateFormat === KFormatType.PEM ? KFormatType.PEM : KFormatType.DER;
100-
const keyEncoding =
101-
privateType === KeyEncoding.PKCS8 ? KeyEncoding.PKCS8 : KeyEncoding.PKCS8;
10291
const exported = priv.handle.exportKey(
10392
format,
104-
keyEncoding,
93+
KeyEncoding.PKCS8,
10594
cipher,
10695
passphrase,
10796
);

packages/react-native-quick-crypto/src/specs/dhKeyPair.nitro.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ export interface DhKeyPair
88
setPrimeLength(primeLength: number): void;
99
setPrime(prime: ArrayBuffer): void;
1010
setGenerator(generator: number): void;
11-
setGroupName(groupName: string): void;
1211

1312
getPublicKey(): ArrayBuffer;
1413
getPrivateKey(): ArrayBuffer;

0 commit comments

Comments
 (0)