7

We use the Spring SAML Security Extension to implement SAML in our application. We now have the following problem:

One of our customers is providing a URL for their identity provider that contains a parameter. The metadata looks like this (heavily abbreviated for brevity):

<EntityDescriptor>
  <IDPSSODescriptor>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
        Location="https://idp.example.com/login?parameter=value"/>
  </IDPSSODescriptor>
</EntityDescriptor>

As can be seen, there is a parameter named "parameter" with a value "value". This parameter is not present in the generated redirect URL. I debugged a bit and found out that SAMLProcessorImpl gets the MessageEncoder from the binding (which is HTTPRedirectDeflateEncoder for HTTP redirect) and delegates encoding the message. The encoder in turn does the following in its buildRedirectURL method:

// endpointURL is https://idp.example.com/login?parameter=value here
URLBuilder urlBuilder = new URLBuilder(endpointURL);

List<Pair<String, String>> queryParams = urlBuilder.getQueryParams();
queryParams.clear(); // whoops

So for some reason, the parameters are stripped intentionally and unconditionally.

Why is this the case and how can I fix this in the most efficient way?

musiKk
  • 14,751
  • 4
  • 55
  • 82

2 Answers2

4

SAML Authentication Request should be only sent by trusted entities and with parameters which cannot be tampered with. Adding a parameter in addition to SAMLAuthnRequest encoded according to HTTP-Redirect binding will mean that a potential attacker can change the value as he/she pleases and IDP will not be able to detect such change - as the parameter will not be covered by digital signature.

SAML provides a mechanism for delivery of secured content in addition to request itself called relayState - and you can set it using WebSSOProfileOptions of Spring SAML.

The above is reason the parameters are cleared (at least I believe so, this logic comes from OpenSAML library which is not written by me), but of course in case you don't mind the security implications, the approach you found is just fine.

Vladimír Schäfer
  • 15,375
  • 2
  • 51
  • 71
  • Thanks, your remarks really give me something to think about. Right now I cannot really think of a scenario where this could have security implications. Would this change if the parameter were encoded as a regular path, i.e., `.../login/parameter/value`? The parameter is not there for security; it's as important as talking to the right host on the right port (in our case it's the selection of a realm on the IdP). Failure to provide it will simply cause authentication to fail. – musiKk Jul 02 '15 at 21:24
  • The URL where IDP receives requests/responses is typically statically defined beforehand, so you'd need to create one endpoint for each parameter value. Another way to send more parameters to IDP (in addition to relayState) is to use Extensions element in AuthnRequest. But essentially, if you believe that there's no risk in someone tampering with the values, go ahead with the parameters in GET request solution, it will require least customization on IDP side. – Vladimír Schäfer Jul 03 '15 at 05:07
  • Both parameter and value are fixed. As stated in the question they are explicitly set in the metadata.xml which is signed. So I guess everything is fine and OpenSAML's behavior is too restrictive. Thanks again! – musiKk Jul 03 '15 at 07:15
  • I now briefly checked the SAML spec and there's no explicit mention that additional parameters should be stripped when using HTTP-Redirect binding - so I agree about OpenSAML being perhaps too restrictive in this case. – Vladimír Schäfer Jul 03 '15 at 07:33
0

For now I solved the problem by extending HTTPRedirectDeflateEncoder, copying the code for buildRedirectURL and remove the queryParams.clear(). Because the URL is created by simple concatenation I explicitly removing all reserved SAML parameters (like SigAlg and the like) in order to avoid potential security issues. After that, it's a simple Spring configuration change to wire everything together.

I still don't know why the list is cleared in the first place, though.

musiKk
  • 14,751
  • 4
  • 55
  • 82
  • Hi musiKk. I tried to do the same but my extended HttpRedirectDeflateEncoder wasn't taking affect and I wondered if you had an example of what you did? I am trying to wire with Spring Boot auto-configuration as described in SO https://stackoverflow.com/questions/48551925/saml-http-request-intercept-with-spring-boot Thanks! – Fratt Feb 02 '18 at 15:40
  • @Fratt Sorry, would really love to help you but I have long since left the company. – musiKk Feb 05 '18 at 16:04