search

OWASP / CheatSheetSeries

The OWASP Cheat Sheet Series was created to provide a concise collection of high value information on specific application security topics.
https://cheatsheetseries.owasp.org
Creative Commons Attribution Share Alike 4.0 International
27.15k stars 3.8k forks source link

Update: Section 2.5 of cheatsheets/Secrets_Management_Cheat_Sheet.md #1265

Closed kwwall closed 4 months ago

kwwall commented 6 months ago

What is missing or needs to be updated?

In https://github.com/OWASP/CheatSheetSeries/pull/1257#issuecomment-1848220253, I had previously noted some additional content that I wanted to see made to section 2.5 of the Secrets Management Cheat Sheet where I specifically proposed adding some verbiage about doing prior threat modeling on this to see if attempting to protect secrets in memory is a beneficial endeavor for a developer's specific use case.

As it is currently presented, I think section 2.5 doesn't make it clear to developers lacking experience in security how nuanced this problem really is. For instance, many developers were taught that the operating system is sufficient to isolate process memory, but attacks like Rowhammer, Spectre, and Meltdown demonstrate this is not the case. Doing a quick and dirty threat model with one's security team ultimately seems better than wasting time on fixing the unfixable or worse, getting a false sense of security by missing threats that adversaries can now use, especially in cloud-based hosting environments.

How should this be resolved?

I have made relatively minor revisions to section 2.5 to try to make these things more obvious. It's not perfect, and certainly not as succinct as I'd like, but I thought I would at least get the ball rolling and see what comes of it.

I therefore am submitting a PR to address this issue.

szh commented 6 months ago

I added some grammar nits though it's already merged.

kwwall commented 6 months ago

@szh - That's why I allowed reviewers to make direct edits to this PR, so you could directly edit your "nits". I trust all of you to do the right thing.

That said, maybe we should require 2 people to approve a PR. It doesn't slow things down a bit, but it also prevents rework. However, in hindsight, who would like to create a new PR to apply Shlomo's suggested changes? I thought @jmanico volunteered (see https://github.com/OWASP/CheatSheetSeries/pull/1266#discussion_r1436575706), but given some of them were contextual, I will do it if it is easier. Just let me know.

jmanico commented 6 months ago

Thanks Kevin, go for it

szh commented 6 months ago

I think requiring two reviewers is a good idea, or we could also try to make a norm of waiting a day or two before merging anything consequential to give everyone time to comment.

mackowski commented 4 months ago

@jmanico @szh @kwwall I changed the setting so PRs now require two reviews :)

jmanico commented 4 months ago

Booooo. Damn you Kevin! 😁