proposal/discussion: OAuth - separate requirement for redirect_uri string-match registration and handling

[elarlang]

spin-off from https://github.com/OWASP/ASVS/issues/1916 "Discussion/Proposal 3"

Probably I was the first one to say that redirect_uri validation is a duplicate of general open-redirect but now I think it's important to have them as a separate requirement:

redirect_uri must be validated with the string-match method, which means no "wildcard" validations.

There are 2 parts:

• Authorization Server must not accept anything else than (one of) the precise URL from the pre-registered list
• As a precondition: the OAuth Client must not send business logic URL to the Authorization Server. It is pretty much the same as Referrer leakage.

-- Feedback from @tghosth in https://github.com/OWASP/ASVS/issues/1916#issuecomment-2024985876

Suggest you propose an updated/added requirement.

[jmanico]

Probably I was the first one to say that redirect_uri validation is a duplicate of general open-redirect but now I think it's important to have them as a separate requirement:

I agree here, Elar. FYI: Redirect flows (login, etc) are often less likely to be amendable to allow-list validation than OAuth flows.

[csfreak92]

@elarlang, is this being addressed by my latest PR #1971? Or am I missing something about it?

[elarlang]

Hi @csfreak92 , let's find the agreement first in the issue and do PR. Discussion over PR's its complicated to follow.

We also need to think, should we have one common requirement for OAuth and OIDC or not.

I prefer requirement text with the idea like:

Verify that Authorisation Server accepts the redirect uri value from the Client that belongs to the pre-registered list of allowed values using the string-match method, e.g. wildcards are not in use.

[csfreak92]

Understood, but I just pushed my PR since it has been sitting in my local for a while and I thought better to have it out there than get lost somewhere as I have worked on it the past few months. :)

Can we agree over discussion and if they are already covered in the PR then that's good and then if not or needs some modifications, then I would modify them as needed?

[csfreak92]

Verify that Authorization Server accepts the redirect URI value from the Client that belongs to the pre-registered list of allowed values using the string-match method, e.g. wildcards are not in use.

@elarlang, I like this text though, sorry missed it. How would a pre-registered list of allowed values be handled? Should we add a text in the chapter/section how this should be done? Also, string-match method seems like regex, right?

[elarlang]

How would a pre-registered list of allowed values be handled? Should we add a text in the chapter/section how this should be done?

I don't think we need to provide guidance for OAuth Client configuration.

Also, string-match method seems like regex, right?

No, it's the opposite. string-match against pre-registered list of full values says that you should not use any regex, substring, wildcard, etc. Provided redirect_uri value must exists in the pre-registered addresses list as it is.

[TobiasAhnoff]

Based on OAuth2 RFCs (where the redirect uri is mentioned on several places), OIDC FAPI 2.0 and own experiences from pentests, I suggest that ASVS should capture that redirect URIs must be:

• Exact and pre-registered (for each client), see https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps#section-6.3.3.2.1
• Absolute, see https://www.rfc-editor.org/rfc/rfc6749#section-3.1.2
• https (if not local loopback for native clients), see https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html#section-5.3.2.2
• Fully validated using "simple" string comparison methods (no wildcards, starts-with or regexps etc), see https://www.rfc-editor.org/rfc/rfc6749#section-3.1.2.3

Perhaps write this as "Verify that redirect uri:s in authorization requests are pre-registerd, absolute, https (if not local loopback for native clients) and validated (by AS) using simple exact string comparison methods (no wildcards, reg exps or e g contains operations)"

