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: Consider adding more general OAuth verifications #1925

Closed
TobiasAhnoff opened this issue Apr 15, 2024 · 8 comments
Closed

V51 OAuth: Consider adding more general OAuth verifications #1925

TobiasAhnoff opened this issue Apr 15, 2024 · 8 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 2) Awaiting response Awaiting a response from the original poster V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@TobiasAhnoff
Copy link

TobiasAhnoff commented Apr 15, 2024

There are of course many candidates, it is hard to summarize all relevant OAuth and OIDC specs in just a small set of verifications for V51 OAuth.

Here are a few suggestions on some more general verifications that, based on my experience with security reviews and penetration tests, would be interesting to discuss (maybe add as individual items with motivation etc, if that would be a good way to contribute):

  • 1 Verify that AS is built using hardened secure libraries, products or services (implemented according to BCPs)
  • 2 Verify that JWTs are verified by RS using a secure hardened library
  • 3 Verify that client configuration asserts least privilege (e g scopes and flows)
  • 4 Verify that only access-tokens are used for authorization by the RS (not id-tokens or other kinds of tokens)
  • 5 Verify that AS singing keys are rotated according to threat model
  • 6 Verify that AS and clients supports client-secret rotation (at least manually)
  • 7 Verify that refresh-tokens expires according to threat model and business requirements
  • 8 Verify that access and refresh tokens are not accessible by Javascript
  • 9 Verify that only strong asymmetric signing keys are used

(edit: added numbers for organizing the feedback)

Perhaps even have a general verification that simply states:
10 Verify recommendations for OAuth2 from https://oauth.net/2.1/ and OIDC FAPI 2.0 from https://openid.net/wg/fapi/specifications/ according to your threat model and business requirements.

Or just recommend to follow this guidance in the in the text before the verifications (not expressing this as a verification).

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

elarlang commented Apr 16, 2024

Thank you @TobiasAhnoff for your input!

It is a nice "validation round", do we have pointed out proposals and requirements covered in one way or another in the ASVS.

A general point of view - OAuth is "logic" based on a list of other technologies, and if some requirement is clearly related to the underlying technology, it should be covered there.

proposal 1

1 Verify that AS is built using hardened secure libraries, products or services (implemented according to BCPs)

Not OAuth specific, covered by section V14.1 Build and Deploy

proposal 2

2 Verify that JWTs are verified by RS using a secure hardened library

Not OAuth specific, covered by section V3.4 Cookie-based Session Management.

One option is to move JWT part away from session management, but this discussion you can follow in the issue #1917

proposal 3

3 Verify that client configuration asserts least privilege (e g scopes and flows)

This is something to address. How much detail or level of abstract we can use, is good question.

I opened the same topic in #1916 "Discussion/Proposal 2". Probably it makes sense to open a separate issue for this to keep the focus.

edit: I opened separate issue: #1964 - if it matches with your problem statement, we can move discussion there, if it does not, then we handle it separately.

proposal 4

4 Verify that only access-tokens are used for authorization by the RS (not id-tokens or other kinds of tokens)

Not OAuth specific, general JWT.

I think we should add typ (RFC7519) to this requirement:

# Description L1 L2 L3 CWE NIST §
3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. 287

Also note, that the access token does not need to be a JWT. First random response from related search

proposal 5

5 Verify that AS singing keys are rotated according to threat model

I don't think it is OAuth specific. I'm not sure we have it addressed, need to anaylze chapters:

... but those are in the authentication section.

proposal 6

6 Verify that AS and clients supports client-secret rotation (at least manually)

Manually it is anyway doable. How do you verify that or what the requirement gives extra?

proposal 7

7 Verify that refresh-tokens expires according to threat model and business requirements

The refresh_token topic requires more attention. But there is more than one point of view.

It is different attitude, is the OAuth used as first-party or 3rd party solution. For the first-party and "session management replacement" (which should be disallowed or not recommended) I would say we can apply V3.3 Session Timeout and V3.8 Session Termination requirements.

