Skip to content

Commit 216f123

Browse files
anonrigaduh95
authored andcommitted
crypto: cache digests for synchronous operations
createHmac(), createSign(), createVerify() and the synchronous one-shot sign, verify, HKDF and PBKDF2 functions looked up their digest with Digest::FromName(). That returns OpenSSL's legacy EVP_MD, so every EVP_DigestInit_ex() and HMAC init had to fetch the provider implementation again through the method store. Look these digests up through the per-Environment cache that createHash() and crypto.hash() already use. Jobs on the thread pool keep using Digest::FromName(): with the cache, Web Crypto digest and HMAC throughput went down, because workers that finish sooner have to be woken up for each new job more often. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Grok PR-URL: #67109 Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent 464ab31 commit 216f123

8 files changed

Lines changed: 53 additions & 7 deletions

File tree

‎deps/ncrypto/ncrypto.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8942,6 +8942,16 @@ const Digest Digest::Fetch(const char* name) {
89428942
return Digest();
89438943
}
89448944

8945+
const Digest Digest::Ref(const EVP_MD* md) {
8946+
#if NCRYPTO_USE_OPENSSL_PROVIDER
8947+
if (md != nullptr && EVP_MD_get0_provider(md) != nullptr) {
8948+
if (EVP_MD_up_ref(const_cast<EVP_MD*>(md)) != 1) return Digest();
8949+
return Digest(DeleteFnPtr<EVP_MD, EVP_MD_free>(const_cast<EVP_MD*>(md)));
8950+
}
8951+
#endif
8952+
return Digest(md);
8953+
}
8954+
89458955
// ============================================================================
89468956
// KEM Implementation
89478957
#if OPENSSL_WITH_KEM

