Skip to content

Conversation

@timlegge
Copy link
Contributor

@timlegge timlegge commented Aug 9, 2022

After discussion I figured we should add this as an addition since there may be use cases where something in-line (lighttpd) for instance incorrectly "normalizes" the URL and there is no simple workaround in the in-line change. The woraround is fairly simple and we can lose the sls_double_encoded_response option as the only known IdP was Ping and they fixed the issue after I reported it.

@timlegge timlegge requested a review from waterkip August 9, 2022 22:34
@waterkip
Copy link
Collaborator

I'm conflicted tbh. I think we should be correct and accept the URI as is with no special logic to transfor the URI and let the caller mess with that. However RFC1 say the URI must be normalized so uppercasing %2f to %2F is correct behavior. Can we name this boolean: microsoft-is-being-wrong-for-lowercasing-the-encoded-uri-before-signing-it. We should probably address it in the IdP where we need to know the type so based on the type we can lowercase it for the user.

Footnotes

  1. https://www.rfc-editor.org/rfc/rfc3986#section-6.2.2.1

@timlegge
Copy link
Contributor Author

I was as well - however https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf line 620

Further, note that URL-encoding is not canonical; that is, there are multiple legal encodings for a given
value. The relying party MUST therefore perform the verification step using the original URL-encoded
values it received on the query string. It is not sufficient to re-encode the parameters after they have been
processed by software because the resulting encoding may not match the signer's encoding.

That being said, we ARE processing the "query string" as received. However, there is the possibility that something in line - such as lighttpd can and does modify the query string by normalizing that URI. Continuing to have the option to deal with that without having to change the http server would be useful.

@waterkip
Copy link
Collaborator

waterkip commented Aug 10, 2022

Yes, the SAML spec and the URI spec are contradicting each other. Mixed case , eg %2f and %2F is not something we can detect nor account for unless we try each combination possible when we get a signature failure.

We've now hit two implementations who both claim to be correct and we are in the middle of it. TBF, I'm more on the side of MS which signs the lower case variant and think lighttpd should expose the original URI. This allows for mixed case situations.

My stand point is that the integration that uses Lighttp with normalization and uses MS with the lower casing should either deal with it by 1) changing MS or 2) changing Lighttpd or 3) implement a work around in their code. Net::SAML2, with #85, can deal with upper, lower and mixed case without the need to tell it anything provided the original URL encoded is given (as defined in the SAML spec).

@timlegge timlegge merged commit 5185a1a into perl-net-saml2:master Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants