SAML-Toolkits / java-saml

Java SAML toolkit
MIT License
633 stars 396 forks source link

Auth.processSLO() should fast-fail on non-GET requests #299

Open veselov opened 3 years ago

veselov commented 3 years ago

Signatures generated for logout response in opensaml (2.6.6) can not be validated by java-saml.

I'm still in the process of investigating this, and not really sure who to blame.

The reason for signature mismatch is that Opensaml puts the original message contents into the signing buffer, and java-saml uses base64 encoded version of it. It actually seems to use percent-encoded base64 encoded message.

However, I also could not validate the message using https://www.samltool.com/validate_logout_res.php. Interestingly, the latter doesn't accept base64 logout response contents.

I also can't find where is this described in SAML documentation. That documentation only talks about XML signatures, which whatever is being used here is not.

I'm loaded with questions, though:

pitbulk commented 3 years ago

samltool.com uses the php-saml toolkit as base, not java-saml, and the validate_logout_res.php endpoint accepts the XML but not the base64encoded form, you can just decode it with https://www.samltool.com/decode.php.

At java-saml, the method to validate the Signature of HTTP-Redirect bindings is validateBinarySignature

The parameters are retrieved using request.getEncodedParameter unless the signature which is retrieved first with request.getParameter("Signature"); and later decoded with Util.base64decoder that executes

Base64.decodeBase64(input)

Based on its documentation, the Base64 class provides Base64 encoding and decoding as defined by RFC 2045.

The getEncodedParameter uses Utils.urlEncoder uses java.net.URLEncoder.encode

The SAML standard specifies how sign on the HTTP-Redirect binding protocol at https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf

3.4.4.1 DEFLATE Encoding
...
The signature value MUST be encoded using the base64 encoding (see RFC 2045 [RFC2045]) with
any whitespace removed.

....

Further, note that URL-encoding is not canonical; that is, there are multiple legal encodings for a given
value. The relying party MUST therefore perform the verification step using the original URL-encoded
values it received on the query string. It is not sufficient to re-encode the parameters after they have been
processed by software because the resulting encoding may not match the signer's encoding.
veselov commented 3 years ago

@pitbulk Thank you for your patience, Sixto.

I was apparently using HTTP Post bindings, where I should have been using HTTP Redirect bindings. Auth.processSLO() just gets request parameter, and can't know whether those are post parameters or URL parameters, so problems only become apparent during signature verification.

veselov commented 3 years ago

On the other hand, would it be an improvement if bindings type was checked before the rest of the validation started, so this is easier to figure out?

pitbulk commented 3 years ago

java-saml generates and receives all SAML messages by the HTTP-Redirect binding with the exception of the SAMLResponse received via HTTP-POST.

veselov commented 3 years ago

But how can the SP side control which bindings it wants?

In my case I'm the SP, and after sending a logout request, the logout response was received as an HTTP Post binding, simply because IDP decided so. But then it's just a mock IDP, so it's not following a whole lot of rules.

I've changed it to use HTTP Redirect and that fixed things, but it would have been a lot easier to realize what's wrong if the response processing code checked the binding and threw an error before going into signature check.

pitbulk commented 3 years ago

SP exposes SAML metadata where are defined the endpoints and its associated binding. If you registered the SP metadata properly at the IdP, the IdP should not send an HTTP-POST Logout request to an HTTP-Redirect endpoint.

I wonder why you have not experienced the error defined here: https://github.com/onelogin/java-saml/blob/master/toolkit/src/main/java/com/onelogin/saml2/Auth.java#L954

String errorMsg = "SAML LogoutRequest/LogoutResponse not found. Only supported HTTP_REDIRECT Binding";

which is exactly the error that you wanted to receive instead the signature validation error.

veselov commented 3 years ago

SP exposes SAML metadata where are defined the endpoints and its associated binding.

That's fair. But yes, I was dealing with Mujina, which I then hacked myself, no SP metadata, so no wonder I stepped into this.

I wonder why you have not experienced the error defined here

That's what I was talking about. Using HTTPRequest.getParameter() there is no way to differentiate between form parameters and query parameters. Since there was a (validish) SAML response returned from httpRequest.getParameter("SAMLResponse") (provided by the HTTP post request into the SLS), the code just happily marched forward.

pitbulk commented 3 years ago

Oh I see, I think in the past this kind of error was raised...but when the httpRequest class was added, we missed it. I agree could be cool to have some kind of check on processSLO to verify it is a GET and not a POST. Do you want to provide a PR?

veselov commented 3 years ago

Yes, let me.