# Description L1 L2 L3 CWE NIST §
3.3.2 [MODIFIED, SPLIT TO 3.3.5] Verify that there is an absolute maximum session lifetime such that re-authentication is required at least every 30 days for L1 applications or every 12 hours for L2 and L3 applications. 613 7.2

For 3rd party solution, we have a requirement currently located in 3.5.1 (it is subject to change via #1917 (comment))

# Description L1 L2 L3 CWE NIST §
3.5.1 [GRAMMAR] Verify that the application allows users to revoke OAuth tokens that form trust relationships with linked applications. 290 7.1.2

I think we need to be really precise, about what architecture and solution we address with the requirement.

One extra issue to cover with the refresh_token topic expiration is that with new refresh_token the AS must keep the exp value like it was before (and not extend it).

proposal 8

8 Verify that access and refresh tokens are not accessible by Javascript

It requires quite a strong change to widespread attitude, as with using SPA architecture, often the browser uses directly OAuth service, including token endpoint.

I have addressed the same topic in the issue #1916 "Discussion/Proposal 1"

edit: I opened separate issue: #1963

proposal 9

9 Verify that only strong asymmetric signing keys are used

Can you find the matching requirement from [V6 Stored Cryptography](V6 Stored Cryptography)?

proposal 10

10 Verify recommendations for OAuth2 from https://oauth.net/2.1/ and OIDC FAPI 2.0 from https://openid.net/wg/fapi/specifications/ according to your threat model and business requirements.

Yes, with many requirements we took the direction of that style. We could save months of @csfreak92 time :) But now we are here and we go with the list of OAuth-related requirements to v5.0.

Before any further action (open mentioned separate issues) I wait "first wave" of feedback :)

@elarlang elarlang added the 2) Awaiting response Awaiting a response from the original poster label Apr 16, 2024
@csfreak92
Copy link
Collaborator

I'll take a look @elarlang and @tghosth. Thanks for the suggestions too @TobiasAhnoff! I'll work on this when I get back home from traveling. :) Pardon the delayed responses.

@elarlang
Copy link
Collaborator

elarlang commented May 9, 2024

Hi @TobiasAhnoff - I would like to hear your comments on my feedback. Then we can open separate issues to discuss some topics further and "close" topics, that are already covered or do not require further discussion.

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented May 20, 2024

Thank you @TobiasAhnoff for your input!

It is a nice "validation round", do we have pointed out proposals and requirements covered in one way or another in the ASVS.

A general point of view - OAuth is "logic" based on a list of other technologies, and if some requirement is clearly related to the underlying technology, it should be covered there.

proposal 1

1 Verify that AS is built using hardened secure libraries, products or services (implemented according to BCPs)

Not OAuth specific, covered by section V14.1 Build and Deploy

I agree that this is could be part of other chapters, but this is perhaps not a Build/Deploy issue, more of a design/architecture decision or best practice like 6.2.2 - "Verify that industry proven or government approved cryptographic algorithms, modes, and libraries are used, instead of custom coded cryptography". You should avoid implementing your own custom code for token-endpoints (AS). It is hard to implement an AS according to BCPs and I believe it would be good if ASVS recommends using hardened secure libraries, products or services (implemented according to BCPs) for OAuth/OIDC solutions.

proposal 2

2 Verify that JWTs are verified by RS using a secure hardened library

Not OAuth specific, covered by section V3.4 Cookie-based Session Management.

One option is to move JWT part away from session management, but this discussion you can follow in the issue #1917

I agree, probably good to move, in example when using JWTs (or other kinds of self-contained tokens) in a service-to-service scenario (OAuth client credentials) there is no session, just token-based request authentication and authorization. It will be good to continue this in #1917

proposal 3

3 Verify that client configuration asserts least privilege (e g scopes and flows)

This is something to address. How much detail or level of abstract we can use, is good question.

I opened the same topic in #1916 "Discussion/Proposal 2". Probably it makes sense to open a separate issue for this to keep the focus.