Note that if using PAR, pre-registered uri:s are no longer mandatory (given confidential clients that first do client authentication as part of PAR, see https://datatracker.ietf.org/doc/html/rfc9126#section-2.4), but maybe that is too much details for ASVS? A good discussion from a NDC Security talk on this is found here https://youtu.be/IZyXOYNXbPk?t=2138

As a side note, which shows things that can go wrong when being relaxed about redirect uri:s, a colleague of mine did some research on redirect issues in Keycloak during a pentest. A write up (as part of responsible disclosure) can be found at https://securityblog.omegapoint.se/en/writeup-keycloak-cve-2023-6927/ and we presented this at NDC Security as part of a longer OAuth talk, which can be found here https://youtu.be/dj02Cr9S1FE

[jmanico]

I agree 100%

[elarlang]

Latest proposal from @TobiasAhnoff (acronym "AS" avoided):

Verify that redirect uri:s in authorization requests are pre-registerd, absolute, https (if not local loopback for native clients) and validated by the authorization server using simple exact string comparison methods (no wildcards, reg exps or e g contains operations)

Are we ready to go with that or any errors or modifications needed? (We can do changes later as well)

[csfreak92]

Are we ready to go with that or any errors or modifications needed? (We can do changes later as well)

I like it! Just to wordsmith and fix the capitalization/spacing, etc just for consistency with the other OAuth 2.0 language and make it ready for anything:

Verify that redirect URIs in authorization requests are pre-registered, absolute, sent over HTTPS (if not local loopback for native clients) and validated by the Authorization Server using simple exact string comparison methods (i.e, no wildcards, starts-with, regular expressions or contain string operations)

How does that look @TobiasAhnoff, @elarlang?

[randomstuff]

sent over HTTPS

"using the HTTPS scheme"?

[jmanico]

If you're using a strict allow-list then worrying about local IP addresses is not necessary. I suggest:

Verify that redirect URIs in authorization requests are pre-registered, absolute, sent over HTTPS, and validated by the Authorization Server using an exact string comparison based allow list. (i.e., no wildcards, starts-with, regular expressions, or contains operations).

[TobiasAhnoff]

I like exact string comparison based allow list, more clear how to implement/verify than simple exact string comparison methods, which raises the question on what "simple" is? (even if this is described in RFC).

To align with RFC for Native apps I think a note on "native client loopback" is needed, perhaps put in another way.

I also like using the HTTPS scheme, sent over HTTPS works as well but it might be unclear if the redirect URI must be HTTPS or if the authorization request must be sent over HTTPS or both.

A suggestion is:

Verify that redirect URIs in authorization requests are pre-registered, absolute, using the HTTPS scheme, and validated by the Authorization Server using an exact string comparison based allow list. (i.e., no wildcards, regular expressions, starts-with or contains operations). Note that for native clients (mobile apps) using the HTTPS scheme for loopback interfaces is preferred (not required).

Perhaps consider writing "avoid using wildcards etc" instead of "no wildcards etc"? Is "no" too strict for some scenarios? Perhaps, but "avoid" would be harder to verify...I believe "no" is good to make the point that wildcards, regular expressions, starts-with and contains operations introduce risk (could not be considered "simple"), which sometimes leads to vulnerabilities.

[jmanico]

I suggest dropping the avoid section and just focus on the positive. Perhaps:

Verify that redirect URIs in authorization requests are pre-registered, absolute, using the HTTPS scheme, and validated by the Authorization Server using an exact string comparison based allow list.

[TobiasAhnoff]

I suggest dropping the avoid section and just focus on the positive. Perhaps:

Verify that redirect URIs in authorization requests are pre-registered, absolute, using the HTTPS scheme, and validated by the Authorization Server using an exact string comparison based allow list.

I agree, short and clear, and for mobile app scenarios the RFC will explain that http loopback URIs are ok to use if https is not supported (too much details for ASVS, perhaps it could be considered for MASVS?).

I´m not sure if this is better, but perhaps it is good to also make it clear that the allow list need to be per client?

Verify that redirect URIs in authorization requests are absolute, using the HTTPS scheme, and validated by the Authorization Server using exact string comparison based on a client specific allow list of pre-registered URIs.

[jmanico]

I like your addition - very clear. I think we are close to a PR.

[csfreak92]

Verify that redirect URIs in authorization requests are absolute, using the HTTPS scheme, and validated by the Authorization Server using exact string comparison based on a client specific allow list of pre-registered URIs.

I concur with this @TobiasAhnoff and @jmanico. Let's wait for the community and then we will create a PR once agreed on.

[elarlang]

In practice, there is no other community to wait for in the issue as already presented in the issue. It catches more attention when included in the document.

It's PR time, as a new requirement to section 50.1.

[jmanico]

I'll submit a PR now.

[elarlang]

re-open due the feedback https://github.com/OWASP/ASVS/issues/2041#issuecomment-2341855336 by @randomstuff

Should this allow the usage of http://127.0.0.1:{port}/{path} and http://[::1]:{port}/{path}?

Also, should there be a discussion about private-Use URIs? (Is this in scope?)

[elarlang]

Current requirement:

#	Description	L1	L2	L3
51.1.7	[ADDED] Verify that redirect URIs in authorization requests are absolute, using the HTTPS scheme, and validated by the Authorization Server using exact string comparison based on a client-specific allow list of pre-registered URIs.	✓	✓	✓

Is it HTTPS or not, is not in the scope for this requirement, it is covered by

V9.1.1 [MODIFIED] Verify that TLS is used for all connectivity between a client and external facing, HTTP-based services, and does not fall back to insecure or unencrypted communications.

The goal for 51.1.7 is to have string-match validation against pre-registered list of URL's. To avoid further confusions, I propose / agree with proposals, to remove the HTTPS part from the requirement.

I would just use:

Verify that the Authorization Server validates redirect URIs based on a client-specific allow list of pre-registered URIs using exact string comparison.