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

V51 OAuth: Add verifications for Authorization Server client configuration #2043

Closed
TobiasAhnoff opened this issue Aug 31, 2024 · 23 comments
Closed
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

@TobiasAhnoff
Copy link

TobiasAhnoff commented Aug 31, 2024

The following verifications are suggested to address least privilege client configuration for the Authorization Server and to follow recommendations from https://oauth.net/2.1/ (not to use ROPC or Implicit flows).

Note that not using the ROPC flow is addressed in 51.1.6, but if the verifications in this issue and in #2041 are added then 51.1.6 is included and could be replaced/removed.

V51.2 Authorization Server

Verify that all clients are configured to only allow the required grant. Avoid reusing clients for different purposes (allowing e g both 'code' and 'client-credentials' for a given client). Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be supported for any client. (L1, L2, L3)

Verify that allowed scopes for each client asserts least privilege. (L1, L2, L3)

@elarlang elarlang added the V51 Group issues related to OAuth label Aug 31, 2024
@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Sep 2, 2024
@randomstuff
Copy link
Contributor

Verify that all clients are configured to only allow the required grant. Avoid reusing clients for different purposes (allowing e g both 'code' and 'client-credentials' for a given client).

I am not sure about this one. Is that really bad?

If the usage of the "client credentials" flow is tightly related to the usage of "code" flow, it might make much sense to only have a single client_id.

Moreover, having separate "FooApp B2B", "FooApp User" client IDs would only clutter the list of registered client (in the AS user interface) and would make it more difficult to deprovision a client application: "I disabled FooApp." ,"Did you disable FooApp B2B as well?"

@TobiasAhnoff
Copy link
Author

The important thing is "least privilege" client configuration, maybe this is better

Verify that all clients are configured to only allow the required grant. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be supported for any client. (L1, L2, L3)

I agree that that the "Avoid reusing clients for different purposes (allowing e g both 'code' and 'client-credentials' for a given client)"-remark makes sense in some scenarios, but in other contexts it does not.

The thing I wanted to point out with that remark was that combining grants (code and client-credentials) in one client (to limit No of clients) might give users access to scopes (if not using PAR) that only system integrations should be granted, which is problematic if authorization is based on just a valid token and scopes...

But that is in a way addressed by

Verify that allowed scopes for each client asserts least privilege. (L1, L2, L3)

Maybe modify that to

Verify that allowed scopes for each client asserts least privilege for any allowed client grant. (L1, L2, L3)

@jmanico
Copy link
Member

jmanico commented Sep 7, 2024 via email

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Sep 8, 2024

I believe both aspects are important for limiting attack vectors, the reason for not suggesting a separate AS verification like

Verify that AS does not support Implicit or ROPC flow (L2, L3)

is that many AS supports this today and it is unlikely to change in the near future, so all you can do as an application developer is to assert that your client configuration does not allow for using those flows. In example, if multiple flows are allowed, an attacker could by simply adding "token" to the "id-token" or "code" authorization-request or change it to "password", extract an access-token or initiate ROPC (PAR mitigates this)

I agree that ASVS should make it clear that AS should no longer support Implicit and ROPC flows, perhaps this could be expressed better than in the note?

@elarlang
Copy link
Collaborator

elarlang commented Sep 13, 2024

Proposed it in #2048 (comment) but I thik it belongs here:

Verify that OAuth / OIDC Client ID is unique and used only by the client.

(requires wordsmithing)

edit: I think it is already discussed in

@elarlang
Copy link
Collaborator

Let's move on with those:

Verify that all clients are configured to only allow the required grant. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be supported for any client.

Verify that allowed scopes for each client asserts least privilege.

@TobiasAhnoff
Copy link
Author

Verify that OAuth / OIDC Client ID is unique and used only by the client.

@elarlang I read this as "Verify that a given (unique) OAuth/OIDC client is used according to least privilege, for a single application scope". Is that correct? Then I think it belongs to the Client section more than the Authorization Server section.

@elarlang
Copy link
Collaborator

elarlang commented Sep 16, 2024

I would not put least privilege topic into this, just unique.

The client only uses what is configured for it and can not control, how it is configured. In my opinion it is authorization server responsibility to configure it correctly.

edit: my context is 1st party solutions, it may be a different need when clients can register themselves on 3rd party solutions.

@elarlang
Copy link
Collaborator

elarlang commented Sep 17, 2024

A bit update to this one:

Verify that the client can only use the required grant. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be supported for any client.

(note for myself: it replaces 51.1.6 (and the number is maybe changed for that time)

This requirement is a bit vague. By principle I can understand that you should not have more permissions than you need, but how do I test it?

Verify that allowed scopes for each client asserts least privilege.

usual ping @randomstuff @TobiasAhnoff :)

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Sep 17, 2024
@randomstuff
Copy link
Contributor

Verify that the client can only use the required grant.

“required” in this sentence feels a bit weird to me. Is that clear enough? Maybe something like: “Verify that for a given client, the authorization server only allows the usage of grants that this client actually needs to use."

@elarlang
Copy link
Collaborator

