crypto: preserve OpenSSL errors from KDF failures

The ncrypto KDF helpers cleared the OpenSSL error queue on return, and
the traits insert their own message, which makes DeriveBitsJob skip
errors->Capture(). Argon2, HKDF, PBKDF2 and scrypt failures were
therefore bare Errors with no code and no opensslErrorStack.

Drop the guard, which DeriveBitsJob already provides, and capture
before inserting since Capture() clears the store.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: #64776
Backport-PR-URL: #65087
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: panva

deps/ncrypto/ncrypto.cc

@@ -2642,8 +2642,6 @@ DataPointer hkdf(const Digest& md,
                  const Buffer<const unsigned char>& info,
                  const Buffer<const unsigned char>& salt,
                  size_t length) {
-  ClearErrorOnReturn clearErrorOnReturn;
-
   if (!checkHkdfLength(md, length) || info.len > INT_MAX ||
       salt.len > INT_MAX) {
     return {};
@@ -2714,8 +2712,6 @@ DataPointer scrypt(const Buffer<const char>& pass,
                    uint64_t p,
                    uint64_t maxmem,
                    size_t length) {
-  ClearErrorOnReturn clearErrorOnReturn;
-
   if (pass.len > INT_MAX || salt.len > INT_MAX) {
     return {};
   }
@@ -2742,8 +2738,6 @@ DataPointer pbkdf2(const Digest& md,
                    const Buffer<const unsigned char>& salt,
                    uint32_t iterations,
                    size_t length) {
-  ClearErrorOnReturn clearErrorOnReturn;
-
   if (pass.len > INT_MAX || salt.len > INT_MAX || length > INT_MAX) {
     return {};
   }
@@ -2775,8 +2769,6 @@ DataPointer argon2(const Buffer<const char>& pass,
                    const Buffer<const unsigned char>& secret,
                    const Buffer<const unsigned char>& ad,
                    Argon2Type type) {
-  ClearErrorOnReturn clearErrorOnReturn;
-
   std::string_view algorithm;
   switch (type) {
     case Argon2Type::ARGON2I:

test/parallel/test-crypto-argon2-job.js

@@ -29,19 +29,30 @@ const empty = Buffer.alloc(0);
 
 // Parameters that OpenSSL's Argon2 KDF rejects.
 const badParams = [
-  { lanes: 0, keylen: 32, memcost: 16, iter: 1 },  // lanes < 1
-  { lanes: 1, keylen: 32, memcost: 0, iter: 1 },   // memcost == 0
-  { lanes: 1, keylen: 32, memcost: 16, iter: 0 },  // iter == 0
+  { lanes: 0, keylen: 32, memcost: 16, iter: 1,
+    reason: /invalid thread pool size/ },
+  { lanes: 1, keylen: 32, memcost: 0, iter: 1,
+    reason: /invalid memory size/ },
+  { lanes: 1, keylen: 32, memcost: 16, iter: 0,
+    reason: /invalid iteration count/ },
 ];
 
-for (const { lanes, keylen, memcost, iter } of badParams) {
+function assertError(err, { reason }) {
+  assert.ok(err);
+  const details = [err.message, ...(err.opensslErrorStack ?? [])];
+  assert.ok(details.some((msg) => reason.test(msg)),
+            `did not find ${reason} in ${details}`);
+}
+
+for (const params of badParams) {
+  const { lanes, keylen, memcost, iter } = params;
+
   {
     const job = new Argon2Job(
       kCryptoJobSync, pass, salt, lanes, keylen, memcost, iter,
       empty, empty, kTypeArgon2id);
     const { 0: err, 1: result } = job.run();
-    assert.ok(err);
-    assert.match(err.message, /Deriving bits failed/);
+    assertError(err, params);
     assert.strictEqual(result, undefined);
   }
 
@@ -50,8 +61,7 @@ for (const { lanes, keylen, memcost, iter } of badParams) {
       kCryptoJobAsync, pass, salt, lanes, keylen, memcost, iter,
       empty, empty, kTypeArgon2id);
     job.ondone = common.mustCall((err, result) => {
-      assert.ok(err);
-      assert.match(err.message, /Deriving bits failed/);
+      assertError(err, params);
       assert.strictEqual(result, undefined);
     });
     job.run();

test/parallel/test-crypto-no-algorithm.js

@@ -26,6 +26,23 @@ if (isMainThread) {
              `did not find ${expected} in ${err.opensslErrorStack}`);
     }
   }));
+
+  const derivations = [
+    ['HKDF', () => crypto.hkdfSync('sha256', Buffer.alloc(32), Buffer.alloc(8),
+                                   Buffer.alloc(0), 32)],
+    ['PBKDF2', () => crypto.pbkdf2Sync('secret', Buffer.alloc(16), 1000, 32,
+                                       'sha256')],
+  ];
+  for (const { 0: name, 1: derive } of derivations) {
+    try {
+      derive();
+    } catch (err) {
+      const expected = /digital envelope routines::unsupported/;
+      const details = [err.message, ...(err.opensslErrorStack ?? [])];
+      assert(details.some((msg) => expected.test(msg)),
+             `${name}: did not find ${expected} in ${details}`);
+    }
+  }
 }
 
 {