From 922f2f0eb279d4d073ad2bb604a69630c65f866e Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Wed, 24 Feb 2021 13:13:42 +0100
Subject: [PATCH] crypto: add optional callback to crypto.sign and
 crypto.verify

PR-URL: https://github.com/nodejs/node/pull/37500
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/crypto.md                             |  26 +++-
 lib/internal/crypto/dsa.js                    |   4 +-
 lib/internal/crypto/ec.js                     |   4 +-
 lib/internal/crypto/rsa.js                    |   3 +-
 lib/internal/crypto/sig.js                    | 117 +++++++++++++---
 src/crypto/crypto_ec.cc                       | 127 +++++++++++++-----
 src/crypto/crypto_ec.h                        |   4 +-
 src/crypto/crypto_sig.cc                      |  36 ++++-
 src/crypto/crypto_sig.h                       |   4 +-
 src/crypto/crypto_util.h                      |   2 +-
 .../parallel/test-crypto-async-sign-verify.js | 108 +++++++++++++++
 11 files changed, 361 insertions(+), 74 deletions(-)
 create mode 100644 test/parallel/test-crypto-async-sign-verify.js

diff --git a/doc/api/crypto.md b/doc/api/crypto.md
index 4c55bcaa9cab81..96dcac7f2c460e 100644
--- a/doc/api/crypto.md
+++ b/doc/api/crypto.md
@@ -3822,10 +3822,13 @@ added: v10.0.0
 Enables the FIPS compliant crypto provider in a FIPS-enabled Node.js build.
 Throws an error if FIPS mode is not available.
 
-### `crypto.sign(algorithm, data, key)`
+### `crypto.sign(algorithm, data, key[, callback])`
 <!-- YAML
 added: v12.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37500
+    description: Optional callback argument added.
   - version:
      - v13.2.0
      - v12.16.0
