Skip to content

Commit 115679a

Browse files
Merge commit from fork
* Draft fix * Fix * Migrate to xml-crypto v6.1.0
1 parent d9ba89d commit 115679a

File tree

3 files changed

+97
-49
lines changed

3 files changed

+97
-49
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "samlify",
3-
"version": "2.9.1",
3+
"version": "2.9.0",
44
"description": "High-level API for Single Sign On (SAML 2.0)",
55
"main": "build/index.js",
66
"keywords": [
@@ -39,7 +39,7 @@
3939
"pako": "^1.0.10",
4040
"uuid": "^8.3.2",
4141
"xml": "^1.0.1",
42-
"xml-crypto": "^3.2.1",
42+
"xml-crypto": "^6.1.0",
4343
"xml-escape": "^1.1.0",
4444
"xpath": "^0.0.32"
4545
},

src/libsaml.ts

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ import { algorithms, wording, namespace } from './urn';
99
import { select } from 'xpath';
1010
import { MetadataInterface } from './metadata';
1111
import nrsa, { SigningSchemeHash } from 'node-rsa';
12-
import { SignedXml, FileKeyInfo } from 'xml-crypto';
12+
import { SignedXml } from 'xml-crypto';
1313
import * as xmlenc from '@authenio/xml-encryption';
1414
import { extract } from './extractor';
1515
import camelCase from 'camelcase';
1616
import { getContext } from './api';
1717
import xmlEscape from 'xml-escape';
18+
import * as fs from 'fs';
19+
import {DOMParser} from '@xmldom/xmldom';
1820

