Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix encryption for @xmldom/xmldom 0.8.6 upgrade #511

Merged
merged 3 commits into from Feb 26, 2023
Merged

Fix encryption for @xmldom/xmldom 0.8.6 upgrade #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f04359d
fix: encryption for @xmldom/xmldom 0.8.6
mastermatt Feb 14, 2023
5c78b94
fix test: remove sessionIndex from failing logout request tests
mastermatt Feb 14, 2023
ab696df
review feedback tweaks
mastermatt Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 16 additions & 13 deletions src/libsaml.ts
View file
Open in desktop
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) {
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
42 changes: 23 additions & 19 deletions test/flow.ts
View file
Open in desktop
Expand Up @@ -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');
Expand All @@ -1019,15 +1019,14 @@ 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');
t.is(extract.issuer, 'https://idp.example.com/metadata');
});

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);
Expand All @@ -1042,37 +1041,34 @@ 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');
t.is(extract.signature, null); // redirect binding doesn't embed the signature
});

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')
tngan marked this conversation as resolved.
Show resolved Hide resolved
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');
t.is(extract.signature, null);
});

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');
Expand All @@ -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');
Expand Down Expand Up @@ -1133,6 +1129,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 +1148,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 +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);
Expand All @@ -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');
}
Expand All @@ -1217,15 +1217,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 +1236,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 +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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
9 changes: 6 additions & 3 deletions test/index.ts
View file
Open in desktop
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 Expand Up @@ -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();
Expand Down