From 86650bd1ce21cef8df915bfa937b789a169cce65 Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 09:34:05 +0100 Subject: [PATCH 1/8] Move all xpath select to xml module It adds better typings and already allowed to catch an incorrect type. It will also make the code safer since the returned type is checked at runtime. --- src/node-saml/saml.ts | 48 +++++++++++++++++++++++-------------------- src/node-saml/xml.ts | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 src/node-saml/xml.ts diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index 7ff1e2e2..1038131c 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -29,6 +29,7 @@ import { } from "./types"; import { AuthenticateOptions, AuthorizeOptions, Profile, SamlConfig } from "../passport-saml/types"; import { assertRequired } from "./utility"; +import { xpath } from "./xml"; const inflateRawAsync = util.promisify(zlib.inflateRaw); const deflateRawAsync = util.promisify(zlib.deflateRaw); @@ -94,7 +95,7 @@ async function processValidlySignedSamlLogoutAsync( } async function promiseWithNameID(nameid: Node): Promise { - const format = xmlCrypto.xpath(nameid, "@Format") as Node[]; + const format = xpath.selectAttributes(nameid, "@Format"); return { value: nameid.textContent, format: format && format[0] && format[0].nodeValue, @@ -683,7 +684,7 @@ class SAML { // // See https://github.com/bergie/passport-saml/issues/19 for references to some of the attack // vectors against SAML signature verification. - validateSignature(fullXml: string, currentNode: HTMLElement, certs: string[]): boolean { + validateSignature(fullXml: string, currentNode: Element, certs: string[]): boolean { const xpathSigQuery = ".//*[" + "local-name(.)='Signature' and " + @@ -692,7 +693,7 @@ class SAML { currentNode.getAttribute("ID") + "']" + "]"; - const signatures = xmlCrypto.xpath(currentNode, xpathSigQuery); + const signatures = xpath.selectElements(currentNode, xpathSigQuery); // This function is expecting to validate exactly one signature, so if we find more or fewer // than that, reject. if (signatures.length !== 1) { @@ -701,16 +702,16 @@ class SAML { const signature = signatures[0]; return certs.some((certToCheck) => { - return this.validateSignatureForCert(signature as string, certToCheck, fullXml, currentNode); + return this.validateSignatureForCert(signature, certToCheck, fullXml, currentNode); }); } // This function checks that the |signature| is signed with a given |cert|. validateSignatureForCert( - signature: string | Node, + signature: Node, cert: string, fullXml: string, - currentNode: HTMLElement + currentNode: Element ): boolean { const sig = new xmlCrypto.SignedXml(); sig.keyInfoProvider = { @@ -718,8 +719,8 @@ class SAML { getKeyInfo: () => "", getKey: () => Buffer.from(this._certToPEM(cert)), }; - signature = this.normalizeNewlines(signature.toString()); - sig.loadSignature(signature); + const signatureStr = this.normalizeNewlines(signature.toString()); + sig.loadSignature(signatureStr); // We expect each signature to contain exactly one reference to the top level of the xml we // are validating, so if we see anything else, reject. if (sig.references.length != 1) return false; @@ -730,7 +731,7 @@ class SAML { if (currentNode.getAttribute(idAttribute) != refId) return false; // If we find any extra referenced nodes, reject. (xml-crypto only verifies one digest, so // multiple candidate references is bad news) - const totalReferencedNodes = xmlCrypto.xpath( + const totalReferencedNodes = xpath.selectElements( currentNode.ownerDocument, "//*[@" + idAttribute + "='" + refId + "']" ); @@ -755,10 +756,10 @@ class SAML { if (!Object.prototype.hasOwnProperty.call(doc, "documentElement")) throw new Error("SAMLResponse is not valid base64-encoded XML"); - const inResponseToNodes = xmlCrypto.xpath( + const inResponseToNodes = xpath.selectAttributes( doc, "/*[local-name()='Response']/@InResponseTo" - ) as Attr[]; + ); if (inResponseToNodes) { inResponseTo = inResponseToNodes.length ? inResponseToNodes[0].nodeValue : null; @@ -772,11 +773,11 @@ class SAML { validSignature = true; } - const assertions = xmlCrypto.xpath( + const assertions = xpath.selectElements( doc, "/*[local-name()='Response']/*[local-name()='Assertion']" - ) as HTMLElement[]; - const encryptedAssertions = xmlCrypto.xpath( + ); + const encryptedAssertions = xpath.selectElements( doc, "/*[local-name()='Response']/*[local-name()='EncryptedAssertion']" ); @@ -815,10 +816,10 @@ class SAML { xmlencOptions ); const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); - const decryptedAssertions = xmlCrypto.xpath( + const decryptedAssertions = xpath.selectElements( decryptedDoc, "/*[local-name()='Assertion']" - ) as HTMLElement[]; + ); if (decryptedAssertions.length != 1) throw new Error("Invalid EncryptedAssertion content"); if ( @@ -1300,14 +1301,14 @@ class SAML { } async _getNameIdAsync(self: SAML, doc: Node): Promise { - const nameIds = xmlCrypto.xpath( + const nameIds = xpath.selectElements( doc, "/*[local-name()='LogoutRequest']/*[local-name()='NameID']" - ) as Node[]; - const encryptedIds = xmlCrypto.xpath( + ); + const encryptedIds = xpath.selectElements( doc, "/*[local-name()='LogoutRequest']/*[local-name()='EncryptedID']" - ) as Node[]; + ); if (nameIds.length + encryptedIds.length > 1) { throw new Error("Invalid LogoutRequest"); @@ -1321,7 +1322,10 @@ class SAML { "No decryption key found getting name ID for encrypted SAML response" ); - const encryptedDatas = xmlCrypto.xpath(encryptedIds[0], "./*[local-name()='EncryptedData']"); + const encryptedDatas = xpath.selectElements( + encryptedIds[0], + "./*[local-name()='EncryptedData']" + ); if (encryptedDatas.length !== 1) { throw new Error("Invalid LogoutRequest"); @@ -1334,7 +1338,7 @@ class SAML { xmlencOptions ); const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); - const decryptedIds = xmlCrypto.xpath(decryptedDoc, "/*[local-name()='NameID']") as Node[]; + const decryptedIds = xpath.selectElements(decryptedDoc, "/*[local-name()='NameID']"); if (decryptedIds.length !== 1) { throw new Error("Invalid EncryptedAssertion content"); } diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts new file mode 100644 index 00000000..8f626ff7 --- /dev/null +++ b/src/node-saml/xml.ts @@ -0,0 +1,40 @@ +import * as xmlCrypto from "xml-crypto"; + +type SelectedValue = string | number | boolean | Node; + +const selectXPath = ( + guard: (values: SelectedValue[]) => values is T[], + node: Node, + xpath: string +): T[] => { + const result = xmlCrypto.xpath(node, xpath); + if (!guard(result)) { + throw new Error("invalid xpath return type"); + } + return result; +}; + +const attributesXPathTypeGuard = (values: SelectedValue[]): values is Attr[] => { + return values.every((value) => { + if (typeof value != "object") { + return false; + } + return typeof value.nodeType === "number" && value.nodeType === value.ATTRIBUTE_NODE; + }); +}; + +const elementsXPathTypeGuard = (values: SelectedValue[]): values is Element[] => { + return values.every((value) => { + if (typeof value != "object") { + return false; + } + return typeof value.nodeType === "number" && value.nodeType === value.ELEMENT_NODE; + }); +}; + +export const xpath = { + selectAttributes: (node: Node, xpath: string): Attr[] => + selectXPath(attributesXPathTypeGuard, node, xpath), + selectElements: (node: Node, xpath: string): Element[] => + selectXPath(elementsXPathTypeGuard, node, xpath), +}; From 833fb4aabdc45da07856e0d394ddefa8e3ed0233 Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 10:01:30 +0100 Subject: [PATCH 2/8] Move xml-crypto and xmlenc operations to xml module --- src/node-saml/saml.ts | 75 ++++++------------------------------------- src/node-saml/xml.ts | 60 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 66 deletions(-) diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index 1038131c..01acfc62 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -2,13 +2,11 @@ import Debug from "debug"; const debug = Debug("node-saml"); import * as zlib from "zlib"; import * as xml2js from "xml2js"; -import * as xmlCrypto from "xml-crypto"; import * as crypto from "crypto"; import * as xmldom from "xmldom"; import { URL } from "url"; import * as querystring from "querystring"; import * as xmlbuilder from "xmlbuilder"; -import * as xmlenc from "xml-encryption"; import * as util from "util"; import { CacheProvider as InMemoryCacheProvider } from "./inmemory-cache-provider"; import * as algorithms from "./algorithms"; @@ -29,7 +27,7 @@ import { } from "./types"; import { AuthenticateOptions, AuthorizeOptions, Profile, SamlConfig } from "../passport-saml/types"; import { assertRequired } from "./utility"; -import { xpath } from "./xml"; +import { decryptXml, validateXmlSignatureForCert, xpath } from "./xml"; const inflateRawAsync = util.promisify(zlib.inflateRaw); const deflateRawAsync = util.promisify(zlib.deflateRaw); @@ -702,49 +700,15 @@ class SAML { const signature = signatures[0]; return certs.some((certToCheck) => { - return this.validateSignatureForCert(signature, certToCheck, fullXml, currentNode); + return validateXmlSignatureForCert( + signature, + this._certToPEM(certToCheck), + fullXml, + currentNode + ); }); } - // This function checks that the |signature| is signed with a given |cert|. - validateSignatureForCert( - signature: Node, - cert: string, - fullXml: string, - currentNode: Element - ): boolean { - const sig = new xmlCrypto.SignedXml(); - sig.keyInfoProvider = { - file: "", - getKeyInfo: () => "", - getKey: () => Buffer.from(this._certToPEM(cert)), - }; - const signatureStr = this.normalizeNewlines(signature.toString()); - sig.loadSignature(signatureStr); - // We expect each signature to contain exactly one reference to the top level of the xml we - // are validating, so if we see anything else, reject. - if (sig.references.length != 1) return false; - const refUri = sig.references[0].uri!; - const refId = refUri[0] === "#" ? refUri.substring(1) : refUri; - // If we can't find the reference at the top level, reject - const idAttribute = currentNode.getAttribute("ID") ? "ID" : "Id"; - if (currentNode.getAttribute(idAttribute) != refId) return false; - // If we find any extra referenced nodes, reject. (xml-crypto only verifies one digest, so - // multiple candidate references is bad news) - const totalReferencedNodes = xpath.selectElements( - currentNode.ownerDocument, - "//*[@" + idAttribute + "='" + refId + "']" - ); - - if (totalReferencedNodes.length > 1) { - return false; - } - // normalize XML to replace XML-encoded carriage returns with actual carriage returns - fullXml = this.normalizeXml(fullXml); - fullXml = this.normalizeNewlines(fullXml); - return sig.checkSignature(fullXml); - } - async validatePostResponseAsync( container: Record ): Promise<{ profile?: Profile | null; loggedOut?: boolean }> { @@ -810,11 +774,7 @@ class SAML { const encryptedAssertionXml = encryptedAssertions[0].toString(); - const xmlencOptions = { key: this.options.decryptionPvk }; - const decryptedXml: string = await util.promisify(xmlenc.decrypt).bind(xmlenc)( - encryptedAssertionXml, - xmlencOptions - ); + const decryptedXml = await decryptXml(encryptedAssertionXml, this.options.decryptionPvk); const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); const decryptedAssertions = xpath.selectElements( decryptedDoc, @@ -1332,11 +1292,7 @@ class SAML { } const encryptedDataXml = encryptedDatas[0].toString(); - const xmlencOptions = { key: self.options.decryptionPvk }; - const decryptedXml: string = await util.promisify(xmlenc.decrypt).bind(xmlenc)( - encryptedDataXml, - xmlencOptions - ); + const decryptedXml = await decryptXml(encryptedDataXml, self.options.decryptionPvk); const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); const decryptedIds = xpath.selectElements(decryptedDoc, "/*[local-name()='NameID']"); if (decryptedIds.length !== 1) { @@ -1464,19 +1420,6 @@ class SAML { throw new Error("Invalid key"); } - - normalizeNewlines(xml: string): string { - // we can use this utility before passing XML to `xml-crypto` - // we are considered the XML processor and are responsible for newline normalization - // https://github.com/node-saml/passport-saml/issues/431#issuecomment-718132752 - return xml.replace(/\r\n?/g, "\n"); - } - - normalizeXml(xml: string): string { - // we can use this utility to parse and re-stringify XML - // `DOMParser` will take care of normalization tasks, like replacing XML-encoded carriage returns with actual carriage returns - return new xmldom.DOMParser({}).parseFromString(xml).toString(); - } } export { SAML }; diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 8f626ff7..880d1900 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -1,4 +1,7 @@ +import * as util from "util"; import * as xmlCrypto from "xml-crypto"; +import * as xmlenc from "xml-encryption"; +import * as xmldom from "xmldom"; type SelectedValue = string | number | boolean | Node; @@ -38,3 +41,60 @@ export const xpath = { selectElements: (node: Node, xpath: string): Element[] => selectXPath(elementsXPathTypeGuard, node, xpath), }; + +export const decryptXml = async (xml: string, decryptionKey: string | Buffer) => + util.promisify(xmlenc.decrypt).bind(xmlenc)(xml, { key: decryptionKey }); + +const normalizeNewlines = (xml: string): string => { + // we can use this utility before passing XML to `xml-crypto` + // we are considered the XML processor and are responsible for newline normalization + // https://github.com/node-saml/passport-saml/issues/431#issuecomment-718132752 + return xml.replace(/\r\n?/g, "\n"); +}; + +const normalizeXml = (xml: string): string => { + // we can use this utility to parse and re-stringify XML + // `DOMParser` will take care of normalization tasks, like replacing XML-encoded carriage returns with actual carriage returns + return new xmldom.DOMParser({}).parseFromString(xml).toString(); +}; + +/** + * This function checks that the |signature| is signed with a given |cert|. + */ +export const validateXmlSignatureForCert = ( + signature: Node, + certPem: string, + fullXml: string, + currentNode: Element +): boolean => { + const sig = new xmlCrypto.SignedXml(); + sig.keyInfoProvider = { + file: "", + getKeyInfo: () => "", + getKey: () => Buffer.from(certPem), + }; + const signatureStr = normalizeNewlines(signature.toString()); + sig.loadSignature(signatureStr); + // We expect each signature to contain exactly one reference to the top level of the xml we + // are validating, so if we see anything else, reject. + if (sig.references.length != 1) return false; + const refUri = sig.references[0].uri!; + const refId = refUri[0] === "#" ? refUri.substring(1) : refUri; + // If we can't find the reference at the top level, reject + const idAttribute = currentNode.getAttribute("ID") ? "ID" : "Id"; + if (currentNode.getAttribute(idAttribute) != refId) return false; + // If we find any extra referenced nodes, reject. (xml-crypto only verifies one digest, so + // multiple candidate references is bad news) + const totalReferencedNodes = xpath.selectElements( + currentNode.ownerDocument, + "//*[@" + idAttribute + "='" + refId + "']" + ); + + if (totalReferencedNodes.length > 1) { + return false; + } + // normalize XML to replace XML-encoded carriage returns with actual carriage returns + fullXml = normalizeXml(fullXml); + fullXml = normalizeNewlines(fullXml); + return sig.checkSignature(fullXml); +}; From b8767725ade355c493aee16216118dae303e1d1e Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 10:18:35 +0100 Subject: [PATCH 3/8] Move xmldom parsing to xml module --- src/node-saml/saml.ts | 13 ++++++------- src/node-saml/xml.ts | 4 ++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index 01acfc62..c7218872 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -3,7 +3,6 @@ const debug = Debug("node-saml"); import * as zlib from "zlib"; import * as xml2js from "xml2js"; import * as crypto from "crypto"; -import * as xmldom from "xmldom"; import { URL } from "url"; import * as querystring from "querystring"; import * as xmlbuilder from "xmlbuilder"; @@ -27,7 +26,7 @@ import { } from "./types"; import { AuthenticateOptions, AuthorizeOptions, Profile, SamlConfig } from "../passport-saml/types"; import { assertRequired } from "./utility"; -import { decryptXml, validateXmlSignatureForCert, xpath } from "./xml"; +import { decryptXml, parseDomFromString, validateXmlSignatureForCert, xpath } from "./xml"; const inflateRawAsync = util.promisify(zlib.inflateRaw); const deflateRawAsync = util.promisify(zlib.deflateRaw); @@ -715,7 +714,7 @@ class SAML { let xml: string, doc: Document, inResponseTo: string | null; try { xml = Buffer.from(container.SAMLResponse, "base64").toString("utf8"); - doc = new xmldom.DOMParser({}).parseFromString(xml); + doc = parseDomFromString(xml); if (!Object.prototype.hasOwnProperty.call(doc, "documentElement")) throw new Error("SAMLResponse is not valid base64-encoded XML"); @@ -775,7 +774,7 @@ class SAML { const encryptedAssertionXml = encryptedAssertions[0].toString(); const decryptedXml = await decryptXml(encryptedAssertionXml, this.options.decryptionPvk); - const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); + const decryptedDoc = parseDomFromString(decryptedXml); const decryptedAssertions = xpath.selectElements( decryptedDoc, "/*[local-name()='Assertion']" @@ -897,7 +896,7 @@ class SAML { const data = Buffer.from(container[samlMessageType] as string, "base64"); const inflated = await inflateRawAsync(data); - const dom = new xmldom.DOMParser().parseFromString(inflated.toString()); + const dom = parseDomFromString(inflated.toString()); const parserConfig = { explicitRoot: true, explicitCharkey: true, @@ -1245,7 +1244,7 @@ class SAML { container: Record ): Promise<{ profile?: Profile; loggedOut?: boolean }> { const xml = Buffer.from(container.SAMLRequest, "base64").toString("utf8"); - const dom = new xmldom.DOMParser().parseFromString(xml); + const dom = parseDomFromString(xml); const parserConfig = { explicitRoot: true, explicitCharkey: true, @@ -1293,7 +1292,7 @@ class SAML { const encryptedDataXml = encryptedDatas[0].toString(); const decryptedXml = await decryptXml(encryptedDataXml, self.options.decryptionPvk); - const decryptedDoc = new xmldom.DOMParser().parseFromString(decryptedXml); + const decryptedDoc = parseDomFromString(decryptedXml); const decryptedIds = xpath.selectElements(decryptedDoc, "/*[local-name()='NameID']"); if (decryptedIds.length !== 1) { throw new Error("Invalid EncryptedAssertion content"); diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 880d1900..7802197e 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -98,3 +98,7 @@ export const validateXmlSignatureForCert = ( fullXml = normalizeNewlines(fullXml); return sig.checkSignature(fullXml); }; + +export const parseDomFromString = (xml: string): Document => { + return new xmldom.DOMParser().parseFromString(xml); +}; From beda308a333a78ee6c98d71111ece58f46fbc90c Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 10:52:52 +0100 Subject: [PATCH 4/8] Move xml2js to xml module --- src/node-saml/saml.ts | 65 +++++++++++++++----------------------- src/node-saml/xml.ts | 19 +++++++++++ src/passport-saml/types.ts | 6 ++++ 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index c7218872..da0302a0 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -1,7 +1,6 @@ import Debug from "debug"; const debug = Debug("node-saml"); import * as zlib from "zlib"; -import * as xml2js from "xml2js"; import * as crypto from "crypto"; import { URL } from "url"; import * as querystring from "querystring"; @@ -24,9 +23,22 @@ import { XMLObject, XMLOutput, } from "./types"; -import { AuthenticateOptions, AuthorizeOptions, Profile, SamlConfig } from "../passport-saml/types"; +import { + AuthenticateOptions, + AuthorizeOptions, + Profile, + SamlConfig, + ErrorWithXmlStatus, +} from "../passport-saml/types"; import { assertRequired } from "./utility"; -import { decryptXml, parseDomFromString, validateXmlSignatureForCert, xpath } from "./xml"; +import { + buildXml2JsObject, + decryptXml, + parseDomFromString, + parseXml2JsFromString, + validateXmlSignatureForCert, + xpath, +} from "./xml"; const inflateRawAsync = util.promisify(zlib.inflateRaw); const deflateRawAsync = util.promisify(zlib.deflateRaw); @@ -798,13 +810,7 @@ class SAML { // If there's no assertion, fall back on xml2js response parsing for the status & // LogoutResponse code. - const parserConfig = { - explicitRoot: true, - explicitCharkey: true, - tagNameProcessors: [xml2js.processors.stripPrefix], - }; - const parser = new xml2js.Parser(parserConfig); - const xmljsDoc = await parser.parseStringPromise(xml); + const xmljsDoc = await parseXml2JsFromString(xml); const response = xmljsDoc.Response; if (response) { const assertion = response.Assertion; @@ -840,14 +846,11 @@ class SAML { } else if (statusCode[0].StatusCode) { msg = statusCode[0].StatusCode[0].$.Value.match(/[^:]*$/)[0]; } - const error = new Error("SAML provider returned " + msgType + " error: " + msg); - const builderOpts = { - rootName: "Status", - headless: true, - }; - // @ts-expect-error adding extra attr to default Error object - error.statusXml = new xml2js.Builder(builderOpts).buildObject(status[0]); - throw error; + const statusXml = buildXml2JsObject("Status", status[0]); + throw new ErrorWithXmlStatus( + "SAML provider returned " + msgType + " error: " + msg, + statusXml + ); } } } @@ -897,13 +900,7 @@ class SAML { const inflated = await inflateRawAsync(data); const dom = parseDomFromString(inflated.toString()); - const parserConfig = { - explicitRoot: true, - explicitCharkey: true, - tagNameProcessors: [xml2js.processors.stripPrefix], - }; - const parser = new xml2js.Parser(parserConfig); - const doc: XMLOutput = await parser.parseStringPromise(inflated); + const doc: XMLOutput = await parseXml2JsFromString(inflated); samlMessageType === "SAMLResponse" ? await this.verifyLogoutResponse(doc) : this.verifyLogoutRequest(doc); @@ -1019,20 +1016,14 @@ class SAML { } private async processValidlySignedAssertionAsync( - xml: xml2js.convertableToString, + xml: string, samlResponseXml: string, inResponseTo: string ) { let msg; - const parserConfig = { - explicitRoot: true, - explicitCharkey: true, - tagNameProcessors: [xml2js.processors.stripPrefix], - }; const nowMs = new Date().getTime(); const profile = {} as Profile; - const parser = new xml2js.Parser(parserConfig); - const doc: XMLOutput = await parser.parseStringPromise(xml); + const doc: XMLOutput = await parseXml2JsFromString(xml); const parsedAssertion: XMLOutput = doc; const assertion: XMLOutput = doc.Assertion; getInResponseTo: { @@ -1245,13 +1236,7 @@ class SAML { ): Promise<{ profile?: Profile; loggedOut?: boolean }> { const xml = Buffer.from(container.SAMLRequest, "base64").toString("utf8"); const dom = parseDomFromString(xml); - const parserConfig = { - explicitRoot: true, - explicitCharkey: true, - tagNameProcessors: [xml2js.processors.stripPrefix], - }; - const parser = new xml2js.Parser(parserConfig); - const doc = await parser.parseStringPromise(xml); + const doc = await parseXml2JsFromString(xml); const certs = await this.certsToCheck(); if (!this.validateSignature(xml, dom.documentElement, certs)) { throw new Error("Invalid signature on documentElement"); diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 7802197e..3963b5f8 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -2,6 +2,7 @@ import * as util from "util"; import * as xmlCrypto from "xml-crypto"; import * as xmlenc from "xml-encryption"; import * as xmldom from "xmldom"; +import * as xml2js from "xml2js"; type SelectedValue = string | number | boolean | Node; @@ -102,3 +103,21 @@ export const validateXmlSignatureForCert = ( export const parseDomFromString = (xml: string): Document => { return new xmldom.DOMParser().parseFromString(xml); }; + +export const parseXml2JsFromString = async (xml: string | Buffer): Promise => { + const parserConfig = { + explicitRoot: true, + explicitCharkey: true, + tagNameProcessors: [xml2js.processors.stripPrefix], + }; + const parser = new xml2js.Parser(parserConfig); + return parser.parseStringPromise(xml); +}; + +export const buildXml2JsObject = (rootName: string, xml: any): string => { + const builderOpts = { + rootName, + headless: true, + }; + return new xml2js.Builder(builderOpts).buildObject(xml); +}; diff --git a/src/passport-saml/types.ts b/src/passport-saml/types.ts index b5dbd72d..bdc209d1 100644 --- a/src/passport-saml/types.ts +++ b/src/passport-saml/types.ts @@ -67,3 +67,9 @@ interface BaseMultiSamlConfig { } export type MultiSamlConfig = Partial & StrategyOptions & BaseMultiSamlConfig; + +export class ErrorWithXmlStatus extends Error { + constructor(message: string, public readonly xmlStatus: string) { + super(message); + } +} From 512fb6126f525b4e5429beca85f82fe778d7eec4 Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 10:59:52 +0100 Subject: [PATCH 5/8] Move xmlbuilder to xml module --- src/node-saml/saml.ts | 12 +++++------- src/node-saml/xml.ts | 6 ++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index da0302a0..bc7b767d 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -4,7 +4,6 @@ import * as zlib from "zlib"; import * as crypto from "crypto"; import { URL } from "url"; import * as querystring from "querystring"; -import * as xmlbuilder from "xmlbuilder"; import * as util from "util"; import { CacheProvider as InMemoryCacheProvider } from "./inmemory-cache-provider"; import * as algorithms from "./algorithms"; @@ -33,6 +32,7 @@ import { import { assertRequired } from "./utility"; import { buildXml2JsObject, + buildXmlBuilderObject, decryptXml, parseDomFromString, parseXml2JsFromString, @@ -354,7 +354,7 @@ class SAML { request["samlp:AuthnRequest"]["samlp:Scoping"] = scoping; } - let stringRequest = xmlbuilder.create((request as unknown) as Record).end(); + let stringRequest = buildXmlBuilderObject(request, false); if (isHttpPostBinding && this.options.privateKey != null) { stringRequest = signAuthnRequestPost(stringRequest, this.options); } @@ -400,7 +400,7 @@ class SAML { } await this.cacheProvider.saveAsync(id, instant); - return xmlbuilder.create((request as unknown) as Record).end(); + return buildXmlBuilderObject(request, false); } _generateLogoutResponse(logoutRequest: Profile) { @@ -427,7 +427,7 @@ class SAML { }, }; - return xmlbuilder.create(request).end(); + return buildXmlBuilderObject(request, false); } async _requestToUrlAsync( @@ -1379,9 +1379,7 @@ class SAML { "@Binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", "@Location": this.getCallbackUrl(), }; - return xmlbuilder - .create((metadata as unknown) as Record) - .end({ pretty: true, indent: " ", newline: "\n" }); + return buildXmlBuilderObject(metadata, true); } _keyToPEM(key: string | Buffer): typeof key extends string | Buffer ? string | Buffer : Error { diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 3963b5f8..1615f74a 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -3,6 +3,7 @@ import * as xmlCrypto from "xml-crypto"; import * as xmlenc from "xml-encryption"; import * as xmldom from "xmldom"; import * as xml2js from "xml2js"; +import * as xmlbuilder from "xmlbuilder"; type SelectedValue = string | number | boolean | Node; @@ -121,3 +122,8 @@ export const buildXml2JsObject = (rootName: string, xml: any): string => { }; return new xml2js.Builder(builderOpts).buildObject(xml); }; + +export const buildXmlBuilderObject = (xml: Record, pretty: boolean): string => { + const options = pretty ? { pretty: true, indent: " ", newline: "\n" } : {}; + return xmlbuilder.create(xml).end(options); +}; From 0816f14d1e5292e5eafe84196cb79612ed2a966f Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 22:00:53 +0100 Subject: [PATCH 6/8] Improve signature tests to better detect regressions --- .../node-saml/saml-post-signing-tests.spec.ts | 299 ++++++++++++++++-- 1 file changed, 267 insertions(+), 32 deletions(-) diff --git a/test/node-saml/saml-post-signing-tests.spec.ts b/test/node-saml/saml-post-signing-tests.spec.ts index c713aa39..0eae5181 100644 --- a/test/node-saml/saml-post-signing-tests.spec.ts +++ b/test/node-saml/saml-post-signing-tests.spec.ts @@ -1,28 +1,129 @@ import * as fs from "fs"; -import * as should from "should"; import { signSamlPost, signAuthnRequestPost } from "../../src/node-saml/saml-post-signing"; import { SamlSigningOptions } from "../../src/node-saml/types"; +import { parseXml2JsFromString } from "../../src/node-saml/xml"; const signingKey = fs.readFileSync(__dirname + "/../static/key.pem"); describe("SAML POST Signing", function () { - it("should sign a simple saml request", function () { + it("should sign a simple saml request", async function () { const xml = 'http://example.com'; const result = signSamlPost(xml, "/SAMLRequest", { privateKey: signingKey }); - result.should.match(/[A-Za-z0-9/+=]+<\/DigestValue>/); - result.should.match(/[A-Za-z0-9/+=]+<\/SignatureValue>/); + const doc = await parseXml2JsFromString(result); + doc.should.be.deepEqual({ + SAMLRequest: { + $: { Id: "_0" }, + Issuer: [ + { + _: "http://example.com", + $: { "xmlns:saml2": "urn:oasis:names:tc:SAML:2.0:assertion" }, + }, + ], + Signature: [ + { + $: { xmlns: "http://www.w3.org/2000/09/xmldsig#" }, + SignedInfo: [ + { + CanonicalizationMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + SignatureMethod: [ + { $: { Algorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1" } }, + ], + Reference: [ + { + $: { URI: "#_0" }, + Transforms: [ + { + Transform: [ + { + $: { + Algorithm: "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + }, + }, + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + }, + ], + DigestMethod: [{ $: { Algorithm: "http://www.w3.org/2000/09/xmldsig#sha1" } }], + DigestValue: [{ _: "1yis05FW/NgGxi12sn/bW3GP9co=" }], + }, + ], + }, + ], + SignatureValue: [ + { + _: + "Oa5ST39rynnUH6XN4tnjoK2luRlKGOq4VHPAKqSgEjzEymTFQRhMwqwQTuFI+AwHSn0qd4wc7GGLIHn0BmUsk/CBZ51nvgjiyTQo+Gkc2/24QlCAwpOM35hgOEaMMvJXgzkFwxvnV/3TGA2J+jrrcQ0q2l6nSuDe27JnCCzbo1vFiHIuWG91pZnS0ZQKnJ593jG5ozo2m2a7l/KvCXIWCGs91KR43IKgmQmOIkVk4i170Ep2trlyj5651LFlT4LShDkkrf4tvWAmeC7rZgf97j58m9vTYXY7zZt5URIvmlE9SZH6NmUdrryZjfZin4Xf7FqpfK/sLzVfBCSLvCse8A==", + }, + ], + }, + ], + }, + }); }); - it("should place the Signature element after the Issuer element", function () { + it("should place the Signature element after the Issuer element", async function () { const xml = 'http://example.com'; const result = signSamlPost(xml, "/SAMLRequest", { privateKey: signingKey }); - result.should.match(/<\/saml2:Issuer>/); - result.should.match( - // - ); - result.should.match( - // - ); + const doc = await parseXml2JsFromString(result); + doc.should.be.deepEqual({ + SAMLRequest: { + $: { Id: "_0" }, + Issuer: [ + { + _: "http://example.com", + $: { "xmlns:saml2": "urn:oasis:names:tc:SAML:2.0:assertion" }, + }, + ], + Signature: [ + { + $: { xmlns: "http://www.w3.org/2000/09/xmldsig#" }, + SignedInfo: [ + { + CanonicalizationMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + SignatureMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" } }, + ], + Reference: [ + { + $: { URI: "#_0" }, + Transforms: [ + { + Transform: [ + { + $: { + Algorithm: "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + }, + }, + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + }, + ], + DigestMethod: [{ $: { Algorithm: "http://www.w3.org/2001/04/xmlenc#sha256" } }], + DigestValue: [{ _: "DeVk/na+V3reUnB3kJPBXoeA12QBCaPNSr/J/1+g8X0=" }], + }, + ], + }, + ], + SignatureValue: [ + { + _: + "N1vamg3kKL4lvk+i/ZPltfZRIvFPO4J+CpNslFCKcuOpVTtgxhbvaHEnmU1gTpfEmFHw2js8isKWbEWepsP+aOfQMFDTnlZM2X7HtuB6uKntpS6bOUnG4mx+P2stbRyhLzJIsDwHTvzZM5+L63O551afjZxYCJBwD2bsvUk1A/1N6dG9+AB6QP/x/Fl6OjZE9J/kQWVZbRyty48p3sIBkO1L0rVk7ekHj5f83JGRtyKt9nlK7ke8dX+BItPQ/CU353RRumQ6rSkv+MZVzqfGWcg6wIc4x5+euS9zA80eBrYOvIU9vjzK8Bd+Lv9ltAAtISMRrVCVWW0XgnKJ4fzZGg==", + }, + ], + }, + ], + }, + }); }); - it("should sign and digest with SHA256 when specified and using privateKey", function () { + it("should sign and digest with SHA256 when specified and using privateKey", async function () { const xml = 'http://example.com'; const options: SamlSigningOptions = { @@ -52,23 +195,115 @@ describe("SAML POST Signing", function () { privateKey: signingKey, }; const result = signSamlPost(xml, "/SAMLRequest", options); - result.should.match( - //); - result.should.match( - // - ); - result.should.match( - // - ); + const doc = await parseXml2JsFromString(result); + doc.should.be.deepEqual({ + SAMLRequest: { + $: { Id: "_0" }, + Issuer: [ + { + _: "http://example.com", + $: { "xmlns:saml2": "urn:oasis:names:tc:SAML:2.0:assertion" }, + }, + ], + Signature: [ + { + $: { xmlns: "http://www.w3.org/2000/09/xmldsig#" }, + SignedInfo: [ + { + CanonicalizationMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + SignatureMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" } }, + ], + Reference: [ + { + $: { URI: "#_0" }, + Transforms: [ + { + Transform: [ + { + $: { + Algorithm: "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + }, + }, + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + }, + ], + DigestMethod: [{ $: { Algorithm: "http://www.w3.org/2001/04/xmlenc#sha256" } }], + DigestValue: [{ _: "DeVk/na+V3reUnB3kJPBXoeA12QBCaPNSr/J/1+g8X0=" }], + }, + ], + }, + ], + SignatureValue: [ + { + _: + "N1vamg3kKL4lvk+i/ZPltfZRIvFPO4J+CpNslFCKcuOpVTtgxhbvaHEnmU1gTpfEmFHw2js8isKWbEWepsP+aOfQMFDTnlZM2X7HtuB6uKntpS6bOUnG4mx+P2stbRyhLzJIsDwHTvzZM5+L63O551afjZxYCJBwD2bsvUk1A/1N6dG9+AB6QP/x/Fl6OjZE9J/kQWVZbRyty48p3sIBkO1L0rVk7ekHj5f83JGRtyKt9nlK7ke8dX+BItPQ/CU353RRumQ6rSkv+MZVzqfGWcg6wIc4x5+euS9zA80eBrYOvIU9vjzK8Bd+Lv9ltAAtISMRrVCVWW0XgnKJ4fzZGg==", + }, + ], + }, + ], + }, + }); }); - it("should sign an AuthnRequest", function () { + it("should sign an AuthnRequest", async function () { const xml = 'http://example.com'; const result = signAuthnRequestPost(xml, { privateKey: signingKey }); - result.should.match(/[A-Za-z0-9/+=]+<\/DigestValue>/); - result.should.match(/[A-Za-z0-9/+=]+<\/SignatureValue>/); + const doc = await parseXml2JsFromString(result); + doc.should.be.deepEqual({ + AuthnRequest: { + $: { xmlns: "urn:oasis:names:tc:SAML:2.0:protocol", Id: "_0" }, + Issuer: [ + { + _: "http://example.com", + $: { "xmlns:saml2": "urn:oasis:names:tc:SAML:2.0:assertion" }, + }, + ], + Signature: [ + { + $: { xmlns: "http://www.w3.org/2000/09/xmldsig#" }, + SignedInfo: [ + { + CanonicalizationMethod: [ + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + SignatureMethod: [ + { $: { Algorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1" } }, + ], + Reference: [ + { + $: { URI: "#_0" }, + Transforms: [ + { + Transform: [ + { + $: { + Algorithm: "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + }, + }, + { $: { Algorithm: "http://www.w3.org/2001/10/xml-exc-c14n#" } }, + ], + }, + ], + DigestMethod: [{ $: { Algorithm: "http://www.w3.org/2000/09/xmldsig#sha1" } }], + DigestValue: [{ _: "wHDDyV7rEQ/AQLYeLgsEUXX+Zxw=" }], + }, + ], + }, + ], + SignatureValue: [ + { + _: + "t6Vg5DrOQiwfVv1IBzhPXMwoRGdNY1lIKbvcZOXr9EeFEEaI8I8qPs9Ibl+Hj3eCC0aDVLg/Uhg9/NCygfYuQuJjFdji0/rEFve/DEgGDscCS42+0J5fM55wNyVLglly9D+hJdZChmHg5IQltFcvOsNHYxbUiPywbOSLSHHFqOfdL4bqYNO/nwhhHMRuA6VQGRSC8EGJkjF9kwuFVjF7XvXyV2aTRJgZYmUB3fzIlokUfBNg2PpvexLipOb1K14ZV0nORewOCPjulJWnd+WSJkHBY1jA/OGiJNCeokOw7XTOLrAZ9+d4/JJ7T3XthWwHrfP3gEljoNTUdQV/gBNNqA==", + }, + ], + }, + ], + }, + }); }); }); From 4a4ab1590d712af71d538c069783a47f9029e9f7 Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 22 Mar 2021 18:01:08 +0100 Subject: [PATCH 7/8] Move signXML function to xml module --- src/node-saml/saml-post-signing.ts | 29 +++-------------------- src/node-saml/saml.ts | 4 +++- src/node-saml/types.ts | 10 ++++++-- src/node-saml/utility.ts | 38 ++++++------------------------ src/node-saml/xml.ts | 37 +++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/node-saml/saml-post-signing.ts b/src/node-saml/saml-post-signing.ts index df55ec2d..74a036ce 100644 --- a/src/node-saml/saml-post-signing.ts +++ b/src/node-saml/saml-post-signing.ts @@ -1,42 +1,19 @@ -import { SignedXml } from "xml-crypto"; -import * as algorithms from "./algorithms"; import { SamlSigningOptions } from "./types"; +import { signXml } from "./xml"; const authnRequestXPath = '/*[local-name(.)="AuthnRequest" and namespace-uri(.)="urn:oasis:names:tc:SAML:2.0:protocol"]'; const issuerXPath = '/*[local-name(.)="Issuer" and namespace-uri(.)="urn:oasis:names:tc:SAML:2.0:assertion"]'; -const defaultTransforms = [ - "http://www.w3.org/2000/09/xmldsig#enveloped-signature", - "http://www.w3.org/2001/10/xml-exc-c14n#", -]; export function signSamlPost( samlMessage: string, xpath: string, options: SamlSigningOptions ): string { - if (!samlMessage) throw new Error("samlMessage is required"); - if (!xpath) throw new Error("xpath is required"); - if (!options) { - options = {} as SamlSigningOptions; - } - - if (options.privateKey == null) throw new Error("options.privateKey is required"); - - const transforms = options.xmlSignatureTransforms || defaultTransforms; - const sig = new SignedXml(); - if (options.signatureAlgorithm) { - sig.signatureAlgorithm = algorithms.getSigningAlgorithm(options.signatureAlgorithm); - } - sig.addReference(xpath, transforms, algorithms.getDigestAlgorithm(options.digestAlgorithm)); - sig.signingKey = options.privateKey; - sig.computeSignature(samlMessage, { - location: { reference: xpath + issuerXPath, action: "after" }, - }); - return sig.getSignedXml(); + return signXml(samlMessage, xpath, { reference: xpath + issuerXPath, action: "after" }, options); } -export function signAuthnRequestPost(authnRequest: string, options: SamlSigningOptions) { +export function signAuthnRequestPost(authnRequest: string, options: SamlSigningOptions): string { return signSamlPost(authnRequest, authnRequestXPath, options); } diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index bc7b767d..38891263 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -10,6 +10,7 @@ import * as algorithms from "./algorithms"; import { signAuthnRequestPost } from "./saml-post-signing"; import { ParsedQs } from "qs"; import { + isValidSamlSigningOptions, AudienceRestrictionXML, AuthorizeRequestXML, CertCallback, @@ -355,7 +356,8 @@ class SAML { } let stringRequest = buildXmlBuilderObject(request, false); - if (isHttpPostBinding && this.options.privateKey != null) { + // TODO: maybe we should always sign here + if (isHttpPostBinding && isValidSamlSigningOptions(this.options)) { stringRequest = signAuthnRequestPost(stringRequest, this.options); } return stringRequest; diff --git a/src/node-saml/types.ts b/src/node-saml/types.ts index 3767154b..e7dd83ef 100644 --- a/src/node-saml/types.ts +++ b/src/node-saml/types.ts @@ -3,12 +3,18 @@ import type { CacheProvider } from "./inmemory-cache-provider"; export type SignatureAlgorithm = "sha1" | "sha256" | "sha512"; export interface SamlSigningOptions { - privateKey?: string | Buffer; + privateKey: string | Buffer; signatureAlgorithm?: SignatureAlgorithm; xmlSignatureTransforms?: string[]; digestAlgorithm?: string; } +export const isValidSamlSigningOptions = ( + options: Partial +): options is SamlSigningOptions => { + return options.privateKey != null; +}; + export interface AudienceRestrictionXML { Audience?: XMLObject[]; } @@ -75,7 +81,7 @@ interface SamlScopingConfig { * The options required to use a SAML strategy * These may be provided by means of defaults specified in the constructor */ -export interface SamlOptions extends SamlSigningOptions, MandatorySamlOptions { +export interface SamlOptions extends Partial, MandatorySamlOptions { // Core callbackUrl?: string; path: string; diff --git a/src/node-saml/utility.ts b/src/node-saml/utility.ts index e789271f..5e31302c 100644 --- a/src/node-saml/utility.ts +++ b/src/node-saml/utility.ts @@ -1,6 +1,5 @@ -import { SignedXml } from "xml-crypto"; import { SamlSigningOptions } from "./types"; -import * as algorithms from "./algorithms"; +import { signXml } from "./xml"; export function assertRequired(value: T | null | undefined, error?: string): T { if (value === undefined || value === null || (typeof value === "string" && value.length === 0)) { @@ -10,37 +9,14 @@ export function assertRequired(value: T | null | undefined, error?: string): } } -export function signXml(samlMessage: string, xpath: string, options: SamlSigningOptions): string { - const defaultTransforms = [ - "http://www.w3.org/2000/09/xmldsig#enveloped-signature", - "http://www.w3.org/2001/10/xml-exc-c14n#", - ]; - - if (!samlMessage) throw new Error("samlMessage is required"); - if (!xpath) throw new Error("xpath is required"); - if (!options) { - options = {} as SamlSigningOptions; - } - - if (!options.privateKey) throw new Error("options.privateKey is required"); - - const transforms = options.xmlSignatureTransforms || defaultTransforms; - const sig = new SignedXml(); - if (options.signatureAlgorithm) { - sig.signatureAlgorithm = algorithms.getSigningAlgorithm(options.signatureAlgorithm); - } - sig.addReference(xpath, transforms, algorithms.getDigestAlgorithm(options.digestAlgorithm)); - sig.signingKey = options.privateKey; - sig.computeSignature(samlMessage, { - location: { reference: xpath, action: "append" }, - }); - - return sig.getSignedXml(); -} - export function signXmlResponse(samlMessage: string, options: SamlSigningOptions): string { const responseXpath = '//*[local-name(.)="Response" and namespace-uri(.)="urn:oasis:names:tc:SAML:2.0:protocol"]'; - return signXml(samlMessage, responseXpath, options); + return signXml( + samlMessage, + responseXpath, + { reference: responseXpath, action: "append" }, + options + ); } diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 1615f74a..474f6c89 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -4,6 +4,8 @@ import * as xmlenc from "xml-encryption"; import * as xmldom from "xmldom"; import * as xml2js from "xml2js"; import * as xmlbuilder from "xmlbuilder"; +import { isValidSamlSigningOptions, SamlSigningOptions } from "./types"; +import * as algorithms from "./algorithms"; type SelectedValue = string | number | boolean | Node; @@ -101,6 +103,41 @@ export const validateXmlSignatureForCert = ( return sig.checkSignature(fullXml); }; +interface XmlSignatureLocation { + reference: string; + action: "append" | "prepend" | "before" | "after"; +} + +export const signXml = ( + xml: string, + xpath: string, + location: XmlSignatureLocation, + options: SamlSigningOptions +): string => { + const defaultTransforms = [ + "http://www.w3.org/2000/09/xmldsig#enveloped-signature", + "http://www.w3.org/2001/10/xml-exc-c14n#", + ]; + + if (!xml) throw new Error("samlMessage is required"); + if (!location) throw new Error("location is required"); + if (!options) throw new Error("options is required"); + if (!isValidSamlSigningOptions(options)) throw new Error("options.privateKey is required"); + + const transforms = options.xmlSignatureTransforms ?? defaultTransforms; + const sig = new xmlCrypto.SignedXml(); + if (options.signatureAlgorithm != null) { + sig.signatureAlgorithm = algorithms.getSigningAlgorithm(options.signatureAlgorithm); + } + sig.addReference(xpath, transforms, algorithms.getDigestAlgorithm(options.digestAlgorithm)); + sig.signingKey = options.privateKey; + sig.computeSignature(xml, { + location, + }); + + return sig.getSignedXml(); +}; + export const parseDomFromString = (xml: string): Document => { return new xmldom.DOMParser().parseFromString(xml); }; From 3a6f50c728d50fed5cb9cdfb600c7f7190fe75db Mon Sep 17 00:00:00 2001 From: Quentin Barbe Date: Mon, 5 Apr 2021 22:47:41 +0200 Subject: [PATCH 8/8] Factorize parseDomFromString in xml module --- src/node-saml/xml.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node-saml/xml.ts b/src/node-saml/xml.ts index 474f6c89..fd7d361d 100644 --- a/src/node-saml/xml.ts +++ b/src/node-saml/xml.ts @@ -59,7 +59,7 @@ const normalizeNewlines = (xml: string): string => { const normalizeXml = (xml: string): string => { // we can use this utility to parse and re-stringify XML // `DOMParser` will take care of normalization tasks, like replacing XML-encoded carriage returns with actual carriage returns - return new xmldom.DOMParser({}).parseFromString(xml).toString(); + return parseDomFromString(xml).toString(); }; /**