1921
const signatureAlgorithms = algorithms.signature;
2022
const digestAlgorithms = algorithms.digest;
@@ -95,6 +97,7 @@ export interface LibSamlInterface {
9597
verifySignature: (xml: string, opts: SignatureVerifierOptions) => [boolean, any];
9698
createKeySection: (use: KeyUse, cert: string | Buffer) => {};
9799
constructMessageSignature: (octetString: string, key: string, passphrase?: string, isBase64?: boolean, signingAlgorithm?: string) => string;
100+
98101
verifyMessageSignature: (metadata, octetString: string, signature: string | Buffer, verifyAlgorithm?: string) => boolean;
99102
getKeyInfo: (x509Certificate: string, signatureConfig?: any) => void;
100103
encryptAssertion: (sourceEntity, targetEntity, entireXML: string) => Promise<string>;
@@ -326,28 +329,28 @@ const libSaml = () => {
326329
} = opts;
327330
const sig = new SignedXml();
328331
// Add assertion sections as reference
332+
const digestAlgorithm = getDigestMethod(signatureAlgorithm);
329333
if (referenceTagXPath) {
330-
sig.addReference(
331-
referenceTagXPath,
332-
transformationAlgorithms,
333-
getDigestMethod(signatureAlgorithm)
334-
);
334+
sig.addReference({
335+
xpath: referenceTagXPath,
336+
transforms: transformationAlgorithms,
337+
digestAlgorithm: digestAlgorithm
338+
});
335339
}
336340
if (isMessageSigned) {
337-
sig.addReference(
341+
sig.addReference({
338342
// reference to the root node
339-
'/*',
340-
transformationAlgorithms,
341-
getDigestMethod(signatureAlgorithm),
342-
'',
343-
'',
344-
'',
345-
false,
346-
);
343+
xpath: '/*',
344+
transforms: transformationAlgorithms,
345+
digestAlgorithm
346+
});
347347
}
348348
sig.signatureAlgorithm = signatureAlgorithm;
349-
sig.keyInfoProvider = new this.getKeyInfo(signingCert, signatureConfig);
350-
sig.signingKey = utility.readPrivateKey(privateKey, privateKeyPass, true);
349+
sig.publicCert = this.getKeyInfo(signingCert, signatureConfig).getKey();
350+
sig.getKeyInfoContent = this.getKeyInfo(signingCert, signatureConfig).getKeyInfo;
351+
sig.privateKey = utility.readPrivateKey(privateKey, privateKeyPass, true);
352+
sig.canonicalizationAlgorithm = 'http://www.w3.org/2001/10/xml-exc-c14n#';
353+
351354
if (signatureConfig) {
352355
sig.computeSignature(rawSamlMessage, signatureConfig);
353356
} else {
@@ -359,11 +362,15 @@ const libSaml = () => {
359362
* @desc Verify the XML signature
360363
* @param {string} xml xml
361364
* @param {SignatureVerifierOptions} opts cert declares the X509 certificate
362-
* @return {boolean} verification result
363-
*/
365+
* @return {[boolean, string | null]} - A tuple where:
366+
* - The first element is `true` if the signature is valid, `false` otherwise.
367+
* - The second element is the cryptographically authenticated assertion node as a string, or `null` if not found.
368+
*/
364369
verifySignature(xml: string, opts: SignatureVerifierOptions) {
365370
const { dom } = getContext();
366371
const doc = dom.parseFromString(xml);
372+
373+
const docParser = new DOMParser();
367374
// In order to avoid the wrapping attack, we have changed to use absolute xpath instead of naively fetching the signature element
368375
// message signature (logout response / saml response)
369376
const messageSignatureXpath = "/*[contains(local-name(), 'Response') or contains(local-name(), 'Request')]/*[local-name(.)='Signature']";
@@ -374,7 +381,6 @@ const libSaml = () => {
374381

375382
// select the signature node
376383
let selection: any = [];
377-
let assertionNode: string | null = null;
378384
const messageSignatureNode = select(messageSignatureXpath, doc);
379385
const assertionSignatureNode = select(assertionSignatureXpath, doc);
380386
const wrappingElementNode = select(wrappingElementsXPath, doc);
@@ -392,10 +398,11 @@ const libSaml = () => {
392398
throw new Error('ERR_ZERO_SIGNATURE');
393399
}
394400

395-
const sig = new SignedXml();
396-
let verified = true;
401+
397402
// need to refactor later on
398-
selection.forEach(signatureNode => {
403+
for (const signatureNode of selection){
404+
const sig = new SignedXml();
405+
let verified = false;
399406

400407
sig.signatureAlgorithm = opts.signatureAlgorithm!;
401408

@@ -404,7 +411,7 @@ const libSaml = () => {
404411
}
405412

406413
if (opts.keyFile) {
407-
sig.keyInfoProvider = new FileKeyInfo(opts.keyFile);
414+
sig.publicCert = fs.readFileSync(opts.keyFile)
408415
}
409416

410417
if (opts.metadata) {
@@ -440,28 +447,56 @@ const libSaml = () => {
440447
throw new Error('ERROR_UNMATCH_CERTIFICATE_DECLARATION_IN_METADATA');
441448
}
442449

443-
sig.keyInfoProvider = new this.getKeyInfo(x509Certificate);
450+
sig.publicCert = this.getKeyInfo(x509Certificate).getKey();
444451

445452
} else {
446453
// Select first one from metadata
447-
sig.keyInfoProvider = new this.getKeyInfo(metadataCert[0]);
454+
sig.publicCert = this.getKeyInfo(metadataCert[0]).getKey();
448455
}
449-
450456
}
451457

452458
sig.loadSignature(signatureNode);
453459

454460
doc.removeChild(signatureNode);
455461

456-
verified = verified && sig.checkSignature(doc.toString());
462+
verified = sig.checkSignature(doc.toString());
457463

458464
// immediately throw error when any one of the signature is failed to get verified
459465
if (!verified) {
460466
throw new Error('ERR_FAILED_TO_VERIFY_SIGNATURE');
461467
}
468+
// attempt is made to get the signed Reference as a string();
469+
// note, we don't have access to the actual signedReferences API unfortunately
470+
// mainly a sanity check here for SAML. (Although ours would still be secure, if multiple references are used)
471+
if (!(sig.getReferences().length >= 1)) {
472+
throw new Error('NO_SIGNATURE_REFERENCES')
473+
}
474+
const signedVerifiedXML = sig.getSignedReferences()[0];
475+
const rootNode = docParser.parseFromString(signedVerifiedXML, 'text/xml').documentElement;
476+
// process the verified signature:
477+
// case 1, rootSignedDoc is a response:
478+
if (rootNode.localName === 'Response') {
479+
480+
// try getting the Xml from the first assertion
481+
const assertions = select(
482+
"./*[local-name()='Assertion']",
483+
rootNode
484+
);
485+
// now we can process the assertion as an assertion
486+
if (assertions.length === 1) {
487+
return [true, assertions[0].toString()];
488+
}
489+
} else if (rootNode.localName === 'Assertion') {
490+
return [true, rootNode.toString()];
491+
} else {
492+
return [true, null]; // signature is valid. But there is no assertion node here. It could be metadata node, hence return null
493+
}
494+
};
462495

463-
});
496+
// something has gone seriously wrong if we are still here
497+
throw new Error('ERR_ZERO_SIGNATURE');
464498

499+
/*
465500
// response must be signed, either entire document or assertion
466501
// default we will take the assertion section under root
467502
if (messageSignatureNode.length === 1) {
@@ -503,7 +538,7 @@ const libSaml = () => {
503538
assertionNode = verifiedDoc.assertion.toString();
504539
}
505540
506-
return [verified, assertionNode];
541+
return [verified, assertionNode];*/
507542
},
508543
/**
509544
* @desc Helper function to create the key section in metadata (abstraction for signing and encrypt use)
@@ -586,12 +621,14 @@ const libSaml = () => {
586621
* @return {string} public key
587622
*/
588623
getKeyInfo(x509Certificate: string, signatureConfig: any = {}) {
589-
this.getKeyInfo = key => {
590-
const prefix = signatureConfig.prefix ? `${signatureConfig.prefix}:` : '';
591-
return `<${prefix}X509Data><${prefix}X509Certificate>${x509Certificate}</${prefix}X509Certificate></${prefix}X509Data>`;
592-
};
593-
this.getKey = keyInfo => {
594-
return utility.getPublicKeyPemFromCertificate(x509Certificate).toString();
624+
const prefix = signatureConfig.prefix ? `${signatureConfig.prefix}:` : '';
625+
return {
626+
getKeyInfo: () => {
627+
return `<${prefix}X509Data><${prefix}X509Certificate>${x509Certificate}</${prefix}X509Certificate></${prefix}X509Data>`;
628+
},
629+
getKey: () => {
630+
return utility.getPublicKeyPemFromCertificate(x509Certificate).toString();
631+
},
595632
};
596633
},
597634
/**

yarn.lock

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -446,16 +446,21 @@
446446
resolved "https://registry.yarnpkg.com/@types/xmldom/-/xmldom-0.1.31.tgz#519b647cfc66debf82cdf50e49763c8fdee553d6"
447447
integrity sha512-bVy7s0nvaR5D1mT1a8ZkByHWNOGb6Vn4yi5TWhEdmyKlAG+08SA7Md6+jH+tYmMLueAwNeWvHHpeKrr6S4c4BA==
448448

449-
"@xmldom/xmldom@^0.8.6":
450-
version "0.8.6"
451-
resolved "https://registry.yarnpkg.com/@xmldom/xmldom/-/xmldom-0.8.6.tgz#8a1524eb5bd5e965c1e3735476f0262469f71440"
452-
integrity sha512-uRjjusqpoqfmRkTaNuLJ2VohVr67Q5YwDATW3VU7PfzTj6IRaihGrYI7zckGZjxQPBIp63nfvJbM+Yu5ICh0Bg==
449+
"@xmldom/is-dom-node@^1.0.1":
450+
version "1.0.1"
451+
resolved "https://registry.yarnpkg.com/@xmldom/is-dom-node/-/is-dom-node-1.0.1.tgz#83b9f3e1260fb008061c6fa787b93a00f9be0629"
452+
integrity sha512-CJDxIgE5I0FH+ttq/Fxy6nRpxP70+e2O048EPe85J2use3XKdatVM7dDVvFNjQudd9B49NPoZ+8PG49zj4Er8Q==
453453

454-
"@xmldom/xmldom@^0.8.8":
454+
"@xmldom/xmldom@^0.8.10":
455455
version "0.8.10"
456456
resolved "https://registry.yarnpkg.com/@xmldom/xmldom/-/xmldom-0.8.10.tgz#a1337ca426aa61cef9fe15b5b28e340a72f6fa99"
457457
integrity sha512-2WALfTl4xo2SkGCYRt6rDTFfk9R1czmBvUQy12gK2KuRKIpWEhcbbzy8EZXtz/jkRqHX8bFEc6FC1HjX4TUWYw==
458458

459+
"@xmldom/xmldom@^0.8.6":
460+
version "0.8.6"
461+
resolved "https://registry.yarnpkg.com/@xmldom/xmldom/-/xmldom-0.8.6.tgz#8a1524eb5bd5e965c1e3735476f0262469f71440"
462+
integrity sha512-uRjjusqpoqfmRkTaNuLJ2VohVr67Q5YwDATW3VU7PfzTj6IRaihGrYI7zckGZjxQPBIp63nfvJbM+Yu5ICh0Bg==
463+
459464
acorn-walk@^8.1.1, acorn-walk@^8.2.0:
460465
version "8.2.0"
461466
resolved "https://registry.yarnpkg.com/acorn-walk/-/acorn-walk-8.2.0.tgz#741210f2e2426454508853a2f44d0ab83b7f69c1"
@@ -2517,13 +2522,14 @@ write-file-atomic@^4.0.1:
25172522
imurmurhash "^0.1.4"
25182523
signal-exit "^3.0.7"
25192524

2520-
xml-crypto@^3.2.1:
2521-
version "3.2.1"
2522-
resolved "https://registry.yarnpkg.com/xml-crypto/-/xml-crypto-3.2.1.tgz#df2511a95275b43e18924693ff932af7de3217a4"
2523-
integrity sha512-0GUNbPtQt+PLMsC5HoZRONX+K6NBJEqpXe/lsvrFj0EqfpGPpVfJKGE7a5jCg8s2+Wkrf/2U1G41kIH+zC9eyQ==
2525+
xml-crypto@^6.1.0:
2526+
version "6.1.0"
2527+
resolved "https://registry.yarnpkg.com/xml-crypto/-/xml-crypto-6.1.0.tgz#c8224808525e5f15478c50b9fe706112a4e6ef1b"
2528+
integrity sha512-0TYPBRPwXLnRGc2F0f9Zc/H076YcP7tkCa2US4jpguuPTEx7TWFqSysIfJ1hP4r2KF82IYzhnzepnsUEsOjlOw==
25242529
dependencies:
2525-
"@xmldom/xmldom" "^0.8.8"
2526-
xpath "0.0.32"
2530+
"@xmldom/is-dom-node" "^1.0.1"
2531+
"@xmldom/xmldom" "^0.8.10"
2532+
xpath "^0.0.33"
25272533

25282534
xml-escape@^1.1.0:
25292535
version "1.1.0"
@@ -2540,6 +2546,11 @@ xpath@0.0.32, xpath@^0.0.32:
25402546
resolved "https://registry.yarnpkg.com/xpath/-/xpath-0.0.32.tgz#1b73d3351af736e17ec078d6da4b8175405c48af"
25412547
integrity sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw==
25422548

2549+
xpath@^0.0.33:
2550+
version "0.0.33"
2551+
resolved "https://registry.yarnpkg.com/xpath/-/xpath-0.0.33.tgz#5136b6094227c5df92002e7c3a13516a5074eb07"
2552+
integrity sha512-NNXnzrkDrAzalLhIUc01jO2mOzXGXh1JwPgkihcLLzw98c0WgYDmmjSh1Kl3wzaxSVWMuA+fe0WTWOBDWCBmNA==
2553+
25432554
y18n@^4.0.0:
25442555
version "4.0.3"
25452556
resolved "https://registry.yarnpkg.com/y18n/-/y18n-4.0.3.tgz#b5f259c82cd6e336921efd7bfd8bf560de9eeedf"

0 commit comments

Comments
 (0)