Bug #69673
Invalid ProxyRestriction generated when Count is set
0%
Description
Lasso 2.8.0 generates invalid AuthnRequests when a ProxyRestriction element is added with a set Count
...
<saml:Conditions>
<saml:ProxyRestriction>
<saml:Audience>http://test.example.com/saml</saml:Audience>
<saml:Count>0</saml:Count>
</saml:ProxyRestriction>
</saml:Conditions>
...
This XML is invalid per the SAML specification (and is rejected as such by Onelogin's samltool.com, keycloak, and any implementation that checks the SAML XML schema
In http://docs.oasis-open.org/security/saml/v2.0/saml-schema-assertion-2.0.xsd, Count is defined as an attribute of ProxyRestriction, and not a subelement, the correct result should be:
<saml:Conditions>
<saml:ProxyRestriction Count="0">
<saml:Audience>http://test.example.com/saml</saml:Audience>
</saml:ProxyRestriction>
</saml:Conditions>
This feature (ProxyRestriction) is used by LemonLDAP::NG (which relies on Lasso) and causes compatibility issues with some SAML IDPs
I have attached a C test case to show the issue
Files
Associated revisions
History
Updated by Benjamin Dauvergne about 1 year ago
Great catch. Would you provide a patch to fix it ?
Updated by Benjamin Dauvergne about 1 year ago
- Status changed from Nouveau to Information nécessaire
- Assignee set to Maxime Besson
Updated by Maxime Besson about 1 year ago
The attached patch solves my use case but I'm not sure if anything else needs to be done in the lib.
Perhaps a release note should mention the fact that Lasso systems connected to each other should be upgraded at the same time to avoid errors when receiving incorrect messages from an older version?
Updated by Benjamin Dauvergne about 1 year ago
Maxime Besson a écrit :
The attached patch solves my use case but I'm not sure if anything else needs to be done in the lib.
Perhaps a release note should mention the fact that Lasso systems connected to each other should be upgraded at the same time to avoid errors when receiving incorrect messages from an older version?
Of course, I'll add it but I'm pretty sure you are the sole users of this SAML feature :-) For my education, what is the use case which mandates the use of this ProxyRestriction and with which software on the other party do you use it ? Or is it used solely among LemondLDAP instances ?
Updated by Benjamin Dauvergne about 1 year ago
- Tracker changed from Support to Bug
- Status changed from Information nécessaire to Fermé
- Target version set to 2.8.1
commit 3a7ad3610f64cfa4a231fb543712371c1828e5e4 (HEAD -> main, origin/main, origin/HEAD)
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Wed Sep 28 18:18:36 2022 +0200
Fix parsing of Count attribute of saml:ProxyRestriction (#69673)
Updated by Maxime Besson about 1 year ago
Benjamin Dauvergne a écrit :
Of course, I'll add it but I'm pretty sure you are the sole users of this SAML feature :-) For my education, what is the use case which mandates the use of this ProxyRestriction and with which software on the other party do you use it ? Or is it used solely among LemondLDAP instances ?
LemonLDAP sets a ProxyRestriction by default when sending an AuthnRequest, the intended purpose of this is to prevent the IDP from using another IDP to authenticate the user. However, after reading the spec carefully, I think this is not what ProxyCondition is for.
ProxyCondition prevents a SP receiving an assertion (LemonLDAP) from using it on another service (such as a LemonLDAP-protected app) which is what we do in every case. So I think we will end up removing it.
This patch however might help with old LLNG versions, and makes Lasso more spec-compliant, so thanks for including it!
Fix parsing of Count attribute of saml:ProxyRestriction (#69673)