From b2f8ba712372006f47b97e265458686f34f16634 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 19 Mar 2021 17:08:51 +0100 Subject: [PATCH] crypto: reconsile oneshot sign/verify sync and async implementations --- benchmark/crypto/oneshot-sign-verify.js | 51 +++++ lib/internal/crypto/dsa.js | 7 +- lib/internal/crypto/ec.js | 7 +- lib/internal/crypto/rsa.js | 4 + lib/internal/crypto/sig.js | 91 ++++----- src/crypto/crypto_sig.cc | 233 ++++------------------- src/crypto/crypto_sig.h | 2 +- test/parallel/test-crypto-sign-verify.js | 14 +- 8 files changed, 151 insertions(+), 258 deletions(-) create mode 100644 benchmark/crypto/oneshot-sign-verify.js diff --git a/benchmark/crypto/oneshot-sign-verify.js b/benchmark/crypto/oneshot-sign-verify.js new file mode 100644 index 00000000000000..7b5a5189ece650 --- /dev/null +++ b/benchmark/crypto/oneshot-sign-verify.js @@ -0,0 +1,51 @@ +'use strict'; + +const common = require('../common.js'); +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); +const fixtures_keydir = path.resolve(__dirname, '../../test/fixtures/keys/'); +const keys = { + publicKey: crypto.createPublicKey( + fs.readFileSync(`${fixtures_keydir}/ed25519_public.pem`)), + privateKey: crypto.createPrivateKey( + fs.readFileSync(`${fixtures_keydir}/ed25519_private.pem`)), +}; + +const data = crypto.randomBytes(256); +const args = [null, data]; + +const bench = common.createBenchmark(main, { + sync: [0, 1], + n: [1e4], +}); + +function measureSync(n) { + bench.start(); + for (let i = 0; i < n; ++i) { + crypto.verify(...args, keys.publicKey, + crypto.sign(...args, keys.privateKey)); + } + bench.end(n); +} + +function measureAsync(n) { + let remaining = n; + function done() { + if (--remaining === 0) + bench.end(n); + } + bench.start(); + for (let i = 0; i < n; ++i) { + crypto.sign(...args, keys.privateKey, (err, signature) => { + crypto.verify(...args, keys.publicKey, signature, done); + }); + } +} + +function main({ n, sync }) { + if (sync) + measureSync(n); + else + measureAsync(n); +} diff --git a/lib/internal/crypto/dsa.js b/lib/internal/crypto/dsa.js index c2a528aa94e056..b615c3a9cb932f 100644 --- a/lib/internal/crypto/dsa.js +++ b/lib/internal/crypto/dsa.js @@ -251,12 +251,15 @@ function dsaSignVerify(key, data, algorithm, signature) { kCryptoJobAsync, signature === undefined ? kSignJobModeSign : kSignJobModeVerify, key[kKeyObject][kHandle], + undefined, + undefined, + undefined, data, normalizeHashName(key.algorithm.hash.name), undefined, // Salt-length is not used in DSA undefined, // Padding is not used in DSA - signature, - kSigEncDER)); + kSigEncDER, + signature)); } module.exports = { diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index 47e50f1b4fdfbb..ee14eed7d083a7 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -467,12 +467,15 @@ function ecdsaSignVerify(key, data, { name, hash }, signature) { kCryptoJobAsync, mode, key[kKeyObject][kHandle], + undefined, + undefined, + undefined, data, hashname, undefined, // Salt length, not used with ECDSA undefined, // PSS Padding, not used with ECDSA - signature, - kSigEncP1363)); + kSigEncP1363, + signature)); } module.exports = { diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 06a9e802717c54..3bca75dbd7970a 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -356,10 +356,14 @@ function rsaSignVerify(key, data, { saltLength }, signature) { kCryptoJobAsync, signature === undefined ? kSignJobModeSign : kSignJobModeVerify, key[kKeyObject][kHandle], + undefined, + undefined, + undefined, data, normalizeHashName(key.algorithm.hash.name), saltLength, padding, + undefined, signature)); } diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js index d5c4c46192cd17..a204f35e60a15b 100644 --- a/lib/internal/crypto/sig.js +++ b/lib/internal/crypto/sig.js @@ -24,9 +24,8 @@ const { Sign: _Sign, SignJob, Verify: _Verify, - signOneShot: _signOneShot, - verifyOneShot: _verifyOneShot, kCryptoJobAsync, + kCryptoJobSync, kSigEncDER, kSigEncP1363, kSignJobModeSign, @@ -40,10 +39,6 @@ const { } = require('internal/crypto/util'); const { - createPrivateKey, - createPublicKey, - isCryptoKey, - isKeyObject, preparePrivateKey, preparePublicOrPrivateKey, } = require('internal/crypto/keys'); @@ -162,38 +157,34 @@ function signOneShot(algorithm, data, key, callback) { // Options specific to (EC)DSA const dsaSigEnc = getDSASignatureEncoding(key); - 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 { + data: keyData, + format: keyFormat, + type: keyType, + passphrase: keyPassphrase + } = preparePrivateKey(key); const job = new SignJob( - kCryptoJobAsync, + callback ? kCryptoJobAsync : kCryptoJobSync, kSignJobModeSign, keyData, + keyFormat, + keyType, + keyPassphrase, data, algorithm, pssSaltLength, rsaPadding, - undefined, dsaSigEnc); + if (!callback) { + const { 0: err, 1: signature } = job.run(); + if (err !== undefined) + throw err; + + return Buffer.from(signature); + } + job.ondone = (error, signature) => { if (error) return FunctionPrototypeCall(callback, job, error); FunctionPrototypeCall(callback, job, null, Buffer.from(signature)); @@ -272,38 +263,34 @@ function verifyOneShot(algorithm, data, key, signature, callback) { ); } - 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 { + data: keyData, + format: keyFormat, + type: keyType, + passphrase: keyPassphrase + } = preparePublicOrPrivateKey(key); const job = new SignJob( - kCryptoJobAsync, + callback ? kCryptoJobAsync : kCryptoJobSync, kSignJobModeVerify, keyData, + keyFormat, + keyType, + keyPassphrase, data, algorithm, pssSaltLength, rsaPadding, - signature, - dsaSigEnc); + dsaSigEnc, + signature); + + if (!callback) { + const { 0: err, 1: result } = job.run(); + if (err !== undefined) + throw err; + + return result; + } job.ondone = (error, result) => { if (error) return FunctionPrototypeCall(callback, job, error); diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 24b1a7f4315f76..694126046c68f5 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -333,8 +333,6 @@ void Sign::Initialize(Environment* env, Local target) { env->SetConstructorFunction(target, "Sign", t); - env->SetMethod(target, "signOneShot", Sign::SignSync); - SignJob::Initialize(env, target); constexpr int kSignJobModeSign = SignConfiguration::kSign; @@ -452,8 +450,6 @@ void Verify::Initialize(Environment* env, Local target) { env->SetProtoMethod(t, "verify", VerifyFinal); env->SetConstructorFunction(target, "Verify", t); - - env->SetMethod(target, "verifyOneShot", Verify::VerifySync); } void Verify::New(const FunctionCallbackInfo& args) { @@ -560,165 +556,6 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } -void Sign::SignSync(const FunctionCallbackInfo& args) { - ClearErrorOnReturn clear_error_on_return; - Environment* env = Environment::GetCurrent(args); - - unsigned int offset = 0; - ManagedEVPPKey key = ManagedEVPPKey::GetPrivateKeyFromJs(args, &offset, true); - if (!key) - return; - - if (!ValidateDSAParameters(key.get())) - return crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); - - ArrayBufferOrViewContents data(args[offset]); - if (UNLIKELY(!data.CheckSizeInt32())) - return THROW_ERR_OUT_OF_RANGE(env, "data is too big"); - - const EVP_MD* md; - if (args[offset + 1]->IsNullOrUndefined()) { - md = nullptr; - } else { - const node::Utf8Value sign_type(args.GetIsolate(), args[offset + 1]); - md = EVP_get_digestbyname(*sign_type); - if (md == nullptr) - return crypto::CheckThrow(env, SignBase::Error::kSignUnknownDigest); - } - - int rsa_padding = GetDefaultSignPadding(key); - if (!args[offset + 2]->IsUndefined()) { - CHECK(args[offset + 2]->IsInt32()); - rsa_padding = args[offset + 2].As()->Value(); - } - - Maybe rsa_salt_len = Nothing(); - if (!args[offset + 3]->IsUndefined()) { - CHECK(args[offset + 3]->IsInt32()); - rsa_salt_len = Just(args[offset + 3].As()->Value()); - } - - CHECK(args[offset + 4]->IsInt32()); - DSASigEnc dsa_sig_enc = - static_cast(args[offset + 4].As()->Value()); - - EVP_PKEY_CTX* pkctx = nullptr; - EVPMDPointer mdctx(EVP_MD_CTX_new()); - - if (!mdctx || - !EVP_DigestSignInit(mdctx.get(), &pkctx, md, nullptr, key.get())) { - return crypto::CheckThrow(env, SignBase::Error::kSignInit); - } - - if (!ApplyRSAOptions(key, pkctx, rsa_padding, rsa_salt_len)) - return crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); - - const unsigned char* input = - reinterpret_cast(data.data()); - size_t sig_len; - if (!EVP_DigestSign(mdctx.get(), nullptr, &sig_len, input, data.size())) - return crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); - - AllocatedBuffer signature = AllocatedBuffer::AllocateManaged(env, sig_len); - if (!EVP_DigestSign(mdctx.get(), - reinterpret_cast(signature.data()), - &sig_len, - input, - data.size())) { - return crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); - } - - signature.Resize(sig_len); - - if (dsa_sig_enc == kSigEncP1363) { - signature = ConvertSignatureToP1363(env, key, std::move(signature)); - } - - args.GetReturnValue().Set(signature.ToBuffer().FromMaybe(Local())); -} - -void Verify::VerifySync(const FunctionCallbackInfo& args) { - ClearErrorOnReturn clear_error_on_return; - Environment* env = Environment::GetCurrent(args); - - unsigned int offset = 0; - ManagedEVPPKey key = - ManagedEVPPKey::GetPublicOrPrivateKeyFromJs(args, &offset); - if (!key) - return; - - ArrayBufferOrViewContents sig(args[offset]); - ArrayBufferOrViewContents data(args[offset + 1]); - - if (UNLIKELY(!sig.CheckSizeInt32())) - return THROW_ERR_OUT_OF_RANGE(env, "sig is too big"); - if (UNLIKELY(!data.CheckSizeInt32())) - return THROW_ERR_OUT_OF_RANGE(env, "data is too big"); - - const EVP_MD* md; - if (args[offset + 2]->IsNullOrUndefined()) { - md = nullptr; - } else { - const node::Utf8Value sign_type(args.GetIsolate(), args[offset + 2]); - md = EVP_get_digestbyname(*sign_type); - if (md == nullptr) - return crypto::CheckThrow(env, SignBase::Error::kSignUnknownDigest); - } - - int rsa_padding = GetDefaultSignPadding(key); - if (!args[offset + 3]->IsUndefined()) { - CHECK(args[offset + 3]->IsInt32()); - rsa_padding = args[offset + 3].As()->Value(); - } - - Maybe rsa_salt_len = Nothing(); - if (!args[offset + 4]->IsUndefined()) { - CHECK(args[offset + 4]->IsInt32()); - rsa_salt_len = Just(args[offset + 4].As()->Value()); - } - - CHECK(args[offset + 5]->IsInt32()); - DSASigEnc dsa_sig_enc = - static_cast(args[offset + 5].As()->Value()); - - EVP_PKEY_CTX* pkctx = nullptr; - EVPMDPointer mdctx(EVP_MD_CTX_new()); - if (!mdctx || - !EVP_DigestVerifyInit(mdctx.get(), &pkctx, md, nullptr, key.get())) { - return crypto::CheckThrow(env, SignBase::Error::kSignInit); - } - - if (!ApplyRSAOptions(key, pkctx, rsa_padding, rsa_salt_len)) - return crypto::CheckThrow(env, SignBase::Error::kSignPublicKey); - - ByteSource sig_bytes = ByteSource::Foreign(sig.data(), sig.size()); - if (dsa_sig_enc == kSigEncP1363) { - sig_bytes = ConvertSignatureToDER(key, sig.ToByteSource()); - if (!sig_bytes) - return crypto::CheckThrow(env, SignBase::Error::kSignMalformedSignature); - } - - bool verify_result; - const int r = EVP_DigestVerify( - mdctx.get(), - sig_bytes.data(), - sig_bytes.size(), - reinterpret_cast(data.data()), - data.size()); - switch (r) { - case 1: - verify_result = true; - break; - case 0: - verify_result = false; - break; - default: - return crypto::CheckThrow(env, SignBase::Error::kSignPublicKey); - } - - args.GetReturnValue().Set(verify_result); -} - SignConfiguration::SignConfiguration(SignConfiguration&& other) noexcept : job_mode(other.job_mode), mode(other.mode), @@ -739,7 +576,7 @@ SignConfiguration& SignConfiguration::operator=( } void SignConfiguration::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("key", key.get()); + tracker->TrackField("key", key); if (job_mode == kCryptoJobAsync) { tracker->TrackFieldWithSize("data", data.size()); tracker->TrackFieldWithSize("signature", signature.size()); @@ -751,21 +588,28 @@ Maybe SignTraits::AdditionalConfig( const FunctionCallbackInfo& args, unsigned int offset, SignConfiguration* params) { + ClearErrorOnReturn clear_error_on_return; Environment* env = Environment::GetCurrent(args); params->job_mode = mode; CHECK(args[offset]->IsUint32()); // Sign Mode - CHECK(args[offset + 1]->IsObject()); // Key params->mode = static_cast(args[offset].As()->Value()); - KeyObjectHandle* key; - ASSIGN_OR_RETURN_UNWRAP(&key, args[offset + 1], Nothing()); - params->key = key->Data(); + ManagedEVPPKey key; + unsigned int keyParamOffset = offset + 1; + if (params->mode == SignConfiguration::kVerify) { + key = ManagedEVPPKey::GetPublicOrPrivateKeyFromJs(args, &keyParamOffset); + } else { + key = ManagedEVPPKey::GetPrivateKeyFromJs(args, &keyParamOffset, true); + } + if (!key) + return Nothing(); + params->key = key; - ArrayBufferOrViewContents data(args[offset + 2]); + ArrayBufferOrViewContents data(args[offset + 5]); if (UNLIKELY(!data.CheckSizeInt32())) { THROW_ERR_OUT_OF_RANGE(env, "data is too big"); return Nothing(); @@ -774,8 +618,8 @@ Maybe SignTraits::AdditionalConfig( ? data.ToCopy() : data.ToByteSource(); - if (args[offset + 3]->IsString()) { - Utf8Value digest(env->isolate(), args[offset + 3]); + if (args[offset + 6]->IsString()) { + Utf8Value digest(env->isolate(), args[offset + 6]); params->digest = EVP_get_digestbyname(*digest); if (params->digest == nullptr) { THROW_ERR_CRYPTO_INVALID_DIGEST(env); @@ -783,18 +627,18 @@ Maybe SignTraits::AdditionalConfig( } } - if (args[offset + 4]->IsInt32()) { // Salt length + if (args[offset + 7]->IsInt32()) { // Salt length params->flags |= SignConfiguration::kHasSaltLength; - params->salt_length = args[offset + 4].As()->Value(); + params->salt_length = args[offset + 7].As()->Value(); } - if (args[offset + 5]->IsUint32()) { // Padding + if (args[offset + 8]->IsUint32()) { // Padding params->flags |= SignConfiguration::kHasPadding; - params->padding = args[offset + 5].As()->Value(); + params->padding = args[offset + 8].As()->Value(); } - if (args[offset + 7]->IsUint32()) { // DSA Encoding + if (args[offset + 9]->IsUint32()) { // DSA Encoding params->dsa_encoding = - static_cast(args[offset + 7].As()->Value()); + static_cast(args[offset + 9].As()->Value()); if (params->dsa_encoding != kSigEncDER && params->dsa_encoding != kSigEncP1363) { THROW_ERR_OUT_OF_RANGE(env, "invalid signature encoding"); @@ -803,18 +647,17 @@ Maybe SignTraits::AdditionalConfig( } if (params->mode == SignConfiguration::kVerify) { - ArrayBufferOrViewContents signature(args[offset + 6]); + ArrayBufferOrViewContents signature(args[offset + 10]); if (UNLIKELY(!signature.CheckSizeInt32())) { THROW_ERR_OUT_OF_RANGE(env, "signature is too big"); return Nothing(); } // If this is an EC key (assuming ECDSA) we need to convert the // the signature from WebCrypto format into DER format... - ManagedEVPPKey m_pkey = params->key->GetAsymmetricKey(); - Mutex::ScopedLock lock(*m_pkey.mutex()); - if (UseP1363Encoding(m_pkey, params->dsa_encoding)) { + Mutex::ScopedLock lock(*key.mutex()); + if (UseP1363Encoding(key, params->dsa_encoding)) { params->signature = - ConvertSignatureToDER(m_pkey, signature.ToByteSource()); + ConvertSignatureToDER(key, signature.ToByteSource()); } else { params->signature = mode == kCryptoJobAsync ? signature.ToCopy() @@ -829,56 +672,54 @@ bool SignTraits::DeriveBits( Environment* env, const SignConfiguration& params, ByteSource* out) { + ClearErrorOnReturn clear_error_on_return; EVPMDPointer context(EVP_MD_CTX_new()); EVP_PKEY_CTX* ctx = nullptr; - ManagedEVPPKey m_pkey = params.key->GetAsymmetricKey(); - Mutex::ScopedLock lock(*m_pkey.mutex()); + Mutex::ScopedLock lock(*params.key.mutex()); switch (params.mode) { case SignConfiguration::kSign: - CHECK_EQ(params.key->GetKeyType(), kKeyTypePrivate); if (!EVP_DigestSignInit( context.get(), &ctx, params.digest, nullptr, - m_pkey.get())) { - return false; + params.key.get())) { + crypto::CheckThrow(env, SignBase::Error::kSignInit); } break; case SignConfiguration::kVerify: - CHECK_NE(params.key->GetKeyType(), kKeyTypeSecret); if (!EVP_DigestVerifyInit( context.get(), &ctx, params.digest, nullptr, - m_pkey.get())) { - return false; + params.key.get())) { + crypto::CheckThrow(env, SignBase::Error::kSignInit); } break; } int padding = params.flags & SignConfiguration::kHasPadding ? params.padding - : GetDefaultSignPadding(m_pkey); + : GetDefaultSignPadding(params.key); Maybe salt_length = params.flags & SignConfiguration::kHasSaltLength ? Just(params.salt_length) : Nothing(); if (!ApplyRSAOptions( - m_pkey, + params.key, ctx, padding, salt_length)) { - return false; + crypto::CheckThrow(env, SignBase::Error::kSignPrivateKey); } switch (params.mode) { case SignConfiguration::kSign: { size_t len; unsigned char* data = nullptr; - if (IsOneShot(m_pkey)) { + if (IsOneShot(params.key)) { EVP_DigestSign( context.get(), nullptr, @@ -909,8 +750,8 @@ bool SignTraits::DeriveBits( if (!EVP_DigestSignFinal(context.get(), data, &len)) return false; - if (UseP1363Encoding(m_pkey, params.dsa_encoding)) { - *out = ConvertSignatureToP1363(env, m_pkey, buf); + if (UseP1363Encoding(params.key, params.dsa_encoding)) { + *out = ConvertSignatureToP1363(env, params.key, buf); } else { buf.Resize(len); *out = std::move(buf); diff --git a/src/crypto/crypto_sig.h b/src/crypto/crypto_sig.h index 45c3d344d92f33..fa44811c3ee44d 100644 --- a/src/crypto/crypto_sig.h +++ b/src/crypto/crypto_sig.h @@ -111,7 +111,7 @@ struct SignConfiguration final : public MemoryRetainer { CryptoJobMode job_mode; Mode mode; - std::shared_ptr key; + ManagedEVPPKey key; ByteSource data; ByteSource signature; const EVP_MD* digest = nullptr; diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index dc90e5b311bdf3..f68c18584d5c1d 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -528,11 +528,15 @@ assert.throws( // Test invalid signature lengths. for (const i of [-2, -1, 1, 2, 4, 8]) { sig = crypto.randomBytes(length + i); - assert.throws(() => { - crypto.verify('sha1', data, opts, sig); - }, { - message: 'Malformed signature' - }); + let result; + try { + result = crypto.verify('sha1', data, opts, sig); + } catch (err) { + assert.match(err.message, /asn1 encoding/); + assert.strictEqual(err.library, 'asn1 encoding routines'); + continue; + } + assert.strictEqual(result, false); } }