OWASP / ASVS

Application Security Verification Standard
Creative Commons Attribution Share Alike 4.0 International
2.71k stars 662 forks source link

V51 OAuth: Consider adding more general OAuth verifications #1925

Open TobiasAhnoff opened 5 months ago

TobiasAhnoff commented 5 months ago

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

(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 commented 5 months ago

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 https://github.com/OWASP/ASVS/issues/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 https://github.com/OWASP/ASVS/issues/1916 "Discussion/Proposal 2". Probably it makes sense to open a separate issue for this to keep the focus.

edit: I opened separate issue: https://github.com/OWASP/ASVS/issues/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 https://github.com/OWASP/ASVS/issues/1917#issuecomment-2027058608)

# 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 https://github.com/OWASP/ASVS/issues/1916 "Discussion/Proposal 1"

edit: I opened separate issue: https://github.com/OWASP/ASVS/issues/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 :)

csfreak92 commented 5 months ago

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 commented 4 months ago

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 commented 4 months ago

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.

elarlang commented 4 months ago

Covered or mapped to issues:

need to open separate issue for proposal

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.

TobiasAhnoff commented 2 months ago

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 commented 2 days ago

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