Skip to content

Commit

Permalink
[Fix] sign: throw on unsupported padding scheme
Browse files Browse the repository at this point in the history
Do not silently apply the wrong padding scheme.
  • Loading branch information
btj authored and ljharb committed Sep 18, 2023
1 parent 5f6fb17 commit 8767739
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 11 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"rules": {
"func-style": "warn",
"indent": ["error", 2],
"multiline-comment-style": "off",
"sort-keys": "off",
},

Expand Down
3 changes: 3 additions & 0 deletions browser/sign.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ var BN = require('bn.js');
var parseKeys = require('parse-asn1');
var curves = require('./curves.json');

var RSA_PKCS1_PADDING = 1;

function sign(hash, key, hashType, signType, tag) {
var priv = parseKeys(key);
if (priv.curve) {
Expand All @@ -20,6 +22,7 @@ function sign(hash, key, hashType, signType, tag) {
return dsaSign(hash, priv, hashType);
}
if (signType !== 'rsa' && signType !== 'ecdsa/rsa') { throw new Error('wrong private key type'); }
if (key.padding !== undefined && key.padding !== RSA_PKCS1_PADDING) { throw new Error('illegal or unsupported padding mode'); }

hash = Buffer.concat([tag, hash]);
var len = priv.modulus.byteLength();
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"tests-only": "nyc tape 'test/**/*.js'",
"pretest": "npm run lint",
"test": "npm run tests-only",
"posttest": "aud --production"
"posttest": "aud --production"
},
"dependencies": {
"bn.js": "^5.2.1",
Expand All @@ -39,6 +39,7 @@
"aud": "^2.0.3",
"eslint": "=8.8.0",
"nyc": "^10.3.2",
"semver": "^6.3.1",
"tape": "^5.6.6"
},
"browser": "browser/index.js",
Expand Down
41 changes: 31 additions & 10 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,15 @@ var Buffer = require('safe-buffer').Buffer;
var asn1 = require('parse-asn1/asn1');
var test = require('tape').test;
var nCrypto = require('crypto');
var semver = require('semver');
var bCrypto = require('../browser');
var fixtures = require('./fixtures');

function isNode10() {
return parseInt(process.version.split('.')[1], 10) <= 10;
}

fixtures.valid.rsa.forEach(function (f) {
var message = Buffer.from(f.message);
var pub = Buffer.from(f['public'], 'base64');
var priv;

// skip passphrase tests in node 10
if (f.passphrase && isNode10()) { return; }

if (f.passphrase) {
priv = {
key: Buffer.from(f['private'], 'base64'),
Expand Down Expand Up @@ -63,14 +57,41 @@ fixtures.valid.rsa.forEach(function (f) {
});
});

// node has padding support since 8.0
// TODO: figure out why node v8.0 - v8.6 is broken
(semver.satisfies(process.versions.node, '>= 8.6') ? test : test.skip)('padding option', function (t) {
var f = fixtures.valid.rsa[0];
var message = Buffer.from(f.message);
var priv = {
key: Buffer.from(f['private'], 'base64'),
padding: 11646841 // Some invalid value
};

t.test('invalid padding option', function (st) {
var bSign = bCrypto.createSign(f.scheme);
var nSign = nCrypto.createSign(f.scheme);
st['throws'](
function () { bSign.update(message).sign(priv); },
/illegal or unsupported padding mode/,
'browser throws exception with proper message'
);
st['throws'](
function () { nSign.update(message).sign(priv); },
/illegal or unsupported padding mode/,
'node throws exception with proper message'
);

st.end();
});

t.end();
});

fixtures.valid.ec.forEach(function (f) {
var message = Buffer.from(f.message);
var pub = Buffer.from(f['public'], 'base64');
var priv;

// skip passphrase tests in node 10
if (f.passphrase && isNode10()) { return; }

if (f.passphrase) {
priv = {
key: Buffer.from(f['private'], 'base64'),
Expand Down

0 comments on commit 8767739

Please sign in to comment.