From 4540c1a4d84eb2af9bba40648bb9efa74f9ad596 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Sat, 25 Feb 2023 18:50:43 -0700 Subject: [PATCH] Fix encryption for @xmldom/xmldom 0.8.6 upgrade (#511) --- src/libsaml.ts | 29 ++++++++++++++++------------- test/flow.ts | 42 +++++++++++++++++++++++------------------- test/index.ts | 9 ++++++--- 3 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/libsaml.ts b/src/libsaml.ts index 56f1b4b..d04b964 100644 --- a/src/libsaml.ts +++ b/src/libsaml.ts @@ -606,19 +606,20 @@ const libSaml = () => { const targetEntityMetadata = targetEntity.entityMeta; const doc = new dom().parseFromString(xml); const assertions = select("//*[local-name(.)='Assertion']", doc) as Node[]; - if (!Array.isArray(assertions)) { + if (!Array.isArray(assertions) || assertions.length === 0) { throw new Error('ERR_NO_ASSERTION'); } - if (assertions.length !== 1) { + if (assertions.length > 1) { throw new Error('ERR_MULTIPLE_ASSERTION'); } + const rawAssertionNode = assertions[0]; // Perform encryption depends on the setting, default is false if (sourceEntitySetting.isAssertionEncrypted) { const publicKeyPem = utility.getPublicKeyPemFromCertificate(targetEntityMetadata.getX509Certificate(certUse.encrypt)); - xmlenc.encrypt(assertions[0].toString(), { + xmlenc.encrypt(rawAssertionNode.toString(), { // use xml-encryption module rsa_pub: Buffer.from(publicKeyPem), // public key from certificate pem: Buffer.from(`-----BEGIN CERTIFICATE-----${targetEntityMetadata.getX509Certificate(certUse.encrypt)}-----END CERTIFICATE-----`), @@ -633,8 +634,8 @@ const libSaml = () => { return reject(new Error('ERR_UNDEFINED_ENCRYPTED_ASSERTION')); } const { encryptedAssertion: encAssertionPrefix } = sourceEntitySetting.tagPrefix; - const encryptAssertionNode = new dom().parseFromString(`<${encAssertionPrefix}:EncryptedAssertion xmlns:${encAssertionPrefix}="${namespace.names.assertion}">${res}`); - doc.replaceChild(encryptAssertionNode, assertions[0]); + const encryptAssertionDoc = new dom().parseFromString(`<${encAssertionPrefix}:EncryptedAssertion xmlns:${encAssertionPrefix}="${namespace.names.assertion}">${res}`); + doc.documentElement.replaceChild(encryptAssertionDoc.documentElement, rawAssertionNode); return resolve(utility.base64Encode(doc.toString())); }); } else { @@ -658,15 +659,17 @@ const libSaml = () => { } // Perform encryption depends on the setting of where the message is sent, default is false const hereSetting = here.entitySetting; - const xml = new dom().parseFromString(entireXML); - const encryptedAssertions = select("/*[contains(local-name(), 'Response')]/*[local-name(.)='EncryptedAssertion']", xml) as Node[]; - if (!Array.isArray(encryptedAssertions)) { + const doc = new dom().parseFromString(entireXML); + const encryptedAssertions = select("/*[contains(local-name(), 'Response')]/*[local-name(.)='EncryptedAssertion']", doc) as Node[]; + if (!Array.isArray(encryptedAssertions) || encryptedAssertions.length === 0) { throw new Error('ERR_UNDEFINED_ENCRYPTED_ASSERTION'); } - if (encryptedAssertions.length !== 1) { + if (encryptedAssertions.length > 1) { throw new Error('ERR_MULTIPLE_ASSERTION'); } - return xmlenc.decrypt(encryptedAssertions[0].toString(), { + const encAssertionNode = encryptedAssertions[0]; + + return xmlenc.decrypt(encAssertionNode.toString(), { key: utility.readPrivateKey(hereSetting.encPrivateKey, hereSetting.encPrivateKeyPass), }, (err, res) => { if (err) { @@ -676,9 +679,9 @@ const libSaml = () => { if (!res) { return reject(new Error('ERR_UNDEFINED_ENCRYPTED_ASSERTION')); } - const assertionNode = new dom().parseFromString(res); - xml.replaceChild(assertionNode, encryptedAssertions[0]); - return resolve([xml.toString(), res]); + const rawAssertionDoc = new dom().parseFromString(res); + doc.documentElement.replaceChild(rawAssertionDoc.documentElement, encAssertionNode); + return resolve([doc.toString(), res]); }); }); }, diff --git a/test/flow.ts b/test/flow.ts index 19bf553..4ff6175 100644 --- a/test/flow.ts +++ b/test/flow.ts @@ -1007,7 +1007,7 @@ test('send login response with [custom template] encrypted signed assertion + si // simulate idp-init slo test('idp sends a redirect logout request without signature and sp parses it', async t => { - const { id, context } = idp.createLogoutRequest(sp, 'redirect', { logoutNameID: 'user@esaml2.com', sessionIndex: '_664ade6a050f55a2c7cb2fb0571df7280365c0c7' }); + const { id, context } = idp.createLogoutRequest(sp, 'redirect', { logoutNameID: 'user@esaml2.com' }); const query = url.parse(context).query; t.is(query!.includes('SAMLRequest='), true); t.is(typeof id, 'string'); @@ -1019,7 +1019,6 @@ test('idp sends a redirect logout request without signature and sp parses it', a t.is(result.sigAlg, null); t.is(typeof samlContent, 'string'); t.is(extract.nameID, 'user@esaml2.com'); - t.is(extract.sessionIndex, '_664ade6a050f55a2c7cb2fb0571df7280365c0c7') t.is(extract.signature, null); t.is(typeof extract.request.id, 'string'); t.is(extract.request.destination, 'https://sp.example.org/sp/slo'); @@ -1027,7 +1026,7 @@ test('idp sends a redirect logout request without signature and sp parses it', a }); test('idp sends a redirect logout request with signature and sp parses it', async t => { - const { id, context } = idp.createLogoutRequest(spWantLogoutReqSign, 'redirect', { logoutNameID: 'user@esaml2.com', sessionIndex: '_664ade6a050f55a2c7cb2fb0571df7280365c0c7' }); + const { id, context } = idp.createLogoutRequest(spWantLogoutReqSign, 'redirect', { logoutNameID: 'user@esaml2.com' }); const query = url.parse(context).query; t.is(query!.includes('SAMLRequest='), true); t.is(query!.includes('SigAlg='), true); @@ -1042,7 +1041,6 @@ test('idp sends a redirect logout request with signature and sp parses it', asyn const octetString = Object.keys(originalURL.query).map(q => q + '=' + encodeURIComponent(originalURL.query[q] as string)).join('&'); const { extract } = await spWantLogoutReqSign.parseLogoutRequest(idp, 'redirect', { query: { SAMLRequest, Signature, SigAlg }, octetString}); t.is(extract.nameID, 'user@esaml2.com'); - t.is(extract.sessionIndex, '_664ade6a050f55a2c7cb2fb0571df7280365c0c7') t.is(extract.issuer, 'https://idp.example.com/metadata'); t.is(typeof extract.request.id, 'string'); t.is(extract.request.destination, 'https://sp.example.org/sp/slo'); @@ -1050,14 +1048,13 @@ test('idp sends a redirect logout request with signature and sp parses it', asyn }); test('idp sends a post logout request without signature and sp parses it', async t => { - const { relayState, type, entityEndpoint, id, context } = idp.createLogoutRequest(sp, 'post', { logoutNameID: 'user@esaml2.com', sessionIndex: '_664ade6a050f55a2c7cb2fb0571df7280365c0c7' }) as PostBindingContext; + const { type, entityEndpoint, id, context } = idp.createLogoutRequest(sp, 'post', { logoutNameID: 'user@esaml2.com' }) as PostBindingContext; t.is(typeof id, 'string'); t.is(typeof context, 'string'); t.is(typeof entityEndpoint, 'string'); t.is(type, 'SAMLRequest'); const { extract } = await sp.parseLogoutRequest(idp, 'post', { body: { SAMLRequest: context } }); t.is(extract.nameID, 'user@esaml2.com'); - t.is(extract.sessionIndex, '_664ade6a050f55a2c7cb2fb0571df7280365c0c7') t.is(extract.issuer, 'https://idp.example.com/metadata'); t.is(typeof extract.request.id, 'string'); t.is(extract.request.destination, 'https://sp.example.org/sp/slo'); @@ -1065,14 +1062,13 @@ test('idp sends a post logout request without signature and sp parses it', async }); test('idp sends a post logout request with signature and sp parses it', async t => { - const { relayState, type, entityEndpoint, id, context } = idp.createLogoutRequest(spWantLogoutReqSign, 'post', { logoutNameID: 'user@esaml2.com', sessionIndex: '_664ade6a050f55a2c7cb2fb0571df7280365c0c7' }) as PostBindingContext; + const { type, entityEndpoint, id, context } = idp.createLogoutRequest(spWantLogoutReqSign, 'post', { logoutNameID: 'user@esaml2.com' }) as PostBindingContext; t.is(typeof id, 'string'); t.is(typeof context, 'string'); t.is(typeof entityEndpoint, 'string'); t.is(type, 'SAMLRequest'); const { extract } = await spWantLogoutReqSign.parseLogoutRequest(idp, 'post', { body: { SAMLRequest: context } }); t.is(extract.nameID, 'user@esaml2.com'); - t.is(extract.sessionIndex, '_664ade6a050f55a2c7cb2fb0571df7280365c0c7') t.is(extract.issuer, 'https://idp.example.com/metadata'); t.is(extract.request.destination, 'https://sp.example.org/sp/slo'); t.is(typeof extract.request.id, 'string'); @@ -1082,7 +1078,7 @@ test('idp sends a post logout request with signature and sp parses it', async t // simulate init-slo test('sp sends a post logout response without signature and parse', async t => { const { context: SAMLResponse } = sp.createLogoutResponse(idp, null, 'post', '', createTemplateCallback(idp, sp, binding.post, {})) as PostBindingContext; - const { samlContent, extract } = await idp.parseLogoutResponse(sp, 'post', { body: { SAMLResponse }}); + const { extract } = await idp.parseLogoutResponse(sp, 'post', { body: { SAMLResponse }}); t.is(extract.signature, null); t.is(extract.issuer, 'https://sp.example.org/metadata'); t.is(typeof extract.response.id, 'string'); @@ -1133,6 +1129,7 @@ test('avoid malformatted response', async t => { const attackResponse = `evil@evil.com${rawResponse}`; try { await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: utility.base64Encode(attackResponse) } }); + t.fail(); } catch (e) { // it must throw an error t.is(true, true); @@ -1151,9 +1148,10 @@ test('avoid malformatted response with redirect binding', async t => { const rawResponse = utility.inflateString(SAMLResponse as string); const attackResponse = `evil@evil.com${rawResponse}`; - const octetString = "SAMLResponse=" + encodeURIComponent(utility.base64Encode(utility.deflateString(attackResponse))) + "&SigAlg=" + encodeURIComponent(sigAlg as string); + const octetString = 'SAMLResponse=' + encodeURIComponent(utility.base64Encode(utility.deflateString(attackResponse))) + '&SigAlg=' + encodeURIComponent(sigAlg as string); try { await sp.parseLoginResponse(idpNoEncrypt, 'redirect', { query :{ SAMLResponse, SigAlg: sigAlg, Signature: signature}, octetString }); + t.fail(); } catch (e) { // it must throw an error t.is(true, true); @@ -1169,6 +1167,7 @@ test('avoid malformatted response with simplesign binding', async t => { const octetString = buildSimpleSignOctetString(type, SAMLResponse, sigAlg, relayState, signature); try { await sp.parseLoginResponse(idpNoEncrypt, 'simpleSign', { body: { SAMLResponse: utility.base64Encode(attackResponse), Signature: signature, SigAlg:sigAlg }, octetString }); + t.fail(); } catch (e) { // it must throw an error t.is(true, true); @@ -1194,6 +1193,7 @@ test('should reject signature wrapped response - case 1', async t => { const wrappedResponse = Buffer.from(xmlWrapped).toString('base64'); try { await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: wrappedResponse } }); + t.fail(); } catch (e) { t.is(e.message, 'ERR_POTENTIAL_WRAPPING_ATTACK'); } @@ -1217,7 +1217,8 @@ test('should reject signature wrapped response - case 2', async t => { const xmlWrapped = outer.replace(/<\/saml:Conditions>/, '' + stripped.replace('', '') + ''); const wrappedResponse = Buffer.from(xmlWrapped).toString('base64'); try { - const result = await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: wrappedResponse } }); + await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: wrappedResponse } }); + t.fail(); } catch (e) { t.is(e.message, 'ERR_POTENTIAL_WRAPPING_ATTACK'); } @@ -1225,7 +1226,8 @@ test('should reject signature wrapped response - case 2', async t => { test('should throw two-tiers code error when the response does not return success status', async t => { try { - const _result = await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: utility.base64Encode(failedResponse) } }); + await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: utility.base64Encode(failedResponse) } }); + t.fail(); } catch (e) { t.is(e.message, 'ERR_FAILED_STATUS with top tier code: urn:oasis:names:tc:SAML:2.0:status:Requester, second tier code: urn:oasis:names:tc:SAML:2.0:status:InvalidNameIDPolicy'); } @@ -1234,10 +1236,11 @@ test('should throw two-tiers code error when the response does not return succes test('should throw two-tiers code error when the response by redirect does not return success status', async t => { try { const SAMLResponse = utility.base64Encode(utility.deflateString(failedResponse)); - const sigAlg = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + const sigAlg = 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'; const encodedSigAlg = encodeURIComponent(sigAlg); - const octetString = "SAMLResponse=" + encodeURIComponent(SAMLResponse) + "&SigAlg=" + encodedSigAlg; - const _result = await sp.parseLoginResponse(idpNoEncrypt, 'redirect',{ query :{ SAMLResponse, SigAlg: encodedSigAlg} , octetString} ); + const octetString = 'SAMLResponse=' + encodeURIComponent(SAMLResponse) + '&SigAlg=' + encodedSigAlg; + await sp.parseLoginResponse(idpNoEncrypt, 'redirect',{ query :{ SAMLResponse, SigAlg: encodedSigAlg} , octetString} ); + t.fail(); } catch (e) { t.is(e.message, 'ERR_FAILED_STATUS with top tier code: urn:oasis:names:tc:SAML:2.0:status:Requester, second tier code: urn:oasis:names:tc:SAML:2.0:status:InvalidNameIDPolicy'); } @@ -1245,7 +1248,8 @@ test('should throw two-tiers code error when the response by redirect does not r test('should throw two-tiers code error when the response over simpleSign does not return success status', async t => { try { - const _result = await sp.parseLoginResponse(idpNoEncrypt, 'simpleSign', { body: { SAMLResponse: utility.base64Encode(failedResponse) } }); + await sp.parseLoginResponse(idpNoEncrypt, 'simpleSign', { body: { SAMLResponse: utility.base64Encode(failedResponse) } }); + t.fail(); } catch (e) { t.is(e.message, 'ERR_FAILED_STATUS with top tier code: urn:oasis:names:tc:SAML:2.0:status:Requester, second tier code: urn:oasis:names:tc:SAML:2.0:status:InvalidNameIDPolicy'); } @@ -1266,7 +1270,7 @@ test.serial('should throw ERR_SUBJECT_UNCONFIRMED for the expired SAML response tk.freeze(fiveMinutesOneSecLater); await sp.parseLoginResponse(idp, 'post', { body: { SAMLResponse } }); // test failed, it shouldn't happen - t.is(true, false); + t.fail(); } catch (e) { t.is(e, 'ERR_SUBJECT_UNCONFIRMED'); } finally { @@ -1289,7 +1293,7 @@ test.serial('should throw ERR_SUBJECT_UNCONFIRMED for the expired SAML response tk.freeze(fiveMinutesOneSecLater); await sp.parseLoginResponse(idp, 'redirect', parseRedirectUrlContextCallBack(SAMLResponse)); // test failed, it shouldn't happen - t.is(true, false); + t.fail(); } catch (e) { t.is(e, 'ERR_SUBJECT_UNCONFIRMED'); } finally { @@ -1311,7 +1315,7 @@ test.serial('should throw ERR_SUBJECT_UNCONFIRMED for the expired SAML response tk.freeze(fiveMinutesOneSecLater); await sp.parseLoginResponse(idp, 'simpleSign', { body: { SAMLResponse, Signature: signature, SigAlg:sigAlg }, octetString }); // test failed, it shouldn't happen - t.is(true, false); + t.fail(); } catch (e) { t.is(e, 'ERR_SUBJECT_UNCONFIRMED'); } finally { diff --git a/test/index.ts b/test/index.ts index e10e0f4..e5ad77c 100644 --- a/test/index.ts +++ b/test/index.ts @@ -261,11 +261,11 @@ test('getAssertionConsumerService with two bindings', t => { }); test('encrypt assertion response without assertion returns error', async t => { const error = await t.throwsAsync(() => libsaml.encryptAssertion(idp, sp, wrongResponse)); - t.is(error?.message, 'ERR_MULTIPLE_ASSERTION'); + t.is(error?.message, 'ERR_NO_ASSERTION'); }); test('encrypt assertion with invalid xml syntax returns error', async t => { const error = await t.throwsAsync(() => libsaml.encryptAssertion(idp, sp, 'This is not a xml format string')); - t.is(error?.message, 'ERR_MULTIPLE_ASSERTION'); + t.is(error?.message, 'ERR_NO_ASSERTION'); }); test('encrypt assertion with empty string returns error', async t => { const error = await t.throwsAsync(() => libsaml.encryptAssertion(idp, sp, '')); @@ -392,7 +392,10 @@ test('verify time with and without drift tolerance', t => { }); -test('metadata with multiple entity descriptors is invalid', t => { +// new versions of xmldom realizes multiple_entitydescriptor.xml is invalid XML and doesn't parse it anymore. +// It just logs an error and ignores the rest of the file, so this test is no longer a valid test case. +// [xmldom error] element parse error: Error: Hierarchy request error: Only one element can be added and only after doctype +test.skip('metadata with multiple entity descriptors is invalid', t => { try { identityProvider({ ...defaultIdpConfig, metadata: readFileSync('./test/misc/multiple_entitydescriptor.xml') }); t.fail();