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 OAuth verifications for token management #2038

Open
TobiasAhnoff opened this issue Aug 31, 2024 · 17 comments
Open

V51 OAuth: Add OAuth verifications for token management #2038

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

To address BCPs from https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps#name-application-architecture-pa a suggestion is to add the following verifications. Note that this address (closes) #1963.

V51.1 Generic OAuth2 security

Verify that tokens are only managed by nodes where it's strictly needed, in example avoid having tokens accessible for JavaScript frontends. (L1, L2, L3)

V51.2 Authorization Server

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)

Verify that all token requests requires client authentication and that only sender-constrained (Proof-of-Posession) access-tokens are issued, either using 'mTLS certifcate binding' or 'DPoP'. (L3)

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

Is there any reason to open a separate issue or we can keep this discussion in #1963?

@TobiasAhnoff
Copy link
Author

We can keep it in #1963, should I add this issue text as a comment there? And then close this one?

@elarlang
Copy link
Collaborator

elarlang commented Sep 1, 2024

If here is no new perspective or reasoning, it's better to continue the discussion where it is already happening.

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

Closing this thread as there is already a discussion in another issue mentioned above.

@elarlang elarlang reopened this Sep 5, 2024
@elarlang
Copy link
Collaborator

elarlang commented Sep 5, 2024

I leave to @TobiasAhnoff to close to be sure all points and contents are transferred to another issue.

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

randomstuff commented Sep 5, 2024

Verify that tokens are only managed by nodes where it's strictly needed, in example avoid having tokens accessible for JavaScript frontends. (L1, L2, L3)

"managed" is somewhat vague?

What about something like: "Verify that tokens are not sent to components which do not strictly need them. For avoid having tokens accessible for the frontend when they are only needed by the backend. (L1, L2, L3)"

Doesn't this also applies to refresh tokens? → Sorry, I misread your text :)

@TobiasAhnoff
Copy link
Author

@randomstuff I like replacing the word "managed" and yes, this applies to refresh tokens as well (especially long lived tokens should be avoided in the frontend). How about this?

Verify that tokens are not sent to components which do not strictly need them. For example avoid having access or refresh tokens accessible for the frontend when they are only needed by the backend. (L1, L2, L3)

@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something and removed 2) Awaiting response Awaiting a response from the original poster labels Sep 9, 2024
@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Sep 9, 2024

Based on the discussion in #2044 a suggestion is to modify/split

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)

to also make it clear that client-authentication (for all confidential clients, including code and clients-credentials) must be sufficient for the level of security. Perhaps have two verifications like this (where the first one for all levels and the second only for L3):

Verify that all confidential clients are configured to use secure client authentication. For L1 cryptographically secure client secrets using basic authentication can be used, while stronger methods (i. e. mTLS or private-key-jwt) are only recommended. (L1,L2,L3)

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'. (L3)

I think this works, but then we have native mobile apps where it is common to have public clients (where all OAuth is done in the app). Some might use OIDC CIBA or integrate authorization for the app with other authentication protocols (in the Nordic countries this is often called BankID) which might not use OAuth at all, but sometimes this is also integrated with OAuth/OIDC.

I think it is hard to capture this in verifications because it depends on context and threat models etc...

I like having the L3 verification to say just:

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'. (L3)

And then (for L3 applications and maybe also L2?) you have to do your own risk assessment and mitigations (e g PKCE, Reference-tokens, DPoP) if you don´t follow this and introduce public client solutions or use other (weaker) client authentication methods. Or should we try and add something about native mobile apps to capture this?

@elarlang
Copy link
Collaborator

elarlang commented Sep 9, 2024

Instead of technical checklist I prefer to go with something abstract.

For the context, we talk about machine-to-machine communication and mTLS authentication between services. It is away from the first level defense. We talk here about the likelihood component - what is the difference to reach to client credentials vs mTLS certificates.

We also have requirement (the category is wrong for this, but this is a separate problem):

# Description L1 L2 L3 CWE
1.2.2 [MODIFIED] Verify that communications between back-end application components, including APIs, middleware and data layers, are authenticated and use individual user accounts. 306

We also have requirements:

# Description L1 L2 L3 CWE
9.2.2 [MODIFIED] Verify that an encrypted protocol such as TLS is used for all inbound and outbound connections to and from the application, including monitoring systems, management tools, remote access and SSH, middleware, databases, mainframes, partner systems, or external APIs. The server must not fall back to insecure or unencrypted protocols. 319
9.3.3 [ADDED] Verify that mutual TLS (mTLS) is used by services communicating internally within a system or "intra-service communications" to ensure all the involved parties at each end of a network connection are who they claim to be. 295

