Response destination presence check

[madorin]

Hello,

ValidationError::EMPTY_DESTINATION or a potential MISSING_DESTINATION is not triggered if destination is not specified:

php-saml/src/Saml2/Response.php

Line 274 in 341215d

if ($this->document->documentElement->hasAttribute('Destination')) {

The code checks if Destination attribute is present, then only in this case it does validation.
In case if the attribute is not specified the security config option relaxDestinationValidation is ignored.

Our integration fails QA test because of this..

[madorin]

@pitbulk , I created a MR on this, please check and merge if it's fine to go

not sure if trim(destination) is needed, in response it does trim, but not in logout Request/Response
probably best to remove the trim, as spaces also belong to node value

[pitbulk]

Hi @madorin,

As you can check in the SAML spec Page 36,

Destination [Optional]
A URI reference indicating the address to which this request has been sent. This is useful to prevent
malicious forwarding of requests to unintended recipients, a protection that is required by some
protocol bindings. If it is present, the actual recipient MUST check that the URI reference identifies the
location at which the message was received. If it does not, the request MUST be discarded. Some
protocol bindings may require the use of this attribute (see [SAMLBind]).

That element is marked as optional, so the toolkit validates it only if provided.

[madorin]

Thanks, @pitbulk, for SAML reference, will check with IdP provider on it,
although the idea with one place check for destinations in MR makes sense.

[pitbulk]

Yes, I noted the idea of the validator to reuse code.

[madorin]

@pitbulk , i checked a bit more on it:

The SAML 2.0 Bindings Specification includes a critical security rule for the HTTP-Redirect and HTTP-POST bindings:

If the message is signed, the Destination attribute MUST be present in the root SAML element of the protocol message.
The recipient MUST then verify that the value matches the URL where the message was received.

Any thoughts on it?

[pitbulk]

In that SAML 2.0 Bindings Specification spec I read the following:

If the SAML protocol message is signed using SimpleSign, the Destination XML attribute in the root SAML element of the SAML 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 SAML protocol message has been received. Also, the signer's certificate or other keying information MAY be included in a form control named KeyInfo. This form control, if present, MUST contain a base-64 encoded <ds:KeyInfo> element [XMLSig] (base-64 encoding is done as in step 1, above).

If the SAML protocol messages are signed (using the SimpleSign approach or [XMLSig]) then the Destination XML attribute in the root SAML element of the SAML 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.

But I assume it indicates that if present, it MUST contain the URL. But it does not indicates that MUST be present.

which aligns with the description on the spec I previously shared:

If it is present, the actual recipient MUST check ...

[madorin]

@pitbulk , at least ChatGPT, Gemini says in practice for signed messages destination attribute is "practically required".

Many SP implementations (like Azure AD, Okta, Shibboleth SP) will reject a SAML response that lacks the Destination attribute or has a mismatch.
In these systems, the validation logic treats absence of Destination as a potential security issue. It prevents redirection/replay attacks

It's the case also with the IdP we did the SAML integration ..

Can we add the requireDestination option in security config with false as default?
I'd prefer this way, instead of keeping a separate fork..