SAML2 HTTP-Redirect: Missing Signature and SigAlg parameters in SAMLRequest Url (AuthNRequest)

[berschmoe]

Summary

I'm currently update to Spring Security 5.2.1. and start to use the integrated SAML 2 implementation.

During the integration I noticed that my identity provider (Keycloak) does not accept the signed AuthNRequest.

The reason is that SAML 2 expects different signature for different bindings (POST or Redirect) - at least that's how I understand it.

• If a POST binding is used the signature is embedded in the XML.
• If a Redirect binding is used the signature is part of the URL query parameters.
  (e.g. https://idp/?SAMLRequest=...&RelayState=...&SigAlg=...&Signature=...)

I checked the Spring Security SAML Extension online demo (https://saml-federation.appspot.com) and here it works as expected.

GET Parameters:
SAMLRequest: fZLLbsIwEEV/JfI...
SigAlg: http://www.w3.org/2000/09/xmldsig#rsa-sha1
Signature: LAB/NahduGHr5ew...

and a none signed AuthNRequest

<saml2p:AuthnRequest xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol"
                     AssertionConsumerServiceURL="https://saml-federation.appspot.com:443/saml/SSO"
                     Destination="https://idp.ssocircle.com:443/sso/SSORedirect/metaAlias/ssocircle"
                     [...]
                     >
    <saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">saml-federation.appspot.com</saml2:Issuer>
</saml2p:AuthnRequest>

Currently the URL is created while using createSamlRequestRedirectUrl in Saml2WebSsoAuthenticationRequestFilter and these parameters aren't set.

private String createSamlRequestRedirectUrl(HttpServletRequest request, RelyingPartyRegistration relyingParty) {
	[...]
	String redirect = UriComponentsBuilder
			.fromUriString(relyingParty.getIdpWebSsoUrl())
			.queryParam("SAMLRequest", UriUtils.encode(encoded, StandardCharsets.ISO_8859_1))
			.queryParam("RelayState", UriUtils.encode(relayState, StandardCharsets.ISO_8859_1))
			.build(true)
			.toUriString();
	return redirect;
}

Expected Behavior

Using HTTP-Redirect binding SigAlg and Signature parameters are added to SAMLRequest Url and AuthNRequest XML is not signed.

Version

5.2.1.RELEASE

[fhanik]

@berschmoe

Thank you for your report. You are absolutely correct. I had originally implemented this as a POST binding and when I switched it to REDIRECT I didn't go back to the spec to check the signatures.

The BINDING spec covers this in detail in section 3.4.4.1 DEFLATE Encoding

Some Identity Providers, like Okta, ignore all signatures on the AuthNRequest message because they require the Service Provider ACS URL to be whitelisted. There is a possibility that Keycloak requires the same thing, and thus as a temporary mitigation you could turn off the signature requirement. You would have to double check this. Other providers, such as SimpleSAMLPhp, accept XML signatures in the message itself.

And that's probably why we haven't seen this bug reported until now.

I have prototyped two different solutions, in two different PRs for review.

1. Option 1 - Simply fix the bug, changing current behavior, with a fallback to existing SAML AuthNRequest Signatures - Step 1 #7758
2. Option 2 - Provide a configuration option for what BINDING should be used. And through configuration, remain backwards compatible. SAML AuthNRequest Signatures - Step 2 #7759 (this PR adds configuration on top of SAML AuthNRequest Signatures - Step 1 #7758)

Flagging @rwinch for consideration.

PS. During testing I discovered that java.util.Base64 is not sufficient for all IDPs, and we had a message that failed. So we changed the encoder/decoder back to Apache Commons Codec.
Each PR has this commit as a rider.

Test configuration as a gist

[lilalinux]

Hi, thanks for fixing. When can we expect the fix to be released? Is there a workaround?

[fhanik]

@lilalinux Most IDPs don't require signatures because they have the SSO URLs white listed and preconfigrued. The work around is to not require signatures. This will be part of the 5.3 release.

[lilalinux]

Unfortunately in SAP idP we can't disable that requirement 😕
Are there alternatives? Can we switch from Redirect to POST? (How?)

[rwinch]

Hi, thanks for fixing. When can we expect the fix to be released? Is there a workaround?

This is in the 5.3 release which will be out tomorrow. You can figure it out by looking at the milestone on the right hand side of the issue and clicking on it to see the scheduled date.

[fpagliar]

My understanding is that this fix is in 5.3.x and not in 5.2.x, is that correct?

[rwinch]

@fpagliar Yes it is only in 5.3.x As an enhancement it does not get backported to patch releases (which are only bug fixes)

[fpagliar]

I'm fine with it not being backported, but I'm trying to figure out the state and what to expect, so sorry to bother but I need to clarify the status.
My understanding on this issue is that 5.2 creates signed AuthNRequests that are not respecting the SAML standard. Is that correct?

[jzheaux]

Yes, @fpagliar, 5.2 deviates from HTTP-Redirect by omitting the SigAlg and Signature query parameters.