So do we have it already covered, just in in the OAuth context?

@csfreak92
Copy link
Collaborator

@elarlang, I agree we have those mTLS and TLS matters covering the points raised by @TobiasAhnoff even if they are OAuth context.

I think the requirement we have here is for:
Verify that tokens are not sent to components which do not strictly need them. For example avoid having access or refresh tokens accessible for the frontend when they are only needed by the backend. (L1, L2, L3)

Right? Or am I missing something else?

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Sep 16, 2024

9.3.3 covers the mTLS parts of

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)

Verify that all token requests requires client authentication and that only sender-constrained (Proof-of-Posession) access-tokens are issued, either using 'mTLS certifcate binding' or 'DPoP'. (L3)

but 9.3.3 does not address OAuth sender-constrained tokens and strong client authentication (without mTLS).

A suggestion is to try and rewrite this (perhaps merge as one) in a more general way as @elarlang suggested, and also check for overlap with other suggested OAuth verifications like in #2040

@elarlang
Copy link
Collaborator

elarlang commented Sep 17, 2024

I try to get the general "token scope" requirement in. We should have "positive" requirement to say, what must be achieved (instead of saying, what must not be done).

So my proposal for this is:

Verify that tokens are only sent to components that strictly need them. For example, avoid having access or refresh tokens accessible for the frontend when they are only needed by the backend.

Section: "Generic OAuth2 and OIDC security"

ping @randomstuff @TobiasAhnoff

@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something labels Sep 17, 2024
@randomstuff
Copy link

@elarlang, Looks good to me.

@elarlang
Copy link
Collaborator

elarlang commented Sep 18, 2024

One requirement is now in the document as:

# Description L1 L2 L3
51.1.2 [ADDED] Verify that tokens are only sent to components that strictly need them. For example, avoid having access or refresh tokens accessible for the frontend when they are only needed by the backend.

If there is need to have changes into this requirement, I prefer we open a separate issue for that.

This issue stays open as here is another one or two requirements (#2038 (comment)) to discuss.

@elarlang
Copy link
Collaborator

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)

Verify that all token requests requires client authentication and that only sender-constrained (Proof-of-Posession) access-tokens are issued, either using 'mTLS certifcate binding' or 'DPoP'. (L3)

With those I'm not technically that familiar (yet), but some feedback/questions.

The "Note" part we can skip

The point for the first one is to ask confidential OAuth client and strong authentication and the second, to use sender-constrained access tokens? If so, it seems that there can be some deduplication done from authentication part for the second requirement.

I really don't like the idea to allow using public OAuth clients for web applications, the same idea was risen also by @TobiasAhnoff here #2038 (comment)

And then (for L3 applications and maybe also L2?) you have to do your own risk assessment and mitigations (e g PKCE, Reference-tokens, DPoP) if you don´t follow this and introduce public client solutions or use other (weaker) client authentication methods. Or should we try and add something about native mobile apps to capture this?

@TobiasAhnoff
Copy link
Author

Yes, the authentication part can be removed from the second verification, since the original OAuth RFC states that "If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server" (see https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.3).

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'.

Verify that only sender-constrained (Proof-of-Posession) access-tokens are issued, either using 'mTLS certifcate binding' or 'DPoP'.

But note that sender-constrained does not require client authentication (and that is why the authentication part was present in the second verification to begin with).

For L3 I think the two verification are clear, but it is not uncommon to allow native mobile apps (not web or hybrid apps) to be public clients, even for L3 applications, so maybe it is too strict and ASVS should address the native mobile apps exception?

Verify that all clients are confidential and configured to use strong client authentication methods, i. e. 'mTLS' or 'private-key-jwt'. Note that native mobile apps might be public clients, given that other token misuse mitigations are in place like PKCE, DPoP and reference access tokens.

@randomstuff
Copy link

For L3 I think the two verification are clear, but it is not uncommon to allow native mobile apps (not web or hybrid apps) to be public clients, even for L3 applications, so maybe it is too strict and ASVS should address the native mobile apps exception?

I'm not so convinced that "native mobile" is such an exception compared to native apps in general. Moreover, if you allow web mobile apps to be public client, why couldn't you allow some web applications to do so (such as a local JupyterLab instance). Moreover, native apps could use dynamic client registration in order to be confidential.

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