V51 OAuth: Add verifications for Authorization Server client configuration
TobiasAhnoff opened this issue · 23 comments
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)
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?"
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)
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?
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
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.
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.
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.
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 :)
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."
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?
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.
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.
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?
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.