Skip to content

Commit 8830a23

Browse files
david-renaud-oktadavid-renaud-okta
authored
fix!: handle poorly formatted PEM files (#85)
* fix!: handle poorly formatted PEM files Added seamless handling of poorly formatted PEM files for signing and encryption
1 parent 925c39a commit 8830a23

File tree

8 files changed

+529
-66
lines changed

8 files changed

+529
-66
lines changed

‎lib/utils.js‎

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
var fs = require('fs');
2-
var Parser = require('@xmldom/xmldom').DOMParser;
1+
const fs = require('fs');
2+
const Parser = require('@xmldom/xmldom').DOMParser;
33

44
exports.pemToCert = function(pem) {
5-
var cert = /-----BEGIN CERTIFICATE-----([^-]*)-----END CERTIFICATE-----/g.exec(pem.toString());
5+
const cert = /-----BEGIN CERTIFICATE-----([^-]*)-----END CERTIFICATE-----/g.exec(pem.toString());
66
if (cert && cert.length > 0) {
77
return cert[1].replace(/[\n|\r\n]/g, '');
88
}
@@ -29,28 +29,27 @@ exports.reportError = function(err, callback){
2929
* @api private
3030
*/
3131
exports.uid = function(len) {
32-
var buf = []
33-
, chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
34-
, charlen = chars.length;
32+
const buf = []
33+
, chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
34+
, charlen = chars.length;
3535

36-
for (var i = 0; i < len; ++i) {
36+
for (let i = 0; i < len; ++i) {
3737
buf.push(chars[getRandomInt(0, charlen - 1)]);
3838
}
3939

4040
return buf.join('');
4141
};
4242

4343
exports.removeWhitespace = function(xml) {
44-
var trimmed = xml
45-
.replace(/\r\n/g, '')
46-
.replace(/\n/g,'')
47-
.replace(/>(\s*)</g, '><') //unindent
48-
.trim();
49-
return trimmed;
44+
return xml
45+
.replace(/\r\n/g, '')
46+
.replace(/\n/g, '')
47+
.replace(/>(\s*)</g, '><') //unindent
48+
.trim();
5049
};
5150

5251
/**
53-
* Retrun a random int, used by `utils.uid()`
52+
* Return a random int, used by `utils.uid()`
5453
*
5554
* @param {Number} min
5655
* @param {Number} max
@@ -69,10 +68,29 @@ function getRandomInt(min, max) {
6968
* @return {function(): Node}
7069
*/
7170
exports.factoryForNode = function factoryForNode(pathToTemplate) {
72-
var template = fs.readFileSync(pathToTemplate)
73-
var prototypeDoc = new Parser().parseFromString(template.toString())
71+
const template = fs.readFileSync(pathToTemplate);
72+
const prototypeDoc = new Parser().parseFromString(template.toString());
7473

7574
return function () {
7675
return prototypeDoc.cloneNode(true);
7776
};
7877
};
78+
79+
/**
80+
* Standardizes PEM content to match the spec (best effort)
81+
*
82+
* @param pem {Buffer} The PEM content to standardize
83+
* @returns {Buffer} The standardized PEM. Original will be returned unmodified if the content is not PEM.
84+
*/
85+
exports.fixPemFormatting = function (pem) {
86+
let pemEntries = pem.toString().matchAll(/([-]{5}[^-\r\n]+[-]{5})([^-]*)([-]{5}[^-\r\n]+[-]{5})/g);
87+
let fixedPem = ''
88+
for (const pemParts of pemEntries) {
89+
fixedPem = fixedPem.concat(`${pemParts[1]}\n${pemParts[2].replaceAll(/[\r\n]/g, '')}\n${pemParts[3]}\n`)
90+
}
91+
if (fixedPem.length === 0) {
92+
return pem;
93+
}
94+
95+
return Buffer.from(fixedPem)
96+
}

‎lib/xml/encrypt.js‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
var xmlenc = require('xml-encryption');
1+
const xmlenc = require('xml-encryption');
22

3-
var utils = require('../utils');
3+
const utils = require('../utils');
44

55
exports.fromEncryptXmlOptions = function (options) {
66
if (!options.encryptionCert) {
77
return this.unencrypted;
88
} else {
9-
var encryptOptions = {
9+
const encryptOptions = {
1010
rsa_pub: options.encryptionPublicKey,
1111
pem: options.encryptionCert,
1212
encryptionAlgorithm: options.encryptionAlgorithm || 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
@@ -29,8 +29,26 @@ exports.unencrypted = function (xml, callback) {
2929
exports.encrypted = function (encryptOptions) {
3030
return function encrypt(xml, callback) {
3131
xmlenc.encrypt(xml, encryptOptions, function (err, encrypted) {
32-
if (err) return callback(err);
33-
callback(null, utils.removeWhitespace(encrypted));
32+
if (err) {
33+
// Attempt to fix errors and retry
34+
xmlenc.encrypt(
35+
xml,
36+
{
37+
...encryptOptions,
38+
rsa_pub: utils.fixPemFormatting(encryptOptions.rsa_pub),
39+
pem: utils.fixPemFormatting(encryptOptions.pem),
40+
},
41+
function (retryErr, retryEncrypted) {
42+
if (retryErr) {
43+
return callback(retryErr);
44+
}
45+
46+
callback(null, utils.removeWhitespace(retryEncrypted));
47+
}
48+
);
49+
} else {
50+
callback(null, utils.removeWhitespace(encrypted));
51+
}
3452
});
3553
};
3654
};

‎lib/xml/sign.js‎

Lines changed: 54 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
var utils = require('../utils');
2-
var SignedXml = require('xml-crypto').SignedXml;
1+
const utils = require('../utils');
2+
const SignedXml = require('xml-crypto').SignedXml;
33

4-
var algorithms = {
4+
const algorithms = {
55
signature: {
66
'rsa-sha256': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256',
7-
'rsa-sha1': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
7+
'rsa-sha1': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'
88
},
99
digest: {
1010
'sha256': 'http://www.w3.org/2001/04/xmlenc#sha256',
@@ -22,61 +22,80 @@ exports.fromSignXmlOptions = function (options) {
2222
if (!options.xpathToNodeBeforeSignature)
2323
throw new Error('xpathToNodeBeforeSignature is required')
2424

25-
var key = options.key;
26-
var pem = options.cert;
27-
var signatureAlgorithm = options.signatureAlgorithm || 'rsa-sha256';
28-
var digestAlgorithm = options.digestAlgorithm || 'sha256';
29-
var signatureNamespacePrefix = (function (prefix) {
25+
const key = options.key;
26+
const pem = options.cert;
27+
const signatureAlgorithm = options.signatureAlgorithm || 'rsa-sha256';
28+
const digestAlgorithm = options.digestAlgorithm || 'sha256';
29+
const signatureNamespacePrefix = (function (prefix) {
3030
// 0.10.1 added prefix, but we want to name it signatureNamespacePrefix - This is just to keep supporting prefix
3131
return typeof prefix === 'string' ? prefix : '';
3232
})(options.signatureNamespacePrefix || options.prefix);
33-
var xpathToNodeBeforeSignature = options.xpathToNodeBeforeSignature;
34-
var idAttribute = options.signatureIdAttribute;
33+
const xpathToNodeBeforeSignature = options.xpathToNodeBeforeSignature;
34+
const idAttribute = options.signatureIdAttribute;
3535

3636
/**
3737
* @param {Document} doc
3838
* @param {Function} [callback]
3939
* @return {string}
4040
*/
4141
return function signXmlDocument(doc, callback) {
42-
var unsigned = exports.unsigned(doc);
43-
var cert = utils.pemToCert(pem);
42+
function sign(key) {
43+
const unsigned = exports.unsigned(doc);
44+
const cert = utils.pemToCert(pem);
4445

45-
var sig = new SignedXml(null, { signatureAlgorithm: algorithms.signature[signatureAlgorithm], idAttribute: idAttribute });
46-
sig.addReference("//*[local-name(.)='Assertion']",
47-
["http://www.w3.org/2000/09/xmldsig#enveloped-signature", "http://www.w3.org/2001/10/xml-exc-c14n#"],
48-
algorithms.digest[digestAlgorithm]);
46+
const sig = new SignedXml(null, {
47+
signatureAlgorithm: algorithms.signature[signatureAlgorithm],
48+
idAttribute: idAttribute
49+
});
50+
sig.addReference("//*[local-name(.)='Assertion']",
51+
["http://www.w3.org/2000/09/xmldsig#enveloped-signature", "http://www.w3.org/2001/10/xml-exc-c14n#"],
52+
algorithms.digest[digestAlgorithm]);
4953

50-
sig.signingKey = key;
54+
sig.signingKey = key;
5155

52-
sig.keyInfoProvider = {
53-
getKeyInfo: function (key, prefix) {
54-
prefix = prefix ? prefix + ':' : prefix;
55-
return "<" + prefix + "X509Data><" + prefix + "X509Certificate>" + cert + "</" + prefix + "X509Certificate></" + prefix + "X509Data>";
56-
}
57-
};
56+
sig.keyInfoProvider = {
57+
getKeyInfo: function (key, prefix) {
58+
prefix = prefix ? prefix + ':' : prefix;
59+
return "<" + prefix + "X509Data><" + prefix + "X509Certificate>" + cert + "</" + prefix + "X509Certificate></" + prefix + "X509Data>";
60+
}
61+
};
62+
63+
sig.computeSignature(unsigned, {
64+
location: {reference: xpathToNodeBeforeSignature, action: 'after'},
65+
prefix: signatureNamespacePrefix
66+
});
67+
68+
return sig.getSignedXml();
69+
}
5870

59-
sig.computeSignature(unsigned, {
60-
location: { reference: xpathToNodeBeforeSignature, action: 'after' },
61-
prefix: signatureNamespacePrefix
62-
});
71+
let signed
72+
try {
73+
try {
74+
signed = sign(key)
75+
} catch (err) {
76+
signed = sign(utils.fixPemFormatting(key))
77+
}
6378

64-
var signed = sig.getSignedXml();
65-
if (callback) {
66-
setImmediate(callback, null, signed);
67-
} else {
68-
return signed;
79+
if (callback) {
80+
setImmediate(callback, null, signed);
81+
} else {
82+
return signed;
83+
}
84+
} catch (e) {
85+
if (callback) {
86+
setImmediate(callback, e)
87+
}
88+
throw e
6989
}
7090
};
7191
};
72-
7392
/**
7493
* @param {Document} doc
7594
* @param {Function} [callback]
7695
* @return {string}
7796
*/
7897
exports.unsigned = function (doc, callback) {
79-
var xml = utils.removeWhitespace(doc.toString());
98+
const xml = utils.removeWhitespace(doc.toString());
8099
if (callback) {
81100
setImmediate(callback, null, xml)
82101
} else {

‎test/saml11.tests.js‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,32 @@ describe('saml 1.1', function () {
4646
assertSignature(signedAssertion, options);
4747
});
4848

49+
it('should not error when cert is missing newlines', function () {
50+
// cert created with:
51+
// openssl req -x509 -new -newkey rsa:2048 -nodes -subj '/CN=auth0.auth0.com/O=Auth0 LLC/C=US/ST=Washington/L=Redmond' -keyout auth0.key -out auth0.pem
52+
53+
var options = {
54+
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),
55+
key: fs.readFileSync(__dirname + '/test-auth0.key')
56+
};
57+
58+
var signedAssertion = saml11[createAssertion]({...options, cert: Buffer.from(options.cert.toString().replaceAll(/[\r\n]/g, ''))});
59+
assertSignature(signedAssertion, options);
60+
});
61+
62+
it('should not error when key is missing newlines', function () {
63+
// cert created with:
64+
// openssl req -x509 -new -newkey rsa:2048 -nodes -subj '/CN=auth0.auth0.com/O=Auth0 LLC/C=US/ST=Washington/L=Redmond' -keyout auth0.key -out auth0.pem
65+
66+
var options = {
67+
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),
68+
key: fs.readFileSync(__dirname + '/test-auth0.key')
69+
};
70+
71+
var signedAssertion = saml11[createAssertion]({...options, key: Buffer.from(options.key.toString().replaceAll(/[\r\n]/g, ''))});
72+
assertSignature(signedAssertion, options);
73+
});
74+
4975
it('should support specifying Issuer property', function () {
5076
var options = {
5177
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),
@@ -350,6 +376,44 @@ describe('saml 1.1', function () {
350376
});
351377
});
352378

379+
it('should not error when encryptionPublicKey is missing newlines', function (done) {
380+
var options = {
381+
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),
382+
key: fs.readFileSync(__dirname + '/test-auth0.key'),
383+
encryptionPublicKey: Buffer.from(fs.readFileSync(__dirname + '/test-auth0_rsa.pub').toString().replaceAll(/[\r\n]/g, '')),
384+
encryptionCert: fs.readFileSync(__dirname + '/test-auth0.pem')
385+
};
386+
387+
saml11[createAssertion](options, function(err, encrypted) {
388+
if (err) return done(err);
389+
390+
xmlenc.decrypt(encrypted, { key: fs.readFileSync(__dirname + '/test-auth0.key')}, function(err, decrypted) {
391+
if (err) return done(err);
392+
assertSignature(decrypted, options);
393+
done();
394+
});
395+
});
396+
});
397+
398+
it('should not error when encryptionCert is missing newlines', function (done) {
399+
var options = {
400+
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),
401+
key: fs.readFileSync(__dirname + '/test-auth0.key'),
402+
encryptionPublicKey: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
403+
encryptionCert: Buffer.from(fs.readFileSync(__dirname + '/test-auth0.pem').toString().replaceAll(/[\r\n]/g, ''))
404+
};
405+
406+
saml11[createAssertion](options, function(err, encrypted) {
407+
if (err) return done(err);
408+
409+
xmlenc.decrypt(encrypted, { key: fs.readFileSync(__dirname + '/test-auth0.key')}, function(err, decrypted) {
410+
if (err) return done(err);
411+
assertSignature(decrypted, options);
412+
done();
413+
});
414+
});
415+
});
416+
353417
it('should support holder-of-key suject confirmationmethod', function (done) {
354418
var options = {
355419
cert: fs.readFileSync(__dirname + '/test-auth0.pem'),

0 commit comments

Comments
 (0)