Skip to content

crypto: add sign/verify support for RSASSA-PSS #11705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tniessen wants to merge 17 commits into from
Closed

crypto: add sign/verify support for RSASSA-PSS #11705

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
a361f53
crypto: add sign/verify support for RSASSA-PSS
tniessen Mar 5, 2017
e467227
Various improvements related to PR #11705
tniessen Mar 7, 2017
5a874a5
Minor linting fixes
tniessen Mar 7, 2017
8a23882
Fix API description, error message
tniessen Mar 8, 2017
15abaae
Use defined constants instead of values
tniessen Mar 8, 2017
d75601e
Improvements requested by sam-github
tniessen Mar 8, 2017
5e78d9c
Changes requested by bnoordhuis
tniessen Mar 11, 2017
5e5f2e4
Move application of RSA options out of sign/verify
tniessen Mar 14, 2017
b9a6779
Fix linter error
tniessen Mar 14, 2017
fcd7053
Move RSA option parsing to crypto.js
tniessen Mar 14, 2017
287e2d6
Remove unused using
tniessen Mar 14, 2017
8dc383e
Strict checks and more tests
tniessen Mar 15, 2017
1679e0c
Fix linting
tniessen Mar 15, 2017
8b64c04
Minor improvements
tniessen Mar 16, 2017
44b71e9
Note about MGF1, test vectors
tniessen Mar 17, 2017
4f695aa
Add RSASSA-PSS sign test
tniessen Mar 28, 2017
8772278
Add changes to doc
tniessen Apr 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Strict checks and more tests
  • Loading branch information
