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

51.2.1 OAuth authorization code - prevent replay and limit the lifetime #2090

Closed
elarlang opened this issue Sep 18, 2024 · 13 comments
Closed
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

The requirement was initially discussed in #2041 and added via #2089

# Description L1 L2 L3
51.2.1 [ADDED] Verify that, if the authorization server returns the authorization code, it can be used only once for a token request and it is only valid for up to 10 minutes.

Further discussion to solve (#2041 (comment)) - 10 minutes as lifetime is written to the specification, but FAPI requires just one minute, although it's for "financial grade" applications and should be considered as level 3.

@elarlang elarlang 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 V51 Group issues related to OAuth labels Sep 18, 2024
@TobiasAhnoff
Copy link

A suggestion is to keep one verification and add a recommendation for L3

Verify that, if the authorization server returns the authorization code, it can be used only once for a token request and it is only valid for up to 10 minutes. Note that for high security requirements (L3) this should be at most 60 seconds.

@elarlang
Copy link
Collaborator Author

FAPI quotes:

https://openid.net/specs/fapi-2_0-security-profile.html#section-5.3.1.1-2.11.1

Authorization servers shall issue authorization codes with a maximum lifetime of 60 seconds

https://openid.net/specs/fapi-2_0-security-profile.html#section-5.3.1.2-3

NOTE: If replay identification of the authorization code is not possible, it is desirable to set the validity period of the authorization code to one minute or a suitable short period of time. The validity period may act as a cache control indicator of when to clear the authorization code cache if one is used

@elarlang
Copy link
Collaborator Author

As the code lifetime part of the requirement is getting longer, it is maybe better to split the requirement.

Verify that, if the authorization server returns the authorization code, it can be used only once for a token request.

The message I would like to send is:

  • as short time as possible for given environment
  • required for L3 and recommended for L1, L2 - max lifetime is 60 seconds
  • based on specs max lifetime is allowed max 10 minutes ( written as a recommendation in the spec released in 2012)

For a starter

Verify that the authorization code is valid short period of time - maximum lifetime for L1 and L2 is 10 minutes and for L3 1 minute.

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 5, 2024

Any wording improvements for the proposal?

ping @randomstuff as well

@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 5, 2024
@elarlang
Copy link
Collaborator Author

elarlang commented Oct 9, 2024

Requirement use it once - from #2090 (comment)

Verify that, if the authorization server returns the authorization code, it can be used only once for a token request.

Requirement die fast - one more update for the language check:

Verify that the authorization code lifetime is short-lived. The maximum lifetime for L1 and L2 is 10 minutes and for L3 1 minute.

@tghosth
Copy link
Collaborator

tghosth commented Oct 9, 2024

I would say:

Verify that the authorization code is short-lived. The maximum lifetime should be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

@tghosth
Copy link
Collaborator

tghosth commented Oct 9, 2024

otherwise LGTM

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 9, 2024

... I think it requires further development.

I feel that it does not carry the points I want to send - #2090 (comment)

  • "should be" is recommendation, but the 10 minutes is extremely long time for authorization code lifetime (written to the specification 12 years ago)
  • it does not cover the "it should be as short as the environment allows, but with maximum values L1/L2 ... and L3 ..."

@elarlang elarlang removed 4) proposal for review Issue contains clear proposal for add/change something 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 9, 2024
@randomstuff
Copy link
Contributor

randomstuff commented Oct 9, 2024

Nitpick: "Verify that the authorization code lifetime is short-lived." is somewhat redundant? Shouldn't it be "Verify that the authorization code is short-lived."?

(Otherwise LGTM.)

@TobiasAhnoff
Copy link

TobiasAhnoff commented Oct 13, 2024

"should be" is recommendation, but the 10 minutes is extremely long time for authorization code lifetime

I agree, perhaps in this case ASVS can have a slightly more strict requirement with a small change from should to shall?

Verify that the authorization code is short-lived. The maximum lifetime shall be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

it does not cover the "it should be as short as the environment allows

I agree, but this might be hard to add without making the requirement less clear? Or is this better?

Verify that the authorization code expires as soon as possible. The maximum lifetime shall be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

But I also think this works

Verify that the authorization code is short-lived. The maximum lifetime should be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

@elarlang
Copy link
Collaborator Author

Thank you proposals, I think the last one is good-n-clear enough, changed "should" > "can"

Verify that the authorization code is short-lived. The maximum lifetime can be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Oct 14, 2024
@elarlang
Copy link
Collaborator Author

Just in case reminder for myself, that there is split for current requirement and other requirement must be added/updated as well:

Verify that, if the authorization server returns the authorization code, it can be used only once for a token request.

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 16, 2024

Via #2157

# Description L1 L2 L3
51.2.1 [ADDED] Verify that, if the authorization server returns the authorization code, it can be used only once for a token request.
51.2.2 [ADDED] Verify that the authorization code is short-lived. The maximum lifetime can be 10 minutes for L1 and L2 applications and 1 minute for L3 applications.

elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 16, 2024
@elarlang elarlang added 6) PR awaiting review and removed 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 16, 2024
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 6) PR awaiting review 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