redirect_uri matching does not follow RFC3986
jgresty opened this issue · 2 comments
Preflight checklist
- I could not find a solution in the existing issues, docs, nor discussions.
- I agree to follow this project's Code of Conduct.
- I have read and am following this repository's Contribution Guidelines.
- This issue affects my Ory Network project.
- I have joined the Ory Community Slack.
- I am signed up to the Ory Security Patch Newsletter.
Describe the bug
From [RFC6749] Section 3.1.2.3
When a redirection URI is included in an authorization request, the
authorization server MUST compare and match the value received
against at least one of the registered redirection URIs (or URI
components) as defined in [RFC3986] Section 6
Fosite seems to be doing an exact string comparison instead:
Line 106 in 45a6785
Reproducing the bug
An easy test is for hostnames, which are case insensitive in [RFC3986] Section 6.2.2.1:
- Create a client with a redirect uri which has a lower case hostname
- Try to authorize with that client, specifying the redirect uri hostname in upper case
eg if a client has a valid redirect uri:
https://example.com
and a request is sent with redirect_uri HTTPS://EXAMPLE.com
, that should be valid.
Relevant log output
No response
Relevant configuration
No response
Version
0.44.0
On which operating system are you observing this issue?
None
In which environment are you deploying?
None
Additional Context
There are far more rules than just hostname case specified in the RFC, I have just included that one as it is the easiest to describe and test.
Simple String Comparison is the current best security practice in OAuth 2.0 and is the requirement in the OpenID Connect 1.0 standard, as well as the upcoming OAuth 2.1 standard it's (likely but it's not published yet) becoming a requirement similar to in OpenID Connect 1.0. As such it's probably "most correct" to specifically look at what Simple String Comparison actually means. Syntax Based Normalization from what I can see is a different string comparison type.
Edit: For these reasons I'd argue that fosite is actually following the spec accurately, and the client implementations should normalize the redirect_uri
prior to making the request or during the configuration provisioning steps. I do however wonder if there is merit in adding a method to the configurator that returns the redirect uri matching func as it may be useful in other scenarios which are not traditionally compliant.
RFC3986 Section 6.2 is the comparison ladder, but neither the OAuth 2.0 Current Best Practices, OAuth 2.1, or OpenID Connect 1.0 mention use of this comparison ladder; just the use of the singular comparison type within the ladder.
Thanks James, I agree with that. It's also mandated by the conformity test suite. Closing