Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 10a7035

Browse filesBrowse files
davidbenMylesBorins
davidben
authored and
MylesBorins
committed
crypto: fix Node_SignFinal
PR #11705 switched Node away from using using OpenSSL's legacy EVP_Sign* and EVP_Verify* APIs. Instead, it computes a hash normally via EVP_Digest* and then uses EVP_PKEY_sign and EVP_PKEY_verify to verify the hash directly. This change corrects two problems: 1. The documentation still recommends the signature algorithm EVP_MD names of OpenSSL's legacy APIs. OpenSSL has since moved away from thosee, which is why ECDSA was strangely inconsistent. (This is why "ecdsa-with-SHA256" was missing.) 2. Node_SignFinal copied some code from EVP_SignFinal's internals. This is problematic for OpenSSL 1.1.0 and is missing a critical check that prevents pkey->pkey.ptr from being cast to the wrong type. To resolve this, remove the non-EVP_PKEY_sign codepath. This codepath is no longer necessary. PR #11705's verify half was already assuming all EVP_PKEYs supported EVP_PKEY_sign and EVP_PKEY_verify. Also, in the documentation, point users towards using hash function names which are more consisent. This avoids an ECDSA special-case and some strangeness around RSA-PSS ("RSA-SHA256" is the OpenSSL name of the sha256WithRSAEncryption OID which is not used for RSA-PSS). PR-URL: #15024 Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 9cebe82 commit 10a7035
Copy full SHA for 10a7035

File tree

Expand file treeCollapse file tree

10 files changed

+110
-99
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+110
-99
lines changed
Open diff view settings
Collapse file

‎benchmark/crypto/rsa-sign-verify-throughput.js‎

