From 18ec8a84be650869a2e53e32fde75a6814acfab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Wed, 21 Aug 2019 00:05:55 +0200 Subject: [PATCH] crypto: add support for IEEE-P1363 DSA signatures PR-URL: https://github.com/nodejs/node/pull/29292 Reviewed-By: Ben Noordhuis Reviewed-By: Daniel Bevenius Reviewed-By: James M Snell --- doc/api/crypto.md | 18 +++ lib/internal/crypto/sig.js | 37 +++++- src/node_crypto.cc | 136 ++++++++++++++++++++++- src/node_crypto.h | 20 +++- test/parallel/test-crypto-sign-verify.js | 90 +++++++++++++-- 5 files changed, 277 insertions(+), 24 deletions(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index 91eefa7f161119..6bf1dcea365d93 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1405,6 +1405,7 @@ changes: --> * `privateKey` {Object | string | Buffer | KeyObject} + * `dsaEncoding` {string} * `padding` {integer} * `saltLength` {integer} * `outputEncoding` {string} The [encoding][] of the return value. @@ -1417,6 +1418,10 @@ If `privateKey` is not a [`KeyObject`][], this function behaves as if `privateKey` had been passed to [`crypto.createPrivateKey()`][]. If it is an object, the following additional properties can be passed: +* `dsaEncoding` {string} For DSA and ECDSA, this option specifies the + format of the generated signature. It can be one of the following: + * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. + * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. * `padding` {integer} Optional padding value for RSA, one of the following: * `crypto.constants.RSA_PKCS1_PADDING` (default) * `crypto.constants.RSA_PKCS1_PSS_PADDING` @@ -1513,6 +1518,7 @@ changes: --> * `object` {Object | string | Buffer | KeyObject} + * `dsaEncoding` {string} * `padding` {integer} * `saltLength` {integer} * `signature` {string | Buffer | TypedArray | DataView} @@ -1526,6 +1532,10 @@ If `object` is not a [`KeyObject`][], this function behaves as if `object` had been passed to [`crypto.createPublicKey()`][]. If it is an object, the following additional properties can be passed: +* `dsaEncoding` {string} For DSA and ECDSA, this option specifies the + format of the generated signature. It can be one of the following: + * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. + * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. * `padding` {integer} Optional padding value for RSA, one of the following: * `crypto.constants.RSA_PKCS1_PADDING` (default) * `crypto.constants.RSA_PKCS1_PSS_PADDING` @@ -2891,6 +2901,10 @@ If `key` is not a [`KeyObject`][], this function behaves as if `key` had been passed to [`crypto.createPrivateKey()`][]. If it is an object, the following additional properties can be passed: +* `dsaEncoding` {string} For DSA and ECDSA, this option specifies the + format of the generated signature. It can be one of the following: + * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. + * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. * `padding` {integer} Optional padding value for RSA, one of the following: * `crypto.constants.RSA_PKCS1_PADDING` (default) * `crypto.constants.RSA_PKCS1_PSS_PADDING` @@ -2944,6 +2958,10 @@ If `key` is not a [`KeyObject`][], this function behaves as if `key` had been passed to [`crypto.createPublicKey()`][]. If it is an object, the following additional properties can be passed: +* `dsaEncoding` {string} For DSA and ECDSA, this option specifies the + format of the generated signature. It can be one of the following: + * `'der'` (default): DER-encoded ASN.1 signature structure encoding `(r, s)`. + * `'ieee-p1363'`: Signature format `r || s` as proposed in IEEE-P1363. * `padding` {integer} Optional padding value for RSA, one of the following: * `crypto.constants.RSA_PKCS1_PADDING` (default) * `crypto.constants.RSA_PKCS1_PSS_PADDING` diff --git a/lib/internal/crypto/sig.js b/lib/internal/crypto/sig.js index 9b9c32e59c8fd0..6eda8455643848 100644 --- a/lib/internal/crypto/sig.js +++ b/lib/internal/crypto/sig.js @@ -11,6 +11,8 @@ const { validateString } = require('internal/validators'); const { Sign: _Sign, Verify: _Verify, + kSigEncDER, + kSigEncP1363, signOneShot: _signOneShot, verifyOneShot: _verifyOneShot } = internalBinding('crypto'); @@ -59,6 +61,20 @@ function getSaltLength(options) { return getIntOption('saltLength', options); } +function getDSASignatureEncoding(options) { + if (typeof options === 'object') { + const { dsaEncoding = 'der' } = options; + if (dsaEncoding === 'der') + return kSigEncDER; + else if (dsaEncoding === 'ieee-p1363') + return kSigEncP1363; + else + throw new ERR_INVALID_OPT_VALUE('dsaEncoding', dsaEncoding); + } + + return kSigEncDER; +} + function getIntOption(name, options) { const value = options[name]; if (value !== undefined) { @@ -81,8 +97,11 @@ Sign.prototype.sign = function sign(options, encoding) { const rsaPadding = getPadding(options); const pssSaltLength = getSaltLength(options); + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(options); + const ret = this[kHandle].sign(data, format, type, passphrase, rsaPadding, - pssSaltLength); + pssSaltLength, dsaSigEnc); encoding = encoding || getDefaultEncoding(); if (encoding && encoding !== 'buffer') @@ -117,8 +136,11 @@ function signOneShot(algorithm, data, key) { const rsaPadding = getPadding(key); const pssSaltLength = getSaltLength(key); + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(key); + return _signOneShot(keyData, keyFormat, keyType, keyPassphrase, data, - algorithm, rsaPadding, pssSaltLength); + algorithm, rsaPadding, pssSaltLength, dsaSigEnc); } function Verify(algorithm, options) { @@ -149,13 +171,15 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) { // Options specific to RSA const rsaPadding = getPadding(options); - const pssSaltLength = getSaltLength(options); + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(options); + signature = getArrayBufferView(signature, 'signature', sigEncoding); return this[kHandle].verify(data, format, type, passphrase, signature, - rsaPadding, pssSaltLength); + rsaPadding, pssSaltLength, dsaSigEnc); }; function verifyOneShot(algorithm, data, key, signature) { @@ -181,6 +205,9 @@ function verifyOneShot(algorithm, data, key, signature) { const rsaPadding = getPadding(key); const pssSaltLength = getSaltLength(key); + // Options specific to (EC)DSA + const dsaSigEnc = getDSASignatureEncoding(key); + if (!isArrayBufferView(signature)) { throw new ERR_INVALID_ARG_TYPE( 'signature', @@ -190,7 +217,7 @@ function verifyOneShot(algorithm, data, key, signature) { } return _verifyOneShot(keyData, keyFormat, keyType, keyPassphrase, signature, - data, algorithm, rsaPadding, pssSaltLength); + data, algorithm, rsaPadding, pssSaltLength, dsaSigEnc); } module.exports = { diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 20cc52fbff7910..fa85f7855371b5 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4910,6 +4910,9 @@ void CheckThrow(Environment* env, SignBase::Error error) { case SignBase::Error::kSignNotInitialised: return env->ThrowError("Not initialised"); + case SignBase::Error::kSignMalformedSignature: + return env->ThrowError("Malformed signature"); + case SignBase::Error::kSignInit: case SignBase::Error::kSignUpdate: case SignBase::Error::kSignPrivateKey: @@ -5007,6 +5010,89 @@ static int GetDefaultSignPadding(const ManagedEVPPKey& key) { RSA_PKCS1_PADDING; } +static const unsigned int kNoDsaSignature = static_cast(-1); + +// Returns the maximum size of each of the integers (r, s) of the DSA signature. +static unsigned int GetBytesOfRS(const ManagedEVPPKey& pkey) { + int bits, base_id = EVP_PKEY_base_id(pkey.get()); + + if (base_id == EVP_PKEY_DSA) { + DSA* dsa_key = EVP_PKEY_get0_DSA(pkey.get()); + // Both r and s are computed mod q, so their width is limited by that of q. + bits = BN_num_bits(DSA_get0_q(dsa_key)); + } else if (base_id == EVP_PKEY_EC) { + EC_KEY* ec_key = EVP_PKEY_get0_EC_KEY(pkey.get()); + const EC_GROUP* ec_group = EC_KEY_get0_group(ec_key); + bits = EC_GROUP_order_bits(ec_group); + } else { + return kNoDsaSignature; + } + + return (bits + 7) / 8; +} + +static AllocatedBuffer ConvertSignatureToP1363(Environment* env, + const ManagedEVPPKey& pkey, + AllocatedBuffer&& signature) { + unsigned int n = GetBytesOfRS(pkey); + if (n == kNoDsaSignature) + return std::move(signature); + + const unsigned char* sig_data = + reinterpret_cast(signature.data()); + + ECDSA_SIG* asn1_sig = d2i_ECDSA_SIG(nullptr, &sig_data, signature.size()); + if (asn1_sig == nullptr) + return AllocatedBuffer(); + + AllocatedBuffer buf = env->AllocateManaged(2 * n); + unsigned char* data = reinterpret_cast(buf.data()); + + const BIGNUM* r = ECDSA_SIG_get0_r(asn1_sig); + const BIGNUM* s = ECDSA_SIG_get0_s(asn1_sig); + CHECK_EQ(n, BN_bn2binpad(r, data, n)); + CHECK_EQ(n, BN_bn2binpad(s, data + n, n)); + + ECDSA_SIG_free(asn1_sig); + + return buf; +} + +static ByteSource ConvertSignatureToDER( + const ManagedEVPPKey& pkey, + const ArrayBufferViewContents& signature) { + unsigned int n = GetBytesOfRS(pkey); + if (n == kNoDsaSignature) + return ByteSource::Foreign(signature.data(), signature.length()); + + const unsigned char* sig_data = + reinterpret_cast(signature.data()); + + if (signature.length() != 2 * n) + return ByteSource(); + + ECDSA_SIG* asn1_sig = ECDSA_SIG_new(); + CHECK_NOT_NULL(asn1_sig); + BIGNUM* r = BN_new(); + CHECK_NOT_NULL(r); + BIGNUM* s = BN_new(); + CHECK_NOT_NULL(s); + CHECK_EQ(r, BN_bin2bn(sig_data, n, r)); + CHECK_EQ(s, BN_bin2bn(sig_data + n, n, s)); + CHECK_EQ(1, ECDSA_SIG_set0(asn1_sig, r, s)); + + unsigned char* data = nullptr; + int len = i2d_ECDSA_SIG(asn1_sig, &data); + ECDSA_SIG_free(asn1_sig); + + if (len <= 0) + return ByteSource(); + + CHECK_NOT_NULL(data); + + return ByteSource::Allocated(reinterpret_cast(data), len); +} + static AllocatedBuffer Node_SignFinal(Environment* env, EVPMDPointer&& mdctx, const ManagedEVPPKey& pkey, @@ -5066,7 +5152,8 @@ static inline bool ValidateDSAParameters(EVP_PKEY* key) { Sign::SignResult Sign::SignFinal( const ManagedEVPPKey& pkey, int padding, - const Maybe& salt_len) { + const Maybe& salt_len, + DSASigEnc dsa_sig_enc) { if (!mdctx_) return SignResult(kSignNotInitialised); @@ -5078,6 +5165,10 @@ Sign::SignResult Sign::SignFinal( AllocatedBuffer buffer = Node_SignFinal(env(), std::move(mdctx), pkey, padding, salt_len); Error error = buffer.data() == nullptr ? kSignPrivateKey : kSignOk; + if (error == kSignOk && dsa_sig_enc == kSigEncP1363) { + buffer = ConvertSignatureToP1363(env(), pkey, std::move(buffer)); + CHECK_NOT_NULL(buffer.data()); + } return SignResult(error, std::move(buffer)); } @@ -5105,10 +5196,15 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { salt_len = Just(args[offset + 1].As()->Value()); } + CHECK(args[offset + 2]->IsInt32()); + DSASigEnc dsa_sig_enc = + static_cast(args[offset + 2].As()->Value()); + SignResult ret = sign->SignFinal( key, padding, - salt_len); + salt_len, + dsa_sig_enc); if (ret.error != kSignOk) return sign->CheckThrow(ret.error); @@ -5152,6 +5248,10 @@ void SignOneShot(const FunctionCallbackInfo& args) { 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 || @@ -5179,6 +5279,10 @@ void SignOneShot(const FunctionCallbackInfo& args) { signature.Resize(sig_len); + if (dsa_sig_enc == kSigEncP1363) { + signature = ConvertSignatureToP1363(env, key, std::move(signature)); + } + args.GetReturnValue().Set(signature.ToBuffer().ToLocalChecked()); } @@ -5284,6 +5388,17 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { salt_len = Just(args[offset + 2].As()->Value()); } + CHECK(args[offset + 3]->IsInt32()); + DSASigEnc dsa_sig_enc = + static_cast(args[offset + 3].As()->Value()); + + ByteSource signature = ByteSource::Foreign(hbuf.data(), hbuf.length()); + if (dsa_sig_enc == kSigEncP1363) { + signature = ConvertSignatureToDER(pkey, hbuf); + if (signature.get() == nullptr) + return verify->CheckThrow(Error::kSignMalformedSignature); + } + bool verify_result; Error err = verify->VerifyFinal(pkey, hbuf.data(), hbuf.length(), padding, salt_len, &verify_result); @@ -5327,6 +5442,10 @@ void VerifyOneShot(const FunctionCallbackInfo& args) { 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 || @@ -5337,11 +5456,18 @@ void VerifyOneShot(const FunctionCallbackInfo& args) { if (!ApplyRSAOptions(key, pkctx, rsa_padding, rsa_salt_len)) return CheckThrow(env, SignBase::Error::kSignPublicKey); + ByteSource sig_bytes = ByteSource::Foreign(sig.data(), sig.length()); + if (dsa_sig_enc == kSigEncP1363) { + sig_bytes = ConvertSignatureToDER(key, sig); + if (!sig_bytes) + return CheckThrow(env, SignBase::Error::kSignMalformedSignature); + } + bool verify_result; const int r = EVP_DigestVerify( mdctx.get(), - reinterpret_cast(sig.data()), - sig.length(), + reinterpret_cast(sig_bytes.get()), + sig_bytes.size(), reinterpret_cast(data.data()), data.length()); switch (r) { @@ -7129,6 +7255,8 @@ void Initialize(Local target, NODE_DEFINE_CONSTANT(target, kKeyTypeSecret); NODE_DEFINE_CONSTANT(target, kKeyTypePublic); NODE_DEFINE_CONSTANT(target, kKeyTypePrivate); + NODE_DEFINE_CONSTANT(target, kSigEncDER); + NODE_DEFINE_CONSTANT(target, kSigEncP1363); env->SetMethod(target, "randomBytes", RandomBytes); env->SetMethod(target, "signOneShot", SignOneShot); env->SetMethod(target, "verifyOneShot", VerifyOneShot); diff --git a/src/node_crypto.h b/src/node_crypto.h index 777ba5d302a536..56a9ad3104dbeb 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -326,6 +326,13 @@ class ByteSource { const char* get() const; size_t size() const; + inline operator bool() const { + return data_ != nullptr; + } + + static ByteSource Allocated(char* data, size_t size); + static ByteSource Foreign(const char* data, size_t size); + static ByteSource FromStringOrBuffer(Environment* env, v8::Local value); @@ -350,9 +357,6 @@ class ByteSource { size_t size_ = 0; ByteSource(const char* data, char* allocated_data, size_t size); - - static ByteSource Allocated(char* data, size_t size); - static ByteSource Foreign(const char* data, size_t size); }; enum PKEncodingType { @@ -628,7 +632,8 @@ class SignBase : public BaseObject { kSignNotInitialised, kSignUpdate, kSignPrivateKey, - kSignPublicKey + kSignPublicKey, + kSignMalformedSignature } Error; SignBase(Environment* env, v8::Local wrap) @@ -649,6 +654,10 @@ class SignBase : public BaseObject { EVPMDPointer mdctx_; }; +enum DSASigEnc { + kSigEncDER, kSigEncP1363 +}; + class Sign : public SignBase { public: static void Initialize(Environment* env, v8::Local target); @@ -666,7 +675,8 @@ class Sign : public SignBase { SignResult SignFinal( const ManagedEVPPKey& pkey, int padding, - const v8::Maybe& saltlen); + const v8::Maybe& saltlen, + DSASigEnc dsa_sig_enc); protected: static void New(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-crypto-sign-verify.js b/test/parallel/test-crypto-sign-verify.js index 66c7ac7d8014df..a16d25f540e1a8 100644 --- a/test/parallel/test-crypto-sign-verify.js +++ b/test/parallel/test-crypto-sign-verify.js @@ -500,21 +500,91 @@ common.expectsError( }); { - const privKey = fixtures.readKey('ec-key.pem'); const data = Buffer.from('Hello world'); - [ - crypto.createSign('sha1').update(data).sign(privKey), - crypto.sign('sha1', data, privKey) - ].forEach((sig) => { - // Signature length variability due to DER encoding - assert.strictEqual(sig.length >= 68, true); + const keys = [['ec-key.pem', 64], ['dsa_private_1025.pem', 40]]; + + for (const [file, length] of keys) { + const privKey = fixtures.readKey(file); + [ + crypto.createSign('sha1').update(data).sign(privKey), + crypto.sign('sha1', data, privKey), + crypto.sign('sha1', data, { key: privKey, dsaEncoding: 'der' }) + ].forEach((sig) => { + // Signature length variability due to DER encoding + assert(sig.length >= length + 4 && sig.length <= length + 8); + + assert.strictEqual( + crypto.createVerify('sha1').update(data).verify(privKey, sig), + true + ); + assert.strictEqual(crypto.verify('sha1', data, privKey, sig), true); + }); + // Test (EC)DSA signature conversion. + const opts = { key: privKey, dsaEncoding: 'ieee-p1363' }; + let sig = crypto.sign('sha1', data, opts); + // Unlike DER signatures, IEEE P1363 signatures have a predictable length. + assert.strictEqual(sig.length, length); + assert.strictEqual(crypto.verify('sha1', data, opts, sig), true); + + // Test invalid signature lengths. + for (const i of [-2, -1, 1, 2, 4, 8]) { + sig = crypto.randomBytes(length + i); + common.expectsError(() => { + crypto.verify('sha1', data, opts, sig); + }, { + message: 'Malformed signature' + }); + } + } + + // Test verifying externally signed messages. + const extSig = Buffer.from('494c18ab5c8a62a72aea5041966902bcfa229821af2bf65' + + '0b5b4870d1fe6aebeaed9460c62210693b5b0a300033823' + + '33d9529c8abd8c5948940af944828be16c', 'hex'); + for (const ok of [true, false]) { assert.strictEqual( - crypto.createVerify('sha1').update(data).verify(privKey, sig), - true + crypto.verify('sha256', data, { + key: fixtures.readKey('ec-key.pem'), + dsaEncoding: 'ieee-p1363' + }, extSig), + ok ); - assert.strictEqual(crypto.verify('sha1', data, privKey, sig), true); + + extSig[Math.floor(Math.random() * extSig.length)] ^= 1; + } + + // Non-(EC)DSA keys should ignore the option. + const sig = crypto.sign('sha1', data, { + key: keyPem, + dsaEncoding: 'ieee-p1363' }); + assert.strictEqual(crypto.verify('sha1', data, certPem, sig), true); + assert.strictEqual( + crypto.verify('sha1', data, { + key: certPem, + dsaEncoding: 'ieee-p1363' + }, sig), + true + ); + assert.strictEqual( + crypto.verify('sha1', data, { + key: certPem, + dsaEncoding: 'der' + }, sig), + true + ); + + for (const dsaEncoding of ['foo', null, {}, 5, true, NaN]) { + common.expectsError(() => { + crypto.sign('sha1', data, { + key: certPem, + dsaEncoding + }); + }, { + code: 'ERR_INVALID_OPT_VALUE' + }); + } }