Skip to content

chore: add security logging and cwe fields#10256

Merged
crenshaw-dev merged 3 commits intoargoproj:masterfrom
notfromstatefarm:chore/security-logging
Aug 17, 2022
Merged

chore: add security logging and cwe fields#10256
crenshaw-dev merged 3 commits intoargoproj:masterfrom
notfromstatefarm:chore/security-logging

Conversation

@notfromstatefarm
Copy link
Contributor

@notfromstatefarm notfromstatefarm commented Aug 9, 2022

As discussed with the security SIG, this PR implements a security and cwe field to be used in logs and standardizes which level should be used. Obviously there is much more work to be done as I have only added this to a couple of logs, but this lays the foundations so that hopefully we can cover a good amount of ground in separate PRs by the v2.5 release.

This will be 'good first issue' heaven. :shipit:

Proposed levels:

Level Friendly Level Description Example
1 Low Unexceptional, non-malicious events Successful access
2 Medium Could indicate malicious events, but has a high likelihood of being user/system error Access denied
3 High Likely malicious events but one that had no side effects or was blocked Out of bounds symlinks in repo
4 Critical Any malicious or exploitable event that had a side effect Secrets being left behind on the filesystem
5 Emergency Unmistakably malicious events that should NEVER occur accidentally and indicates an active attack Brute forcing of accounts

credit to @crenshaw-dev for the idea!

cc @crenshaw-dev @jessesuen @todaywasawesome

Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
@notfromstatefarm notfromstatefarm changed the title chore: add security logging and cve fields chore: add security logging and cwe fields Aug 9, 2022
@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Attention: Patch coverage is 33.33333% with 16 lines in your changes missing coverage. Please review.

Project coverage is 46.16%. Comparing base (af40d52) to head (6ded9d3).
Report is 2654 commits behind head on master.

Files Patch % Lines
util/gpg/gpg.go 0.00% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10256      +/-   ##
==========================================
- Coverage   46.18%   46.16%   -0.02%     
==========================================
  Files         226      226              
  Lines       27581    27595      +14     
==========================================
+ Hits        12737    12739       +2     
- Misses      13124    13136      +12     
  Partials     1720     1720              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of the changes as presented. Thanks @notfromstatefarm!

@notfromstatefarm
Copy link
Contributor Author

notfromstatefarm commented Aug 11, 2022

Here's a flowchart on how to decide which level to use. Do you think we should include this in the docs @crenshaw-dev ?
ArgoCD Security Log Flowchart

@crenshaw-dev
Copy link
Member

@notfromstatefarm I like that! The phrase "has a vulnerability been exposed" seems slightly ambiguous to me though. I'm not sure whether there's a succinct way to clarify.

@crenshaw-dev
Copy link
Member

Ariana Grande singing "I see it, I like it, I want it, I got it"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants