Skip to content

Commit

Permalink
crypto: refactor crypto subsystem and introduce WebCrypto API
Browse files Browse the repository at this point in the history 
nodejs/node#35093
  • Loading branch information
@codebytere
codebytere committed May 20, 2021
1 parent cad014e commit df61546
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 148 deletions.
116 changes: 40 additions & 76 deletions patches/node/fix_comment_out_incompatible_crypto_modules.patch
View file
Expand Up @@ -8,86 +8,50 @@ and https://github.com/nodejs/node/pull/31178 that is not currently compatible
with what's exposed through BoringSSL. I plan to upstream parts of this or
otherwise introduce shims to reduce friction.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index c119b2314f18d1710bb3cbf1910c86ff994ec951..58554799b50097972405e40f593d089236bca961 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5207,11 +5207,11 @@ bool DiffieHellman::Init(int primeLength, int g) {
bool DiffieHellman::Init(const char* p, int p_len, int g) {
dh_.reset(DH_new());
if (p_len <= 0) {
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
return false;
}
if (g <= 1) {
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
return false;
}
BIGNUM* bn_p =
@@ -5230,18 +5230,18 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
dh_.reset(DH_new());
if (p_len <= 0) {
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
return false;
}
if (g_len <= 0) {
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
return false;
}
BIGNUM* bn_g =
BN_bin2bn(reinterpret_cast<const unsigned char*>(g), g_len, nullptr);
if (BN_is_zero(bn_g) || BN_is_one(bn_g)) {
BN_free(bn_g);
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
return false;
diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc
index 1c48f98656fd211403354bb88331450e51ffb3e5..5c50976b663db560d758c7bb6f97d3e35d5dcb55 100644
--- a/src/crypto/crypto_dh.cc
+++ b/src/crypto/crypto_dh.cc
@@ -469,6 +469,7 @@ Maybe<bool> DhKeyGenTraits::AdditionalConfig(
EVPKeyCtxPointer DhKeyGenTraits::Setup(DhKeyPairGenConfig* params) {
EVPKeyPointer key_params;
if (params->params.prime_fixed_value) {
+#if 0
DHPointer dh(DH_new());
if (!dh)
return EVPKeyCtxPointer();
@@ -485,9 +486,11 @@ EVPKeyCtxPointer DhKeyGenTraits::Setup(DhKeyPairGenConfig* params) {
key_params = EVPKeyPointer(EVP_PKEY_new());
CHECK(key_params);
EVP_PKEY_assign_DH(key_params.get(), dh.release());
+#endif
} else {
EVPKeyCtxPointer param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
EVP_PKEY* raw_params = nullptr;
+#if 0
if (!param_ctx ||
EVP_PKEY_paramgen_init(param_ctx.get()) <= 0 ||
EVP_PKEY_CTX_set_dh_paramgen_prime_len(
@@ -499,7 +502,7 @@ EVPKeyCtxPointer DhKeyGenTraits::Setup(DhKeyPairGenConfig* params) {
EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0) {
return EVPKeyCtxPointer();
}
-
+#endif
key_params = EVPKeyPointer(raw_params);
}
BIGNUM* bn_p =
@@ -5734,7 +5734,7 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
if (!EC_KEY_set_public_key(new_key.get(), pub.get()))
return env->ThrowError("Failed to set generated public key");

diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
index ea4c70ad5d8c844860ba3480fc7ef4205f0a3cdc..cdf8dd47d6e2a5894066cec01fbe347af079ec22 100644
--- a/src/crypto/crypto_ec.cc
+++ b/src/crypto/crypto_ec.cc
@@ -314,7 +314,7 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
return THROW_ERR_CRYPTO_OPERATION_FAILED(env,
"Failed to set generated public key");

- EC_KEY_copy(ecdh->key_.get(), new_key.get());
+ ecdh->key_.reset(EC_KEY_dup(new_key.get()));
ecdh->group_ = EC_KEY_get0_group(ecdh->key_.get());
}

@@ -6222,6 +6222,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
EVPKeyCtxPointer Setup() override {
EVPKeyPointer params;
if (prime_info_.fixed_value_) {
+#if 0
DHPointer dh(DH_new());
if (!dh)
return nullptr;
@@ -6238,6 +6239,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
params = EVPKeyPointer(EVP_PKEY_new());
CHECK(params);
EVP_PKEY_assign_DH(params.get(), dh.release());
+#endif
} else {
EVPKeyCtxPointer param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
if (!param_ctx)
@@ -6245,7 +6247,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {

if (EVP_PKEY_paramgen_init(param_ctx.get()) <= 0)
return nullptr;
-
+#if 0
if (EVP_PKEY_CTX_set_dh_paramgen_prime_len(param_ctx.get(),
prime_info_.prime_size_) <= 0)
return nullptr;
@@ -6253,7 +6255,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
if (EVP_PKEY_CTX_set_dh_paramgen_generator(param_ctx.get(),
generator_) <= 0)
return nullptr;
-
+#endif
EVP_PKEY* raw_params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0)
return nullptr;

125 changes: 62 additions & 63 deletions patches/node/fix_key_gen_apis_are_not_available_in_boringssl.patch
View file
Expand Up @@ -5,72 +5,20 @@ Subject: fix: key gen APIs are not available in BoringSSL

This will make Node's key pair generation APIs fail.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index bd40705e6b1ae0927239a06c40a2181e4458b1c4..79e781fb3e6ec63334c2c5d4b24d2a6049be79fc 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -291,24 +291,14 @@ Maybe<bool> Decorate(Environment* env, Local<Object> obj,
V(BIO) \
V(PKCS7) \
V(X509V3) \
- V(PKCS12) \
V(RAND) \
- V(DSO) \
V(ENGINE) \
V(OCSP) \
V(UI) \
V(COMP) \
V(ECDSA) \
V(ECDH) \
- V(OSSL_STORE) \
- V(FIPS) \
- V(CMS) \
- V(TS) \
V(HMAC) \
- V(CT) \
- V(ASYNC) \
- V(KDF) \
- V(SM2) \
V(USER) \

#define V(name) case ERR_LIB_##name: lib = #name "_"; break;
@@ -6138,6 +6128,7 @@ class DSAKeyPairGenerationConfig : public KeyPairGenerationConfig {
if (EVP_PKEY_paramgen_init(param_ctx.get()) <= 0)
return nullptr;

+#ifndef OPENSSL_IS_BORINGSSL
if (EVP_PKEY_CTX_set_dsa_paramgen_bits(param_ctx.get(), modulus_bits_) <= 0)
return nullptr;

@@ -6148,6 +6139,7 @@ class DSAKeyPairGenerationConfig : public KeyPairGenerationConfig {
return nullptr;
}
}
+#endif

EVP_PKEY* raw_params = nullptr;
if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0)
diff --git a/src/node_crypto_common.cc b/src/node_crypto_common.cc
index 6473b652ac95609aff555d99be38b48a5aa513a5..caaaf19dc02101c2024b511780c94fc85476b7a2 100644
--- a/src/node_crypto_common.cc
+++ b/src/node_crypto_common.cc
@@ -240,10 +240,10 @@ int UseSNIContext(const SSLPointer& ssl, BaseObjectPtr<SecureContext> context) {
}

const char* GetClientHelloALPN(const SSLPointer& ssl) {
+#ifndef OPENSSL_IS_BORINGSSL
diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc
index f4b7bd3ad8548a0b69943ddea669e6f1991b7a49..221d652fa7de246e5f69fcf392e334087bac0199 100644
--- a/src/crypto/crypto_common.cc
+++ b/src/crypto/crypto_common.cc
@@ -242,7 +242,7 @@ const char* GetClientHelloALPN(const SSLPointer& ssl) {
const unsigned char* buf;
size_t len;
size_t rem;
-
+#ifndef OPENSSL_IS_BORINGSSL
if (!SSL_client_hello_get0_ext(
ssl.get(),
TLSEXT_TYPE_application_layer_protocol_negotiation,
@@ -252,17 +252,18 @@ const char* GetClientHelloALPN(const SSLPointer& ssl) {
rem < 2) {
return nullptr;
}
-
@@ -255,13 +255,15 @@ const char* GetClientHelloALPN(const SSLPointer& ssl) {
len = (buf[0] << 8) | buf[1];
if (len + 2 != rem) return nullptr;
return reinterpret_cast<const char*>(buf + 3);
Expand All @@ -79,15 +27,15 @@ index 6473b652ac95609aff555d99be38b48a5aa513a5..caaaf19dc02101c2024b511780c94fc8
}

const char* GetClientHelloServerName(const SSLPointer& ssl) {
+#ifndef OPENSSL_IS_BORINGSSL
const unsigned char* buf;
size_t len;
size_t rem;
-
+#ifndef OPENSSL_IS_BORINGSSL
if (!SSL_client_hello_get0_ext(
ssl.get(),
TLSEXT_TYPE_server_name,
@@ -284,6 +285,8 @@ const char* GetClientHelloServerName(const SSLPointer& ssl) {
@@ -283,6 +285,8 @@ const char* GetClientHelloServerName(const SSLPointer& ssl) {
if (len + 2 > rem)
return nullptr;
return reinterpret_cast<const char*>(buf + 5);
Expand All @@ -96,7 +44,7 @@ index 6473b652ac95609aff555d99be38b48a5aa513a5..caaaf19dc02101c2024b511780c94fc8
}

const char* GetServerName(SSL* ssl) {
@@ -291,7 +294,10 @@ const char* GetServerName(SSL* ssl) {
@@ -290,7 +294,10 @@ const char* GetServerName(SSL* ssl) {
}

bool SetGroups(SecureContext* sc, const char* groups) {
Expand All @@ -107,7 +55,7 @@ index 6473b652ac95609aff555d99be38b48a5aa513a5..caaaf19dc02101c2024b511780c94fc8
}

const char* X509ErrorCode(long err) { // NOLINT(runtime/int)
@@ -768,13 +774,13 @@ MaybeLocal<Array> GetClientHelloCiphers(
@@ -757,13 +764,13 @@ MaybeLocal<Array> GetClientHelloCiphers(
Environment* env,
const SSLPointer& ssl) {
EscapableHandleScope scope(env->isolate());
Expand All @@ -124,3 +72,54 @@ index 6473b652ac95609aff555d99be38b48a5aa513a5..caaaf19dc02101c2024b511780c94fc8
buf += 2;
Local<Object> obj = Object::New(env->isolate());
if (!Set(env->context(),
diff --git a/src/crypto/crypto_dsa.cc b/src/crypto/crypto_dsa.cc
index 271db427fa8539feb30c1712574976fb1f623e91..05ec6103007b0c687c1c713c44f5e00f1f2f33c3 100644
--- a/src/crypto/crypto_dsa.cc
+++ b/src/crypto/crypto_dsa.cc
@@ -29,7 +29,7 @@ namespace crypto {
EVPKeyCtxPointer DsaKeyGenTraits::Setup(DsaKeyPairGenConfig* params) {
EVPKeyCtxPointer param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DSA, nullptr));
EVP_PKEY* raw_params = nullptr;
-
+#ifndef OPENSSL_IS_BORINGSSL
if (!param_ctx ||
EVP_PKEY_paramgen_init(param_ctx.get()) <= 0 ||
EVP_PKEY_CTX_set_dsa_paramgen_bits(
@@ -49,7 +49,7 @@ EVPKeyCtxPointer DsaKeyGenTraits::Setup(DsaKeyPairGenConfig* params) {
return EVPKeyCtxPointer();
}
}
-
+#endif
if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0)
return EVPKeyCtxPointer();

diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index 0d533ce42531d147a4c99fef5a72c311e0796150..da66cde877c6ce045d907205bb2a6e369234fb8e 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -488,24 +488,14 @@ Maybe<bool> Decorate(Environment* env, Local<Object> obj,
V(BIO) \
V(PKCS7) \
V(X509V3) \
- V(PKCS12) \
V(RAND) \
- V(DSO) \
V(ENGINE) \
V(OCSP) \
V(UI) \
V(COMP) \
V(ECDSA) \
V(ECDH) \
- V(OSSL_STORE) \
- V(FIPS) \
- V(CMS) \
- V(TS) \
V(HMAC) \
- V(CT) \
- V(ASYNC) \
- V(KDF) \
- V(SM2) \
V(USER) \

#define V(name) case ERR_LIB_##name: lib = #name "_"; break;
22 changes: 13 additions & 9 deletions patches/node/fix_use_crypto_impls_for_compat.patch
View file
Expand Up @@ -16,20 +16,24 @@ We can revisit this once that happens and determine a more mutually
compatible path forward either by upstreaming a shim to BoringSSL or
adapting Node.js.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 79e781fb3e6ec63334c2c5d4b24d2a6049be79fc..c119b2314f18d1710bb3cbf1910c86ff994ec951 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -4574,7 +4574,7 @@ static unsigned int GetBytesOfRS(const ManagedEVPPKey& pkey) {
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
index 7b113a8dcb06b0b0e1329ce0daf7305598ea6545..b04e53a7f24885ffb6639430988d0ffb524b028e 100644
--- a/src/crypto/crypto_sig.cc
+++ b/src/crypto/crypto_sig.cc
@@ -110,7 +110,7 @@ unsigned int GetBytesOfRS(const ManagedEVPPKey& pkey) {
if (base_id == EVP_PKEY_DSA) {
DSA* dsa_key = EVP_PKEY_get0_DSA(pkey.get());
const DSA* dsa_key = EVP_PKEY_get0_DSA(pkey.get());
// Both r and s are computed mod q, so their width is limited by that of q.
- bits = BN_num_bits(DSA_get0_q(dsa_key));
+ bits = BN_num_bits(dsa_key->q);
} else if (base_id == EVP_PKEY_EC) {
EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(pkey.get());
const EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(pkey.get());
const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key);
@@ -6949,7 +6949,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index da66cde877c6ce045d907205bb2a6e369234fb8e..fbe4c0f06c10df7c8a492711594381c27c6f81f7 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -655,7 +655,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsUint32());
Environment* env = Environment::GetCurrent(args);
uint32_t len = args[0].As<Uint32>()->Value();
Expand All @@ -38,7 +42,7 @@ index 79e781fb3e6ec63334c2c5d4b24d2a6049be79fc..c119b2314f18d1710bb3cbf1910c86ff
if (data == nullptr) {
// There's no memory available for the allocation.
// Return nothing.
@@ -6961,7 +6961,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
@@ -667,7 +667,7 @@ void SecureBuffer(const FunctionCallbackInfo<Value>& args) {
data,
len,
[](void* data, size_t len, void* deleter_data) {
Expand Down

0 comments on commit df61546

Please sign in to comment.