fixup! crypto: add optional callback to crypto.sign and crypto.verify

nodejs · Mar 8, 2021 · b7dead3 · b7dead3
1 parent f85e838
commit b7dead3
Show file tree

Hide file tree

Showing 10 changed files with 147 additions and 65 deletions.
diff --git 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,8 +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
- kSigEncDER, // TODO(@jasnell): revise the inconsistency with WebCrypto's EC
- signature));
+ signature,
+ kSigEncDER));
 }
 
 module.exports = {

diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -435,8 +435,8 @@ function ecdsaSignVerify(key, data, { name, hash }, signature) {
  hashname,
  undefined, // Salt length, not used with ECDSA
  undefined, // PSS Padding, not used with ECDSA
- kSigEncP1363,
- signature));
+ signature,
+ kSigEncP1363));
 }
 
 module.exports = {

diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js
@@ -360,7 +360,6 @@ function rsaSignVerify(key, data, { saltLength }, signature) {
  normalizeHashName(key.algorithm.hash.name),
  saltLength,
  padding,
- undefined, // EC(DSA) Signature Encoding is not used in RSA
  signature));
 }
 

diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js
@@ -183,7 +183,6 @@ function signOneShot(algorithm, data, key, callback) {
  keyData = createPrivateKey(key)[kHandle];
  }
 
- // TODO: add dsaSigEnc to SignJob
  // TODO: recognize pssSaltLength RSA_PSS_SALTLEN_* constants in SignJob
  const job = new SignJob(
  kCryptoJobAsync,
@@ -193,6 +192,7 @@ function signOneShot(algorithm, data, key, callback) {
  algorithm,
  pssSaltLength,
  rsaPadding,
+ undefined,
  dsaSigEnc);
 
  job.ondone = (error, signature) => {
@@ -295,7 +295,6 @@ function verifyOneShot(algorithm, data, key, signature, callback) {
  keyData = createPublicKey(key)[kHandle];
  }
 
- // TODO: add dsaSigEnc to SignJob
  // TODO: recognize pssSaltLength RSA_PSS_SALTLEN_* constants in SignJob
  const job = new SignJob(
  kCryptoJobAsync,
@@ -305,8 +304,8 @@ function verifyOneShot(algorithm, data, key, signature, callback) {
  algorithm,
  pssSaltLength,
  rsaPadding,
- dsaSigEnc,
- signature);
+ signature,
+ dsaSigEnc);
 
  job.ondone = (error, result) => {
  if (error) return FunctionPrototypeCall(callback, job, error);

diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc
@@ -926,62 +926,114 @@ 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;
 
- if (!BN_bn2binpad(pr, ptr, order_size_bytes) ||
- !BN_bn2binpad(ps, ptr + order_size_bytes, order_size_bytes)) {
+ 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 = BN_num_bytes(pr);
+ }
+ }
+
+ CHECK_GT(len, 0);
+
+ char* outdata = MallocOpenSSL<char>(len * 2);
+ ByteSource out = ByteSource::Allocated(outdata, len * 2);
+ unsigned char* ptr = reinterpret_cast<unsigned char*>(outdata);
+
+ if (!BN_bn2binpad(pr, ptr, len) || !BN_bn2binpad(ps, ptr + len, len)) {
  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
@@ -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
@@ -233,6 +233,14 @@ bool IsOneShot(const ManagedEVPPKey& key) {
  return false;
  }
 }
+
+bool UseP1363Encoding(const SignConfiguration& params) {
+ switch (EVP_PKEY_id(params.key->GetAsymmetricKey().get())) {
+ case EVP_PKEY_EC:
+ case EVP_PKEY_DSA: return params.dsa_encoding == kSigEncP1363;
+ }
+ return false;
+}
 } // namespace
 
 SignBase::Error SignBase::Init(const char* sign_type) {
@@ -297,6 +305,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);
 }
 
@@ -681,7 +691,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 {
@@ -744,6 +755,16 @@ Maybe<bool> SignTraits::AdditionalConfig(
  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())) {
@@ -754,7 +775,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(*params)) {
  params->signature =
  ConvertFromWebCryptoSignature(m_pkey, signature.ToByteSource());
  } else {
@@ -851,9 +872,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(params)) {
  *out = ConvertToWebCryptoSignature(
  params.key->GetAsymmetricKey(), buf);
  } else {

diff --git 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
@@ -70,7 +70,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