feat: Add DH and ECDH support

[athexweb3]

feat: Add DH and ECDH support with benchmarks

Description

This PR implements full support for Diffie-Hellman (DH) and Elliptic Curve Diffie-Hellman (ECDH) key exchanges in react-native-quick-crypto, complete with C++ native implementations, benchmarks, and comprehensive tests.

Changes

Core Implementation

• Diffie-Hellman (DH):

  • Implemented DiffieHellman class mirroring the Node.js API (createDiffieHellman, getDiffieHellman).
  • Added native C++ logic using OpenSSL EVP_PKEY for key generation and shared secret derivation.
  • Supports standard MODP groups (modp14, modp15, etc.) as well as custom prime/generator inputs.

• Elliptic Curve Diffie-Hellman (ECDH):

  • Implemented ECDH class mirroring the Node.js API (createECDH).
  • Added native C++ logic using OpenSSL EC_KEY and EVP_PKEY.
  • Supports standard curves such as secp256k1, prime256v1, and others.

Benchmarks & Tests

• Benchmarks:

  • Added dedicated benchmark suites for DH and ECDH.
  • Measures key generation and shared secret derivation performance.
  • Compared against crypto-browserify (where applicable) and native baselines.

• Tests:

  • Added comprehensive test coverage validating:

    • Key generation (sync and async).
    • Shared secret derivation consistency with Node.js.
    • Proper error handling for invalid keys and groups.
    • Interoperability between multiple instances.

Verification

• iOS: Builds and runs successfully.
• Tests: All new DH and ECDH tests pass.
• Benchmarks: Benchmarks run successfully and demonstrate native performance improvements.
• Linting: No lint or formatting issues remaining.

Checklist

• Implementation matches the Node.js crypto API.
• Native C++ implementation works on both iOS and Android.
• Benchmarks added and verifiable.
• Unit tests pass.
• Rebased on latest main (synced with release 1.0.6).

[dbalatoni13]

Looking forward to this

[athexweb3]

Brad bro is busy currently. hope he will review this soon.

[boorad]

1. Left some comments.
2. There are conflicts to resolve
3. There's only one commit that should be in this PR. Remove the other docs-related commits.

[boorad]

