[BUG] SAML LogoutResponse InResponseTo validation is not working

[eerohakio]

Correct me if I'm wrong but it seems that SAML LogoutResponse InResponseTo validation is not working.

When going through code this one was particularly interesting https://github.com/node-saml/node-saml/blob/master/src/saml.ts#L698

It seems that InResponseTo is always taken from Response rather than LogoutResponse when request is LogoutResponse type.
All the test cases/static mocks are made with only Response format without any tests to test LogoutResponse validations.

Is there a way to overcome this issue with LogoutResponses?

Example from SAML official bindings

<samlp:LogoutResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
xmlns="urn:oasis:names:tc:SAML:2.0:assertion"
 ID="b0730d21b628110d8b7e004005b13a2b"
InResponseTo="d2b7c388cec36fa7c39c28fd298644a8"
 IssueInstant="2004-01-21T19:00:49Z" Version="2.0">
 <Issuer>https://ServiceProvider.com/SAML</Issuer>
 <samlp:Status>
 <samlp:StatusCode
Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
 </samlp:Status>
</samlp:LogoutResponse>

[srd90]

First of all: Issue reporter referenced codebase via via this link:

https://github.com/node-saml/node-saml/blob/master/src/saml.ts#L698

which becomes obsolete once content of that file at master branch changes over period of time. So here is same reference to codebase pinned to the version (e691ccf3) that was at the head of master branch at the time this issue was created:

node-saml/src/saml.ts

Line 698 in e691ccf

"/*[local-name()='Response']/@InResponseTo",

---

Evidence suggests that you debugged code path related to your SP's SingleLogoutService's HTTP-POST binding.

Is there a way to overcome this issue with LogoutResponses?

You did not rule out any solutions so have you tried HTTP-Redirect binding? Based on quick look at codebase

node-saml/src/saml.ts

Lines 866 to 868 in e691ccf

	samlMessageType === "SAMLResponse"
	? await this.verifyLogoutResponse(doc)
	: this.verifyLogoutRequest(doc);

inresponseto is validated for LogoutResponse messages if those arrive via HTTP-Redirect binding

node-saml/src/saml.ts

Lines 952 to 964 in e691ccf

	protected async verifyLogoutResponse(doc: XMLOutput): Promise<void> {
	const statusCode = doc.LogoutResponse.Status[0].StatusCode[0].$.Value;
	if (statusCode !== "urn:oasis:names:tc:SAML:2.0:status:Success")
	throw new Error("Bad status code: " + statusCode);
	this.verifyIssuer(doc.LogoutResponse);
	const inResponseTo = doc.LogoutResponse.$.InResponseTo;
	if (inResponseTo) {
	return this.validateInResponseTo(inResponseTo);
	}
	return;
	}

FYI: You might have better luck with SLO over HTTP-Redirect binding during other circumstances (depending on your IdP's and your SPs domains and how your IdP propagates SLOs). This FYI is related to how modern browsers handle session related cookies (in case your implementation depends on cookies during SLO scenarios and it should not depend on those but if .... ) see node-saml/passport-saml#419 for background information.

---

It seems that InResponseTo is always taken from Response rather than LogoutResponse when request is LogoutResponse type.

It also seems that it (inresponseto validation of LogoutResponse to SP via HTTP-Post binding) might have worked at some point of time or not or it might have become broken after some version update or not. Author of following issue has not left any other traces of versions except timestamps of the issue report: node-saml/passport-saml#438

Timestamp suggest that problem was spotted prior to split of core SAML functionality to @node-saml/node-saml and @node-saml/passport-saml when version 4.0.0 was relesed (latter being passportjs module which uses internally @node-saml/node-saml). One might be able to find how this was handled prior some breaking change from passport-saml codebase.

[eerohakio]

Thanks for your reply!
This explains part of our issue which means I need to investigate codebase more. Also have to investigate if it’s possible to use HTTP-Redirect binding instead which could solve the issue.

[cjbarth]

@eerohakio , if this issue no longer applies, feel free to close it.