Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth, Add Requirement about protection against modification of the RAR authorization_details parameter #2151

Closed
randomstuff opened this issue Oct 15, 2024 · 16 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@randomstuff
Copy link
Contributor

randomstuff commented Oct 15, 2024

Should be add a requirement about the risk of manipulation of the RAR authorization_details parameter?

See the security considerations from the RAR specification (emphasis mine):

The authorization_details parameter is sent through the user agent in case of an OAuth authorization request, which makes them vulnerable to modifications by the user. If the integrity of the authorization_details is a concern, clients MUST protect authorization_details against tampering and swapping. This can be achieved by signing the request using signed request objects as defined in [RFC9101] or using the request_uri authorization request parameter as defined in [RFC9101] in conjunction with [RFC9126] to pass the URI of the request object to the AS.

i.e. when this is a concern, JAR or PAR can be used.

Straw man wording:

Verify that the user cannot tamper with RAR requests. This can be achieved with PAR or JAR/JWS.

Argument against: This type of requirement can be generalized to other requests (outside of OAuth) which is one reason for not including this requirement in the OAuth chapter. A more general requirement could be used instead in V5. Are the requirements already in V5 enough?

Context: #2036

@elarlang
Copy link
Collaborator

In case we have a general requirement to cover that, we don't need to duplicate it into V51.

At the moment I don't have match to provide from existing requirement for this.

Corret me if I'm wrong, but the requirement feels quite a niche and edge case. If rar is used...

But when watching the RFC9396, it seems like another "approved by RFC" way to leak sensitive information (the content of authorization_details) via URL parameters.

@elarlang
Copy link
Collaborator

@randomstuff - is this again "if public client && rar then jar"?

If yes, I really would like to make a requirement "do not use public client" for L2, L3 (#1963).

@randomstuff
Copy link
Contributor Author

"if public client && rar then jar"?

@elarlang, The goal here is to protect against modification from the user. I don't believe it makes much sense to protect the authorization request from the user when we have a public client. For public client, the client is expected to be some application running on the user device so we can't really prevent the user from messing it up or forging it.

So my proposition should probably be completed as:

For server-side clients, verify that the user cannot tamper with RAR requests. This can be achieved with PAR or JAR/JWS.

@elarlang
Copy link
Collaborator

  • The requirement should start with the "Verify that".
  • "server-side" is some new term here, is it meant "confidential client" and if not, what is the difference?
  • "PAR requests" == "pushed authorization request request"

Without "server-side", is this close to the meaning?

Verify that the authorization server protects rich authorization request (RAR) integrity (that the end-user can not tamper the request) by using pushed authorization request (PAR) or JWT-secured authorization request (JAR) with JSON Web Signature (JWS).

@elarlang elarlang added the 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet label Oct 20, 2024
@randomstuff
Copy link
Contributor Author

randomstuff commented Oct 20, 2024

My intent was to talk about backend-side clients i.e. clients which do not execute on the user device but on a backend server:

  • For clients which executes on the user device, this requirement does not make much sense because we cannot prevent the user from tampering with the authorization request.
  • For clients which executes on a backend server, the backend application might want to prevent the user from tampering with the authorization_details.

These two categories are somewhat related with the OAuth concept of public vs confidential clients but clients which executes on the user device can be confidential (with dynamic client registration).

So something like:

Verify that the user cannot tamper with rich authorization request (RAR) authorization_details if the client is not executed on a user device. This can for example be achieved using PAR.

@elarlang
Copy link
Collaborator

The requirement should be positive - what must be achieved. (That is why my proposal in #2151 (comment) was worded differently)

Previously "This can be achieved with PAR or JAR/JWS.", in the last one "This can for example be achieved using PAR.".

@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Oct 22, 2024
@randomstuff
Copy link
Contributor Author

randomstuff commented Oct 26, 2024

Verify that the user cannot tamper with rich authorization request (RAR) authorization_details if the client is not executed on a user device. This can for example be achieved using PAR.

The requirement should be positive

Note that we currently have from #1964:

Verify that for a given client, an attacker may not tamper with the 'response_mode' parameter, for example by having the authorization server validate this value against the expected values or by using pushed authorization request (PAR) or JWT-secured authorization request (JAR).

which is similarly worded?

@randomstuff
Copy link
Contributor Author

Actually this topic is quite similar to #1964 and we could use the same wording and mitigation for both.

@randomstuff
Copy link
Contributor Author

randomstuff commented Oct 26, 2024

Using the same wording as for #1964:

Verify that the user may not tamper with the 'authorization_details' parameter for clients which are not executed on the user device, for example by using pushed authorization request (PAR) or JWT-secured authorization request (JAR).

I'll sync the wording with whatever comes out from #1964 (if applicable).

@TobiasAhnoff
Copy link

I think this is in sync with #1964 but a little hard to read, perhaps this is more clear?

For backend clients (which are not executed on the user device), verify that the user may not tamper with the 'authorization_details' parameter. For example by using pushed authorization request (PAR) or JWT-secured authorization request (JAR).

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Nov 6, 2024
@elarlang
Copy link
Collaborator

elarlang commented Nov 8, 2024

We updated the other requirement (I think), we should come up with positive-wording requirement here as well.

@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos and removed 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR 4) proposal for review Issue contains clear proposal for add/change something labels Nov 9, 2024
@randomstuff
Copy link
Contributor Author

I am coming up with:

Verify that for a given server-side client (which is not executed on the user device), the authorization server enforces that the 'authorization_details' parameter is not chosen by the user but by the client backend. For example by requiring the usage of pushed authorization request (PAR) or JWT-secured authorization request (JAR).

but this can certainly be improved.

@elarlang
Copy link
Collaborator

Some moving around and changes:

Verify that for a server-side client (which is not executed on the user device), the authorization server enforces that the 'authorization_details' parameter value is from the client backend and not tampered by the user. For example by requiring the usage of pushed authorization request (PAR) or JWT-secured authorization request (JAR).

Question as well: "user device" > "end-user device", "user-controlled device"?

@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos labels Nov 16, 2024
@elarlang
Copy link
Collaborator

ping @randomstuff @TobiasAhnoff

@randomstuff
Copy link
Contributor Author

Yes maybe, "end-user device".

@elarlang
Copy link
Collaborator

elarlang commented Nov 18, 2024

Goes in via #2383:

# Description L1 L2 L3
51.2.15 [ADDED] Verify that for a server-side client (which is not executed on the end-user device), the authorization server enforces that the 'authorization_details' parameter value is from the client backend and not tampered by the user. For example by requiring the usage of pushed authorization request (PAR) or JWT-secured authorization request (JAR).

elarlang pushed a commit to elarlang/ASVS that referenced this issue Nov 18, 2024
@elarlang elarlang mentioned this issue Nov 18, 2024
@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Nov 18, 2024
elarlang pushed a commit that referenced this issue Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

4 participants