ASVS icon indicating copy to clipboard operation
ASVS copied to clipboard

3.5.2 doesn't seem like it is in context

Open tghosth opened this issue 2 years ago • 23 comments

3.5.2 sounds more like it should either:

  1. Be in section 2.10 where we discuss intra-service authentication
  2. It should be made more generic to be clear that this is a general guideline that temporary session tokens should be used after authentication and never a static identifier.
# Description L1 L2 L3 CWE NIST §
3.5.2 [GRAMMAR] Verify that the application uses session tokens rather than static API secrets and keys, except with legacy implementations. 798

History:

# Description L1 L2 L3 CWE NIST §
3.5.2 Verify the application uses session tokens rather than static API secrets and keys, except with legacy implementations. - 798
3.5.2 Verify that single factor, static API secrets and keys are not used, except with legacy implementations. - 798
3.3.1 Verify single factor unchanging API secrets and keys are not used, except with legacy implementations. 1.0

tghosth avatar Jan 12 '23 15:01 tghosth

I would just say:

Verify that the application uses framework-specific session tokens or cryptographically signed JSON Web Tokens for session management. Static API secrets and keys should be avoided.

jmanico avatar Jan 12 '23 16:01 jmanico

Probably we need to start from the question: what problem it addresses?

From the text I don't read it as machine-to-machine communication specific.

As a quick-fix I would get rid of phrase ", except with legacy implementations".

elarlang avatar Jan 14 '23 11:01 elarlang

It seems this section is for applications that authenticate with OpenID connect and supply a JWT to the user, instead of a random session cookie. I am not sure how "static API secrets and keys" can be an alternative to that, so in this context this requirement doesn't make sense to me.

For intra-service authentication, static API secrets and keys seem acceptable to me.

Sjord avatar Jan 17 '23 12:01 Sjord

Some more details:

History:

# Description L1 L2 L3 CWE NIST §
3.5.2 Verify the application uses session tokens rather than static API secrets and keys, except with legacy implementations. - 798
3.5.2 Verify that single factor, static API secrets and keys are not used, except with legacy implementations. - 798
3.3.1 Verify single factor unchanging API secrets and keys are not used, except with legacy implementations. 1.0

See also 2.10.1:

# Description L1 L2 L3 CWE NIST §
2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access. 287
2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access. OS assisted HSM 287 5.1.1.1
2.10.1 Verify that intra-service secrets do not rely on unchanging passwords, such as API keys or shared accounts with privileged access. OS assisted HSM 287 5.1.1.1
2.10.1 Verify that integration secrets do not rely on unchanging passwords, such as API keys or shared privileged accounts. OS assisted HSM 287 5.1.1.1
2.11.1 Verify that integration secrets do not rely on unchanging passwords, such as API keys or shared privileged accounts. Software OS assisted HSM 5.1.1.1
2.11.1 Integration secrets SHOULD NOT rely on unchanging passwords, such as API keys or shared privileged accounts. Software OS assisted HSM 5.1.1.1
2.11.1 Integration secrets SHOULD NOT rely on unchanging passwords, such as API keys or shared privileged accounts. If passwords are required, the credential should not be a default account and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 5.1.1.1
2.21 Integration secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 5.1.1.1
2.21 Integration memorized secrets SHOULD NOT rely on unchanging memorized secrets, such as API keys or shared privileged accounts. If memorized secrets are required, the credential should not be a default account, and stored with sufficient protection to prevent offline recovery attacks, including local system access. Software OS assisted HSM 4.0 5.1.1.1

From Andrew's deck: image

From Andrew's deck: image

tghosth avatar May 23 '23 10:05 tghosth

There has been discussion of 2.10.1 at #1032

tghosth avatar May 23 '23 11:05 tghosth

3.5.2 sounds more like it should either:

  1. Be in section 2.10 where we discuss intra-service authentication
  2. It should be made more generic to be clear that this is a general guideline that temporary session tokens should be used after authentication and never a static identifier.

I am leaning towards option 2 here. 2.10.1 seems like it is going to change slightly anyway. The history of 3.5.2 is a bit weird but I think the idea is partially covered by 2.10.1 and the rest is kind of a generic basis for session management.

tghosth avatar May 23 '23 11:05 tghosth

After reading it again, I still have the same question: what problem it solves?

elarlang avatar May 27 '23 18:05 elarlang

I think the problem it solves is that we don't have a basic level requirement which says, use session tokens not fixed values which I think is why Jim came up with:

Verify that the application uses framework-specific session tokens or cryptographically signed JSON Web Tokens for session management. Static API secrets and keys should be avoided.

tghosth avatar Jun 12 '23 15:06 tghosth

@elarlang what do you think about this very basic session management requirement which Jim suggested here https://github.com/OWASP/ASVS/issues/1522#issuecomment-1380700455

Verify that the application uses framework-specific session tokens or cryptographically signed JSON Web Tokens for session management. Static API secrets and keys should be avoided.

