From 352e059e54d24dff56b7d3dbe240bea7b92f3463 Mon Sep 17 00:00:00 2001 From: Nickie Pucel Date: Fri, 26 Mar 2021 08:54:14 -0700 Subject: [PATCH] Add ability to specify a maximum allowed assertion age --- README.md | 1 + src/passport-saml/saml.ts | 51 +++++++++++++++++++++++++++++++++++--- src/passport-saml/types.ts | 1 + test/tests.spec.ts | 45 +++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index db7fd32f..4d38bf38 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,7 @@ type Profile = { - `identifierFormat`: optional name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`) - `wantAssertionsSigned`: if truthy, add `WantAssertionsSigned="true"` to the metadata, to specify that the IdP should always sign the assertions. - `acceptedClockSkewMs`: Time in milliseconds of skew that is acceptable between client and server when checking `OnBefore` and `NotOnOrAfter` assertion condition validity timestamps. Setting to `-1` will disable checking these conditions entirely. Default is `0`. +- `maxAssertionAgeMs`: Amount of time after which the framework should consider an assertion expired. If the limit imposed by this variable is stricter than the limit imposed by `NotOnOrAfter`, this limit will be used when determining if an assertion is expired. - `attributeConsumingServiceIndex`: optional `AttributeConsumingServiceIndex` attribute to add to AuthnRequest to instruct the IDP which attribute set to attach to the response ([link](http://blog.aniljohn.com/2014/01/data-minimization-front-channel-saml-attribute-requests.html)) - `disableRequestedAuthnContext`: if truthy, do not request a specific authentication context. This is [known to help when authenticating against Active Directory](https://github.com/node-saml/passport-saml/issues/226) (AD FS) servers. - `authnContext`: if truthy, name identifier format to request auth context (default: `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport`); array of values is also supported diff --git a/src/passport-saml/saml.ts b/src/passport-saml/saml.ts index 85311fee..edc7a55e 100644 --- a/src/passport-saml/saml.ts +++ b/src/passport-saml/saml.ts @@ -132,6 +132,7 @@ class SAML { skipRequestCompression: ctorOptions.skipRequestCompression ?? false, disableRequestAcsUrl: ctorOptions.disableRequestAcsUrl ?? false, acceptedClockSkewMs: ctorOptions.acceptedClockSkewMs ?? 0, + maxAssertionAgeMs: ctorOptions.maxAssertionAgeMs ?? 0, path: ctorOptions.path ?? "/saml/consume", host: ctorOptions.host ?? "localhost", issuer: ctorOptions.issuer ?? "onelogin_saml", @@ -1116,11 +1117,17 @@ class SAML { if (confirmData && confirmData.$) { const subjectNotBefore = confirmData.$.NotBefore; const subjectNotOnOrAfter = confirmData.$.NotOnOrAfter; + const maxTimeLimitMs = this.processMaxAgeAssertionTime( + this.options.maxAssertionAgeMs, + subjectNotOnOrAfter, + assertion.$.issueInstant + ); const subjErr = this.checkTimestampsValidityError( nowMs, subjectNotBefore, - subjectNotOnOrAfter + subjectNotOnOrAfter, + maxTimeLimitMs ); if (subjErr) { throw subjErr; @@ -1167,10 +1174,16 @@ class SAML { throw new Error(msg); } if (conditions && conditions.$) { + const maxTimeLimitMs = this.processMaxAgeAssertionTime( + this.options.maxAssertionAgeMs, + conditions.$.NotOnOrAfter, + assertion.$.IssueInstant + ); const conErr = this.checkTimestampsValidityError( nowMs, conditions.$.NotBefore, - conditions.$.NotOnOrAfter + conditions.$.NotOnOrAfter, + maxTimeLimitMs ); if (conErr) throw conErr; } @@ -1231,7 +1244,12 @@ class SAML { return { profile, loggedOut: false }; } - checkTimestampsValidityError(nowMs: number, notBefore: string, notOnOrAfter: string) { + checkTimestampsValidityError( + nowMs: number, + notBefore: string, + notOnOrAfter: string, + maxTimeLimitMs?: number + ) { if (this.options.acceptedClockSkewMs == -1) return null; if (notBefore) { @@ -1244,6 +1262,10 @@ class SAML { if (nowMs - this.options.acceptedClockSkewMs >= notOnOrAfterMs) return new Error("SAML assertion expired"); } + if (maxTimeLimitMs) { + if (nowMs - this.options.acceptedClockSkewMs >= maxTimeLimitMs) + return new Error("SAML assertion expired"); + } return null; } @@ -1461,6 +1483,29 @@ class SAML { // https://github.com/node-saml/passport-saml/issues/431#issuecomment-718132752 return xml.replace(/\r\n?/g, "\n"); } + + /** + * Process max age assertion and use it if it is more restrictive than the NotOnOrAfter age + * assertion received in the SAMLResponse. + * + * @param maxAssertionAgeMs Max time after IssueInstant that we will accept assertion, in Ms. + * @param notOnOrAfter Expiration provided in response. + * @param issueInstant Time when response was issued. + * @returns {*} The expiration time to be used, in Ms. + */ + processMaxAgeAssertionTime( + maxAssertionAgeMs: number, + notOnOrAfter: string, + issueInstant: string + ): number { + const notOnOrAfterMs = Date.parse(notOnOrAfter); + if (maxAssertionAgeMs === 0) { + return notOnOrAfterMs; + } + + const maxAssertionTimeMs = Date.parse(issueInstant) + maxAssertionAgeMs; + return maxAssertionTimeMs < notOnOrAfterMs ? maxAssertionTimeMs : notOnOrAfterMs; + } } export { SAML }; diff --git a/src/passport-saml/types.ts b/src/passport-saml/types.ts index e10a5bf8..7cad16bb 100644 --- a/src/passport-saml/types.ts +++ b/src/passport-saml/types.ts @@ -63,6 +63,7 @@ export interface SamlOptions extends SamlSigningOptions, MandatorySamlOptions { audience?: string; scoping?: SamlScopingConfig; wantAssertionsSigned?: boolean; + maxAssertionAgeMs: number; // InResponseTo Validation validateInResponseTo: boolean; diff --git a/test/tests.spec.ts b/test/tests.spec.ts index 8ab2bd40..26e327df 100644 --- a/test/tests.spec.ts +++ b/test/tests.spec.ts @@ -1658,6 +1658,51 @@ describe("passport-saml /", function () { profile!.nameID!.should.startWith("ploer"); }); + it("onelogin xml document with current time after MaxAssertionAge (minus default clock skew) should fail", async () => { + const xml = + 'https://app.onelogin.com/saml/metadata/371755' + + 'https://app.onelogin.com/saml/metadata/371755DCnPTQYBb1hKspbe6fg1U3q8xn4=e0+aFomA0+JAY0f9tKqzIuqIVSSw7LiFUsneEDKPBWdiTz1sMdgr/2y1e9+rjaS2mRmCi/vSQLY3zTYz0hp6nJNU19+TWoXo9kHQyWT4KkeQL4Xs/gZ/AoKC20iHVKtpPps0IQ0Ml/qRoouSitt6Sf/WDz2LV/pWcH2hx5tv3xSw36hK2NQc7qw7r1mEXnvcjXReYo8rrVf7XHGGxNoRIEICUIi110uvsWemSXf0Z0dyb0FVYOWuSsQMDlzNpheADBifFO4UTfSEhFZvn8kVCGZUIwrbOhZ2d/+YEtgyuTg+qtslgfy4dwd4TvEcfuRzQTazeefprSFyiQckAXOjcw==' + + TEST_CERT + + 'ploer@subspacesw.com{audience}urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' + + ""; + const base64xml = Buffer.from(xml).toString("base64"); + const container = { SAMLResponse: base64xml }; + + // Set the maxAssertionAgeMs so that IssueInstant + maxAssertionAgeMs == 2014-05-28T00:16:09Z + // Note that NotOnOrAfter == 2015-05-28T00:19:08Z in the response + const samlObj = new SAML({ ...samlConfig, maxAssertionAgeMs: 1000 }); + + // Fake the current date to be after the time limit set by maxAssertionAgeMs, + // but before the limit set by NotOnOrAfter + fakeClock.restore(); + fakeClock = sinon.useFakeTimers(Date.parse("2014-05-28T00:17:09Z")); + await assert.rejects(samlObj.validatePostResponseAsync(container), { + message: "SAML assertion expired", + }); + }); + + it("onelogin xml document with current time before MaxAssertionAge (minus default clock skew) should pass", async () => { + const xml = + 'https://app.onelogin.com/saml/metadata/371755' + + 'https://app.onelogin.com/saml/metadata/371755DCnPTQYBb1hKspbe6fg1U3q8xn4=e0+aFomA0+JAY0f9tKqzIuqIVSSw7LiFUsneEDKPBWdiTz1sMdgr/2y1e9+rjaS2mRmCi/vSQLY3zTYz0hp6nJNU19+TWoXo9kHQyWT4KkeQL4Xs/gZ/AoKC20iHVKtpPps0IQ0Ml/qRoouSitt6Sf/WDz2LV/pWcH2hx5tv3xSw36hK2NQc7qw7r1mEXnvcjXReYo8rrVf7XHGGxNoRIEICUIi110uvsWemSXf0Z0dyb0FVYOWuSsQMDlzNpheADBifFO4UTfSEhFZvn8kVCGZUIwrbOhZ2d/+YEtgyuTg+qtslgfy4dwd4TvEcfuRzQTazeefprSFyiQckAXOjcw==' + + TEST_CERT + + 'ploer@subspacesw.com{audience}urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' + + ""; + const base64xml = Buffer.from(xml).toString("base64"); + const container = { SAMLResponse: base64xml }; + + // Set the maxAssertionAgeMs so that IssueInstant + maxAssertionAgeMs == 2014-05-28T00:16:09Z + // Note that NotOnOrAfter == 2015-05-28T00:19:08Z in the response + const samlObj = new SAML({ ...samlConfig, maxAssertionAgeMs: 1000 }); + + // Fake the current date to be before the time limit set by maxAssertionAgeMs + fakeClock.restore(); + fakeClock = sinon.useFakeTimers(Date.parse("2014-05-28T00:16:08Z")); + + const { profile } = await samlObj.validatePostResponseAsync(container); + profile!.nameID!.should.startWith("ploer"); + }); + it("onelogin xml document with audience and no AudienceRestriction should not pass", async () => { const signingCert = fs.readFileSync(__dirname + "/static/cert.pem", "utf-8"); const xml = `