Code Review: DH and ECDH Support (PR #862)

Commit: 479ac2245f6e5595d462f790b59fd2e5a6146c01
Repository: margelo/react-native-quick-crypto

Overview

This PR adds Diffie-Hellman (DH) and Elliptic Curve Diffie-Hellman (ECDH) cryptographic support to react-native-quick-crypto. The implementation includes C++ native modules using OpenSSL, TypeScript API wrappers, tests, and benchmarks.

Stats: ~12,300 lines added across 144 files

---

Summary

Aspect	Rating	Notes
Functionality	✅ Good	Complete DH/ECDH API matching Node.js
Security	⚠️ Concerns	Deprecated APIs, missing validations
Code Quality	⚠️ Mixed	Smart pointers inconsistent, memory management issues
Testing	✅ Good	Comprehensive test coverage
Performance	✅ Good	Benchmarks included

---

Critical Issues

1. Use of Deprecated OpenSSL APIs

Files: HybridDiffieHellman.cpp, HybridECDH.cpp

Both files suppress deprecation warnings:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"

The code uses deprecated low-level APIs (DH_new, EC_KEY_new, EVP_PKEY_assign_DH, etc.) that are deprecated in OpenSSL 3.x. Per project guidelines, OpenSSL 3.6+ EVP APIs only should be used.

Recommendation: Migrate to modern EVP-based parameter/key generation using OSSL_PARAM builders and EVP_PKEY_fromdata().

2. Inconsistent Memory Management

File: HybridDiffieHellman.cpp

Raw pointer management with manual EVP_PKEY_free() calls:

EVP_PKEY* _pkey = nullptr;  // Raw pointer as member

// Manual cleanup scattered throughout
if (_pkey) {
    EVP_PKEY_free(_pkey);
    _pkey = nullptr;
}

File: HybridECDH.cpp

Mixed approach - uses std::unique_ptr for locals but raw pointer for member:

std::unique_ptr<EVP_PKEY_CTX, decltype(&EVP_PKEY_CTX_free)> ctx(...);  // Good
EVP_PKEY* _pkey = nullptr;  // Raw pointer member - inconsistent

Recommendation: Use smart pointers consistently for all OpenSSL resources. Consider a wrapper type or alias:

using EVP_PKEY_ptr = std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>;

3. Missing Input Validation

File: HybridDiffieHellman.cpp:60-62

No validation on prime length:

void HybridDiffieHellman::initWithSize(double primeLength, double generator) {
    // No minimum size check - allows weak parameters
    if (EVP_PKEY_CTX_set_dh_paramgen_prime_len(pctx, (int)primeLength) <= 0)

Recommendation: Enforce minimum key sizes (e.g., 2048 bits for DH) per security guidelines.

4. Potential Memory Leak in Error Path

File: HybridDiffieHellman.cpp:136-141

DH_set0_pqg(peer_dh, peer_p, nullptr, peer_g);
DH_set0_key(peer_dh, peer_pub_key, nullptr);

EVP_PKEY* peer_pkey = EVP_PKEY_new();
EVP_PKEY_assign_DH(peer_pkey, peer_dh);

If EVP_PKEY_assign_DH fails, peer_pkey is leaked. No error check after EVP_PKEY_new().

Recommendation: Add error checking and use RAII patterns:

if (!peer_pkey) {
    DH_free(peer_dh);
    throw std::runtime_error("Failed to create peer EVP_PKEY");
}

---

Medium Issues

5. Double Conversion Through String

Files: HybridDiffieHellman.cpp, HybridECDH.cpp

return ToNativeArrayBuffer(std::string(secret.begin(), secret.end()));

Creating an intermediate std::string from binary data is inefficient and semantically incorrect (strings are for text).

Recommendation: Create a ToNativeArrayBuffer overload accepting std::vector<uint8_t> or uint8_t* + size directly.

6. Type Coercion from Double

File: HybridDiffieHellman.cpp:60

void HybridDiffieHellman::initWithSize(double primeLength, double generator)

Using double for integer parameters (prime length, generator) is unusual. Likely a Nitro Modules limitation, but the cast to int should be explicit and validated:

if (primeLength < 0 || primeLength > INT_MAX)
    throw std::runtime_error("Invalid prime length");

7. Commented-Out Code

File: HybridECDH.cpp:65-68

// Use cached group
// std::unique_ptr<EC_GROUP, decltype(&EC_GROUP_free)> group(EC_GROUP_new_by_curve_name(_curveNid), EC_GROUP_free);
// if (!group)
//   throw std::runtime_error("Failed to create group");

Recommendation: Remove commented code; use git history for reference.

8. Missing const Correctness

File: HybridECDH.hpp

std::shared_ptr<ArrayBuffer> getPrivateKey();
std::shared_ptr<ArrayBuffer> getPublicKey();

Getter methods should be const:

std::shared_ptr<ArrayBuffer> getPrivateKey() const;
std::shared_ptr<ArrayBuffer> getPublicKey() const;

---

Minor Issues

9. Inconsistent Error Messages

File: HybridDiffieHellman.cpp

Error messages vary in style:

• "Failed to create DH"
• "Not a DH key"
• "No prime"

Recommendation: Use consistent, descriptive error messages: "DiffieHellman: failed to create DH structure".

10. Magic Numbers in Tests

File: dh_tests.ts:8

const dh = crypto.createDiffieHellman(512);

512-bit DH is cryptographically weak. While acceptable for testing, consider using 2048-bit or adding a comment explaining this is intentional for test speed.

11. Benchmark Fairness

File: ecdh.ts:18-20

.add('@noble/curves', () => {
    p256.utils.randomPrivateKey();
});

This only generates a private key, while the rnqc benchmark creates an ECDH object AND generates keys. Not a fair comparison.

---

Positive Aspects

1. Good test coverage - Tests cover key generation, secret computation, key setting/getting, and standard groups
2. Benchmarks included - Performance comparisons against crypto-browserify and @noble/curves
3. API compatibility - Follows Node.js crypto API conventions
4. TypeScript typing - Proper Buffer/string overloads with encoding support
5. Build system updates - Both iOS (podspec) and Android (CMakeLists) properly configured

---

Recommendations Summary

Priority	Action
🔴 High	Migrate from deprecated OpenSSL APIs to EVP + OSSL_PARAM
🔴 High	Add minimum key size validation (2048-bit DH minimum)
🟡 Medium	Use smart pointers consistently for all OpenSSL resources
🟡 Medium	Fix potential memory leaks in error paths
🟡 Medium	Remove string intermediate in ArrayBuffer conversion
🟢 Low	Remove commented-out code
🟢 Low	Add const to getter methods
🟢 Low	Standardize error messages

---

Files Reviewed

• cpp/dh/HybridDiffieHellman.cpp (293 lines)
• cpp/dh/HybridDiffieHellman.hpp (42 lines)
• cpp/ecdh/HybridECDH.cpp (222 lines)
• cpp/ecdh/HybridECDH.hpp (49 lines)
• src/diffie-hellman.ts (191 lines)
• example/src/tests/dh/dh_tests.ts (76 lines)
• example/src/tests/ecdh/ecdh_tests.ts (75 lines)
• example/src/benchmarks/dh/dh.ts (59 lines)
• example/src/benchmarks/ecdh/ecdh.ts (55 lines)
• Build configuration files (podspec, CMakeLists.txt)

[boorad]

Also, I don't see the following from .docs/implementation-coverage.md:

• createDiffieHellman()
• createDiffieHellmanGroup()
• createECDH()

[athexweb3]

Hey! I’m a bit busy at the moment, but I’ll take a look when I have some free time. Thanks for the review.

[boorad]

Re: QuickCrypto.podspec header paths - The cpp/dh and cpp/ecdh paths follow the same pattern as cpp/utils and cpp/hkdf that were already in the list. These paths are needed because QuickCryptoAutolinking.mm includes the headers with bare names (e.g., #include "HybridDiffieHellman.hpp").