@tniessen
tniessen committed Mar 26, 2017
commit 8dc383e7392b435329d54e2f891ef0067b245275
38 changes: 31 additions & 7 deletions lib/crypto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,22 @@ Sign.prototype.sign = function sign(options, encoding) {
var passphrase = options.passphrase || null;

// Options specific to RSA
var rsaPadding = options.padding || constants.RSA_PKCS1_PADDING;
var pssSaltLength = constants.RSA_PSS_SALTLEN_MAX_SIGN;
if (typeof options.saltLength === 'number') {
pssSaltLength = options.saltLength;
var rsaPadding = constants.RSA_PKCS1_PADDING;
if (options.hasOwnProperty('padding')) {
Copy link
Copy Markdown
Member

@addaleax addaleax Mar 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just check for options.padding !== undefined instead of using hasOwnProperty? The latter fails when options doesn’t inherit from Object, and I think there’s no reason to require padding to be a own property of options.

Copy link
Copy Markdown
Member Author

@tniessen tniessen Mar 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was my first consideration, but @bnoordhuis wrote:

Can you add test cases for obviously invalid inputs (null, undefined, NaN, "boom", an object, etc.)?

If we consider padding: undefined invalid, then checking padding !== undefined is not enough. If Ben doesn't see a problem here, I will change it to !== undefined. Or maybe 'padding' in options instead of options.hasOwnProperty('padding').

if (options.padding === options.padding >> 0) {
rsaPadding = options.padding;
} else {
throw new TypeError('padding must be an integer');
}
}

var pssSaltLength = constants.RSA_PSS_SALTLEN_AUTO;
if (options.hasOwnProperty('saltLength')) {
if (options.saltLength === options.saltLength >> 0) {
pssSaltLength = options.saltLength;
} else {
throw new TypeError('saltLength must be an integer');
}
}

var ret = this._handle.sign(toBuf(key), null, passphrase, rsaPadding,
Expand Down Expand Up @@ -343,10 +355,22 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
sigEncoding = sigEncoding || exports.DEFAULT_ENCODING;

// Options specific to RSA
var rsaPadding = options.padding || constants.RSA_PKCS1_PADDING;
var rsaPadding = constants.RSA_PKCS1_PADDING;
if (options.hasOwnProperty('padding')) {
if (options.padding === options.padding >> 0) {
rsaPadding = options.padding;
} else {
throw new TypeError('padding must be an integer');
}
}

var pssSaltLength = constants.RSA_PSS_SALTLEN_AUTO;
if (typeof options.saltLength === 'number') {
pssSaltLength = options.saltLength;
if (options.hasOwnProperty('saltLength')) {
if (options.saltLength === options.saltLength >> 0) {
pssSaltLength = options.saltLength;
} else {
throw new TypeError('saltLength must be an integer');
}
}

return this._handle.verify(toBuf(key), toBuf(signature, sigEncoding), null,
Expand Down
149 changes: 116 additions & 33 deletions test/parallel/test-crypto-sign-verify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const crypto = require('crypto');
// Test certificates
const certPem = fs.readFileSync(common.fixturesDir + '/test_cert.pem', 'ascii');
const keyPem = fs.readFileSync(common.fixturesDir + '/test_key.pem', 'ascii');
const modSize = 1024;

// Test signing and verifying
{
Expand Down Expand Up @@ -71,44 +72,126 @@ const keyPem = fs.readFileSync(common.fixturesDir + '/test_key.pem', 'ascii');
assert.strictEqual(verified, true, 'sign and verify (stream)');
}

// Special tests for RSA_PKCS1_PSS_PADDING
{
['RSA-SHA1', 'RSA-SHA256'].forEach((algo) => {
[null, -2, -1, 0, 16, 32, 64].forEach((saltLength) => {
let verified;

// Test sign and verify with the given parameters
const s4 = crypto.createSign(algo)
.update('Test123')
.sign({
key: keyPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength
});
verified = crypto.createVerify(algo)
.update('Test')
.update('123')
.verify({
key: certPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength
}, s4);
assert.strictEqual(verified, true, 'sign and verify (buffer, PSS)');

// Setting the salt length to RSA_PSS_SALTLEN_AUTO should always work for
// verification
verified = crypto.createVerify(algo)
.update('Test123')
.verify({
key: certPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: crypto.constants.RSA_PSS_SALTLEN_AUTO
}, s4);
assert.strictEqual(verified, true, 'sign and verify (buffer, PSS)');
function testPSS(algo, hLen) {
// Maximum permissible salt length
let max = modSize / 8 - hLen - 2;

function getEffectiveSaltLength(saltLength) {
switch(saltLength) {
case crypto.constants.RSA_PSS_SALTLEN_DIGEST:
return hLen;
case crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN:
return max;
default:
return saltLength;
}
}

let signSaltLengths = [
crypto.constants.RSA_PSS_SALTLEN_DIGEST,
getEffectiveSaltLength(crypto.constants.RSA_PSS_SALTLEN_DIGEST),
crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN,
getEffectiveSaltLength(crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN),
0, 16, 32, 64, 128
];

let verifySaltLengths = [
crypto.constants.RSA_PSS_SALTLEN_DIGEST,
getEffectiveSaltLength(crypto.constants.RSA_PSS_SALTLEN_DIGEST),
getEffectiveSaltLength(crypto.constants.RSA_PSS_SALTLEN_MAX_SIGN),
0, 16, 32, 64, 128
];

signSaltLengths.forEach((signSaltLength) => {
if(signSaltLength > max) {
// If the salt length is too big, an Error should be thrown
assert.throws(() => {
crypto.createSign(algo)
.update('Test123')
.sign({
key: keyPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: signSaltLength
});
}, /^Error:.*data too large for key size$/);
} else {
// Otherwise, a valid signature should be generated
const s4 = crypto.createSign(algo)
.update('Test123')
.sign({
key: keyPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: signSaltLength
});

let verified;
verifySaltLengths.forEach((verifySaltLength) => {
// Verification should succeed if and only if the salt length is
// correct
verified = crypto.createVerify(algo)
.update('Test123')
.verify({
key: certPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: verifySaltLength
}, s4);
let saltLengthCorrect = getEffectiveSaltLength(signSaltLength) ==
getEffectiveSaltLength(verifySaltLength);
assert.strictEqual(verified, saltLengthCorrect, 'verify (PSS)');
});

// Verification using RSA_PSS_SALTLEN_AUTO should always work
verified = crypto.createVerify(algo)
.update('Test123')
.verify({
key: certPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: crypto.constants.RSA_PSS_SALTLEN_AUTO
}, s4);
assert.strictEqual(verified, true, 'verify (PSS with SALTLEN_AUTO)');

// Verifying an incorrect message should never work
verified = crypto.createVerify(algo)
.update('Test1234')
.verify({
key: certPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: crypto.constants.RSA_PSS_SALTLEN_AUTO
}, s4);
assert.strictEqual(verified, false, 'verify (PSS, incorrect)');
}
});
});
}

testPSS('RSA-SHA1', 20);
testPSS('RSA-SHA256', 32);
}

// Test exceptions for invalid `padding` and `saltLength` values
{
[null, undefined, NaN, "boom", {}, []].forEach((invalidValue) => {
assert.throws(() => {
crypto.createSign('RSA-SHA256')
.update('Test123')
.sign({
key: keyPem,
padding: invalidValue
});
}, /^TypeError: padding must be an integer$/);

assert.throws(() => {
crypto.createSign('RSA-SHA256')
.update('Test123')
.sign({
key: keyPem,
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
saltLength: invalidValue
});
}, /^TypeError: saltLength must be an integer$/);
});

assert.throws(() => {
crypto.createSign('RSA-SHA1')
.update('Test123')
Expand Down