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

chore: add security logging and cwe fields #10256

Merged
merged 3 commits into from Aug 17, 2022

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

Merging #10256 (6ded9d3) into master (af40d52) will decrease coverage by 0.01%.
The diff coverage is 33.33%.

@@            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              
Impacted Files Coverage Δ
util/gpg/gpg.go 64.65% <0.00%> (-2.31%) ⬇️
reposerver/repository/repository.go 64.37% <100.00%> (+0.06%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

common/common.go Outdated Show resolved Hide resolved
docs/operator-manual/security.md Outdated Show resolved Hide resolved
Signed-off-by: notfromstatefarm <86763948+notfromstatefarm@users.noreply.github.com>
Copy link
Collaborator

@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
Collaborator

@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
Collaborator

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.

None yet

2 participants