[keycloak-dev] Keycloak's SAML AuthnResponse uses wrong binding

[Bill Burke]

John, to be honest, you can go on and on if you want, but I view this as 
a low priority and I don't agree with half of your assumptions and I 
just don't have the time to argue with you.  If you want to open some 
jira issues to describe in full your concrens feel free.  If you want to 
provide a patch even better, but we have a ton of more important issues 
in the queue that customers actually care about.  Not one person other 
than you has complained about how we handle importing SP metadata.  
We're a small team.  We have to prioritize resources.  Sorry.

On 4/21/2016 8:02 PM, John Dennis wrote:
> On 04/16/2016 02:39 PM, Bill Burke wrote:
>
> Sorry for the delay in responding, I was out on PTO.
>
>> On 4/16/2016 11:23 AM, John Dennis wrote:
>>> On 04/16/2016 10:04 AM, Bill Burke wrote:
>>>>>> I also noticed that when I used the Web UI to examine the SP 
>>>>>> metadata
>>>>>> (Installation tab of the realm client, selecting the "SAML Metadata
>>>>>> SPSSODescriptor" format) that it did not match the SP metadata that
>>>>>> had been loaded using the client registration service. Not only 
>>>>>> wasn't
>>>>>> it the exact same metadata, but specifically it was missing 
>>>>>> several of
>>>>>> the endpoints the SP declared in it's metadata. Why isn't the 
>>>>>> metadata
>>>>>> the same and why did Keycloak drop essential endpoint/binding
>>>>>> information?
>>>
>>>> Its not going to store the exact SP metadata file that was imported.
>>>> Keycloak only imports SP metadata that Keycloak.  Also, the admin
>>>> console is allowed to modify this imported metadata however it 
>>>> wants. I
>>>> don't see a problem with this behavior at all.
>>>
>>> Here is the issue. I'm an admin and I register my SP with Keycloak.
>>> But either because things are not working as expected or because I
>>> simply can't remember if I updated the SP metadata in the Keycloak IdP
>>> after I made a change in the SP at my end I decide to go the the
>>> Keycloak Web UI and check and see what Keycloak thinks my SP 
>>> metadata is.
>>>
>>> Instead of seeing what I believe my SP metadata should be I see
>>> something completely different and that is a red flag. I wonder why
>>> the metadata I load is not the same metadata Keycloak is operating on.
>>> Is this the source of my problems?
>>>
>>> In particular I'm concerned about Keycloak dropping my SP's SAML
>>> endpoints.
>>>
>>
>> I don't think your concern is valid or a red flag.
>
> Let me try to explain why an IdP modifying an SP's metadata is a concern.
>
> As an illustration of the problem I've attached the SP metadata I 
> loaded into Keycloak (myapp_sp_metadata.xml) and what Keycloak reports 
> as the SP's metadata when queried from the client installation tab in 
> the "SAML Metadata SPSSODescriptor" format. Here are where the 
> metadata differs between the actual SP metadata and Keycloak's version 
> of the SP metadata:
>
> 1) Keycloak has added a protocolSupportEnumeration for 
> urn:oasis:names:tc:SAML:1.1:protocol which the SP never reported 
> support for.
>
> 2) Keycloak has dropped the x509 encryption information.
>
> 3) Keycloak has dropped the SingleLogoutService using the 
> HTTP-Redirect binding.
>
> 4) Keycloak has dropped the AssertionConsumerService using the 
> HTTP-Artifact binding.
>
> 5) Keycloak has dropped the AssertionConsumerService using the PAOS 
> binding.
>

The metadata keycloak produces is the picture of what

> This is clearly not what the SP reported to the IdP and I continue to 
> believe this is a problem. IdP's are not permitted to alter the SP 
> configuration information, rather an IdP is supposed to use the SP 
> information verbatim (provided the IdP trusts the metadata).
>
> It is common practice in SAML for admins to examine metadata. Why? 
> Because most SAML problems are the result of incorrect metadata. When 
> things aren't working as expected the first course of action should be 
> to check the loaded metadata because the metadata drives the entire 
> relationship between entities. A mistake in the metadata will 
> certainly throw things into the weeds. Something not working? Check 
> the metadata! If the IdP reports metadata different than provided by 
> the SP then that absolutely will raise red flags.
>
>> Please correct me if
>> I'm wrong, but there is absolutely nothing within the SP metadata file
>> that defines the preferred way the client wants to communicate if there
>> are multiple options defined.
>
> Actually the SP does indicate it's default AssertionConsumerService, 
> it's desire for signed responses, encrypted responses, etc. in it's 
> metadata. The IdP is supposed to honor them if it's able.
>
>> Also, it is usually the IDP side that
>> drives these preferences, not the client.
>
> The IdP must respect what the SP reported.
>

It does respect what was reported, it ju