Verify that for a given client, the authorization server only allows the usage of grants that this client actually needs to use.

I think we somehow need to send clear message, about password grant etc.

Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be supported for any client.

Should we keep "the note", have a separate requirement or there is some good reason to not have it covered at all?

@TobiasAhnoff
Copy link
Author

I think adding it as a note captures that they are both omitted from best current practices, https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-11#section-10, where ROPC must not be used while Implicit should not be used.
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-27#section-2.4
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-27#section-2.1.2

Verify that for a given client, the authorization server only allows the usage of grants that this client actually needs to use. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be used.

@TobiasAhnoff
Copy link
Author

This requirement is a bit vague. By principle I can understand that you should not have more permissions than you need, but how do I test it?

Verify that allowed scopes for each client asserts least privilege.

Yes, perhaps this is better?

Verify that for a given client, the authorization server only allows the usage of scopes that this client and grant actually needs to use. For example, if supporting the code flow, only scopes that the end-user (resource owner) are allowed to used should be configured for the client.

@elarlang
Copy link
Collaborator

I think the scope and grant topics are different enough to keep them separate. And merging them does not answer my questions

By principle I can understand that you should not have more permissions than you need, but how do I test it?

@elarlang
Copy link
Collaborator

elarlang commented Sep 19, 2024

This I'll handle with PR tomorrow.

Verify that for a given client, the authorization server only allows the usage of grants that this client actually needs to use. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be used.

edit: PR done and the current state is:

# Description L1 L2 L3
51.2.5 [ADDED] Verify that for a given client, the authorization server only allows the usage of grants that this client needs to use. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) should no longer be used.

The requirement above is subject to finetune via #2047.


This one is still waiting to be inserted into the document, but I think it should be a bit more descriptive, why it exists or what problem it solves.

Verify that allowed scopes for each client asserts least privilege.

@TobiasAhnoff
Copy link
Author

Verify that allowed scopes for each client asserts least privilege.

By principle I can understand that you should not have more permissions than you need, but how do I test it?

One way to test this is to try add more scopes to the auth request, then use the more powerful token in API requests and see if you are allowed to access resources you should not have access to.

This one is still waiting to be inserted into the document, but I think it should be a bit more descriptive, why it exists or what problem it solves.

Configuration of scopes for clients is one case where least privilege is important, but sometimes forgotten, and I think it is good to have something in ASVS that address this, to mitigate the risk of "unexpected" privilege escalation for a user/client.

@elarlang
Copy link
Collaborator

A bit different wording

Verify that the OAuth Client has assigned only required scopes using the least privilege principle.

@randomstuff
Copy link
Contributor

Maybe

Verify that the OAuth Client has requested only required scopes using the least privilege principle.

Could be extended:

Verify that the OAuth Client has requested only required scopes (or other authorizations) using the least privilege principle.

in order to include RAR and resource indicators.

@elarlang
Copy link
Collaborator

request and assigned are different things and requirements

For me, current requirement focuses what is available for this OAuth client.

Another topic is, what is the client requesting for certain action.

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Sep 28, 2024

Another topic is, what is the client requesting for certain action.

This is addressed in #2044 by having

Verify that clients asserts least privilege in token requests, by minimizing the set of requested scopes or any other parameter that affects permissions.

But it can be replaced by the requirement suggested here

Verify that the OAuth Client has requested only required scopes (or other authorizations) using the least privilege principle.

For me, current requirement focuses what is available for this OAuth client.

Yes, that is the intent, if not using PAR, the client is not in control of the requested scopes using the code flow, the user/attacker is, we can only limit the set of allowed scopes configured for the client...which is addressed by:

Verify that the OAuth Client is assigned only required scopes using the least privilege principle.

Perhaps both requirements can be made shorter as "only required" and "least privilege" says the same thing or is it good to have "least privilege"?

Verify that the OAuth Client is assigned only required scopes.

Verify that the OAuth Client has requested only required scopes (or other authorization parameters).

@elarlang
Copy link
Collaborator

Trying to make them more understandable for "out of context".

Section OAuth Authorization Server

Verify that the OAuth Client is assigned only the required scopes in the authorization server configuration.

Section OAuth Client

Verify that the OAuth Client has requested only required scopes (or other authorization parameters) in requests to the authorization server.

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Sep 30, 2024
elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 4, 2024
@elarlang
Copy link
Collaborator

elarlang commented Oct 4, 2024

Via #2124

V51.2 OAuth Authorization Server

# Description L1 L2 L3
51.2.8 [ADDED] Verify that the OAuth Client is assigned only the required scopes in the authorization server configuration.

V51.3 OAuth Client

# Description L1 L2 L3
51.3.6 [ADDED] Verify that the OAuth Client has requested only required scopes (or other authorization parameters) in requests to the authorization server.

@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Oct 4, 2024
elarlang pushed a commit that referenced this issue Oct 8, 2024
@elarlang
Copy link
Collaborator

elarlang commented Oct 8, 2024

Requirements are now in. In case those need additional improvement, it's better to open a separate issue.

@elarlang elarlang closed this as completed Oct 8, 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

5 participants