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

Should 8.2.2 specifically allow for encrypted data #2029

Closed
tghosth opened this issue Aug 27, 2024 · 9 comments
Closed

Should 8.2.2 specifically allow for encrypted data #2029

tghosth opened this issue Aug 27, 2024 · 9 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet Community wanted We would like feedback from the community to guide our decision otherwise we will progress V8 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@tghosth
Copy link
Collaborator

tghosth commented Aug 27, 2024

Jim made the following suggestion for 8.2.2 in this comment:

8.2.2: Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data. If sensitive data must be stored, ensure it is encrypted and integrity-protected, and that it is only stored temporarily with appropriate expiration mechanisms. Exceptions are made for session identifiers, provided their handling follows other guidance in the session management section, including the use of secure cookies and robust protection against cross-site scripting (XSS) attacks.

The current requirement says the following: (the session token aspect is under separate discussion)

Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session tokens which should be stored in either cookies or sessionStorage.

Although I don't think we need to expand the current requirement, do we want to specifically allow for encrypted data to be stored in those mechanisms?

If so, do we also need to talk about key management?

@tghosth tghosth 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 Community wanted We would like feedback from the community to guide our decision otherwise we will progress V8 labels Aug 27, 2024
@elarlang
Copy link
Collaborator

elarlang commented Sep 2, 2024

If sensitive data must be stored, ensure it is encrypted and integrity-protected, ...

Integrity protection does not protect against sensitive data leakage, e. g. sensitive data in JWT. In a way related to #1919.

@jmanico
Copy link
Member

jmanico commented Sep 3, 2024

I agree, this is why I suggest encrypted and integrity-protected

@elarlang
Copy link
Collaborator

elarlang commented Sep 3, 2024

From V8 and sensitive information perspective, only the confidentiality-protection matters.

@tghosth
Copy link
Collaborator Author

tghosth commented Sep 3, 2024

To be fair @elarlang, the beginning of the chapter talks about the full CIA triad:
https://github.com/OWASP/ASVS/blob/master/5.0/en/0x16-V8-Data-Protection.md#control-objective

@elarlang
Copy link
Collaborator

elarlang commented Sep 3, 2024

Well, what problem we are solving here? Pointing to CIA... how talking about availability makes sense for this requirement?

The requirement says, that verify that data stored in the client side does not contain sensitive data. It says quite clearly, what is the problem to solve - no plain-text readable sensitive data to the client side...

@elarlang
Copy link
Collaborator

elarlang commented Sep 3, 2024

Everyones favorite and freshly modified 8.2.2 says:

# Description L1 L2 L3 CWE
8.2.2 [MODIFIED, MERGED FROM 3.2.3] Verify that data stored in browser storage (such as localStorage, sessionStorage, IndexedDB, or cookies) does not contain sensitive data, with the exception of session identifiers. 922

Options:

  • to say, that the data stored in the client-side must not be plain-text recoverable
  • to add "..., with the exception of session identifiers or encrypted data that is not plain-text recoverable" + supporting requirements somewhere else to keep the focus for this requirement only to send the message "no sensitive info to the client side".

@tghosth
Copy link
Collaborator Author

tghosth commented Sep 3, 2024

@jmanico do you think it is critical to allow encrypted data stored in browser storage? To be honest, I feel like I'd rather not add it to the requirement.

@jmanico
Copy link
Member

jmanico commented Sep 4, 2024

If you must store sensitive data in the browser long term, then yes. Even better just don't store sensitive data in the browser long term. I think its way more critical to remove sensitive data via cache headers than it is to store it encrypted.

So I think we are in sync, Josh.

@tghosth
Copy link
Collaborator Author

tghosth commented Sep 4, 2024

Ok so I think we will leave the requirement as it is for now without getting into the question of encryption.

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 Community wanted We would like feedback from the community to guide our decision otherwise we will progress V8 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

3 participants