Skip to content

Commit

Permalink
Prevent bypass for InResponseTo (#87)
Browse files Browse the repository at this point in the history 
Addresses #82.
  • Loading branch information
@afurlane
afurlane committed Jun 16, 2022
1 parent abfb20d commit c58d868
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 34 deletions.
60 changes: 32 additions & 28 deletions src/saml.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1074,8 +1074,9 @@ class SAML {
}

const subject = assertion.Subject;
let subjectConfirmation: XMLOutput | null = null;
let subjectConfirmation: XMLOutput | null | undefined;
let confirmData: XMLOutput | null = null;
let subjectConfirmations: XMLOutput[] | null = null;
if (subject) {
const nameID = subject[0].NameID;
if (nameID && nameID[0]._) {
Expand All @@ -1087,33 +1088,31 @@ class SAML {
profile.spNameQualifier = nameID[0].$.SPNameQualifier;
}
}

subjectConfirmation = subject[0].SubjectConfirmation?.find(
(_subjectConfirmation: XMLOutput) => {
const _confirmData = _subjectConfirmation.SubjectConfirmationData?.[0];
if (_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,
maxTimeLimitMs
);
if (subjErr === null) return true;
}

return false;
subjectConfirmations = subject[0].SubjectConfirmation;
subjectConfirmation = subjectConfirmations?.find((_subjectConfirmation: XMLOutput) => {
const _confirmData = _subjectConfirmation.SubjectConfirmationData?.[0];
if (_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,
maxTimeLimitMs
);
if (subjErr === null) return true;
}
);

if (subjectConfirmation) {
return false;
});

if (subjectConfirmation != null) {
confirmData = subjectConfirmation.SubjectConfirmationData[0];
}
}
Expand Down Expand Up @@ -1144,8 +1143,13 @@ class SAML {
}
}
} else {
await this.cacheProvider.removeAsync(inResponseTo);
break getInResponseTo;
if (subjectConfirmations != null && subjectConfirmation == null) {
msg = "No valid subject confirmation found among those available in the SAML assertion";
throw new Error(msg);
} else {
await this.cacheProvider.removeAsync(inResponseTo);
break getInResponseTo;
}
}
} else {
break getInResponseTo;
Expand Down
65 changes: 59 additions & 6 deletions test/tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,33 @@ describe("node-saml /", function () {
expect(profile.nameID).to.equal("vincent.vega@evil-corp.com");
});

it("valid xml document with multiple SubjectConfirmation should fail if no one is valid", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T19:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-double-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
const signingCert = fs.readFileSync(__dirname + "/static/cert.pem", "utf-8");
const privateKey = fs.readFileSync(__dirname + "/static/key.pem", "utf-8");

const samlObj = new SAML({
cert: signingCert,
privateKey: privateKey,
issuer: "onesaml_login",
audience: false,
validateInResponseTo: ValidateInResponseTo.always,
});

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
// The second `SubjectConfirmationData` is invalid, the first one could not be used so we should get
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message:
"No valid subject confirmation found among those available in the SAML assertion",
});
});

it("valid xml document with multiple SubjectConfirmation should validate, first is expired so it should take the second one", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
Expand All @@ -719,15 +746,15 @@ describe("node-saml /", function () {
// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
// The second `SubjectConfirmationData` purposefully has the wrong InResponseTo so we can check for it
assert.rejects(samlObj.validatePostResponseAsync(container), {
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
});
});

it("valid xml document with no SubjectConfirmation should not validate", async () => {
it("valid xml document with multiple SubjectConfirmations should fail if InResponseTo does not match a valid SubjectConfirmation", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-no-subjectconfirmation.xml",
__dirname + "/static/response.root-signed.message-signed-double-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
Expand All @@ -744,11 +771,36 @@ describe("node-saml /", function () {

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
assert.rejects(samlObj.validatePostResponseAsync(container), {
// The second `SubjectConfirmationData` purposefully has the wrong InResponseTo so we can check for it
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
});
});

it("valid xml document with no SubjectConfirmation should validate", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-no-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
const signingCert = fs.readFileSync(__dirname + "/static/cert.pem", "utf-8");
const privateKey = fs.readFileSync(__dirname + "/static/key.pem", "utf-8");

const samlObj = new SAML({
cert: signingCert,
privateKey: privateKey,
issuer: "onesaml_login",
audience: false,
validateInResponseTo: ValidateInResponseTo.always,
});

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
const { profile } = await samlObj.validatePostResponseAsync(container);
assertRequired(profile, "profile must exist");
});

it("valid xml document with only empty SubjectConfirmation should not validate", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
Expand All @@ -769,8 +821,9 @@ describe("node-saml /", function () {

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message:
"No valid subject confirmation found among those available in the SAML assertion",
});
});

Expand Down

0 comments on commit c58d868

Please sign in to comment.