diff --git a/lib/response.ts b/lib/response.ts index 852f1b48..9929979b 100644 --- a/lib/response.ts +++ b/lib/response.ts @@ -129,6 +129,11 @@ const validateInternal = async (rawAssertion, options, cb) => { // rawAssertion is our SAML Response. However, we still call it rawAssertion because of legacy naming convention const responseObj = await xmlToJs(rawAssertion, cb); + // xmlToJs already invoked cb with the parse error; stop so we do not call cb + // twice or dereference an undefined object below. + if (!responseObj) { + return; + } checkStatusCode(responseObj, cb); let signedXml: string | null; @@ -150,6 +155,7 @@ const validateInternal = async (rawAssertion, options, cb) => { const error = new WrapError('Invalid assertion.'); error.inner = e; cb(error); + return; } } @@ -166,34 +172,81 @@ const validateInternal = async (rawAssertion, options, cb) => { // Save the js object derived from xml and check status code const assertionObj = await xmlToJs(decryptedSignedXml, cb); // this is our assertion object + // xmlToJs already invoked cb with the parse error; stop before + // parseResponseAndVersion dereferences an undefined object. + if (!assertionObj) { + return; + } - parseResponseAndVersion(assertionObj, responseObj, function onParse(err, assertion, version) { + parseResponseAndVersion(assertionObj, responseObj, async function onParse(err, assertion, version) { if (err) { cb(err); return; } - const tokenHandler = tokenHandlers[version]; + // onParse is async (the replay validator may be), so any synchronous throw + // here would become an unhandled rejection and leave validate() pending. + // Convert unexpected failures into a clean rejection. + try { + const tokenHandler = tokenHandlers[version]; - if (!options.bypassExpiration && !tokenHandler.validateExpiration(assertion)) { - cb(new Error('Assertion is expired.')); - return; - } + if (!options.bypassExpiration && !tokenHandler.validateExpiration(assertion)) { + cb(new Error('Assertion is expired.')); + return; + } - if ( - options.audience && - !tokenHandler.validateAudience(assertion, options.audience, options.strictAudienceValidation) - ) { - cb(new Error('Invalid audience.')); - return; - } + if ( + options.audience && + !tokenHandler.validateAudience(assertion, options.audience, options.strictAudienceValidation) + ) { + cb(new Error('Invalid audience.')); + return; + } - if (options.inResponseTo && assertion.inResponseTo && assertion.inResponseTo !== options.inResponseTo) { - cb(new Error('Invalid InResponseTo.')); - return; - } + // Fail closed: when the caller supplies an expected InResponseTo (an + // SP-initiated flow), the assertion must carry a matching, signed + // InResponseTo. A missing value is a mismatch, not a reason to skip the + // check, otherwise a captured assertion can be replayed into a different + // login by stripping InResponseTo from the unsigned wrapper. + if (options.inResponseTo && assertion.inResponseTo !== options.inResponseTo) { + cb(new Error('Invalid InResponseTo.')); + return; + } + + // Optional one-time replay protection. The library is stateless, so the + // caller supplies the store. The callback receives the signed assertion + // identifiers and must return true when the assertion has already been + // seen (a replay), keying entries by assertion ID until NotOnOrAfter. + if (typeof options.assertionReplayValidator === 'function') { + let alreadyUsed: boolean; + try { + alreadyUsed = await options.assertionReplayValidator({ + assertionId: tokenHandler.getAssertionId(assertion), + sessionIndex: assertion.AuthnStatement?.['@']?.SessionIndex, + // Effective expiry (Conditions or the latest bearer confirmation), so + // the caller can size the replay-cache TTL to cover the whole window + // an assertion can still validate. + notOnOrAfter: tokenHandler.getNotOnOrAfter(assertion), + inResponseTo: assertion.inResponseTo, + }); + } catch (e) { + const error = new WrapError('An error occurred during assertion replay validation.'); + error.inner = e; + cb(error); + return; + } + if (alreadyUsed) { + cb(new Error('Assertion has already been used (replay detected).')); + return; + } + } - parseAttributes(assertion, tokenHandler, cb); + parseAttributes(assertion, tokenHandler, cb); + } catch (e) { + const error = new WrapError('An error occurred trying to validate the assertion.'); + error.inner = e; + cb(error); + } }); }; @@ -266,7 +319,18 @@ function parseResponseAndVersion(assertionObj, responseObj, cb) { } const tokenHandler = tokenHandlers[version]; - assertion.inResponseTo = tokenHandler.getInResponseTo(responseObj); + + // Derive InResponseTo only from signed content so it cannot be forged by + // tampering with the unsigned wrapper. When the whole Response is + // signed, `assertionObj` is the Response subtree and Response/@InResponseTo is + // trustworthy. In the common assertion-only-signed case, fall back to the + // bearer SubjectConfirmationData/@InResponseTo, which is inside the signed + // assertion. The outer `responseObj` wrapper is never trusted here. + const signedResponseInResponseTo = assertionObj.Response + ? tokenHandler.getInResponseTo(assertionObj) + : undefined; + assertion.inResponseTo = + signedResponseInResponseTo || tokenHandler.getSubjectConfirmationInResponseTo(assertion); cb(null, assertion, version, response); } diff --git a/lib/saml20.ts b/lib/saml20.ts index 59e8701b..298ca1fe 100644 --- a/lib/saml20.ts +++ b/lib/saml20.ts @@ -101,6 +101,8 @@ const parse = (assertion) => { claims: claims, issuer: getProp(assertion, 'Issuer'), sessionIndex: getProp(assertion, 'AuthnStatement.@.SessionIndex'), + assertionId: getAssertionId(assertion), + notOnOrAfter: getNotOnOrAfter(assertion), }; }; @@ -134,23 +136,150 @@ const validateAudience = (assertion, realm, strictValidation = false) => { } }; +const clockSkewMs = 10 * 60 * 1000; // 10 minutes clock skew. + +// Collect every SubjectConfirmationData element across all SubjectConfirmation +// entries. Bearer assertions carry their expiration here rather than (or in +// addition to) Conditions. +const getSubjectConfirmationData = (assertion): Record[] => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let confirmations = getAttribute(assertion, 'Subject.SubjectConfirmation'); + if (!confirmations) { + return []; + } + confirmations = Array.isArray(confirmations) ? confirmations : [confirmations]; + const data: Record[] = []; + for (const confirmation of confirmations) { + let scd = getAttribute | Record[]>( + confirmation, + 'SubjectConfirmationData' + ); + if (!scd) { + continue; + } + scd = Array.isArray(scd) ? scd : [scd]; + data.push(...scd); + } + return data; +}; + +// Check a [NotBefore, NotOnOrAfter] window against now, applying clock skew. +// Returns 'unbounded' when no NotOnOrAfter is present (the window has no upper +// limit), 'valid' when now is inside the window, and 'invalid' when a bound is +// present but unparseable or now falls outside it. +type WindowResult = 'valid' | 'invalid' | 'unbounded'; +const checkWindow = (notBefore?: string, notOnOrAfter?: string): WindowResult => { + const now = Date.now(); + + if (notBefore) { + const ms = new Date(notBefore).getTime(); + if (Number.isNaN(ms) || now < ms - clockSkewMs) { + return 'invalid'; + } + } + + if (!notOnOrAfter) { + return 'unbounded'; + } + const ms = new Date(notOnOrAfter).getTime(); + if (Number.isNaN(ms) || now > ms + clockSkewMs) { + return 'invalid'; + } + return 'valid'; +}; + const validateExpiration = (assertion) => { - const dteNotBefore = getProp(assertion, 'Conditions.@.NotBefore'); - let notBefore: any = new Date(dteNotBefore); - notBefore = notBefore.setMinutes(notBefore.getMinutes() - 10); // 10 minutes clock skew + // The window is an absolute constraint on the assertion: when + // present it must be satisfied. A present-but-unparseable bound is invalid. + const conditionsNotBefore = getAttribute(assertion, 'Conditions.@.NotBefore'); + const conditionsNotOnOrAfter = getAttribute(assertion, 'Conditions.@.NotOnOrAfter'); + const conditionsResult = checkWindow(conditionsNotBefore, conditionsNotOnOrAfter); + if (conditionsResult === 'invalid') { + return false; + } + + // When supplies a satisfied upper bound, the assertion is + // time-boxed and currently within its validity window. + if (conditionsResult === 'valid') { + return true; + } - const dteNotOnOrAfter = getProp(assertion, 'Conditions.@.NotOnOrAfter'); - let notOnOrAfter: any = new Date(dteNotOnOrAfter); - notOnOrAfter = notOnOrAfter.setMinutes(notOnOrAfter.getMinutes() + 10); // 10 minutes clock skew + // Otherwise fall back to the bearer SubjectConfirmationData. Per SAML 2.0 + // core (2.4.1.1) multiple SubjectConfirmation elements are alternatives: + // satisfying any one is sufficient. The assertion is valid if at least one + // confirmation is currently within its window AND carries an upper bound. + // If no satisfied upper bound exists anywhere, the assertion is rejected + // rather than treated as "never expires" (the original NaN defect). + for (const scd of getSubjectConfirmationData(assertion)) { + const attrs = (scd['@'] as Record | undefined) ?? {}; + if (checkWindow(attrs.NotBefore, attrs.NotOnOrAfter) === 'valid') { + return true; + } + } - const now = new Date(); - return !(now < notBefore || now > notOnOrAfter); + return false; }; +// InResponseTo read from the outer wrapper. Only trust this when the +// whole Response is signed; the wrapper is unsigned in the common +// assertion-only-signed case. const getInResponseTo = (xml) => { return getProp(xml, 'Response.@.InResponseTo'); }; -const saml20 = { getInResponseTo, validateExpiration, validateAudience, parse }; +// InResponseTo carried inside the bearer SubjectConfirmationData. This element +// lives inside the and is therefore covered by the assertion +// signature even when the outer wrapper is not signed. +const getSubjectConfirmationInResponseTo = (assertion): string | undefined => { + for (const scd of getSubjectConfirmationData(assertion)) { + const inResponseTo = (scd['@'] as Record | undefined)?.InResponseTo; + if (inResponseTo) { + return inResponseTo; + } + } + return undefined; +}; + +const getAssertionId = (assertion): string | undefined => { + return getAttribute(assertion, '@.ID'); +}; + +// The effective NotOnOrAfter after which the assertion can no longer validate, +// mirroring validateExpiration: the absolute Conditions/@NotOnOrAfter when +// present, otherwise the latest bearer SubjectConfirmationData/@NotOnOrAfter +// (SubjectConfirmations are alternatives, so the assertion remains usable until +// the last of them lapses). Callers key replay-cache TTLs off this value, so it +// must reflect the real upper bound rather than only the Conditions window. +const getNotOnOrAfter = (assertion): string | undefined => { + const conditionsNotOnOrAfter = getAttribute(assertion, 'Conditions.@.NotOnOrAfter'); + if (conditionsNotOnOrAfter) { + return conditionsNotOnOrAfter; + } + + let latest: string | undefined; + let latestMs = -Infinity; + for (const scd of getSubjectConfirmationData(assertion)) { + const value = (scd['@'] as Record | undefined)?.NotOnOrAfter; + if (!value) { + continue; + } + const ms = new Date(value).getTime(); + if (!Number.isNaN(ms) && ms > latestMs) { + latestMs = ms; + latest = value; + } + } + return latest; +}; + +const saml20 = { + getInResponseTo, + getSubjectConfirmationInResponseTo, + getAssertionId, + getNotOnOrAfter, + validateExpiration, + validateAudience, + parse, +}; export default saml20; diff --git a/lib/typings.ts b/lib/typings.ts index 10f2bb05..abfb99b6 100644 --- a/lib/typings.ts +++ b/lib/typings.ts @@ -15,4 +15,24 @@ export interface SAMLProfile { claims: Record; issuer: string; sessionIndex: string; + // Signed assertion identifier and validity bound, exposed so callers can + // implement one-time replay protection keyed by assertionId until notOnOrAfter. + assertionId?: string; + notOnOrAfter?: string; +} + +// Identifiers from the signed assertion passed to a caller-supplied replay +// validator. The validator returns true when the assertion has already been +// used (a replay) and must be rejected. +export interface AssertionReplayInfo { + assertionId?: string; + sessionIndex?: string; + // The assertion's effective NotOnOrAfter (the absolute Conditions bound when + // present, otherwise the latest bearer SubjectConfirmationData bound). This is + // the raw value from the assertion. validateExpiration accepts up to a + // 10-minute clock-skew tolerance beyond it, so a replay cache should keep the + // entry until at least notOnOrAfter + 10 minutes to cover the full window in + // which the assertion can still validate. + notOnOrAfter?: string; + inResponseTo?: string; } diff --git a/test/assets/certificates/testIdpCert.crt b/test/assets/certificates/testIdpCert.crt new file mode 100644 index 00000000..13edf743 --- /dev/null +++ b/test/assets/certificates/testIdpCert.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDBzCCAe+gAwIBAgIUfmiCRt513V6v242KcE4fS3tXnqkwDQYJKoZIhvcNAQEL +BQAwEzERMA8GA1UEAwwIdGVzdC1pZHAwHhcNMjYwNjIyMjM1MzI2WhcNNDYwNjE3 +MjM1MzI2WjATMREwDwYDVQQDDAh0ZXN0LWlkcDCCASIwDQYJKoZIhvcNAQEBBQAD +ggEPADCCAQoCggEBAM6XayR93ey62b2EpioV03K2eW4xB3psPUdQQ5cxqUgjUjKy +jSVleHcCCfHf/uqVYibxlUc5tfTRARNdCx8lB6Ob+MZLdTI0NS2tso5LBHvgywpe +ZqdD3aBHVHczxGVAkTX18XdwR0Ri31gPcZGP+tChKInJeTPo6i/tVbWmON/kP3NJ +mWYHrWfZHe+vKuytAE7Oqqn+LKM45FR6KlFjWuW11voshatRRWnGARIBDiQL2477 +id7d/UnSXZcqYgq34OfpNAy56v25YPFn7t4xtMa/8ZJGfsbKX24JNvlwLXFKT0v5 +y0KOCiAjVRjuMrOB1pobVwDAnjtxQyn1QAuxfCcCAwEAAaNTMFEwHQYDVR0OBBYE +FFhkCT/OWx/bn8P1AQYq8vxSEBH5MB8GA1UdIwQYMBaAFFhkCT/OWx/bn8P1AQYq +8vxSEBH5MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAG2r5QR1 +WRfD25DrjK8zMcm4NxZCmBvrxHO+f1Z9I5pdCazXrRY66jMt+VVayYZ/eZez+tPH +PEeFNaHBQLnpQdI7HQsAYU+f79bgctSRMbeT3Mjdx5b8mhp9I256Zs/NyTYeEp8V +wk9sjvhrr/pBA+t6oQKJwvfK7+u3pE42eIWxPSYqNQRKrhKPX4Ej7/3EQyMNfIwo +KF2WJYnXhe2IyccsOXayHtQkTLXOccIYkLIbgTb6auZKSMo7EvPN+op0Vhvjk49u +VfPLI076c1mSFrVL/x0yQ8c0YFf0eAFCl4pcKrNp125gT9MW1QAMgqKV/7tx8RnW +zEstp1SkQL9ZCa0= +-----END CERTIFICATE----- diff --git a/test/assets/certificates/testIdpKey.pem b/test/assets/certificates/testIdpKey.pem new file mode 100644 index 00000000..2dbedd3f --- /dev/null +++ b/test/assets/certificates/testIdpKey.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDOl2skfd3sutm9 +hKYqFdNytnluMQd6bD1HUEOXMalII1Iyso0lZXh3Agnx3/7qlWIm8ZVHObX00QET +XQsfJQejm/jGS3UyNDUtrbKOSwR74MsKXmanQ92gR1R3M8RlQJE19fF3cEdEYt9Y +D3GRj/rQoSiJyXkz6Oov7VW1pjjf5D9zSZlmB61n2R3vryrsrQBOzqqp/iyjOORU +eipRY1rltdb6LIWrUUVpxgESAQ4kC9uO+4ne3f1J0l2XKmIKt+Dn6TQMuer9uWDx +Z+7eMbTGv/GSRn7Gyl9uCTb5cC1xSk9L+ctCjgogI1UY7jKzgdaaG1cAwJ47cUMp +9UALsXwnAgMBAAECggEAAzkkkEywbjWaGRVdPFHb2zHSoi/8pGHU8OxlKZI6SGhY +q3bSse8r2nt7KT1r7kAHaIEjaZmSZ6/tGt68Qi+jN1/DGWDrAq4C2GQZ4ZN1DfO7 +Zz2Cz4BFEG+cd0GlAkloGpXsPwdO7Ve3kVmoVXOQH7or9j5g+DjdkoLa7/sYbxHK +6FNHQIXOsizZ+MhcIQBX3eraoqcpaWafED0nvwocJhtN7AVmVXulW79Fs8P09egh ++qTY456XXf/bkpMcedc4sWso3bvvA1j+iS0Rr8bHzAPqA1B3Z0iKgkmvJsJlm8fF +vgt7nOFsm8C15SmZikvPShvkxh8A4RHAQbpNY/hK1QKBgQDiZccgF89som7My9DA +ttN9OMclc5QC6mQ0E52PPKGIW3sSuDav03rg3hIyrpFcPa/hq8gq4nzByBL/nnL1 +bO4Fofi52Anicnn/O06M0eHvmP43DZ5mBiu4cf4x6IaQmf1U8JrUZF9lavyW04ai +aNTTRggIZlm7fBLQMhJdFrN4ywKBgQDpmquDvt6xMK+BrXzLQyBGDnaLeE/bt5IZ +kUzGGGxmc9VcZi7UftZOqlbuzS0qfE48Ck0miVdYpt7lL8BMtOXjoBu4MrwdQQMS +jYNynGfpiHY8Zr4zGoPqQDg56FdObo1RB5EfljFGZnTSFPW4oHbJg5loYyBoTMsm +3LTqJkTKlQKBgQCZjXJrP/r9sYX4/VwO+XGkAvh/XE7NU3C3KX66AeOFepaU8cCV +rJgxIC2zllcc+vHp2/sdqxP20t6f5TYPY9xkkaEDW5YIsqAwDmeOd2QIf/ocGO6Q +QCszJI3GB/IM7YS3MaGx4IobXV8IZVtxmCyRR3R3TgQad2LDNtLhtF3x1QKBgQDm +rJfPGYx3dfbo27KOWLOm2iNPJ7fb5BJ98s/YEUgBh0JZ4oE9zh27QlNjrfF6sZLj +kNyMQDSjUuxpblS6qisUMgcNRfQiAw+Qo3L4mt+1aM4waNhKSFWY3F9pNzf3OA2N +xSYWBc6UkRmsVYwrCzEhXjT/MltPAv3cWza+vJlTXQKBgClaYJ7A7Z5USwdTwNJ5 +UZp30uxKFMnSO9DhsmMgGPBQRsS5UPlDRHCQqoyx1aJyPuaaJICZL7QcCWwA1pCD +wO9vxti8MdAk32F5UlR5FQz6bWrsgLWWY7+eA+FAATxSoMZoxQ1aEO0eiKb86K2E +A55k9iw1jIhb1Oc4hjbjDrZ6 +-----END PRIVATE KEY----- diff --git a/test/lib/saml20.inResponseTo.attacks.spec.ts b/test/lib/saml20.inResponseTo.attacks.spec.ts new file mode 100644 index 00000000..caa4cc18 --- /dev/null +++ b/test/lib/saml20.inResponseTo.attacks.spec.ts @@ -0,0 +1,329 @@ +import assert from 'assert'; +import fs from 'fs'; +import { sign } from '../../lib/sign'; +import { validate } from '../../lib/response'; +import saml20 from '../../lib/saml20'; + +// Regression tests for the SP-initiated assertion-injection / InResponseTo +// replay-binding bypass and the validateExpiration NaN defect. +// +// Threat model: an attacker captures one validly signed (assertion-only-signed) +// victim assertion and replays it into their own freshly started SP-initiated +// login, where the caller enforces inResponseTo = attackerSessionId. The outer +// wrapper is unsigned, so the attacker can edit it freely. + +const privateKey = fs.readFileSync('./test/assets/certificates/testIdpKey.pem').toString(); +const cert = fs.readFileSync('./test/assets/certificates/testIdpCert.crt').toString(); + +const AUDIENCE = 'https://sp.jackson.example/saml'; +const VICTIM_REQ_ID = 'ORIGINAL_IDP_REQUEST_ID'; +const ATTACKER_SESSION_ID = '_attacker_fresh_session'; +const NAME_ID_CLAIM = 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'; + +interface BuildOpts { + // Include InResponseTo on the outer wrapper (unsigned). + responseInResponseTo?: string | null; + // Include InResponseTo inside the signed bearer SubjectConfirmationData. + subjectInResponseTo?: string | null; + // Emit a Conditions validity window. + withConditionsWindow?: boolean; + // Emit NotOnOrAfter on the bearer SubjectConfirmationData. + withSubjectNotOnOrAfter?: boolean; +} + +// Build an assertion-only-signed SAML Response. Only the is signed, +// which is the SAML default and what most IdPs emit. +function buildAssertionOnlySigned({ + responseInResponseTo = VICTIM_REQ_ID, + subjectInResponseTo = null, + withConditionsWindow = true, + withSubjectNotOnOrAfter = true, +}: BuildOpts = {}): string { + const now = new Date(); + const past = new Date(now.getTime() - 3600_000).toISOString(); + const future = new Date(now.getTime() + 3600_000).toISOString(); + const t = now.toISOString(); + + const responseIrtAttr = responseInResponseTo ? ` InResponseTo="${responseInResponseTo}"` : ''; + const subjectIrtAttr = subjectInResponseTo ? ` InResponseTo="${subjectInResponseTo}"` : ''; + const subjectNotOnOrAfterAttr = withSubjectNotOnOrAfter ? ` NotOnOrAfter="${future}"` : ''; + const conditions = withConditionsWindow + ? `${AUDIENCE}` + : `${AUDIENCE}`; + + const xml = + `` + + `https://idp` + + `` + + `` + + `https://idp` + + `victim@target.com` + + `` + + conditions + + `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport` + + `victim@target.com` + + `` + + ``; + + return sign(xml, privateKey, cert, '//*[local-name(.)="Assertion"]'); +} + +function stripResponseInResponseTo(xml: string): string { + return xml.replace(new RegExp(`(]*?) InResponseTo="${VICTIM_REQ_ID}"`), '$1'); +} + +async function expectReject(xml: string, message: string, opts: Record) { + await assert.rejects( + () => validate(xml, { audience: AUDIENCE, publicKey: cert, ...opts }), + (err: Error) => { + assert.strictEqual(err.message, message); + return true; + } + ); +} + +describe('saml20.attacks: InResponseTo replay binding (SP-initiated)', () => { + it('rejects an unchanged captured assertion replayed into a different session (negative control)', async () => { + const signed = buildAssertionOnlySigned({ subjectInResponseTo: VICTIM_REQ_ID }); + await expectReject(signed, 'Invalid InResponseTo.', { inResponseTo: ATTACKER_SESSION_ID }); + }); + + it('rejects when the outer InResponseTo is stripped and no signed InResponseTo is present', async () => { + // PoC: InResponseTo lived only on the unsigned wrapper. Stripping it must + // not silently skip the binding check. + const signed = buildAssertionOnlySigned({ subjectInResponseTo: null }); + await expectReject(stripResponseInResponseTo(signed), 'Invalid InResponseTo.', { + inResponseTo: ATTACKER_SESSION_ID, + }); + }); + + it('rejects when the outer InResponseTo is stripped but the signed assertion binds to the victim request', async () => { + // The signed SubjectConfirmationData still names the victim's request id, + // which does not match the attacker's session. + const signed = buildAssertionOnlySigned({ subjectInResponseTo: VICTIM_REQ_ID }); + await expectReject(stripResponseInResponseTo(signed), 'Invalid InResponseTo.', { + inResponseTo: ATTACKER_SESSION_ID, + }); + }); + + it('accepts a legitimate SP-initiated login bound via the signed SubjectConfirmationData', async () => { + // Outer wrapper carries no InResponseTo; the trustworthy value lives in the + // signed assertion and matches the caller-supplied request id. + const signed = buildAssertionOnlySigned({ + responseInResponseTo: null, + subjectInResponseTo: VICTIM_REQ_ID, + }); + const profile = await validate(signed, { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + }); + assert.strictEqual(profile.claims[NAME_ID_CLAIM], 'victim@target.com'); + }); + + it('does not enforce binding for IdP-initiated logins (no inResponseTo supplied)', async () => { + const signed = buildAssertionOnlySigned({ responseInResponseTo: null, subjectInResponseTo: null }); + const profile = await validate(signed, { audience: AUDIENCE, publicKey: cert }); + assert.strictEqual(profile.claims[NAME_ID_CLAIM], 'victim@target.com'); + }); +}); + +describe('saml20: one-time assertion replay protection', () => { + it('rejects a second use of the same assertion when a replay validator is supplied', async () => { + const signed = buildAssertionOnlySigned({ subjectInResponseTo: VICTIM_REQ_ID }); + const seen = new Set(); + const assertionReplayValidator = async ({ assertionId }: { assertionId?: string }) => { + if (!assertionId) return false; + if (seen.has(assertionId)) return true; + seen.add(assertionId); + return false; + }; + + const opts = { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + assertionReplayValidator, + }; + + const profile = await validate(signed, opts); + assert.strictEqual(profile.assertionId, '_assert1'); + + await assert.rejects( + () => validate(signed, opts), + (err: Error) => { + assert.strictEqual(err.message, 'Assertion has already been used (replay detected).'); + return true; + } + ); + }); + + it('passes the Conditions NotOnOrAfter to the replay validator for TTL sizing', async () => { + const signed = buildAssertionOnlySigned({ + subjectInResponseTo: VICTIM_REQ_ID, + withConditionsWindow: true, + }); + let captured: { assertionId?: string; notOnOrAfter?: string } | undefined; + const profile = await validate(signed, { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + assertionReplayValidator: async (info: { assertionId?: string; notOnOrAfter?: string }) => { + captured = info; + return false; + }, + }); + assert.ok(captured, 'replay validator was called'); + assert.strictEqual(captured!.assertionId, '_assert1'); + assert.ok(captured!.notOnOrAfter, 'notOnOrAfter is provided'); + assert.strictEqual(captured!.notOnOrAfter, profile.notOnOrAfter); + assert.ok(new Date(captured!.notOnOrAfter!).getTime() > Date.now()); + }); + + it('falls back to SubjectConfirmationData NotOnOrAfter when there is no Conditions window', async () => { + // The assertion is bearer-bounded only; the replay validator must still get + // a concrete expiry so it does not have to guess a TTL. + const signed = buildAssertionOnlySigned({ + responseInResponseTo: null, + subjectInResponseTo: VICTIM_REQ_ID, + withConditionsWindow: false, + withSubjectNotOnOrAfter: true, + }); + let captured: { notOnOrAfter?: string } | undefined; + const profile = await validate(signed, { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + assertionReplayValidator: async (info: { notOnOrAfter?: string }) => { + captured = info; + return false; + }, + }); + assert.ok(captured!.notOnOrAfter, 'notOnOrAfter is provided from SubjectConfirmationData'); + assert.strictEqual(captured!.notOnOrAfter, profile.notOnOrAfter); + assert.ok(new Date(captured!.notOnOrAfter!).getTime() > Date.now()); + }); + + it('surfaces errors thrown by the replay validator without leaking a valid profile', async () => { + const signed = buildAssertionOnlySigned({ subjectInResponseTo: VICTIM_REQ_ID }); + await assert.rejects( + () => + validate(signed, { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + assertionReplayValidator: async () => { + throw new Error('store unavailable'); + }, + }), + (err: Error) => { + assert.strictEqual(err.message, 'An error occurred during assertion replay validation.'); + return true; + } + ); + }); +}); + +describe('saml20.validateExpiration: at least one enforceable upper bound required', () => { + const past = () => new Date(Date.now() - 3600_000).toISOString(); + const future = () => new Date(Date.now() + 3600_000).toISOString(); + + it('treats an assertion with no expiration anywhere as expired (was "never expires")', () => { + assert.strictEqual(saml20.validateExpiration({}), false); + }); + + it('treats a NotBefore-only Conditions (no upper bound) as expired', () => { + assert.strictEqual(saml20.validateExpiration({ Conditions: { '@': { NotBefore: past() } } }), false); + }); + + it('treats a present-but-unparseable bound as expired', () => { + assert.strictEqual( + saml20.validateExpiration({ Conditions: { '@': { NotBefore: past(), NotOnOrAfter: 'not-a-date' } } }), + false + ); + }); + + it('accepts a current Conditions validity window', () => { + assert.strictEqual( + saml20.validateExpiration({ Conditions: { '@': { NotBefore: past(), NotOnOrAfter: future() } } }), + true + ); + }); + + it('accepts a bearer assertion bounded only by SubjectConfirmationData/@NotOnOrAfter', () => { + // No Conditions window, but a signed bearer expiration is present. This is a + // spec-valid bearer assertion and must not be rejected. + assert.strictEqual( + saml20.validateExpiration({ + Subject: { SubjectConfirmation: { SubjectConfirmationData: { '@': { NotOnOrAfter: future() } } } }, + }), + true + ); + }); + + it('accepts when one SubjectConfirmation is stale but another is still valid (alternatives, OR)', () => { + // SAML 2.0 core 2.4.1.1: multiple SubjectConfirmations are alternatives; + // satisfying any one is sufficient. + assert.strictEqual( + saml20.validateExpiration({ + Subject: { + SubjectConfirmation: [ + { SubjectConfirmationData: { '@': { NotOnOrAfter: '2000-01-01T00:00:00Z' } } }, + { SubjectConfirmationData: { '@': { NotOnOrAfter: future() } } }, + ], + }, + }), + true + ); + }); + + it('rejects when every SubjectConfirmation upper bound is in the past', () => { + assert.strictEqual( + saml20.validateExpiration({ + Subject: { + SubjectConfirmation: [ + { SubjectConfirmationData: { '@': { NotOnOrAfter: '2000-01-01T00:00:00Z' } } }, + { SubjectConfirmationData: { '@': { NotOnOrAfter: '2010-01-01T00:00:00Z' } } }, + ], + }, + }), + false + ); + }); + + it('honors a valid Conditions window even if a bearer confirmation has lapsed', () => { + assert.strictEqual( + saml20.validateExpiration({ + Conditions: { '@': { NotBefore: past(), NotOnOrAfter: future() } }, + Subject: { + SubjectConfirmation: { SubjectConfirmationData: { '@': { NotOnOrAfter: '2000-01-01T00:00:00Z' } } }, + }, + }), + true + ); + }); + + it('accepts a bearer login (no Conditions window) end-to-end via validate()', async () => { + const signed = buildAssertionOnlySigned({ + responseInResponseTo: null, + subjectInResponseTo: VICTIM_REQ_ID, + withConditionsWindow: false, + withSubjectNotOnOrAfter: true, + }); + const profile = await validate(signed, { + audience: AUDIENCE, + publicKey: cert, + inResponseTo: VICTIM_REQ_ID, + }); + assert.strictEqual(profile.claims[NAME_ID_CLAIM], 'victim@target.com'); + }); + + it('rejects an assertion with no expiration bound anywhere end-to-end via validate()', async () => { + const signed = buildAssertionOnlySigned({ + subjectInResponseTo: VICTIM_REQ_ID, + withConditionsWindow: false, + withSubjectNotOnOrAfter: false, + }); + await expectReject(signed, 'Assertion is expired.', { inResponseTo: VICTIM_REQ_ID }); + }); +}); diff --git a/test/lib/saml20.spec.ts b/test/lib/saml20.spec.ts index 656d7181..68f8a34f 100644 --- a/test/lib/saml20.spec.ts +++ b/test/lib/saml20.spec.ts @@ -138,18 +138,23 @@ describe('saml20.ts', function () { }); it('validateExpiration ok', function () { - const value = saml20.validateExpiration(assertion); - assert.strictEqual(value, true); - assert(saml20.validateExpiration(assertion)); + // A parsed assertion with a current, bounded validity window is valid. + const assertionWithWindow = { + Conditions: { + '@': { + NotBefore: new Date(Date.now() - 3600_000).toISOString(), + NotOnOrAfter: new Date(Date.now() + 3600_000).toISOString(), + }, + }, + }; + assert.strictEqual(saml20.validateExpiration(assertionWithWindow), true); }); it('validateExpiration not ok', function () { - try { - const value = saml20.validateExpiration('assertion'); - assert.strictEqual(value, true); - } catch (error) { - assert(error); - } + // A missing validity window must be treated as invalid (expired), not as + // "never expires". + assert.strictEqual(saml20.validateExpiration('assertion'), false); + assert.strictEqual(saml20.validateExpiration({}), false); }); it('getClaims with friendly names', function () { const attributes = [