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 client verifications #2044

Closed
TobiasAhnoff opened this issue Aug 31, 2024 · 16 comments
Closed

V51 OAuth: Add client verifications #2044

TobiasAhnoff opened this issue Aug 31, 2024 · 16 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 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 general client responsibilities.

V51.3 Client

Verify that system integrations clients, service-to-service without (human) user interaction, are required to use the 'client-credentials' grant.

Verify that clients used by (human) users are required to use the 'code' grant or, if considered more appropriate, the 'device_code' grant. Additional custom "flows", not known to be industry standard (preferably defined in RFCs), should be avoided.

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

Verify that clients used by (human) users requires the users consent to initiate an authorization request. Note that the consent can be implicit or explicit, requires sufficient CSRF-protection and should (by default) request a minimal set of scopes.

Notes

Note that adding the second verification above and suggested verification for PKCE in #2041 covers the PKCE part of 51.2.2 and the other parts of 51.2.2 is OIDC which should not be part of OAuth2 verifications, so a suggestion is to remove 51.2.2 (given the scope discussion in #2039 and that PKCE verifications from #2041 are added).

Also note that if the scope will be defined as suggested in #2036 then 51.2.3 and 51.2.4 should be removed as they are not in that scope, or the scope need to include Resource indicators and RAR specs as well.

@elarlang elarlang added the V51 Group issues related to OAuth label Aug 31, 2024
@randomstuff
Copy link

randomstuff commented Sep 1, 2024

Verify that system integrations clients, service-to-service without (human) user interaction, are required to use the 'client-credentials' grant.

In contrast to what? Static provisioning of (access) tokens (and mTLS)?

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Sep 1, 2024

In contrast to what? Static provisioning of (access) tokens (and mTLS)?

I was thinking in the context of OAuth2, there are of course service-to-service integration scenarios where there is no need to use OAuth2, where just e g mTLS is all that is needed. Did that make it clearer?

@randomstuff
Copy link

@TobiasAhnoff I was just wondering which other OAuth flows you are intended to disallow here. Someone might attempt to automate an authorization code flow. Is this what you are trying to disallow here?

On the other hand, we would not want to disallow Token Exchange.

What about something like: « Verify service-to-service authorization requests, without (human) user interaction, do not use flows which are intended to be used for interactive flows such as the authorization code grant, the implicit grant or CIBA. »

@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
@elarlang
Copy link
Collaborator

elarlang commented Sep 5, 2024

Hi @randomstuff - as I can not tag you in the issues you are not taking part, I (mis)use this issue for that.

I would like to get your feedback on every V51 tagged issue, but to set a more clear focus for getting things moving I set focus to:

Nice to have, to get issues closed and document updated:

