From 62a4900e4d575b5c146f8f3889cb8a8c11e00555 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 11 Sep 2021 20:52:01 +0530 Subject: [PATCH] src,crypto: eliminate code duplication between StatelessDiffieHellman* Signed-off-by: Darshan Sen --- src/crypto/crypto_dh.cc | 101 +++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index 1c48f98656fd21..de355266d79d47 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -539,62 +539,68 @@ WebCryptoKeyExportStatus DHKeyExportTraits::DoExport( } namespace { -AllocatedBuffer StatelessDiffieHellman( - Environment* env, - ManagedEVPPKey our_key, - ManagedEVPPKey their_key) { - size_t out_size; - - EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(our_key.get(), nullptr)); +bool CreateStatelessDiffieHellmanContext( + const ManagedEVPPKey& our_key, + const ManagedEVPPKey& their_key, + char** buf, + size_t* out_size) { + const EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(our_key.get(), nullptr)); if (!ctx || EVP_PKEY_derive_init(ctx.get()) <= 0 || EVP_PKEY_derive_set_peer(ctx.get(), their_key.get()) <= 0 || - EVP_PKEY_derive(ctx.get(), nullptr, &out_size) <= 0) - return AllocatedBuffer(); + EVP_PKEY_derive(ctx.get(), nullptr, out_size) <= 0) + return false; - AllocatedBuffer result = AllocatedBuffer::AllocateManaged(env, out_size); - CHECK_NOT_NULL(result.data()); + *buf = MallocOpenSSL(*out_size); + size_t remainder_size = *out_size; + if (EVP_PKEY_derive(ctx.get(), + reinterpret_cast(*buf), + &remainder_size) <= 0) + return false; - unsigned char* data = reinterpret_cast(result.data()); - if (EVP_PKEY_derive(ctx.get(), data, &out_size) <= 0) - return AllocatedBuffer(); + ZeroPadDiffieHellmanSecret(remainder_size, *buf, *out_size); - ZeroPadDiffieHellmanSecret(out_size, &result); - return result; + return true; } -// The version of StatelessDiffieHellman that returns an AllocatedBuffer -// is not threadsafe because of the AllocatedBuffer allocation of a -// v8::BackingStore (it'll cause much crashing if we call it from a -// libuv worker thread). This version allocates a ByteSource instead, -// which we can convert into a v8::BackingStore later. -// TODO(@jasnell): Eliminate the code duplication between these two -// versions of the function. -ByteSource StatelessDiffieHellmanThreadsafe( +Local StatelessDiffieHellman( Environment* env, - ManagedEVPPKey our_key, - ManagedEVPPKey their_key) { + const ManagedEVPPKey& our_key, + const ManagedEVPPKey& their_key) { + char* buf = nullptr; size_t out_size; - - EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(our_key.get(), nullptr)); - if (!ctx || - EVP_PKEY_derive_init(ctx.get()) <= 0 || - EVP_PKEY_derive_set_peer(ctx.get(), their_key.get()) <= 0 || - EVP_PKEY_derive(ctx.get(), nullptr, &out_size) <= 0) - return ByteSource(); - - char* buf = MallocOpenSSL(out_size); - ByteSource out = ByteSource::Allocated(buf, out_size); - - if (EVP_PKEY_derive( - ctx.get(), - reinterpret_cast(buf), - &out_size) <= 0) { + if (!CreateStatelessDiffieHellmanContext(our_key, + their_key, + &buf, + &out_size)) + return Local(); + + return Buffer::New(env, + buf, + out_size, + [](char* data, void* hint) { + OPENSSL_free(data); + }, + nullptr).FromMaybe(Local()); +} + +// The version of StatelessDiffieHellman that returns a Buffer +// is not threadsafe because of the allocation of a v8::BackingStore +// (it'll cause much crashing if we call it from a libuv worker +// thread). This version allocates a ByteSource instead, which we can +// convert into a v8::BackingStore later. +ByteSource StatelessDiffieHellmanThreadsafe( + const ManagedEVPPKey& our_key, + const ManagedEVPPKey& their_key) { + char* buf = nullptr; + size_t out_size; + if (!CreateStatelessDiffieHellmanContext(our_key, + their_key, + &buf, + &out_size)) return ByteSource(); - } - ZeroPadDiffieHellmanSecret(out_size, buf, out.size()); - return out; + return ByteSource::Allocated(buf, out_size); } } // namespace @@ -612,11 +618,11 @@ void DiffieHellman::Stateless(const FunctionCallbackInfo& args) { ManagedEVPPKey our_key = our_key_object->Data()->GetAsymmetricKey(); ManagedEVPPKey their_key = their_key_object->Data()->GetAsymmetricKey(); - AllocatedBuffer out = StatelessDiffieHellman(env, our_key, their_key); - if (out.size() == 0) + Local out = StatelessDiffieHellman(env, our_key, their_key); + if (out.IsEmpty() || Buffer::Length(out) == 0) return ThrowCryptoError(env, ERR_get_error(), "diffieHellman failed"); - args.GetReturnValue().Set(out.ToBuffer().FromMaybe(Local())); + args.GetReturnValue().Set(out); } Maybe DHBitsTraits::AdditionalConfig( @@ -661,7 +667,6 @@ bool DHBitsTraits::DeriveBits( const DHBitsConfig& params, ByteSource* out) { *out = StatelessDiffieHellmanThreadsafe( - env, params.private_key->GetAsymmetricKey(), params.public_key->GetAsymmetricKey()); return true;