@@ -3837,7 +3840,10 @@ changes:
 * `algorithm` {string | null | undefined}
 * `data` {ArrayBuffer|Buffer|TypedArray|DataView}
 * `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|CryptoKey}
-* Returns: {Buffer}
+* `callback` {Function}
+  * `err` {Error}
+  * `signature` {Buffer}
+* Returns: {Buffer} if the `callback` function is not provided.
 <!--lint enable maximum-line-length remark-lint-->
 
 Calculates and returns the signature for `data` using the given private key and
@@ -3864,6 +3870,8 @@ additional properties can be passed:
   size, `crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN` (default) sets it to the
   maximum permissible value.
 
+If the `callback` function is provided this function uses libuv's threadpool.
+
 ### `crypto.timingSafeEqual(a, b)`
 <!-- YAML
 added: v6.6.0
@@ -3894,10 +3902,13 @@ Use of `crypto.timingSafeEqual` does not guarantee that the *surrounding* code
 is timing-safe. Care should be taken to ensure that the surrounding code does
 not introduce timing vulnerabilities.
 
-### `crypto.verify(algorithm, data, key, signature)`
+### `crypto.verify(algorithm, data, key, signature[, callback])`
 <!-- YAML
 added: v12.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37500
+    description: Optional callback argument added.
   - version: v15.0.0
     pr-url: https://github.com/nodejs/node/pull/35093
     description: The data, key, and signature arguments can also be ArrayBuffer.
@@ -3913,7 +3924,12 @@ changes:
 * `data` {ArrayBuffer| Buffer|TypedArray|DataView}
 * `key` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject|CryptoKey}
 * `signature` {ArrayBuffer|Buffer|TypedArray|DataView}
-* Returns: {boolean}
+* `callback` {Function}
+  * `err` {Error}
+  * `result` {boolean}
+* Returns: {boolean} `true` or `false` depending on the validity of the
+  signature for the data and public key if the `callback` function is not
+  provided.
 <!--lint enable maximum-line-length remark-lint-->
 
 Verifies the given signature for `data` using the given key and algorithm. If
@@ -3945,6 +3961,8 @@ The `signature` argument is the previously calculated signature for the `data`.
 Because public keys can be derived from private keys, a private key or a public
 key may be passed for `key`.
 
+If the `callback` function is provided this function uses libuv's threadpool.
+
 ### `crypto.webcrypto`
 <!-- YAML
 added: v15.0.0
diff --git a/lib/internal/crypto/dsa.js b/lib/internal/crypto/dsa.js
index 27cba2139ed691..c2a528aa94e056 100644
--- a/lib/internal/crypto/dsa.js
+++ b/lib/internal/crypto/dsa.js
@@ -10,6 +10,7 @@ const {
   KeyObjectHandle,
   SignJob,
   kCryptoJobAsync,
+  kSigEncDER,
   kKeyTypePrivate,
   kSignJobModeSign,
   kSignJobModeVerify,
@@ -254,7 +255,8 @@ function dsaSignVerify(key, data, algorithm, signature) {
     normalizeHashName(key.algorithm.hash.name),
     undefined,  // Salt-length is not used in DSA
     undefined,  // Padding is not used in DSA
-    signature));
+    signature,
+    kSigEncDER));
 }
 
 module.exports = {
diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
index 19ec84fae3b614..47e50f1b4fdfbb 100644
--- a/lib/internal/crypto/ec.js
+++ b/lib/internal/crypto/ec.js
@@ -17,6 +17,7 @@ const {
   kKeyTypePublic,
   kSignJobModeSign,
   kSignJobModeVerify,
+  kSigEncP1363,
 } = internalBinding('crypto');
 
 const {
@@ -470,7 +471,8 @@ function ecdsaSignVerify(key, data, { name, hash }, signature) {
     hashname,
     undefined,  // Salt length, not used with ECDSA
     undefined,  // PSS Padding, not used with ECDSA
-    signature));
+    signature,
+    kSigEncP1363));
 }
 
 module.exports = {
diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js
index c930c284cd23b2..06a9e802717c54 100644
--- a/lib/internal/crypto/rsa.js
+++ b/lib/internal/crypto/rsa.js
@@ -30,6 +30,7 @@ const {
 } = require('internal/errors');
 
 const {
+  validateInt32,
   validateUint32,
 } = require('internal/validators');
 
@@ -342,7 +343,7 @@ function rsaSignVerify(key, data, { saltLength }, signature) {
     // TODO(@jasnell): Validate maximum size of saltLength
     // based on the key size:
     //   Math.ceil((keySizeInBits - 1)/8) - digestSizeInBytes - 2
-    validateUint32(saltLength, 'algorithm.saltLength');
+    validateInt32(saltLength, 'algorithm.saltLength', -2);
   }
 
   const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify;
diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js
index 783cdb33202202..d5c4c46192cd17 100644
--- a/lib/internal/crypto/sig.js
+++ b/lib/internal/crypto/sig.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  FunctionPrototypeCall,
   ObjectSetPrototypeOf,
   ReflectApply,
 } = primordials;
@@ -14,17 +15,22 @@ const {
 } = require('internal/errors');
 
 const {
+  validateCallback,
   validateEncoding,
   validateString,
 } = require('internal/validators');
 
 const {
   Sign: _Sign,
+  SignJob,
   Verify: _Verify,
   signOneShot: _signOneShot,
   verifyOneShot: _verifyOneShot,
+  kCryptoJobAsync,
   kSigEncDER,
   kSigEncP1363,
+  kSignJobModeSign,
+  kSignJobModeVerify,
 } = internalBinding('crypto');
 
 const {
@@ -34,12 +40,18 @@ const {
 } = require('internal/crypto/util');
 
 const {
-  preparePublicOrPrivateKey,
+  createPrivateKey,
+  createPublicKey,
+  isCryptoKey,
+  isKeyObject,
   preparePrivateKey,
+  preparePublicOrPrivateKey,
 } = require('internal/crypto/keys');
 
 const { Writable } = require('stream');
 
+const { Buffer } = require('buffer');
+
 const {
   isArrayBufferView,
 } = require('internal/util/types');
@@ -131,22 +143,18 @@ Sign.prototype.sign = function sign(options, encoding) {
   return ret;
 };
 
-function signOneShot(algorithm, data, key) {
+function signOneShot(algorithm, data, key, callback) {
   if (algorithm != null)
     validateString(algorithm, 'algorithm');
 
+  if (callback !== undefined)
+    validateCallback(callback);
+
   data = getArrayBufferOrView(data, 'data');
 
   if (!key)
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
 
-  const {
-    data: keyData,
-    format: keyFormat,
-    type: keyType,
-    passphrase: keyPassphrase
-  } = preparePrivateKey(key);
-
   // Options specific to RSA
   const rsaPadding = getPadding(key);
   const pssSaltLength = getSaltLength(key);
@@ -154,8 +162,43 @@ function signOneShot(algorithm, data, key) {
   // Options specific to (EC)DSA
   const dsaSigEnc = getDSASignatureEncoding(key);
 
-  return _signOneShot(keyData, keyFormat, keyType, keyPassphrase, data,
-                      algorithm, rsaPadding, pssSaltLength, dsaSigEnc);
+  if (!callback) {
+    const {
+      data: keyData,
+      format: keyFormat,
+      type: keyType,
+      passphrase: keyPassphrase
+    } = preparePrivateKey(key);
+
+    return _signOneShot(keyData, keyFormat, keyType, keyPassphrase, data,
+                        algorithm, rsaPadding, pssSaltLength, dsaSigEnc);
+  }
+
+  let keyData;
+  if (isKeyObject(key) || isCryptoKey(key)) {
+    ({ data: keyData } = preparePrivateKey(key));
+  } else if (key != null && (isKeyObject(key.key) || isCryptoKey(key.key))) {
+    ({ data: keyData } = preparePrivateKey(key.key));
+  } else {
+    keyData = createPrivateKey(key)[kHandle];
+  }
+
+  const job = new SignJob(
+    kCryptoJobAsync,
+    kSignJobModeSign,
+    keyData,
+    data,
+    algorithm,
+    pssSaltLength,
+    rsaPadding,
+    undefined,
+    dsaSigEnc);
+
+  job.ondone = (error, signature) => {
+    if (error) return FunctionPrototypeCall(callback, job, error);
+    FunctionPrototypeCall(callback, job, null, Buffer.from(signature));
+  };
+  job.run();
 }
 
 function Verify(algorithm, options) {
@@ -197,10 +240,13 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
                               rsaPadding, pssSaltLength, dsaSigEnc);
 };
 
-function verifyOneShot(algorithm, data, key, signature) {
+function verifyOneShot(algorithm, data, key, signature, callback) {
   if (algorithm != null)
     validateString(algorithm, 'algorithm');
 
+  if (callback !== undefined)
+    validateCallback(callback);
+
   data = getArrayBufferOrView(data, 'data');
 
   if (!isArrayBufferView(data)) {
@@ -211,13 +257,6 @@ function verifyOneShot(algorithm, data, key, signature) {
     );
   }
 
-  const {
-    data: keyData,
-    format: keyFormat,
-    type: keyType,
-    passphrase: keyPassphrase
-  } = preparePublicOrPrivateKey(key);
-
   // Options specific to RSA
   const rsaPadding = getPadding(key);
   const pssSaltLength = getSaltLength(key);
@@ -233,8 +272,44 @@ function verifyOneShot(algorithm, data, key, signature) {
     );
   }
 
-  return _verifyOneShot(keyData, keyFormat, keyType, keyPassphrase, signature,
-                        data, algorithm, rsaPadding, pssSaltLength, dsaSigEnc);
+  if (!callback) {
+    const {
+      data: keyData,
+      format: keyFormat,
+      type: keyType,
+      passphrase: keyPassphrase
+    } = preparePublicOrPrivateKey(key);
+
+    return _verifyOneShot(keyData, keyFormat, keyType, keyPassphrase,
+                          signature, data, algorithm, rsaPadding,
+                          pssSaltLength, dsaSigEnc);
+  }
+
+  let keyData;
+  if (isKeyObject(key) || isCryptoKey(key)) {
+    ({ data: keyData } = preparePublicOrPrivateKey(key));
+  } else if (key != null && (isKeyObject(key.key) || isCryptoKey(key.key))) {
+    ({ data: keyData } = preparePublicOrPrivateKey(key.key));
+  } else {
+    keyData = createPublicKey(key)[kHandle];
+  }
+
+  const job = new SignJob(
+    kCryptoJobAsync,
+    kSignJobModeVerify,
+    keyData,
+    data,
+    algorithm,
+    pssSaltLength,
+    rsaPadding,
+    signature,
+    dsaSigEnc);
+
+  job.ondone = (error, result) => {
+    if (error) return FunctionPrototypeCall(callback, job, error);
+    FunctionPrototypeCall(callback, job, null, result);
+  };
+  job.run();
 }
 
 module.exports = {
diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
index ec8b7a864d9d18..b97670b54a62fc 100644
--- a/src/crypto/crypto_ec.cc
+++ b/src/crypto/crypto_ec.cc
@@ -15,6 +15,8 @@
 #include <openssl/ec.h>
 #include <openssl/ecdh.h>
 
+#include <algorithm>
+
 namespace node {
 
 using v8::Array;
@@ -926,62 +928,117 @@ size_t GroupOrderSize(ManagedEVPPKey key) {
   return BN_num_bytes(order.get());
 }
 
+// TODO(@jasnell): Reconcile with ConvertSignatureToP1363 in crypto_sig.cc
+// TODO(@jasnell): Move out of crypto_ec since this is also doing DSA now also
 ByteSource ConvertToWebCryptoSignature(
-    ManagedEVPPKey key,
+    const ManagedEVPPKey& key,
     const ByteSource& signature) {
   const unsigned char* data =
       reinterpret_cast<const unsigned char*>(signature.get());
-  EcdsaSigPointer ecsig(d2i_ECDSA_SIG(nullptr, &data, signature.size()));
-
-  if (!ecsig)
-    return ByteSource();
-
-  size_t order_size_bytes = GroupOrderSize(key);
-  char* outdata = MallocOpenSSL<char>(order_size_bytes * 2);
-  ByteSource out = ByteSource::Allocated(outdata, order_size_bytes * 2);
-  unsigned char* ptr = reinterpret_cast<unsigned char*>(outdata);
 
+  ECDSASigPointer ecsig;
+  DsaSigPointer dsasig;
   const BIGNUM* pr;
   const BIGNUM* ps;
-  ECDSA_SIG_get0(ecsig.get(), &pr, &ps);
+  size_t len = 0;
+
+  switch (EVP_PKEY_id(key.get())) {
+    case EVP_PKEY_EC: {
+      ecsig = ECDSASigPointer(d2i_ECDSA_SIG(nullptr, &data, signature.size()));
+
+      if (!ecsig)
+        return ByteSource();
+
+      len = GroupOrderSize(key);
+
+      ECDSA_SIG_get0(ecsig.get(), &pr, &ps);
+      break;
+    }
+    case EVP_PKEY_DSA: {
+      dsasig = DsaSigPointer(d2i_DSA_SIG(nullptr, &data, signature.size()));
+
+      if (!dsasig)
+        return ByteSource();
+
+      DSA_SIG_get0(dsasig.get(), &pr, &ps);
+      len = std::max(BN_num_bytes(pr), BN_num_bytes(ps));
+    }
+  }
+
+  CHECK_GT(len, 0);
 
-  if (!BN_bn2binpad(pr, ptr, order_size_bytes) ||
-      !BN_bn2binpad(ps, ptr + order_size_bytes, order_size_bytes)) {
+  char* outdata = MallocOpenSSL<char>(len * 2);
+  memset(outdata, 0, len * 2);
+  ByteSource out = ByteSource::Allocated(outdata, len * 2);
+  unsigned char* ptr = reinterpret_cast<unsigned char*>(outdata);
+
+  if (BN_bn2binpad(pr, ptr, len) <= 0 ||
+    BN_bn2binpad(ps, ptr + len, len) <= 0) {
     return ByteSource();
   }
+
   return out;
 }
 
+// TODO(@jasnell): Reconcile with ConvertSignatureToDER in crypto_sig.cc
+// TODO(@jasnell): Move out of crypto_ec since this is also doing DSA now also
 ByteSource ConvertFromWebCryptoSignature(
-    ManagedEVPPKey key,
+    const ManagedEVPPKey& key,
     const ByteSource& signature) {
-  size_t order_size_bytes = GroupOrderSize(key);
+  BignumPointer r(BN_new());
+  BignumPointer s(BN_new());
+  const unsigned char* sig = signature.data<unsigned char>();
 
-  // If the size of the signature is incorrect, verification
-  // will fail.
-  if (signature.size() != 2 * order_size_bytes)
-    return ByteSource();  // Empty!
+  switch (EVP_PKEY_id(key.get())) {
+    case EVP_PKEY_EC: {
+      size_t order_size_bytes = GroupOrderSize(key);
 
-  EcdsaSigPointer ecsig(ECDSA_SIG_new());
-  if (!ecsig)
-    return ByteSource();
+      // If the size of the signature is incorrect, verification
+      // will fail.
+      if (signature.size() != 2 * order_size_bytes)
+        return ByteSource();  // Empty!
 
-  BignumPointer r(BN_new());
-  BignumPointer s(BN_new());
+      ECDSASigPointer ecsig(ECDSA_SIG_new());
+      if (!ecsig)
+        return ByteSource();
 
-  const unsigned char* sig = signature.data<unsigned char>();
+      if (!BN_bin2bn(sig, order_size_bytes, r.get()) ||
+          !BN_bin2bn(sig + order_size_bytes, order_size_bytes, s.get()) ||
+          !ECDSA_SIG_set0(ecsig.get(), r.release(), s.release())) {
+        return ByteSource();
+      }
 
-  if (!BN_bin2bn(sig, order_size_bytes, r.get()) ||
-      !BN_bin2bn(sig + order_size_bytes, order_size_bytes, s.get()) ||
-      !ECDSA_SIG_set0(ecsig.get(), r.release(), s.release())) {
-    return ByteSource();
-  }
+      int size = i2d_ECDSA_SIG(ecsig.get(), nullptr);
+      char* data = MallocOpenSSL<char>(size);
+      unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
+      CHECK_EQ(i2d_ECDSA_SIG(ecsig.get(), &ptr), size);
+      return ByteSource::Allocated(data, size);
+    }
+    case EVP_PKEY_DSA: {
+      size_t len = signature.size() / 2;
+
+      if (signature.size() != 2 * len)
+        return ByteSource();
+
+      DsaSigPointer dsasig(DSA_SIG_new());
+      if (!dsasig)
+        return ByteSource();
+
+      if (!BN_bin2bn(sig, len, r.get()) ||
+          !BN_bin2bn(sig + len, len, s.get()) ||
+          !DSA_SIG_set0(dsasig.get(), r.release(), s.release())) {
+        return ByteSource();
+      }
 
-  int size = i2d_ECDSA_SIG(ecsig.get(), nullptr);
-  char* data = MallocOpenSSL<char>(size);
-  unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
-  CHECK_EQ(i2d_ECDSA_SIG(ecsig.get(), &ptr), size);
-  return ByteSource::Allocated(data, size);
+      int size = i2d_DSA_SIG(dsasig.get(), nullptr);
+      char* data = MallocOpenSSL<char>(size);
+      unsigned char* ptr = reinterpret_cast<unsigned char*>(data);
+      CHECK_EQ(i2d_DSA_SIG(dsasig.get(), &ptr), size);
+      return ByteSource::Allocated(data, size);
+    }
+    default:
+      UNREACHABLE();
+  }
 }
 
 }  // namespace crypto
diff --git a/src/crypto/crypto_ec.h b/src/crypto/crypto_ec.h
index 00d9d0087b0989..e23b28571b4feb 100644
--- a/src/crypto/crypto_ec.h
+++ b/src/crypto/crypto_ec.h
@@ -164,11 +164,11 @@ v8::Maybe<bool> GetEcKeyDetail(
     v8::Local<v8::Object> target);
 
 ByteSource ConvertToWebCryptoSignature(
-  ManagedEVPPKey key,
+  const ManagedEVPPKey& key,
   const ByteSource& signature);
 
 ByteSource ConvertFromWebCryptoSignature(
-    ManagedEVPPKey key,
+    const ManagedEVPPKey& key,
     const ByteSource& signature);
 
 }  // namespace crypto
diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc
index b2e6084d619843..75fa603207cf1f 100644
--- a/src/crypto/crypto_sig.cc
+++ b/src/crypto/crypto_sig.cc
@@ -231,6 +231,17 @@ bool IsOneShot(const ManagedEVPPKey& key) {
       return false;
   }
 }
+
+bool UseP1363Encoding(const ManagedEVPPKey& key,
+                      const DSASigEnc& dsa_encoding) {
+  switch (EVP_PKEY_id(key.get())) {
+    case EVP_PKEY_EC:
+    case EVP_PKEY_DSA:
+      return dsa_encoding == kSigEncP1363;
+    default:
+      return false;
+  }
+}
 }  // namespace
 
 SignBase::Error SignBase::Init(const char* sign_type) {
@@ -295,6 +306,8 @@ void Sign::Initialize(Environment* env, Local<Object> target) {
 
   NODE_DEFINE_CONSTANT(target, kSignJobModeSign);
   NODE_DEFINE_CONSTANT(target, kSignJobModeVerify);
+  NODE_DEFINE_CONSTANT(target, kSigEncDER);
+  NODE_DEFINE_CONSTANT(target, kSigEncP1363);
   NODE_DEFINE_CONSTANT(target, RSA_PKCS1_PSS_PADDING);
 }
 
@@ -679,7 +692,8 @@ SignConfiguration::SignConfiguration(SignConfiguration&& other) noexcept
       digest(other.digest),
       flags(other.flags),
       padding(other.padding),
-      salt_length(other.salt_length) {}
+      salt_length(other.salt_length),
+      dsa_encoding(other.dsa_encoding) {}
 
 SignConfiguration& SignConfiguration::operator=(
     SignConfiguration&& other) noexcept {
@@ -733,15 +747,25 @@ Maybe<bool> SignTraits::AdditionalConfig(
     }
   }
 
-  if (args[offset + 4]->IsUint32()) {  // Salt length
+  if (args[offset + 4]->IsInt32()) {  // Salt length
     params->flags |= SignConfiguration::kHasSaltLength;
-    params->salt_length = args[offset + 4].As<Uint32>()->Value();
+    params->salt_length = args[offset + 4].As<Int32>()->Value();
   }
   if (args[offset + 5]->IsUint32()) {  // Padding
     params->flags |= SignConfiguration::kHasPadding;
     params->padding = args[offset + 5].As<Uint32>()->Value();
   }
 
+  if (args[offset + 7]->IsUint32()) {  // DSA Encoding
+    params->dsa_encoding =
+        static_cast<DSASigEnc>(args[offset + 7].As<Uint32>()->Value());
+    if (params->dsa_encoding != kSigEncDER &&
+        params->dsa_encoding != kSigEncP1363) {
+      THROW_ERR_OUT_OF_RANGE(env, "invalid signature encoding");
+      return Nothing<bool>();
+    }
+  }
+
   if (params->mode == SignConfiguration::kVerify) {
     ArrayBufferOrViewContents<char> signature(args[offset + 6]);
     if (UNLIKELY(!signature.CheckSizeInt32())) {
@@ -752,7 +776,7 @@ Maybe<bool> SignTraits::AdditionalConfig(
     // the signature from WebCrypto format into DER format...
     ManagedEVPPKey m_pkey = params->key->GetAsymmetricKey();
     Mutex::ScopedLock lock(*m_pkey.mutex());
-    if (EVP_PKEY_id(m_pkey.get()) == EVP_PKEY_EC) {
+    if (UseP1363Encoding(m_pkey, params->dsa_encoding)) {
       params->signature =
           ConvertFromWebCryptoSignature(m_pkey, signature.ToByteSource());
     } else {
@@ -849,9 +873,7 @@ bool SignTraits::DeriveBits(
         if (!EVP_DigestSignFinal(context.get(), data, &len))
           return false;
 
-        // If this is an EC key (assuming ECDSA) we have to
-        // convert the signature in to the proper format.
-        if (EVP_PKEY_id(params.key->GetAsymmetricKey().get()) == EVP_PKEY_EC) {
+        if (UseP1363Encoding(m_pkey, params.dsa_encoding)) {
           *out = ConvertToWebCryptoSignature(
               params.key->GetAsymmetricKey(), buf);
         } else {
diff --git a/src/crypto/crypto_sig.h b/src/crypto/crypto_sig.h
index 2713ca4ca6ba4f..45c3d344d92f33 100644
--- a/src/crypto/crypto_sig.h
+++ b/src/crypto/crypto_sig.h
@@ -15,7 +15,8 @@ namespace crypto {
 static const unsigned int kNoDsaSignature = static_cast<unsigned int>(-1);
 
 enum DSASigEnc {
-  kSigEncDER, kSigEncP1363
+  kSigEncDER,
+  kSigEncP1363
 };
 
 class SignBase : public BaseObject {
@@ -117,6 +118,7 @@ struct SignConfiguration final : public MemoryRetainer {
   int flags = SignConfiguration::kHasNone;
   int padding = 0;
   int salt_length = 0;
+  DSASigEnc dsa_encoding = kSigEncDER;
 
   SignConfiguration() = default;
 
diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h
index 82e970bb818a1d..c04b68efbf8c71 100644
--- a/src/crypto/crypto_util.h
+++ b/src/crypto/crypto_util.h
@@ -75,7 +75,7 @@ using HMACCtxPointer = DeleteFnPtr<HMAC_CTX, HMAC_CTX_free>;
 using CipherCtxPointer = DeleteFnPtr<EVP_CIPHER_CTX, EVP_CIPHER_CTX_free>;
 using RsaPointer = DeleteFnPtr<RSA, RSA_free>;
 using DsaPointer = DeleteFnPtr<DSA, DSA_free>;
-using EcdsaSigPointer = DeleteFnPtr<ECDSA_SIG, ECDSA_SIG_free>;
+using DsaSigPointer = DeleteFnPtr<DSA_SIG, DSA_SIG_free>;
 
 // Our custom implementation of the certificate verify callback
 // used when establishing a TLS handshake. Because we cannot perform
diff --git a/test/parallel/test-crypto-async-sign-verify.js b/test/parallel/test-crypto-async-sign-verify.js
new file mode 100644
index 00000000000000..4980c87673bdb5
--- /dev/null
+++ b/test/parallel/test-crypto-async-sign-verify.js
@@ -0,0 +1,108 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const crypto = require('crypto');
+const fixtures = require('../common/fixtures');
+
+function test(
+  publicFixture,
+  privateFixture,
+  algorithm,
+  deterministic,
+  options
+) {
+  let publicPem = fixtures.readKey(publicFixture);
+  let privatePem = fixtures.readKey(privateFixture);
+  let privateKey = crypto.createPrivateKey(privatePem);
+  let publicKey = crypto.createPublicKey(publicPem);
+  const privateDer = {
+    key: privateKey.export({ format: 'der', type: 'pkcs8' }),
+    format: 'der',
+    type: 'pkcs8',
+    ...options
+  };
+  const publicDer = {
+    key: publicKey.export({ format: 'der', type: 'spki' }),
+    format: 'der',
+    type: 'spki',
+    ...options
+  };
+
+  if (options) {
+    publicPem = { ...options, key: publicPem };
+    privatePem = { ...options, key: privatePem };
+    privateKey = { ...options, key: privateKey };
+    publicKey = { ...options, key: publicKey };
+  }
+
+  const data = Buffer.from('Hello world');
+  const expected = crypto.sign(algorithm, data, privateKey);
+
+  for (const key of [privatePem, privateKey, privateDer]) {
+    crypto.sign(algorithm, data, key, common.mustSucceed((actual) => {
+      if (deterministic) {
+        assert.deepStrictEqual(actual, expected);
+      }
+
+      assert.strictEqual(
+        crypto.verify(algorithm, data, key, actual), true);
+    }));
+  }
+
+  for (const key of [publicPem, publicKey, publicDer]) {
+    crypto.verify(algorithm, data, key, expected, common.mustSucceed(
+      (verified) => assert.strictEqual(verified, true)));
+
+    crypto.verify(algorithm, data, key, Buffer.from(''), common.mustSucceed(
+      (verified) => assert.strictEqual(verified, false)));
+  }
+}
+
+// RSA w/ default padding
+test('rsa_public.pem', 'rsa_private.pem', 'sha256', true);
+test('rsa_public.pem', 'rsa_private.pem', 'sha256', true,
+     { padding: crypto.constants.RSA_PKCS1_PADDING });
+
+// RSA w/ PSS_PADDING and default saltLength
+test('rsa_public.pem', 'rsa_private.pem', 'sha256', false,
+     { padding: crypto.constants.RSA_PKCS1_PSS_PADDING });
+test('rsa_public.pem', 'rsa_private.pem', 'sha256', false,
+     {
+       padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
+       saltLength: crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN
+     });
+
+// RSA w/ PSS_PADDING and PSS_SALTLEN_DIGEST
+test('rsa_public.pem', 'rsa_private.pem', 'sha256', false,
+     {
+       padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
+       saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST
+     });
+
+// ED25519
+test('ed25519_public.pem', 'ed25519_private.pem', undefined, true);
+// ED448
+test('ed448_public.pem', 'ed448_private.pem', undefined, true);
+
+// ECDSA w/ der signature encoding
+test('ec_secp256k1_public.pem', 'ec_secp256k1_private.pem', 'sha384',
+     false);
+test('ec_secp256k1_public.pem', 'ec_secp256k1_private.pem', 'sha384',
+     false, { dsaEncoding: 'der' });
+
+// ECDSA w/ ieee-p1363 signature encoding
+test('ec_secp256k1_public.pem', 'ec_secp256k1_private.pem', 'sha384', false,
+     { dsaEncoding: 'ieee-p1363' });
+
+// DSA w/ der signature encoding
+test('dsa_public.pem', 'dsa_private.pem', 'sha256',
+     false);
+test('dsa_public.pem', 'dsa_private.pem', 'sha256',
+     false, { dsaEncoding: 'der' });
+
+// DSA w/ ieee-p1363 signature encoding
+test('dsa_public.pem', 'dsa_private.pem', 'sha256', false,
+     { dsaEncoding: 'ieee-p1363' });