From 341ee31e1537f18bd2d316ea8f3acc35bdb57d57 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 10 Mar 2021 16:26:14 -0800 Subject: [PATCH] crypto: reconcile duplicated code Signed-off-by: James M Snell PR-URL: https://github.com/nodejs/node/pull/37704 Reviewed-By: Filip Skokan Reviewed-By: Antoine du Hamel --- src/crypto/crypto_ec.cc | 114 --------------------------------------- src/crypto/crypto_ec.h | 9 ---- src/crypto/crypto_sig.cc | 67 +++++++++++++++++------ 3 files changed, 50 insertions(+), 140 deletions(-) diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index b97670b54a62fc..3e2c2031adb8e4 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -927,119 +927,5 @@ size_t GroupOrderSize(ManagedEVPPKey key) { CHECK(EC_GROUP_get_order(group, order.get(), nullptr)); 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( - const ManagedEVPPKey& key, - const ByteSource& signature) { - const unsigned char* data = - reinterpret_cast(signature.get()); - - ECDSASigPointer ecsig; - DsaSigPointer dsasig; - const BIGNUM* pr; - const BIGNUM* 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); - - char* outdata = MallocOpenSSL(len * 2); - memset(outdata, 0, len * 2); - ByteSource out = ByteSource::Allocated(outdata, len * 2); - unsigned char* ptr = reinterpret_cast(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( - const ManagedEVPPKey& key, - const ByteSource& signature) { - BignumPointer r(BN_new()); - BignumPointer s(BN_new()); - const unsigned char* sig = signature.data(); - - switch (EVP_PKEY_id(key.get())) { - case EVP_PKEY_EC: { - size_t order_size_bytes = GroupOrderSize(key); - - // If the size of the signature is incorrect, verification - // will fail. - if (signature.size() != 2 * order_size_bytes) - return ByteSource(); // Empty! - - ECDSASigPointer ecsig(ECDSA_SIG_new()); - if (!ecsig) - 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(size); - unsigned char* ptr = reinterpret_cast(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_DSA_SIG(dsasig.get(), nullptr); - char* data = MallocOpenSSL(size); - unsigned char* ptr = reinterpret_cast(data); - CHECK_EQ(i2d_DSA_SIG(dsasig.get(), &ptr), size); - return ByteSource::Allocated(data, size); - } - default: - UNREACHABLE(); - } -} - } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_ec.h b/src/crypto/crypto_ec.h index e23b28571b4feb..444fca58d49779 100644 --- a/src/crypto/crypto_ec.h +++ b/src/crypto/crypto_ec.h @@ -162,15 +162,6 @@ v8::Maybe GetEcKeyDetail( Environment* env, std::shared_ptr key, v8::Local target); - -ByteSource ConvertToWebCryptoSignature( - const ManagedEVPPKey& key, - const ByteSource& signature); - -ByteSource ConvertFromWebCryptoSignature( - const ManagedEVPPKey& key, - const ByteSource& signature); - } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 75fa603207cf1f..946ac250f2fa01 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -117,6 +117,21 @@ unsigned int GetBytesOfRS(const ManagedEVPPKey& pkey) { return (bits + 7) / 8; } +bool ExtractP1363( + const unsigned char* sig_data, + unsigned char* out, + size_t len, + size_t n) { + ECDSASigPointer asn1_sig(d2i_ECDSA_SIG(nullptr, &sig_data, len)); + if (!asn1_sig) + return false; + + const BIGNUM* pr = ECDSA_SIG_get0_r(asn1_sig.get()); + const BIGNUM* ps = ECDSA_SIG_get0_s(asn1_sig.get()); + + return BN_bn2binpad(pr, out, n) > 0 && BN_bn2binpad(ps, out + n, n) > 0; +} + // Returns the maximum size of each of the integers (r, s) of the DSA signature. AllocatedBuffer ConvertSignatureToP1363(Environment* env, const ManagedEVPPKey& pkey, @@ -128,33 +143,49 @@ AllocatedBuffer ConvertSignatureToP1363(Environment* env, const unsigned char* sig_data = reinterpret_cast(signature.data()); - ECDSASigPointer asn1_sig(d2i_ECDSA_SIG(nullptr, &sig_data, signature.size())); - if (!asn1_sig) - return AllocatedBuffer(); - AllocatedBuffer buf = AllocatedBuffer::AllocateManaged(env, 2 * n); unsigned char* data = reinterpret_cast(buf.data()); - const BIGNUM* r = ECDSA_SIG_get0_r(asn1_sig.get()); - const BIGNUM* s = ECDSA_SIG_get0_s(asn1_sig.get()); - CHECK_EQ(n, static_cast(BN_bn2binpad(r, data, n))); - CHECK_EQ(n, static_cast(BN_bn2binpad(s, data + n, n))); + if (!ExtractP1363(sig_data, data, signature.size(), n)) + return std::move(signature); return buf; } +// Returns the maximum size of each of the integers (r, s) of the DSA signature. +ByteSource ConvertSignatureToP1363( + Environment* env, + const ManagedEVPPKey& pkey, + const ByteSource& signature) { + unsigned int n = GetBytesOfRS(pkey); + if (n == kNoDsaSignature) + return ByteSource(); + + const unsigned char* sig_data = + reinterpret_cast(signature.get()); + + char* outdata = MallocOpenSSL(n * 2); + memset(outdata, 0, n * 2); + ByteSource out = ByteSource::Allocated(outdata, n * 2); + unsigned char* ptr = reinterpret_cast(outdata); + + if (!ExtractP1363(sig_data, ptr, signature.size(), n)) + return ByteSource(); + + return out; +} ByteSource ConvertSignatureToDER( const ManagedEVPPKey& pkey, - const ArrayBufferOrViewContents& signature) { + ByteSource&& out) { unsigned int n = GetBytesOfRS(pkey); if (n == kNoDsaSignature) - return signature.ToByteSource(); + return std::move(out); const unsigned char* sig_data = - reinterpret_cast(signature.data()); + reinterpret_cast(out.get()); - if (signature.size() != 2 * n) + if (out.size() != 2 * n) return ByteSource(); ECDSASigPointer asn1_sig(ECDSA_SIG_new()); @@ -511,7 +542,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { ByteSource signature = hbuf.ToByteSource(); if (dsa_sig_enc == kSigEncP1363) { - signature = ConvertSignatureToDER(pkey, hbuf); + signature = ConvertSignatureToDER(pkey, hbuf.ToByteSource()); if (signature.get() == nullptr) return crypto::CheckThrow(env, Error::kSignMalformedSignature); } @@ -657,7 +688,7 @@ void Verify::VerifySync(const FunctionCallbackInfo& args) { ByteSource sig_bytes = ByteSource::Foreign(sig.data(), sig.size()); if (dsa_sig_enc == kSigEncP1363) { - sig_bytes = ConvertSignatureToDER(key, sig); + sig_bytes = ConvertSignatureToDER(key, sig.ToByteSource()); if (!sig_bytes) return crypto::CheckThrow(env, SignBase::Error::kSignMalformedSignature); } @@ -778,7 +809,7 @@ Maybe SignTraits::AdditionalConfig( Mutex::ScopedLock lock(*m_pkey.mutex()); if (UseP1363Encoding(m_pkey, params->dsa_encoding)) { params->signature = - ConvertFromWebCryptoSignature(m_pkey, signature.ToByteSource()); + ConvertSignatureToDER(m_pkey, signature.ToByteSource()); } else { params->signature = mode == kCryptoJobAsync ? signature.ToCopy() @@ -874,8 +905,10 @@ bool SignTraits::DeriveBits( return false; if (UseP1363Encoding(m_pkey, params.dsa_encoding)) { - *out = ConvertToWebCryptoSignature( - params.key->GetAsymmetricKey(), buf); + *out = ConvertSignatureToP1363( + env, + params.key->GetAsymmetricKey(), + buf); } else { buf.Resize(len); *out = std::move(buf);