‎deps/ncrypto/ncrypto.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,9 @@ class Digest final {
368368

369369
static const Digest FromName(const char* name);
370370
static const Digest Fetch(const char* name);
371+
// Returns a Digest that holds its own reference to md if md was fetched from
372+
// a provider.
373+
static const Digest Ref(const EVP_MD* md);
371374

372375
private:
373376
const EVP_MD* md_ = nullptr;

‎src/crypto/crypto_hash.cc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,25 @@ const EVP_MD* GetDigestImplementation(
363363
#endif
364364
}
365365

366+
const ncrypto::Digest GetDigest(Environment* env, const char* name) {
367+
#if NCRYPTO_USE_OPENSSL_PROVIDER
368+
MaybeCachedMD result = FetchAndMaybeCacheMD(env, name);
369+
if (result.cached_md != nullptr) {
370+
return ncrypto::Digest::Ref(result.cached_md);
371+
}
372+
return result.digest;
373+
#elif NCRYPTO_USE_BORINGSSL
374+
return ncrypto::Digest::FromName(name);
375+
#endif
376+
}
377+
378+
const ncrypto::Digest GetDigestForJob(Environment* env,
379+
CryptoJobMode mode,
380+
const char* name) {
381+
if (mode == kCryptoJobSync) return GetDigest(env, name);
382+
return ncrypto::Digest::FromName(name);
383+
}
384+
366385
void MarkInvalidXofLength() {
367386
#if NCRYPTO_USE_OPENSSL_PROVIDER
368387
ERR_raise(ERR_LIB_EVP, EVP_R_NOT_XOF_OR_INVALID_LENGTH);

‎src/crypto/crypto_hash.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@
1212

1313
namespace node {
1414
namespace crypto {
15+
// Like ncrypto::Digest::FromName(), but caches provider implementations per
16+
// Environment so that later lookups do not have to fetch them again.
17+
const ncrypto::Digest GetDigest(Environment* env, const char* name);
18+
19+
// GetDigest() for synchronous jobs. Jobs on the thread pool keep using
20+
// ncrypto::Digest::FromName(): there, the cache lowered Web Crypto digest and
21+
// HMAC throughput, because workers that finish sooner have to be woken up for
22+
// each new job more often.
23+
const ncrypto::Digest GetDigestForJob(Environment* env,
24+
CryptoJobMode mode,
25+
const char* name);
26+
1527
class Hash final : public BaseObject {
1628
public:
1729
static void Initialize(Environment* env, v8::Local<v8::Object> target);

‎src/crypto/crypto_hkdf.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "crypto/crypto_hkdf.h"
22
#include "async_wrap-inl.h"
33
#include "base_object-inl.h"
4+
#include "crypto/crypto_hash.h"
45
#include "crypto/crypto_keys.h"
56
#include "env-inl.h"
67
#include "memory_tracker-inl.h"
@@ -9,7 +10,6 @@
910

1011
namespace node {
1112

12-
using ncrypto::Digest;
1313
using v8::FunctionCallbackInfo;
1414
using v8::JustVoid;
1515
using v8::Maybe;
@@ -54,7 +54,7 @@ Maybe<void> HKDFTraits::AdditionalConfig(
5454
CHECK(args[offset + 4]->IsUint32()); // Length
5555

5656
Utf8Value hash(env->isolate(), args[offset]);
57-
params->digest = Digest::FromName(*hash);
57+
params->digest = GetDigestForJob(env, mode, *hash);
5858
if (!params->digest) [[unlikely]] {
5959
THROW_ERR_CRYPTO_INVALID_DIGEST(env, "Invalid digest: %s", hash);
6060
return Nothing<void>();

‎src/crypto/crypto_hmac.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "crypto/crypto_hmac.h"
22
#include "async_wrap-inl.h"
33
#include "base_object-inl.h"
4+
#include "crypto/crypto_hash.h"
45
#include "crypto/crypto_keys.h"
56
#include "crypto/crypto_sig.h"
67
#include "crypto/crypto_util.h"
@@ -69,7 +70,7 @@ void Hmac::New(const FunctionCallbackInfo<Value>& args) {
6970
void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) {
7071
HandleScope scope(env()->isolate());
7172

72-
Digest md = Digest::FromName(hash_type);
73+
Digest md = GetDigest(env(), hash_type);
7374
if (!md) [[unlikely]] {
7475
return THROW_ERR_CRYPTO_INVALID_DIGEST(
7576
env(), "Invalid digest: %s", hash_type);

‎src/crypto/crypto_pbkdf2.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "crypto/crypto_pbkdf2.h"
22
#include "async_wrap-inl.h"
33
#include "base_object-inl.h"
4+
#include "crypto/crypto_hash.h"
45
#include "crypto/crypto_keys.h"
56
#include "crypto/crypto_util.h"
67
#include "env-inl.h"
@@ -11,7 +12,6 @@
1112

1213
namespace node {
1314

14-
using ncrypto::Digest;
1515
using v8::FunctionCallbackInfo;
1616
using v8::Int32;
1717
using v8::JustVoid;
@@ -104,7 +104,7 @@ Maybe<void> PBKDF2Traits::AdditionalConfig(
104104
}
105105

106106
Utf8Value name(args.GetIsolate(), args[offset + 4]);
107-
params->digest = Digest::FromName(*name);
107+
params->digest = GetDigestForJob(env, mode, *name);
108108
if (!params->digest) [[unlikely]] {
109109
THROW_ERR_CRYPTO_INVALID_DIGEST(env, "Invalid digest: %s", name);
110110
return Nothing<void>();

‎src/crypto/crypto_sig.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "async_wrap-inl.h"
33
#include "base_object-inl.h"
44
#include "crypto/crypto_ec.h"
5+
#include "crypto/crypto_hash.h"
56
#include "crypto/crypto_keys.h"
67
#include "crypto/crypto_util.h"
78
#include "env-inl.h"
@@ -315,7 +316,7 @@ bool VerifyPrehashed(const EVPKeyPointer& key,
315316

316317
SignBase::Error SignBase::Init(const char* digest) {
317318
CHECK_NULL(mdctx_);
318-
auto md = Digest::FromName(digest);
319+
auto md = GetDigest(env(), digest);
319320
if (!md) [[unlikely]]
320321
return Error::UnknownDigest;
321322

@@ -666,7 +667,7 @@ Maybe<void> SignTraits::AdditionalConfig(
666667

667668
if (args[offset + 7]->IsString()) {
668669
Utf8Value digest(env->isolate(), args[offset + 7]);
669-
params->digest = Digest::FromName(*digest);
670+
params->digest = GetDigestForJob(env, mode, *digest);
670671
if (!params->digest) [[unlikely]] {
671672
THROW_ERR_CRYPTO_INVALID_DIGEST(env, "Invalid digest: %s", digest);
672673
return Nothing<void>();

0 commit comments

Comments
 (0)