IdentityPython / SATOSA

Proxy translating between different authentication protocols (SAML2, OpenID Connect and OAuth2)
https://idpy.org
Apache License 2.0
200 stars 123 forks source link

Metadata Signature identifier <ns1:Signature Id=“Signature1”> not working well #185

Open surfnet-niels opened 6 years ago

surfnet-niels commented 6 years ago

When SatoSa generates metadata for either front or backend saml plugins, the signature identifier is generated as

While the value is strictly speaking not incorrect (a unique string is required), the uniqueness is rather poor and some products, including pyFF MDX which is commonly used together with break if a duplicate signature value is in metadata of multiple entities (like say, a frontend and a backend metadata of the same proxy)

Code Version

SaToSa 3.4.8

Expected Behavior

Problem a signature with a bit more randomness should be used

Current Behavior

While the value is strickly speaking not incorrect (a unique string is required), the uniqueness is rather poor and some products, including pyFF break if a duplicate signature value is in metadata of multiple entities (like say, a frontend and a backend metadata of the same proxy)

Possible Solution

Problem a signature with a bit more randomness should be used OR can we remove this entirely, as it does not seem to be used anyway?

Steps to Reproduce

Configure a saml front or backend. Generate metadata

c00kiemon5ter commented 6 years ago

Hi and thanks for filling this issue with details. This is a behaviour that SATOSA does not control; it is dictated by pysaml2. (Maybe we should move the issue there.)

a signature with a bit more randomness should be used

This should be easy to do and I already have it on my TODO list. pysaml2 hardcodes the identifier in many places, and the values are 1 and 2. Search for usages of pre_signature_part where the third argument is the signature-id that becomes part of the formatted string Signature%d (which also means that the id is only allowed to be digits).

can we remove this entirely, as it does not seem to be used anyway?

Quoting the SAML2-metadata spec Section 3.1.2 References:

Signed metadata elements MUST supply a value for the identifier attribute on the signed element. The element may or may not be the root element of the actual XML document containing the signed metadata element. Signatures MUST contain a single containing a URI reference to the identifier attribute value of the metadata element being signed. For example, if the identifier attribute value is "foo", then the URI attribute in the element MUST be "#foo".

(where the identifier attribute refers to the xs:ID attribute)

I don't think we can remove it entirely - but it can certainly be improved.