Copy file name to clipboardExpand all lines: benchmark/crypto/rsa-sign-verify-throughput.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ keylen_list.forEach(function(key) {
1818

1919
var bench = common.createBenchmark(main, {
2020
writes: [500],
21-
algo: ['RSA-SHA1', 'RSA-SHA224', 'RSA-SHA256', 'RSA-SHA384', 'RSA-SHA512'],
21+
algo: ['SHA1', 'SHA224', 'SHA256', 'SHA384', 'SHA512'],
2222
keylen: keylen_list,
2323
len: [1024, 102400, 2 * 102400, 3 * 102400, 1024 * 1024]
2424
});
Collapse file

‎doc/api/crypto.md‎

Copy file name to clipboardExpand all lines: doc/api/crypto.md
+19-21Lines changed: 19 additions & 21 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -842,28 +842,31 @@ of two ways:
842842
- Using the [`sign.update()`][] and [`sign.sign()`][] methods to produce the
843843
signature.
844844

845-
The [`crypto.createSign()`][] method is used to create `Sign` instances. `Sign`
846-
objects are not to be created directly using the `new` keyword.
845+
The [`crypto.createSign()`][] method is used to create `Sign` instances. The
846+
argument is the string name of the hash function to use. `Sign` objects are not
847+
to be created directly using the `new` keyword.
847848

848849
Example: Using `Sign` objects as streams:
849850

850851
```js
851852
const crypto = require('crypto');
852-
const sign = crypto.createSign('RSA-SHA256');
853+
const sign = crypto.createSign('SHA256');
853854

854855
sign.write('some data to sign');
855856
sign.end();
856857

857858
const privateKey = getPrivateKeySomehow();
858859
console.log(sign.sign(privateKey, 'hex'));
859-
// Prints: the calculated signature
860+
// Prints: the calculated signature using the specified private key and
861+
// SHA-256. For RSA keys, the algorithm is RSASSA-PKCS1-v1_5 (see padding
862+
// parameter below for RSASSA-PSS). For EC keys, the algorithm is ECDSA.
860863
```
861864

862865
Example: Using the [`sign.update()`][] and [`sign.sign()`][] methods:
863866

864867
```js
865868
const crypto = require('crypto');
866-
const sign = crypto.createSign('RSA-SHA256');
869+
const sign = crypto.createSign('SHA256');
867870

868871
sign.update('some data to sign');
869872

@@ -872,27 +875,22 @@ console.log(sign.sign(privateKey, 'hex'));
872875
// Prints: the calculated signature
873876
```
874877

875-
A `Sign` instance can also be created by just passing in the digest
876-
algorithm name, in which case OpenSSL will infer the full signature algorithm
877-
from the type of the PEM-formatted private key, including algorithms that
878-
do not have directly exposed name constants, e.g. 'ecdsa-with-SHA256'.
878+
In some cases, a `Sign` instance can also be created by passing in a signature
879+
algorithm name, such as 'RSA-SHA256'. This will use the corresponding digest
880+
algorithm. This does not work for all signature algorithms, such as
881+
'ecdsa-with-SHA256'. Use digest names instead.
879882

880-
Example: signing using ECDSA with SHA256
883+
Example: signing using legacy signature algorithm name
881884

882885
```js
883886
const crypto = require('crypto');
884-
const sign = crypto.createSign('sha256');
887+
const sign = crypto.createSign('RSA-SHA256');
885888

886889
sign.update('some data to sign');
887890

888-
const privateKey =
889-
`-----BEGIN EC PRIVATE KEY-----
890-
MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49
891-
AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2
892-
pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==
893-
-----END EC PRIVATE KEY-----`;
894-
895-
console.log(sign.sign(privateKey).toString('hex'));
891+
const privateKey = getPrivateKeySomehow();
892+
console.log(sign.sign(privateKey, 'hex'));
893+
// Prints: the calculated signature
896894
```
897895

898896
### sign.sign(private_key[, output_format])
@@ -965,7 +963,7 @@ Example: Using `Verify` objects as streams:
965963

966964
```js
967965
const crypto = require('crypto');
968-
const verify = crypto.createVerify('RSA-SHA256');
966+
const verify = crypto.createVerify('SHA256');
969967

970968
verify.write('some data to sign');
971969
verify.end();
@@ -980,7 +978,7 @@ Example: Using the [`verify.update()`][] and [`verify.verify()`][] methods:
980978

981979
```js
982980
const crypto = require('crypto');
983-
const verify = crypto.createVerify('RSA-SHA256');
981+
const verify = crypto.createVerify('SHA256');
984982

985983
verify.update('some data to sign');
986984

Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+19-28Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3993,7 +3993,8 @@ void SignBase::CheckThrow(SignBase::Error error) {
39933993

39943994
static bool ApplyRSAOptions(EVP_PKEY* pkey, EVP_PKEY_CTX* pkctx, int padding,
39953995
int salt_len) {
3996-
if (pkey->type == EVP_PKEY_RSA || pkey->type == EVP_PKEY_RSA2) {
3996+
if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA ||
3997+
EVP_PKEY_id(pkey) == EVP_PKEY_RSA2) {
39973998
if (EVP_PKEY_CTX_set_rsa_padding(pkctx, padding) <= 0)
39983999
return false;
39994000
if (padding == RSA_PKCS1_PSS_PADDING) {
@@ -4102,33 +4103,23 @@ static int Node_SignFinal(EVP_MD_CTX* mdctx, unsigned char* md,
41024103
if (!EVP_DigestFinal_ex(mdctx, m, &m_len))
41034104
return rv;
41044105

4105-
if (mdctx->digest->flags & EVP_MD_FLAG_PKEY_METHOD_SIGNATURE) {
4106-
size_t sltmp = static_cast<size_t>(EVP_PKEY_size(pkey));
4107-
pkctx = EVP_PKEY_CTX_new(pkey, nullptr);
4108-
if (pkctx == nullptr)
4109-
goto err;
4110-
if (EVP_PKEY_sign_init(pkctx) <= 0)
4111-
goto err;
4112-
if (!ApplyRSAOptions(pkey, pkctx, padding, pss_salt_len))
4113-
goto err;
4114-
if (EVP_PKEY_CTX_set_signature_md(pkctx, mdctx->digest) <= 0)
4115-
goto err;
4116-
if (EVP_PKEY_sign(pkctx, md, &sltmp, m, m_len) <= 0)
4117-
goto err;
4118-
*sig_len = sltmp;
4119-
rv = 1;
4120-
err:
4121-
EVP_PKEY_CTX_free(pkctx);
4122-
return rv;
4123-
}
4124-
4125-
if (mdctx->digest->sign == nullptr) {
4126-
EVPerr(EVP_F_EVP_SIGNFINAL, EVP_R_NO_SIGN_FUNCTION_CONFIGURED);
4127-
return 0;
4128-
}
4129-
4130-
return mdctx->digest->sign(mdctx->digest->type, m, m_len, md, sig_len,
4131-
pkey->pkey.ptr);
4106+
size_t sltmp = static_cast<size_t>(EVP_PKEY_size(pkey));
4107+
pkctx = EVP_PKEY_CTX_new(pkey, nullptr);
4108+
if (pkctx == nullptr)
4109+
goto err;
4110+
if (EVP_PKEY_sign_init(pkctx) <= 0)
4111+
goto err;
4112+
if (!ApplyRSAOptions(pkey, pkctx, padding, pss_salt_len))
4113+
goto err;
4114+
if (EVP_PKEY_CTX_set_signature_md(pkctx, EVP_MD_CTX_md(mdctx)) <= 0)
4115+
goto err;
4116+
if (EVP_PKEY_sign(pkctx, md, &sltmp, m, m_len) <= 0)
4117+
goto err;
4118+
*sig_len = sltmp;
4119+
rv = 1;
4120+
err:
4121+
EVP_PKEY_CTX_free(pkctx);
4122+
return rv;
41324123
}
41334124

41344125
SignBase::Error Sign::SignFinal(const char* key_pem,
Collapse file

‎test/fixtures/0-dns/create-cert.js‎

Copy file name to clipboardExpand all lines: test/fixtures/0-dns/create-cert.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const BN = asn1.bignum;
88
const id_at_commonName = [ 2, 5, 4, 3 ];
99
const rsaEncryption = [1, 2, 840, 113549, 1, 1, 1];
1010
const sha256WithRSAEncryption = [1, 2, 840, 113549, 1, 1, 11];
11-
const sigalg = 'RSA-SHA256';
11+
const digest = 'SHA256';
1212

1313
const private_key = fs.readFileSync('./0-dns-key.pem');
1414
// public key file can be generated from the private key with
@@ -59,7 +59,7 @@ const tbs = {
5959

6060
const tbs_der = rfc5280.TBSCertificate.encode(tbs, 'der');
6161

62-
const sign = crypto.createSign(sigalg);
62+
const sign = crypto.createSign(digest);
6363
sign.update(tbs_der);
6464
const signature = sign.sign(private_key);
6565

Collapse file

‎test/parallel/test-crypto-binary-default.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-binary-default.js
+12-12Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -404,28 +404,28 @@ assert.throws(function() {
404404
}, /^Error: Digest method not supported$/);
405405

406406
// Test signing and verifying
407-
const s1 = crypto.createSign('RSA-SHA1')
407+
const s1 = crypto.createSign('SHA1')
408408
.update('Test123')
409409
.sign(keyPem, 'base64');
410-
const s1Verified = crypto.createVerify('RSA-SHA1')
410+
const s1Verified = crypto.createVerify('SHA1')
411411
.update('Test')
412412
.update('123')
413413
.verify(certPem, s1, 'base64');
414414
assert.strictEqual(s1Verified, true, 'sign and verify (base 64)');
415415

416-
const s2 = crypto.createSign('RSA-SHA256')
416+
const s2 = crypto.createSign('SHA256')
417417
.update('Test123')
418418
.sign(keyPem); // binary
419-
const s2Verified = crypto.createVerify('RSA-SHA256')
419+
const s2Verified = crypto.createVerify('SHA256')
420420
.update('Test')
421421
.update('123')
422422
.verify(certPem, s2); // binary
423423
assert.strictEqual(s2Verified, true, 'sign and verify (binary)');
424424

425-
const s3 = crypto.createSign('RSA-SHA1')
425+
const s3 = crypto.createSign('SHA1')
426426
.update('Test123')
427427
.sign(keyPem, 'buffer');
428-
const s3Verified = crypto.createVerify('RSA-SHA1')
428+
const s3Verified = crypto.createVerify('SHA1')
429429
.update('Test')
430430
.update('123')
431431
.verify(certPem, s3);
@@ -569,8 +569,8 @@ const d = crypto.createDiffieHellman(p, 'hex');
569569
assert.strictEqual(d.verifyError, DH_NOT_SUITABLE_GENERATOR);
570570

571571
// Test RSA key signing/verification
572-
const rsaSign = crypto.createSign('RSA-SHA1');
573-
const rsaVerify = crypto.createVerify('RSA-SHA1');
572+
const rsaSign = crypto.createSign('SHA1');
573+
const rsaVerify = crypto.createVerify('SHA1');
574574
assert.ok(rsaSign instanceof crypto.Sign);
575575
assert.ok(rsaVerify instanceof crypto.Verify);
576576

@@ -606,13 +606,13 @@ assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
606606
'8195e0268da7eda23d9825ac43c724e86ceeee0d0d4465678652ccaf6501' +
607607
'0ddfb299bedeb1ad';
608608

609-
const sign = crypto.createSign('RSA-SHA256');
609+
const sign = crypto.createSign('SHA256');
610610
sign.update(input);
611611

612612
const output = sign.sign(privateKey, 'hex');
613613
assert.strictEqual(output, signature);
614614

615-
const verify = crypto.createVerify('RSA-SHA256');
615+
const verify = crypto.createVerify('SHA256');
616616
verify.update(input);
617617

618618
assert.strictEqual(verify.verify(publicKey, signature, 'hex'), true);
@@ -631,11 +631,11 @@ assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
631631

632632
// DSA signatures vary across runs so there is no static string to verify
633633
// against
634-
const sign = crypto.createSign('DSS1');
634+
const sign = crypto.createSign('SHA1');
635635
sign.update(input);
636636
const signature = sign.sign(privateKey, 'hex');
637637

638-
const verify = crypto.createVerify('DSS1');
638+
const verify = crypto.createVerify('SHA1');
639639
verify.update(input);
640640

641641
assert.strictEqual(verify.verify(publicKey, signature, 'hex'), true);
Collapse file

‎test/parallel/test-crypto-rsa-dsa.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto-rsa-dsa.js
+34-12Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ test_rsa('RSA_PKCS1_PADDING');
132132
test_rsa('RSA_PKCS1_OAEP_PADDING');
133133

134134
// Test RSA key signing/verification
135-
let rsaSign = crypto.createSign('RSA-SHA1');
136-
let rsaVerify = crypto.createVerify('RSA-SHA1');
135+
let rsaSign = crypto.createSign('SHA1');
136+
let rsaVerify = crypto.createVerify('SHA1');
137137
assert.ok(rsaSign);
138138
assert.ok(rsaVerify);
139139

@@ -152,19 +152,19 @@ rsaVerify.update(rsaPubPem);
152152
assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
153153

154154
// Test RSA key signing/verification with encrypted key
155-
rsaSign = crypto.createSign('RSA-SHA1');
155+
rsaSign = crypto.createSign('SHA1');
156156
rsaSign.update(rsaPubPem);
157157
assert.doesNotThrow(() => {
158158
const signOptions = { key: rsaKeyPemEncrypted, passphrase: 'password' };
159159
rsaSignature = rsaSign.sign(signOptions, 'hex');
160160
});
161161
assert.strictEqual(rsaSignature, expectedSignature);
162162

163-
rsaVerify = crypto.createVerify('RSA-SHA1');
163+
rsaVerify = crypto.createVerify('SHA1');
164164
rsaVerify.update(rsaPubPem);
165165
assert.strictEqual(rsaVerify.verify(rsaPubPem, rsaSignature, 'hex'), true);
166166

167-
rsaSign = crypto.createSign('RSA-SHA1');
167+
rsaSign = crypto.createSign('SHA1');
168168
rsaSign.update(rsaPubPem);
169169
assert.throws(() => {
170170
const signOptions = { key: rsaKeyPemEncrypted, passphrase: 'wrong' };
@@ -188,16 +188,28 @@ assert.throws(() => {
188188
'8195e0268da7eda23d9825ac43c724e86ceeee0d0d4465678652ccaf6501' +
189189
'0ddfb299bedeb1ad';
190190

191-
const sign = crypto.createSign('RSA-SHA256');
191+
const sign = crypto.createSign('SHA256');
192192
sign.update(input);
193193

194194
const output = sign.sign(privateKey, 'hex');
195195
assert.strictEqual(signature, output);
196196

197-
const verify = crypto.createVerify('RSA-SHA256');
197+
const verify = crypto.createVerify('SHA256');
198198
verify.update(input);
199199

200200
assert.strictEqual(verify.verify(publicKey, signature, 'hex'), true);
201+
202+
// Test the legacy signature algorithm name.
203+
const sign2 = crypto.createSign('RSA-SHA256');
204+
sign2.update(input);
205+
206+
const output2 = sign2.sign(privateKey, 'hex');
207+
assert.strictEqual(signature, output2);
208+
209+
const verify2 = crypto.createVerify('SHA256');
210+
verify2.update(input);
211+
212+
assert.strictEqual(verify2.verify(publicKey, signature, 'hex'), true);
201213
}
202214

203215

@@ -209,14 +221,24 @@ assert.throws(() => {
209221

210222
// DSA signatures vary across runs so there is no static string to verify
211223
// against
212-
const sign = crypto.createSign('DSS1');
224+
const sign = crypto.createSign('SHA1');
213225
sign.update(input);
214226
const signature = sign.sign(dsaKeyPem, 'hex');
215227

216-
const verify = crypto.createVerify('DSS1');
228+
const verify = crypto.createVerify('SHA1');
217229
verify.update(input);
218230

219231
assert.strictEqual(verify.verify(dsaPubPem, signature, 'hex'), true);
232+
233+
// Test the legacy 'DSS1' name.
234+
const sign2 = crypto.createSign('DSS1');
235+
sign2.update(input);
236+
const signature2 = sign2.sign(dsaKeyPem, 'hex');
237+
238+
const verify2 = crypto.createVerify('DSS1');
239+
verify2.update(input);
240+
241+
assert.strictEqual(verify2.verify(dsaPubPem, signature2, 'hex'), true);
220242
}
221243

222244

@@ -226,7 +248,7 @@ assert.throws(() => {
226248
const input = 'I AM THE WALRUS';
227249

228250
{
229-
const sign = crypto.createSign('DSS1');
251+
const sign = crypto.createSign('SHA1');
230252
sign.update(input);
231253
assert.throws(() => {
232254
sign.sign({ key: dsaKeyPemEncrypted, passphrase: 'wrong' }, 'hex');
@@ -236,7 +258,7 @@ const input = 'I AM THE WALRUS';
236258
{
237259
// DSA signatures vary across runs so there is no static string to verify
238260
// against
239-
const sign = crypto.createSign('DSS1');
261+
const sign = crypto.createSign('SHA1');
240262
sign.update(input);
241263

242264
let signature;
@@ -245,7 +267,7 @@ const input = 'I AM THE WALRUS';
245267
signature = sign.sign(signOptions, 'hex');
246268
});
247269

248-
const verify = crypto.createVerify('DSS1');
270+
const verify = crypto.createVerify('SHA1');
249271
verify.update(input);
250272

251273
assert.strictEqual(verify.verify(dsaPubPem, signature, 'hex'), true);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.