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
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.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:

src/crypto/crypto_argon2.cc

@@ -147,6 +147,7 @@ bool Argon2Traits::DeriveBits(Environment* env,
                             config.type);
 
   if (!dp) {
+    errors->Capture();
     errors->Insert(NodeCryptoError::ARGON2_FAILED);
     return false;
   }

src/crypto/crypto_hkdf.cc

@@ -127,6 +127,7 @@ bool HKDFTraits::DeriveBits(Environment* env,
                           },
                           params.length);
   if (!dp) {
+    errors->Capture();
     errors->Insert(NodeCryptoError::HKDF_FAILED);
     return false;
   }

src/crypto/crypto_pbkdf2.cc

@@ -135,6 +135,7 @@ bool PBKDF2Traits::DeriveBits(Environment* env,
                             params.length);
 
   if (!dp) {
+    errors->Capture();
     errors->Insert(NodeCryptoError::PBKDF2_FAILED);
     return false;
   }

src/crypto/crypto_scrypt.cc

@@ -132,6 +132,7 @@ bool ScryptTraits::DeriveBits(Environment* env,
       params.length);
 
   if (!dp) {
+    errors->Capture();
     errors->Insert(NodeCryptoError::SCRYPT_FAILED);
     return false;
   }

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

@@ -29,19 +29,31 @@ 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,
+    code: 'ERR_OSSL_INVALID_THREAD_POOL_SIZE', reason: /invalid thread pool size/ },
+  { lanes: 1, keylen: 32, memcost: 0, iter: 1,
+    code: 'ERR_OSSL_INVALID_MEMORY_SIZE', reason: /invalid memory size/ },
+  { lanes: 1, keylen: 32, memcost: 16, iter: 0,
+    code: 'ERR_OSSL_INVALID_ITERATION_COUNT', reason: /invalid iteration count/ },
 ];
 
-for (const { lanes, keylen, memcost, iter } of badParams) {
+function assertError(err, { code, reason }) {
+  assert.ok(err);
+  assert.match(err.message, /Argon2 derivation failed/);
+  assert.strictEqual(err.code, code);
+  assert.ok(err.opensslErrorStack.some((msg) => reason.test(msg)),
+            `did not find ${reason} in ${err.opensslErrorStack}`);
+}
+
+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, /Argon2 derivation failed/);
+    assertError(err, params);
     assert.strictEqual(result, undefined);
   }
 
@@ -50,8 +62,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, /Argon2 derivation failed/);
+      assertError(err, params);
       assert.strictEqual(result, undefined);
     });
     job.run();

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

@@ -26,6 +26,24 @@ 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) {
+      assert.match(err.message, /derivation failed/);
+      assert.strictEqual(err.code, 'ERR_OSSL_EVP_UNSUPPORTED', `${name}: ${err.code}`);
+      const expected = /digital envelope routines::unsupported/;
+      assert(err.opensslErrorStack.some((msg) => expected.test(msg)),
+             `${name}: did not find ${expected} in ${err.opensslErrorStack}`);
+    }
+  }
 }
 
 {