Skip to content

Commit

Permalink
fix: encryption for @xmldom/xmldom 0.8.6
Browse files Browse the repository at this point in the history 
`@xmldom/xmldom` 0.8.6 included: fix: Properly check nodes before replacement.
xmldom/xmldom#457

Which caused the `.replaceChild` calls in `encryptAssertion` and `decryptAssertion` to error with:
> "Not found: child not in parent"

The root cause is a subtle difference in `@xmldom/xmldom` `Document` vs `Element` instances.
The previous code was asking xmldom to replace an element from a top-level Document with another top-level Document. The patch to xmldom 0.8.6 started the enforcement of `.replaceChild` being passed `Nodes` who share a common parent Node.

e.g. in the case of `encryptAssertion`: `doc.replaceChild(encryptAssertionNode, rawAssertionNode)`.
It's important to distinguish that neither `doc` nor `encryptAssertionNode` are `Element` nodes, but instead `Document` Nodes. Meaning `doc` does _not_ refer to the `<samlp:Response>` node, but instead a meta object one level up. To reference the Response tag, you instead use the `Document#documentElement` attribute.
Changing that line to the following as the same intended affect using the correct node references.
`doc.documentElement.replaceChild(encryptAssertionNode.documentElement, rawAssertionNode)`
I renamed a few of the variables in an attempt to clarify which are `Documents`.

Also fixes an issue where the `ERR_NO_ASSERTION` and `ERR_UNDEFINED_ENCRYPTED_ASSERTION` errors were not being thrown if exactly zero nodes were found.

fixes: tngan#495
  • Loading branch information
@mastermatt
mastermatt committed Feb 14, 2023
1 parent 49295a2 commit f04359d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 25 deletions.
27 changes: 15 additions & 12 deletions src/libsaml.ts
View file
Expand Up @@ -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-----`),
Expand All @@ -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}</${encAssertionPrefix}:EncryptedAssertion>`);
doc.replaceChild(encryptAssertionNode, assertions[0]);
const encryptAssertionDoc = new dom().parseFromString(`<${encAssertionPrefix}:EncryptedAssertion xmlns:${encAssertionPrefix}="${namespace.names.assertion}">${res}</${encAssertionPrefix}:EncryptedAssertion>`);
doc.documentElement.replaceChild(encryptAssertionDoc.documentElement, rawAssertionNode);
return resolve(utility.base64Encode(doc.toString()));
});
} else {
Expand All @@ -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) {
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) {
Expand All @@ -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]);
});
});
},
Expand Down
30 changes: 19 additions & 11 deletions test/flow.ts
View file
Expand Up @@ -1050,7 +1050,7 @@ 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', sessionIndex: '_664ade6a050f55a2c7cb2fb0571df7280365c0c7' }) as PostBindingContext;
t.is(typeof id, 'string');
t.is(typeof context, 'string');
t.is(typeof entityEndpoint, 'string');
Expand Down Expand Up @@ -1133,6 +1133,7 @@ test('avoid malformatted response', async t => {
const attackResponse = `<NameID>evil@evil.com${rawResponse}</NameID>`;
try {
await sp.parseLoginResponse(idpNoEncrypt, 'post', { body: { SAMLResponse: utility.base64Encode(attackResponse) } });
t.fail()
} catch (e) {
// it must throw an error
t.is(true, true);
Expand All @@ -1151,9 +1152,10 @@ test('avoid malformatted response with redirect binding', async t => {

const rawResponse = utility.inflateString(SAMLResponse as string);
const attackResponse = `<NameID>evil@evil.com${rawResponse}</NameID>`;
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);
Expand All @@ -1169,6 +1171,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);
Expand All @@ -1194,6 +1197,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');
}
Expand All @@ -1217,15 +1221,17 @@ test('should reject signature wrapped response - case 2', async t => {
const xmlWrapped = outer.replace(/<\/saml:Conditions>/, '</saml:Conditions><saml:Advice>' + stripped.replace('<?xml version="1.0" encoding="UTF-8"?>', '') + '</saml:Advice>');
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');
}
});

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');
}
Expand All @@ -1234,18 +1240,20 @@ 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');
}
});

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');
}
Expand All @@ -1266,7 +1274,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 {
Expand All @@ -1289,7 +1297,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 {
Expand All @@ -1311,7 +1319,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 {
Expand Down
4 changes: 2 additions & 2 deletions test/index.ts
View file
Expand Up @@ -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, ''));
Expand Down

0 comments on commit f04359d

Please sign in to comment.