How is the ACS Endpoint populated in SAML2MessageContext?

[Jaroslav Kačer]

Hello everyone!

I have run into a strange problem with PAC4J 1.7.1 SAML client and I wonder if someone could explain me what's going on.

The problem occurs with a certain IdP (just one out of cca 10) when a SAML response with a subject confirmation is received. Example:

<ns2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
 <ns2:SubjectConfirmationData
 InResponseTo="_lq77pbf5huvzeifh9v6y15n5uyir3sflelyjchm"
 NotOnOrAfter="2015-09-24T06:58:38Z"
 Recipient="https://MyServer/login?client_name=MySaml2Client"/>
</ns2:SubjectConfirmation>

The above XML looks just fine to me. The problem occurs when such message is validated in SAML2DefaultResponseValidator.isValidBearerSubjectConfirmationData() and and the Recipient URL does not match the Endpoint URL, which is read from the SAML2 Message Context / SAML Endpoint Context.

So I tried to find the place where the ACS Endpoint value is set to the context but failed :-(

1. The context is built in SAML2Client.retrieveCredetials() using a SAML2ContextProvider from a WebContext.
2. --> buildServiceProviderContext() --> addSPContext() - I stopped here because I did not understand how a SPSSODescriptor got created using OpenSaml.

What I know is how the ACS is read from the context in SAML2WebSSOMessageReceiver:

SAML2MessageContext.getSPAssertionConsumerService() --> getSPAssertionConsumerService(null) --> SPSSODescriptorImpl.getAssertionConsumerServices().

But I don't know when/how the SP SSO Descriptor is populated with ACS.

Could someone advice me what I should look at and where I should put a breakpoint to observe what ACS value is put into the context?

And, don't you know if it's theoretically possible that the value is incorrectly set? This looks weird to me because all my other clients are initialized fine, as far as I know, and they all have a very similar Spring definition, all in a single application.

Thank you in advance for your answers!

Jarda

[Jérôme LELEU]

Hi,

It seems strange the problem happens only for an IdP and not for others.

Did you test it with pac4j-saml v1.8?

Thanks.

Best regards,

Jérôme

--
You received this message because you are subscribed to the Google Groups "pac4j-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pac4j-users...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Jaroslav Kačer]

Hi Jerome!

Yes, it's really strange.

I have slightly modified logging in this case, the old statement only showed the value loaded from the SAML response, not the value created from configuration. Now I should see both, so I will know exactly what value got into the context.

Now I am waiting for the partner company to run the test again; they are a few timezones away from me and I cannot run the test on my own, so it's little complicated. I'll keep you updated.

And I haven't tested it with 1.8 yet; I will do it if the problem persists.

Thank you,

   Jarda

Dne úterý 29. září 2015 10:27:42 UTC+2 Jérôme LELEU napsal(a):

[Jérôme LELEU]

Hi,

OK. Thanks. Keep us posted.

Our efforts are now mainly focused on the pac4j v1.8 release (I don't think we'll ever have a new 1.7.x release).

Best regards,

Jérôme

[Jaroslav Kačer]

Hi!

I think I have finally found the cause of the problem: It is the default port numbers (443 for HTTPS etc). While the value from the SAML Endpoint context seems to always have a port number, the value from the SAML Response may or may not have it. The string comparison them may fail in some cases, which will make the whole validation fail.

It all begins with a SAML request sent:

<saml2p:AuthnRequest
    AssertionConsumerServiceURL="https://MyServer:443/login?client_name=XxxxxxxxSaml2Client"
    Destination="https://xxxxxxxxx"
    ForceAuthn="false" ID="_jhp3xpb2zfzbbtduudcqagetbpd1bmsphpvxzg8"
    IsPassive="false" IssueInstant="2015-10-01T18:01:42.665Z"
    ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
    ProviderName="pac4j-saml" Version="2.0" xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol">

The IdP server then receives the request and generates a response. Based on the IdP, the response may or may not contain the port value in <Response>/Destination and <SubjectConfirmationData>/Recipient. Here is my observation:

• MS ADFS takes the value from the request and therefore the response always contains the port number.Response validation then succeeds and everything works just fine.
• Shibboleth already checks the AssertionConsumerServiceURL of the incoming request and reports an error of it is different from the value taken from metadata. (I don't know if this can be configured or not.) So the metadata must already contain the port and everything works fine too - the port number is always present in the incoming SAML Response.
• Other IdP servers (among which the one I am working with now - I don't know the name, sorry, maybe something proprietary) accept a request with a port number but generate a response with "normalized" URLs - either from the metadata or they just normalize it themselves. The SP then receives a URL without the port number but compares it with a URL containing the number --> failure.

Here is an example of SAML response for the above request (ignore the difference of InResponseTo):

<samlp:Response
    Destination="https://MyServer/login?client_name=XxxxxxxxxSaml2Client"
    ID="_560d7506ceaa32.99458778"
    InResponseTo="_mx4guuug7klmjieinalbvmhjciqxfi62jpcxrri"
    IssueInstant="2015-10-01T18:01:42Z" Version="2.0"
    xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
.....
        <saml:SubjectConfirmationData
            NotOnOrAfter="2015-10-01T18:06:42Z"
            Recipient="https://MyServer/login?client_name=XxxxxxxxxSaml2Client"/>
      </saml:SubjectConfirmation>
......

I think this is a regression from version 1.6.0. I worked with the same type of (unknown) IdP server before and responses passed validation just fine. When I compare the 1.6 and 1.7 code, I can see some differences.

Version 1.6 - Saml2ResponseValidator:

if (data.getRecipient() == null) {
   logger.debug("SubjectConfirmationData recipient cannot be null for Bearer confirmation");
   return false;
} else {
   if (!data.getRecipient().equals(context.getAssertionConsumerUrl())) {   // Context is ExtendedSAMLMessageContext from PAC4J
       logger.debug("SubjectConfirmationData recipient {} does not match SP assertion consumer URL, found",
          data.getRecipient());
       return false;
   }
}

Version 1.7 - SAML2DefaultResponseValidator (slightly modified by me for the sake of logging):

if (data.getRecipient() == null) {
   logger.debug("SubjectConfirmationData recipient cannot be null for Bearer confirmation");
   return false;
} else {
    final Endpoint endpoint = context.getSAMLEndpointContext().getEndpoint(); // Context is SAML2MessageContext from PAC4J, SAMLEndpointContext is from OpenSaml
    if (endpoint == null) {
       logger.warn("No endpoint was found in the SAML endpoint context");
       return false;
   }
   final String url = endpoint.getLocation();

   if (!data.getRecipient().equals(url)) {
       logger.debug("SubjectConfirmationData recipient {} does not match SP assertion consumer URL, found. SP ACS URL from context: {}", data.getRecipient(), url);
       return false;
   }
}

At the moment, I see a few possible solutions:

• Always write port numbers in ACS URLs in the metadata. Unfortunately this is not possible for us because we have already distributed our metadata in the past and partner companies would have to update them at the same time we redeploy our application.
  
• Make the comparison of endpoint URLs insensitive to default port numbers. This is probably the way I will go, at least for now.
• Not check this at all.

Do you also think this could be caused by the transition from pure PAC4J message context to the OpenSaml-base context? (I did not find the place where the value was populated, which was my initial question.)

Thank you in advance for any answer!

Jarda

[Jérôme LELEU]

Hi,

Just to be sure: it worked with pac4j 1.6 and the tricky IdP and it doesn't with pac4j 1.7.

Regardless of the source code, I'm not surprised that the check fails if the urls are different (one with a port, the other one without).

Either we have something wrong in the code or we need to update the SAML support to address this edge case.

Would you mind opening a Github issue for that? The source code is unfortunately the same in pac4j v1.8 and v1.7 so I assume there will be the same problem.

@Misagh: do you have a clue about this issue?

Thanks.

Best regards,

Jérôme

--

[Jaroslav Kačer]

Hi!

Dne pátek 2. října 2015 16:23:25 UTC+2 Jérôme LELEU napsal(a):

Hi,

Just to be sure: it worked with pac4j 1.6 and the tricky IdP and it doesn't with pac4j 1.7.

Yes, I am 99% sure, although I cannot prove it.

The partner company has 2 IdPs with the same code base (at least they say it's identical). We have been using the old one for about 6 months with PAC4J 1.6.0. Last week we started testing against the new one with PAC4J 1.7.1.

Regardless of the source code, I'm not surprised that the check fails if the urls are different (one with a port, the other one without).

Either we have something wrong in the code or we need to update the SAML support to address this edge case.

Would you mind opening a Github issue for that? The source code is unfortunately the same in pac4j v1.8 and v1.7 so I assume there will be the same problem.

Sure, I will open it and post the number here.

 

Thank you very much!

Jarda

[Jérôme LELEU]

You're welcome. If you have a working patch, don't hesitate to submit a pull request, we could use it for the release (1.8.0) before a better solution (1.8.1)...

--

[Jaroslav Kačer]

Hi, here is the GitHub bug:

https://github.com/pac4j/pac4j/issues/299

Yes, I have a temporary fix for that, I'll try to submit a merge request on Monday.

Have a nice weekend,

Jarda

Dne pátek 2. října 2015 17:49:09 UTC+2 Jérôme LELEU napsal(a):

[Isa]

Hello Jérôme,

I have the same problem with pac4j 1.7, except in my case it is the HTTP protocol instead of the ports. Our load balancer accepts HTTPS requests and then passes HTTP requests to the underlying redundant servers that incorporate pac4j. There is too much involved to make the change in the architecture, and it would be significantly less work to change the code.

1) Can we further relax the "standard port" check to also ignore the protocol (i.e. https://foo.com and http://foo.com would be treated as equal)?

2) Can we merge this fix back to 1.7? I'm stuck on that due to an older version of Play Framework.. The original relaxation/fix can be found here.

Thanks,

Isa

[Misagh Moayyed]

My vote would be -1.

 

If you want to make the change locally on your end, that’s fine. If you want pac4j to relax its API to allow you to make that change, that’s also fine and we can review your requirements. But we could never ignore the protocol as you suggest. That is a change/risk you have to take on locally. As framework developers, that would be a big violation of the spec.

 

--

You received this message because you are subscribed to the Google Groups "pac4j-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pac4j-users...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

This email has been sent from a virus-free computer protected by Avast. www.avast.com

[Jaroslav Kačer]

Hi Isa!

We use a very similar architecture and it works fine.

We did just three things:

1. We switched HTTP off completely on the load balancer, so only HTTPS is available.
   
2. The SAML metadata always contain URL with HTTPS, never HTTP.
   
3. On Tomcat, we configured the HTTP connector to pretend it is secure. There is an attribute on the <Connector/> element for that. This is specific to the servlet container user, so it may or may not be required/allowed in your case.
   

Is this doable in your case? If it is, you should fine.

Best Regards,

   Jarda

Dne úterý 5. ledna 2016 22:15:23 UTC+1 Isa napsal(a):

[Isa]

Thanks Jarda and Misagh!

Okay so Jarda's approach worked for me, for the most part.. Since we are on play-pac4j 1.2 and the older scala 2.2.4 which does not offer a getScheme() or isSecure() on the HTTP request type, the older version of play-pac4j had hardcoded the scheme getters to return "http" in JavaWebContext and ScalaWebContext. I ended up creating new JARs for play-pac4j_java and play-pac4j_scala that default to "https" instead of "http", and everything seems to be working fine now. This change is only needed for the older version of play-pac4j as the newer version (I checked v1.4) does take the isSecure() request attribute into consideration and properly returns scheme = "https".

Thanks guys for all your help.

Isa

[Jaroslav Kačer]

Nice to hear it's working for you :-)

Best Regards,

   Jarda

Dne čtvrtek 7. ledna 2016 3:53:27 UTC+1 Isa napsal(a):