Response Destination Validation - Query Strings

[dlpetrie]

We are porting an old SAML implementation from PHP over to Go, and so far this library has worked great. I have reused the middleware logic and mixed with our own to satisfy the multi-tenant setup we have.

The issue I'm running into now is our old setup used a few query strings in the ACS URL Location, and we need to maintain that for compatibility. With the library and go, unfortunately, it organizes the query string in alphabetical order and looks for an exact match URL with query strings, and if not matching, it fails. So even if the URL is the same, but the query string appear in a different order, the destination validation fails.

Would you be open to a PR that either:

1. Removes the query string as part of the ACS Location / Destination validation and ignores the query string
2. Removes the query string and verifies the rest of the url. Then additionally validates the query string, regardless of order

saml/service_provider.go

Lines 869 to 873 in 34930b2

	if responseHasSignature || response.Destination != "" {
	if response.Destination != sp.AcsURL.String() {
	return nil, fmt.Errorf("`Destination` does not match AcsURL (expected %q, actual %q)", sp.AcsURL.String(), response.Destination)
	}
	}

[studouglas]

This is affecting us as well. The proper solution here would be to compare response.Destination against the actual URL the request was received at instead of the ACS URL.

This would correctly implement Section "3.4.5.2, Security Considerations" of the SAML spec:

If the message is signed, the Destination XML attribute in the root SAML element of the protocol
message MUST contain the URL to which the sender has instructed the user agent to deliver the
message. The recipient MUST then verify that the value matches the location at which the message has
been received.

This is how SAML-toolkits/java-saml implements the check here.

This can be done by adding a new param for the actual URL to ParseXMLResponse, based on the request object in-scope at the call-site in parseResponseHTTP.