OWASP / ASVS

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

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

Closed elarlang closed 3 weeks ago

elarlang commented 1 month ago

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 (https://github.com/OWASP/ASVS/issues/2041#issuecomment-2354654587) - 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.

TobiasAhnoff commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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:

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 commented 1 month ago

Any wording improvements for the proposal?

ping @randomstuff as well

elarlang commented 1 month ago

Requirement use it once - from https://github.com/OWASP/ASVS/issues/2090#issuecomment-2363454433

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 commented 1 month ago

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 commented 1 month ago

otherwise LGTM

elarlang commented 1 month ago

... I think it requires further development.

I feel that it does not carry the points I want to send - https://github.com/OWASP/ASVS/issues/2090#issuecomment-2363454433

randomstuff commented 1 month ago

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

"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 commented 4 weeks ago

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

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 commented 3 weeks ago

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.