> Let me give another example. Some SAML bindings require the 
> Destination attribute. But if the IdP has thrown away the binding 
> endpoint for an SP how can it set the Destination attribute?
>
> Also the IdP is supposed to validate the AssertionConsumerServiceURL 
> against the value supplied in the SP metadata.
>
>>
>>> I think what you're implying is that Keycloak will parse the SP
>>> metadata and retain only those parts of it which it currently
>>> understands, it then stores the data it picked out of the metadata in
>>> persistent storage and when asked to display the metadata Keycloak
>>> will read it's internal datastore and format it as SAML metadata.
>>> Correct?
>>>
>>
>> No, it parses the SP metadata file and decides how it is going to
>> interact with the SP based on best practices, i.e. POST over 
>> Redirect, etc.
>>
>>> If so what happens when Keycloak grows support for other SAML
>>> features? Are you then going to require every SP to re-register
>>> themselves so that Keycloak can parse the metadata again and rebuild
>>> it's internal data structures? Shouldn't after a Keycloak upgrade
>>> things just work?
>>>
>> If it works before Keycloak is upgraded, why wouldn't it work after?
>
> It's not a question of features no longer working, but rather the 
> inability of new features to work after an upgrade without requiring 
> the all SP's to register once again so that Keycloak can read 
> information which is now necessary but was formerly discarded [1].
>
> Simple. Let's use encryption as an example. I'm guessing Keycloak does 
> not currently support receiving encrypted requests because Keycloak 
> has deleted the encryption information from the SP. Now lets say in a 
> later release Keycloak supports encrypted requests. How are you going 
> to decrypt the request if you didn't preserve the encryption 
> parameters the SP provided to you in it's metadata? Are you going to 
> require every SP to register themselves with Keycloak again? The same 
> holds true for other features.
>
> [1] I'm still working on the assumption Keycloak does not preserve the 
> SP metadata verbatim but rather parses it and stores processed data 
> instead. The SAML IdP's I'm most familiar with (Shibboleth V3 and 
> Ipsilon) store the metadata and parse the stored metadata each time an 
> entity is loaded. This allows them to accurately report the metadata 
> and respond to bug fixes and feature upgrades when a new software 
> version is installed because it's working from the canonical source 
> description of the entity (e.g. the metadata).
>
>> I really don't see the admin wanting to switch to Artifact if Keycloak
>> ever ends up supporting it.    Also, IMO, it is just as likely that the
>> admin defines the client in the admin console and exports the SP
>> metadata to be consumed by mod-auth-mellon, than for somebody to craft
>> an SP metadata file and import it into Keycloak.
>
> Keycloak should never be in the business of generating an SP's metadata.
>
> How can an IdP possibly know anything about an SP? IdP's cannot define 
> what an SP is or how it is configured. Suggesting an IdP can know a 
> prori how an SP is configured or what it's capabilities are is way 
> outside normal SAML practice and is fundamentally backwards. If IdP's 
> were capable of knowing enough about a SP such that it could generate 
> the SP's metadata then why would metadata exchange even exist? 
> Metadata and metadata exchange is a fundamental building block of SAML 
> with it's own specification and exists for a reason.
>
> Let me give a concrete example of why your suggestion that Keycloak 
> would generate the metadata for a mellon SP is flawed. Up until 
> recently mellon did not support ECP. ECP requires the SP export an 
> AssertionConsumerService with a PAOS binding. So how is Keycloak going 
> to generate the correct metadata for a mellon SP if it doesn't know 
> what the version of mellon is and the versions of the libraries that 
> mellon links with on the SP? Those are critical components of what 
> mellon's capabilities are. So how is Keycloak as an IdP which is 
> completely independent of an SP going to generate correct metadata? 
> It's completely backwards to normal SAML. IdP's don't generate SP 
> metadata, rather IdP's consume SP metadata. How is an IdP going to 
> know an SP's security requirements? (e.g. signed responses, encrypted 
> responses, validity period, etc.) How is an IdP going to know what the 
> default service endpoint is or the correct ordering of endpoints? 
> Don't forget that an SP may communicate with multiple IdP's thus the 
> SP metadata must be valid across a set of IdP's (the other IdP's will 
> be unknown to Keycloak).
>
>> It really depends if
>> the admin is migrating from an existing SSO solution to Keycloak or not.
>>
>> IMO, what the bug fixes should be are:
>> * Make sure "Force POST Binding" is on by default (High Priority)
>> * Don't support an import unless there are valid bindings.  i.e. if only
>> artifact is listed, then abort import with an error. (High priority)
>> * Add support for ACSI and ACSU (low priority)
>> * Add support for Artifact Binding (very low priority, to Won't 
>> Implement)
>>
>>
>>> So there are 2 reasons why the entire contents of original metadata
>>> should be preserved:
>>>
>>> 1) Diagnostics, debugging, etc.
>>>
>>> 2) Supporting upgrades.
>>>
>> I completely disagree that the entire contents need to be preserved. And
>> I don't see what a Keycloak upgrade has to do with anything.
>>
>>
>>>
>>>
>>
>
>

-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com