(but should be maybe combined, see #2039 (comment))

Note that, if you agree with proposals, it is also valuable feedback to share.

We also have one dedicated Slack channel for "OAuth working group" but I was not able to find your name from userlist.

@TobiasAhnoff
Copy link
Author

@randomstuff good point! I was thinking perhaps we try to rewrite this in a positive way, what do you think about this?

Verify service-to-service token requests, without (human) user interaction, use flows which are intended to be used for service-to-service, i e client-credentials or token-exchange.

And if we introduce token-exchange here, we should also add that spec to the scope for OAuth (#2036)

@jmanico
Copy link
Member

jmanico commented Sep 7, 2024 via email

@randomstuff
Copy link

Can we perhaps add something like “… with strong client authentication” at the end of this or something similar?

This looks like a separate verification (not only ties to machine-to-machine interaction) and probably something you would not want in L1?

@TobiasAhnoff
Copy link
Author

I think “… with strong client authentication” maybe does not need to be added in this verification as it applies for all levels and mandating strong client authentication methods (mTLS or private-key-twt) is addressed in #2038

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'. Note that L1-L2 applications could also use client-secret authentication and allow public clients. (L3)

Perhaps that need to be rewritten/split to more clearly capture that good (sufficient for the level of security) client authentication is a must on all levels for client-credentials (e g properly managed basic auth or mTLS etc), but for L3 strong mTLS or private-key-jwt is required.

Reading this suggested verification I also think it might be too strict, not allowing public code-flow native mobile app clients on L3, I will make a comment on that in #2038, worth a discussion!

@elarlang elarlang changed the title V5.1 OAuth: Add client verifications V51 OAuth: Add client verifications Sep 10, 2024
@elarlang
Copy link
Collaborator

Verify service-to-service token requests, without (human) user interaction, use flows which are intended to be used for service-to-service, i e client-credentials or token-exchange.

Should it be authorization server requirement to say, that what is allowed for machine-to-machine clients and what is not? (It should not be allowed for the Client to decide)

Verify that clients used by (human) users requires the users consent to initiate an authorization request. Note that the consent can be implicit or explicit, requires sufficient CSRF-protection and should (by default) request a minimal set of scopes.

Here seems to be 2 things - CSRF protection and minimal set of scopes.

We have requirement:

3.2.5 [ADDED] Verify that creating a session for the application requires the user's consent and that the application is protected against a CSRF-style attack where a new application session for the user is created via SSO without user interaction.

Do you think it covers the case? At least I proposed the requirement keeping exactly this scenario in mind.

Please provide list of updated requirements.

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Sep 13, 2024
@elarlang
Copy link
Collaborator

elarlang commented Sep 20, 2024

ping @TobiasAhnoff - please provide latest list of proposed requirements.

@TobiasAhnoff
Copy link
Author

Should it be authorization server requirement to say, that what is allowed for machine-to-machine clients and what is not? (It should not be allowed for the Client to decide)

@elarlang I agree, and from #2043 we also have

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.

I suggest to simply remove

Verify that system integrations clients, service-to-service without (human) user interaction, are required to use the 'client-credentials' grant.

and

Verify that clients used by (human) users are required to use the 'code' grant or, if considered more appropriate, the 'device_code' grant. Additional custom "flows", not known to be industry standard (preferably defined in RFCs), should be avoided.

since they are partly covered by #2043 and more guidance/education on when to use which flow, not mitigations for a known attack-type (the "industry standard" is part is overlap by #2047).

An alternative would be add something about grants to use, perhaps like this:

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

@TobiasAhnoff
Copy link
Author

I think 3.2.5 covers most of it, perhaps modify 3.2.5 to also state that "by default request a minimal set of scopes"?

3.2.5 [ADDED] Verify that creating a session for the application requires the user's consent, by default a minimal set of scopes should be requested, and that the application is protected against a CSRF-style attack where a new application session for the user is created via SSO without user interaction.

@elarlang
Copy link
Collaborator

elarlang commented Sep 22, 2024

I think 3.2.5 covers most of it, perhaps modify 3.2.5 to also state that "by default request a minimal set of scopes"?

Those are different problems and I prefer to not merge them to one requirement. One requires defense against CSRF, another is default behavior for assigning privileges.

I think the scope question is addressed here: #2043 (comment)

@elarlang elarlang removed the 2) Awaiting response Awaiting a response from the original poster label Sep 22, 2024
@elarlang
Copy link
Collaborator

Do I understand correctly, that we don't add anything from initial 4 requirements as those are covered one way or another somewhere else and it is proposed to update current 51.2.5

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

@TobiasAhnoff
Copy link
Author

Almost, I will try to make a summary

The first two could be replaced by a modified 51.2.5

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

or even more specific on valid OAuth grants

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

The third one is not addressed by in example #2043 so I think this should be added to address that clients should not request more scopes than needed on tokens requests (even if the client is allowed to use more scopes).

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

The fourth one is almost covered by 3.2.5 but the aspect of secure defaults for consent pages is not addressed by #2043, since the focus is on configuration of the allowed scope for the client (not the ones selected by default on the consent page). It is a detail, but I think it is worth having this in ASVS since there are many applications that by mistake or with bad intent ask the user to consent for scopes that aren't really need for what the user wants to do (but might be needed for other user consents granted for the client).

So a suggestion is to add this as separate requirement

Verify that user consent by default requested a minimal set of scopes.

@TobiasAhnoff
Copy link
Author

I think both

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

and

Verify that user consent by default requested a minimal set of scopes.

are covered by the two requirements suggested in #2043 (comment) and #2120

As noted earlier other parts of this issue is covered by #2043 (comment) so, as far as I understand, this issue can now be closed (replaced by #2043 and #2120 discussion).

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 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