From 2f89ad9de836086ae2dccd2b8ce92dc64cc816fd Mon Sep 17 00:00:00 2001 From: Deepak Prabhakara Date: Thu, 19 Dec 2024 10:39:40 +0000 Subject: [PATCH] fix for signature checks failing with ` ` chars (#748) * Add test case for issue (#747) Co-authored-by: Riley Wills * fix for signature checks failing with chars --------- Co-authored-by: Riley Wills --- lib/validateSignature.ts | 2 +- ...validResponseSignedMessage-unsanitized.xml | 54 ++++++++++++ test/lib/saml20.responseSignedMessage.spec.ts | 84 +++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 test/assets/saml20.validResponseSignedMessage-unsanitized.xml diff --git a/lib/validateSignature.ts b/lib/validateSignature.ts index 8ab289c7..bed0fc7e 100644 --- a/lib/validateSignature.ts +++ b/lib/validateSignature.ts @@ -58,7 +58,7 @@ const _hasValidSignature = (xml, cert, certThumbprint) => { idAttribute: 'AssertionID', }); - signed.loadSignature(signature.toString()); + signed.loadSignature(signature); let valid; let id, calculatedThumbprint; diff --git a/test/assets/saml20.validResponseSignedMessage-unsanitized.xml b/test/assets/saml20.validResponseSignedMessage-unsanitized.xml new file mode 100644 index 00000000..26198ea6 --- /dev/null +++ b/test/assets/saml20.validResponseSignedMessage-unsanitized.xml @@ -0,0 +1,54 @@ + + + http://idp.example.com/metadata.php + + + o2gfRtA1O8EEy6J8Sl7L4dKsQts=xk5Z0neBXyFACKQztJC5EXMiFCNSFyjlwkwaB8fdz5n4TnwKstzwUianBxCtmMM2dMYjzENnPL3jigcikJawkNrfmV6JP+9VXoK54C/Bb0nH69zwp/Am8VuvJZShgzAjHJ0L34o2l5QcW14qSeKR9+OPhenbyp/Uhxxe/eEGECE= +MIICajCCAdOgAwIBAgIBADANBgkqhkiG9w0BAQ0FADBSMQswCQYDVQQGEwJ1czET + MBEGA1UECAwKQ2FsaWZvcm5pYTEVMBMGA1UECgwMT25lbG9naW4gSW5jMRcwFQYD + VQQDDA5zcC5leGFtcGxlLmNvbTAeFw0xNDA3MTcxNDEyNTZaFw0xNTA3MTcxNDEy + NTZaMFIxCzAJBgNVBAYTAnVzMRMwEQYDVQQIDApDYWxpZm9ybmlhMRUwEwYDVQQK + DAxPbmVsb2dpbiBJbmMxFzAVBgNVBAMMDnNwLmV4YW1wbGUuY29tMIGfMA0GCSqG + SIb3DQEBAQUAA4GNADCBiQKBgQDZx+ON4IUoIWxgukTb1tOiX3bMYzYQiwWPUNMp + +Fq82xoNogso2bykZG0yiJm5o8zv/sd6pGouayMgkx/2FSOdc36T0jGbCHuRSbti + a0PEzNIRtmViMrt3AeoWBidRXmZsxCNLwgIV6dn2WpuE5Az0bHgpZnQxTKFek0BM + KU/d8wIDAQABo1AwTjAdBgNVHQ4EFgQUGHxYqZYyX7cTxKVODVgZwSTdCnwwHwYD + VR0jBBgwFoAUGHxYqZYyX7cTxKVODVgZwSTdCnwwDAYDVR0TBAUwAwEB/zANBgkq + hkiG9w0BAQ0FAAOBgQByFOl+hMFICbd3DJfnp2Rgd/dqttsZG/tyhILWvErbio/D + Ee98mXpowhTkC04ENprOyXi7ZbUqiicF89uAGyt1oqgTUCD1VsLahqIcmrzgumNy + TwLGWo17WDAa1/usDhetWAMhgzF/Cnf5ek0nK00m0YZGyc4LzgD0CROMASTWNg== + + + + + http://idp.example.com/metadata.php + + _ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7 + + + + + + + http://sp.example.com/demo1/metadata.php + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:Password + + + + + test + + + test@example.com + + + users + examplerole1 + + + + \ No newline at end of file diff --git a/test/lib/saml20.responseSignedMessage.spec.ts b/test/lib/saml20.responseSignedMessage.spec.ts index bdec7db1..3b2e4d27 100644 --- a/test/lib/saml20.responseSignedMessage.spec.ts +++ b/test/lib/saml20.responseSignedMessage.spec.ts @@ -4,6 +4,9 @@ import fs from 'fs'; // Tests Configuration const validResponse = fs.readFileSync('./test/assets/saml20.validResponseSignedMessage.xml').toString(); +const validResponseUnsanitized = fs + .readFileSync('./test/assets/saml20.validResponseSignedMessage-unsanitized.xml') + .toString(); const issuerName = 'http://idp.example.com/metadata.php'; const thumbprint = 'e606eced42fa3abd0c5693456384f5931b174707'; @@ -92,3 +95,84 @@ describe('saml20.responseSignedMessage', function () { } }); }); + +describe('saml20.validResponseSignedMessageUnsanitized', function () { + it('Should validate saml 2.0 token using thumbprint', async function () { + const response = await validate(validResponseUnsanitized, { + thumbprint: thumbprint, + bypassExpiration: true, + inResponseTo: inResponseTo, + }); + + assert.strictEqual(response.issuer, issuerName); + assert.strictEqual( + response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], + '_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7' + ); + }); + + it('Should validate saml 2.0 token using certificate', async function () { + const response = await validate(validResponseUnsanitized, { + publicKey: certificate, + bypassExpiration: true, + inResponseTo: inResponseTo, + }); + + assert.strictEqual(response.issuer, issuerName); + assert.strictEqual( + response.claims['http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'], + '_ce3d2948b4cf20146dee0a0b3dd6f69b6cf86f62d7' + ); + }); + + it('Should validate saml 2.0 token and check audience', async function () { + const response = await validate(validResponseUnsanitized, { + publicKey: certificate, + audience: audience, + bypassExpiration: true, + inResponseTo: inResponseTo, + }); + assert.strictEqual(response.issuer, issuerName); + }); + + it('Should fail with invalid audience', async function () { + try { + await validate(validResponseUnsanitized, { + publicKey: certificate, + audience: 'http://any-other-audience.com/', + bypassExpiration: true, + inResponseTo: inResponseTo, + }); + } catch (error) { + const result = (error as Error).message; + assert.strictEqual(result, 'Invalid audience.'); + } + }); + + it('Should fail with missing root element', async function () { + try { + await validate('invalid-assertion', { + publicKey: certificate, + bypassExpiration: true, + inResponseTo: inResponseTo, + }); + } catch (error) { + const result = (error as Error).message; + assert.strictEqual(result, 'missing root element'); + } + }); + + it('Should fail with invalid inResponseTo', async function () { + try { + await validate(validResponseUnsanitized, { + publicKey: certificate, + audience: audience, + bypassExpiration: true, + inResponseTo: 'not-the-right-response-to', + }); + } catch (error) { + const result = (error as Error).message; + assert.strictEqual(result, 'Invalid InResponseTo.'); + } + }); +});