Skip to content

Commit

Permalink
Fix regression in ECDSA_KEY.generateCSR() (#538)
Browse files Browse the repository at this point in the history 
Subject alternative names extensions were not being correctly handled. Moving to non-deprecated jsrsasign API call and implementing some translation between previous extension format and the one required by the new API call.

Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>
  • Loading branch information
@bestbeforetoday
bestbeforetoday authored Dec 7, 2021
1 parent 9e9ca5d commit fc60e77
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 86 deletions.
40 changes: 40 additions & 0 deletions fabric-common/lib/Utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,3 +622,43 @@ module.exports.randomize = (items) => {
module.exports.checkParameter = (name) => {
throw Error(`Missing ${name} parameter`);
};

/**
* Map CSRUtil.newCSRPEM style extensions:
* ```
* {
* subjectAltName: {
* array: [...],
* },
* }
* ```
*
* to CertificationRequest style extensions:
* ```
* {
* extname: 'subjectAltName',
* array: [...],
* }
* ```
* @private
*/
module.exports.mapCSRExtensions = (extensions) => {
if (!Array.isArray(extensions)) {
return extensions;
}

const results = [];
extensions.forEach(extension => {
const isCertificationRequestExtension = typeof extension.extname === 'string';
if (isCertificationRequestExtension) {
results.push(extension);
} else {
Object.entries(extension).forEach(([extname, props]) => {
const extensionRequest = Object.assign({}, props, {extname});
results.push(extensionRequest);
});
}
});

return results;
};
9 changes: 5 additions & 4 deletions fabric-common/lib/impl/ecdsa/key.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
const Key = require('../../Key');
const HashPrimitives = require('../../HashPrimitives');
const jsrsa = require('jsrsasign');
const Utils = require('../../Utils');
const asn1 = jsrsa.asn1;
const KEYUTIL = jsrsa.KEYUTIL;
const ECDSA = jsrsa.ECDSA;
Expand Down Expand Up @@ -118,20 +119,20 @@ class ECDSA_KEY extends Key {
* @throws Will throw an error if CSR generation fails for any other reason
*/
generateCSR(subjectDN, extensions) {

// check to see if this is a private key
if (!this.isPrivate()) {
throw new Error('A CSR cannot be generated from a public key');
}

const csr = asn1.csr.CSRUtil.newCSRPEM({
const extreq = Utils.mapCSRExtensions(extensions);
const csr = new asn1.csr.CertificationRequest({
subject: {str: asn1.x509.X500Name.ldapToOneline(subjectDN)},
sbjpubkey: this.getPublicKey()._key,
sigalg: 'SHA256withECDSA',
sbjprvkey: this._key,
ext: extensions
extreq,
});
return csr;
return csr.getPEM();
}

/**
Expand Down
12 changes: 2 additions & 10 deletions fabric-common/lib/impl/ecdsa/pkcs11_key.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
'use strict';

const Key = require('../../Key');
const Utils = require('../../Utils');
const jsrsa = require('jsrsasign');
const asn1 = jsrsa.asn1;

Expand Down Expand Up @@ -112,16 +113,7 @@ const Pkcs11EcdsaKey = class extends Key {
}
const ecdsa = new EC(this._cryptoSuite._ecdsaCurve);
const pubKey = ecdsa.keyFromPublic(this._pub._ecpt);
const extreq = [];
if (param.ext !== undefined && param.ext.length !== undefined) {
for (const ext of param.ext) {
for (const extname in ext) {
const extObj = ext[extname];
extObj.extname = extname;
extreq.push(extObj);
}
}
}
const extreq = Utils.mapCSRExtensions(param.ext);
const sigAlgName = param.sigalg;
const csr = new _KJUR_asn1_csr.CertificationRequest({
subject: param.subject,
Expand Down
114 changes: 42 additions & 72 deletions fabric-common/test/impl/ecdsa/key.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

'use strict';

const ECDSA_KEY = require('../../../lib/impl/ecdsa/key');
const rewire = require('rewire');
const ECDSA_KEY_REWIRE = rewire('../../../lib/impl/ecdsa/key');
const jsrsa = require('jsrsasign');
Expand Down Expand Up @@ -121,8 +122,9 @@ describe('ECDSA_KEY', () => {
});

describe('#generateCSR', () => {

const {prvKeyObj: privateKey, pubKeyObj: publicKey} = KEYUTIL.generateKeypair('EC', 'P-256');
let revert;

afterEach(() => {
if (revert) {
revert();
Expand All @@ -131,92 +133,60 @@ describe('ECDSA_KEY', () => {
});

it('should throw when trying to generate if public', () => {
(() => {
const fakeKey = {type: 'EC', prvKeyHex: 'privateKey', pubKeyHex: 'publicKey'};
const myKey = new ECDSA_KEY_REWIRE(fakeKey);
myKey._key.should.deep.equal(fakeKey);
myKey.isPrivate = sinon.stub().returns(false);
myKey.generateCSR('CN=publickey');
}).should.throw(/A CSR cannot be generated from a public key/);
const key = new ECDSA_KEY(publicKey);

const test = () => key.generateCSR('CN=PublicKey');

test.should.throw('A CSR cannot be generated from a public key');
});

it('should rethrow internal errors', () => {
(() => {
const pemStub = sinon.stub().throws(new Error('MY_ERROR'));
const fakeAnsn1 = {
csr: {
CSRUtil: {
newCSRPEM: pemStub
}
},
x509: {
X500Name: {
ldapToOneline: sinon.stub()
}
}
};
const key = new ECDSA_KEY(privateKey);
const badExt = {
extname: 'FAIL',
};

revert = ECDSA_KEY_REWIRE.__set__('asn1', fakeAnsn1);
const fakeKey = {type: 'EC', prvKeyHex: 'privateKey', pubKeyHex: 'publicKey'};
const myKey = new ECDSA_KEY_REWIRE(fakeKey);
myKey._key.should.deep.equal(fakeKey);
myKey.isPrivate = sinon.stub().returns(true);
myKey.generateCSR('CN=publickey');
}).should.throw(/MY_ERROR/);
});
const test = () => key.generateCSR('CN=name', [badExt]);

it('should call into jsra lib if private', () => {
test.should.throw(badExt.extname);
});

const pemStub = sinon.stub().returns('your PEM sir');
const fakeAnsn1 = {
csr: {
CSRUtil: {
newCSRPEM: pemStub
}
it('should include extensions', () => {
const subjectAltNames = [
{dns: 'host1'},
{dns: 'host2'},
];
const subjectAltNameExt = {
subjectAltName: {
array: subjectAltNames,
},
x509: {
X500Name: {
ldapToOneline: sinon.stub()
}
}
};
const key = new ECDSA_KEY(privateKey);

revert = ECDSA_KEY_REWIRE.__set__('asn1', fakeAnsn1);
const fakeKey = {type: 'EC', prvKeyHex: 'privateKey', pubKeyHex: 'publicKey'};
const myKey = new ECDSA_KEY_REWIRE(fakeKey);
myKey._key.should.deep.equal(fakeKey);
myKey.isPrivate = sinon.stub().returns(true);
const csr = myKey.generateCSR('CN=publickey');
const csrPem = key.generateCSR('CN=name', [subjectAltNameExt]);
const csr = jsrsa.asn1.csr.CSRUtil.getParam(csrPem);

csr.should.equal('your PEM sir');
sinon.assert.calledOnce(pemStub);
const expected = {
extname: 'subjectAltName',
array: subjectAltNames,
};
csr.should.have.property('extreq').that.deep.includes(expected);
});

it('should call into jsrsa lib if extensions are passed', () => {

const extensions = [{subjectAltName: {array: [{dns: 'host1'}, {dns: 'host2'}]}}];
const pemStub = sinon.stub().returns('your PEM sir');
const fakeAnsn1 = {
csr: {
CSRUtil: {
newCSRPEM: pemStub
}
},
x509: {
X500Name: {
ldapToOneline: sinon.stub()
}
}
it('should include extension request format extensions', () => {
const subjectAltName = {
extname: 'subjectAltName',
array: [
{dns: 'host1'},
{dns: 'host2'},
],
};
const key = new ECDSA_KEY(privateKey);

revert = ECDSA_KEY_REWIRE.__set__('asn1', fakeAnsn1);
const fakeKey = {type: 'EC', prvKeyHex: 'privateKey', pubKeyHex: 'publicKey'};
const myKey = new ECDSA_KEY_REWIRE(fakeKey);
myKey.isPrivate = sinon.stub().returns(true);
const csr = myKey.generateCSR('CN=publickey', extensions);
const csrPem = key.generateCSR('CN=name', [subjectAltName]);
const csr = jsrsa.asn1.csr.CSRUtil.getParam(csrPem);

csr.should.equal('your PEM sir');
sinon.assert.calledOnceWithExactly(pemStub, sinon.match.has('ext', extensions));
csr.should.have.property('extreq').that.deep.includes(subjectAltName);
});
});

Expand Down

0 comments on commit fc60e77

Please sign in to comment.