tghosth avatar Sep 14 '23 10:09 tghosth

Is the context here machine-to-machine integrations or it covers all "usual" end-users with their browsers as well? If it is machine to machine, How it's different from current 2.10.1?

# Description L1 L2 L3 CWE NIST §
2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys or shared accounts with privileged access. 287

For 2.10.1 there is separate issue opened: #1032

At the moment the requirement is in "Token based session management", but I can not read this category out from the requirement text.

What is "framework-specific" session token?

elarlang avatar Sep 14 '23 10:09 elarlang

Is the context here machine-to-machine integrations or it covers all "usual" end-users with their browsers as well? If it is machine to machine, How it's different from current 2.10.1?

I think we are talking about usual end users here and that service to service is something else handled in 2.10.1.

What is "framework-specific" session token?

This is a good question and I think actually we should be more consistent with the wording we are using in 3.2 so I would propose the following:

# Description L1 L2 L3 CWE NIST §
3.1.3 [ADDED] Verify that the application uses opaque or cryptographically signed tokens for session management. Static API secrets and keys should be avoided.

tghosth avatar Sep 18 '23 09:09 tghosth

@elarlang do you approve this wording?

tghosth avatar Sep 28 '23 11:09 tghosth

We already have those requirements:

# Description L1 L2 L3 CWE NIST §
3.2.2 [MODIFIED] Verify that opaque session tokens possess at least 128 bits of entropy. (C6) 331 7.1
3.2.4 [MODIFIED] Verify that opaque session tokens are generated using a secure random function. (C6) 330 7.1
3.5.2 [GRAMMAR] Verify that the application uses session tokens rather than static API secrets and keys, except with legacy implementations. 798

Why we need proposed 3.1.3? What problem it solves? Feels like some overlapping.

elarlang avatar Sep 28 '23 12:09 elarlang

Ah yeah, I think it is intended to replace 3.5.2 with a generic requirement for the need for session management so it should be this:

# Description L1 L2 L3 CWE NIST §
3.1.3 [MOVED FROM 3.5.2, MODIFIED] Verify that the application uses opaque or cryptographically signed tokens for session management. Static API secrets and keys should be avoided.

@elarlang

tghosth avatar Sep 28 '23 13:09 tghosth

Maybe some duplication with "session tokens" Verify that the application uses opaque session tokens or cryptographically signed tokens for session management. Static API secrets and keys should be avoided.

Otherwise (I personally) may read it like: Verify that the application uses ( opaque OR cryptographically ) signed tokens for session management. Static API secrets and keys should be avoided.

... and it rises the question - why we need to sign opaque tokens,

elarlang avatar Sep 28 '23 17:09 elarlang

Fair point, how about this @elarlang:

# Description L1 L2 L3 CWE NIST §
3.1.3 [MOVED FROM 3.5.2, MODIFIED] Verify that the application uses either cryptographically signed or opaque tokens for session management. Static API secrets and keys should be avoided.

tghosth avatar Oct 01 '23 09:10 tghosth

ok with text. no CWE?

elarlang avatar Oct 01 '23 09:10 elarlang

I cannot find a good CWE match so I would rather just get this in for now.

tghosth avatar Oct 03 '23 12:10 tghosth

Although actually 3.5.2 originally had CWE-798 which is not ideal but it is something.

tghosth avatar Oct 03 '23 12:10 tghosth

Opened #1743 to resolve

tghosth avatar Oct 03 '23 12:10 tghosth

Sorry to rehash this, but I think the original concern was that "Static API secrets and keys should be avoided." would get interpreted as almost a stand alone statement. I think this is a valid concern as while reading this, though I did eventually lean toward "this only applies to client-side sessions", there was some thought of whether or not ASVS was wanting me to ditch static API secrets m2m.

craig-shony avatar Mar 13 '24 15:03 craig-shony

@elarlang what do you think?

tghosth avatar Mar 13 '24 18:03 tghosth

ASVS was wanting me to ditch static API secrets m2m.

There is more discussion about this in 2.10.1 Verify that intra-service secrets do not rely on unchanging credentials such as passwords, API keys · Issue #1032 · OWASP/ASVS.

Sjord avatar Mar 14 '24 08:03 Sjord

@set-reminder 10 minutes Josh to look at this one

tghosth avatar Mar 21 '24 11:03 tghosth

Reminder Thursday, March 21, 2024 1:09 PM (GMT+01:00)

Josh to look at this one

octo-reminder[bot] avatar Mar 21 '24 11:03 octo-reminder[bot]

🔔 @tghosth

Josh to look at this one

octo-reminder[bot] avatar Mar 21 '24 12:03 octo-reminder[bot]

Hi @craig-shony, I agree with @Sjord that the best place to discuss this would be in #1032 where I think we are already deep into this discussion :)

If you don't agree, tag me with some reasoning and I can reopen this

tghosth avatar Apr 03 '24 05:04 tghosth