edit: I opened separate issue: #1964 - if it matches with your problem statement, we can move discussion there, if it does not, then we handle it separately.

I agree, we continue this in #1964

proposal 4

4 Verify that only access-tokens are used for authorization by the RS (not id-tokens or other kinds of tokens)

Not OAuth specific, general JWT.

I think we should add typ (RFC7519) to this requirement:

Description L1 L2 L3 CWE NIST §

3.5.6 [ADDED] Verify that other, security-sensitive attributes of a stateless token are being verified. For example, in a JWT this may include issuer, subject, and audience. ✓ ✓ ✓ 287

Also note, that the access token does not need to be a JWT. First random response from related search

I agree, this could be part of 3.5.6

proposal 5

5 Verify that AS singing keys are rotated according to threat model

I don't think it is OAuth specific. I'm not sure we have it addressed, need to anaylze chapters:

* [V2.9 Cryptographic Verifier](https://github.com/OWASP/ASVS/blob/master/5.0/en/0x11-V2-Authentication.md#v29-cryptographic-verifier)

* [V2.10](https://github.com/OWASP/ASVS/blob/master/5.0/en/0x11-V2-Authentication.md#v210-service-authentication)

... but those are in the authentication section.

I agree, this is better as a more general verification, it is always important to rotate key material, not just when signing access tokens, but is is good to mention AS as an example of when it is important and sometimes forgotten. 2.10.1 captures this in a way, but it needs some works since it is focused on service authentication.

proposal 6

6 Verify that AS and clients supports client-secret rotation (at least manually)

Manually it is anyway doable. How do you verify that or what the requirement gives extra?

This is also related to 2.10.1, perhaps part of the #1032 discussion?

proposal 7

7 Verify that refresh-tokens expires according to threat model and business requirements

The refresh_token topic requires more attention. But there is more than one point of view.

It is different attitude, is the OAuth used as first-party or 3rd party solution. For the first-party and "session management replacement" (which should be disallowed or not recommended) I would say we can apply V3.3 Session Timeout and V3.8 Session Termination requirements.

Description L1 L2 L3 CWE NIST §

3.3.2 [MODIFIED, SPLIT TO 3.3.5] Verify that there is an absolute maximum session lifetime such that re-authentication is required at least every 30 days for L1 applications or every 12 hours for L2 and L3 applications. ✓ ✓ ✓ 613 7.2

For 3rd party solution, we have a requirement currently located in 3.5.1 (it is subject to change via #1917 (comment))

Description L1 L2 L3 CWE NIST §

3.5.1 [GRAMMAR] Verify that the application allows users to revoke OAuth tokens that form trust relationships with linked applications. ✓ ✓ 290 7.1.2

I think we need to be really precise, about what architecture and solution we address with the requirement.

One extra issue to cover with the refresh_token topic expiration is that with new refresh_token the AS must keep the exp value like it was before (and not extend it).

Refresh-tokens and sessions etc are need elaboration, I need to get back on this later this week...

proposal 8

8 Verify that access and refresh tokens are not accessible by Javascript

It requires quite a strong change to widespread attitude, as with using SPA architecture, often the browser uses directly OAuth service, including token endpoint.

I have addressed the same topic in the issue #1916 "Discussion/Proposal 1"

edit: I opened separate issue: #1963

I agree, it will be good to continue this in #1963

proposal 9

9 Verify that only strong asymmetric signing keys are used

Can you find the matching requirement from [V6 Stored Cryptography](V6 Stored Cryptography)?

No, as I understand this is not present in V6. Besides using crypto properly, this is also a least privilege issue, a mistake made when configuring an AS is to use a symmetric signing key (HMACs) without realizing that it will be shared with all RS (who now also will be able to create valid tokens). Depending on the organization, trust boundaries etc this might be a critical issue, leaking a very sensitive key...which might be good to point out in ASVS

proposal 10

10 Verify recommendations for OAuth2 from https://oauth.net/2.1/ and OIDC FAPI 2.0 from https://openid.net/wg/fapi/specifications/ according to your threat model and business requirements.

Yes, with many requirements we took the direction of that style. We could save months of @csfreak92 time :) But now we are here and we go with the list of OAuth-related requirements to v5.0.

Before any further action (open mentioned separate issues) I wait "first wave" of feedback :)

I believe it would be good to reference both https://oauth.net/2.1/ and https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps in the OAuth chapter, and https://openid.bitbucket.io/fapi/fapi-2_0-security-profile.html from the OIDC chapter, perhaps not as verifications, but as part of the text at the beginning of the chapters. Since these documents are important for building secure OAuth/OIDC applications and it is important that ASVS verifications are aligned with them.

@TobiasAhnoff
Copy link
Author

TobiasAhnoff commented Jul 17, 2024

Covered or mapped to issues:

* proposal 2 - section V3.4 / [cleanup V3.5 Token-based Session Management #1917](https://github.com/OWASP/ASVS/issues/1917)

* proposal 3 - [proposal/discussion: OAuth - (for 1st party usage) only used (by the client) communication options must be allowed by authorization server #1964](https://github.com/OWASP/ASVS/issues/1964)

* proposal 4 - [proposal/discussion: JWT - 3.5.6 add "type", and rephrase it to describe the goal #1967](https://github.com/OWASP/ASVS/issues/1967)

* proposal 6 - [2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys #1032](https://github.com/OWASP/ASVS/issues/1032)

* proposal 7 - [proposal/discussion: OAuth: requirement for refresh_token lifetime #1968](https://github.com/OWASP/ASVS/issues/1968)

* proposal 8 - [proposal/discussion: OAuth - disallow web application to be OAuth public client (and to have direct communication with OAuth token endpoint) #1963](https://github.com/OWASP/ASVS/issues/1963)

need to open separate issue for proposal

* proposal 1

* proposal 5

* proposal 9

Proposal 10 is change for documentation, not with requirements. As it is written in a different format (compared to other paragraphs), I need to fix that part first.

For proposal 1 I suggest to not add this aspect as a separate verification, it could be part of suggested verfication for proposal 10 (below) and (in my opinion) it is also covered by verifications 1.2.3 and 1.4.4, which might be clearer if the OAuth chapter also had the same text as chapter 13:
"Please read this chapter in combination with all other chapters at this same level; we do not duplicate authentication, session management, or general input validation concerns. Rather, the general requirements from those chapters always apply and therefore this chapter can not be taken out of context and be tested separately."

For proposal 5 I suggest to make it part of 6.4 and add this new verification:
6.4.3 - Verify that cryptographic keys, such as access token signing keys, are rotated on a schedule (according to threat model and requirements.)

For proposal 9 I suggest to alter it a bit, make that part of 6.4 and add this new verification:
6.4.4 - Verify that all access to cryptographic keys asserts least privilege. In example if entities required to validate a signed message (or token) should not be able to sign new messages (or tokens,) then asymmetric keys must be used (not e g HMACs with shared keys.)

For proposal 10 (and 1) I suggest to add a new verification (or maybe address this in text for the new OAuth/OIDC chapter):
Verify that industry proven OAuth2 authorization servers implementing OAuth2 Best Current Practices (https://oauth.net/2.1/) are used (instead of custom coded services.) Preferably these services are listed at https://openid.net/certification/#OPENID-OP-P. For L3 applications they should also be implement High Security OAuth specifications at https://oauth.net/2/ (e g support for mTLS and OIDC FAPI).

@elarlang elarlang self-assigned this Jul 23, 2024
@elarlang
Copy link
Collaborator

@TobiasAhnoff - if you think those proposals are not covered yet but should be, please open issues for each proposal separately.

@TobiasAhnoff
Copy link
Author

All proposals are now covered by other issues, so this can be closed

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 2) Awaiting response Awaiting a response from the original poster 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