-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[feat] - cache duplicate creds during detection #2276
Draft
ahrav
wants to merge
32
commits into
main
Choose a base branch
from
feat-cache-dupe-creds
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
f2f62ab
Extend memory cache to allow for configuring custom expiration and pu…
ahrav 9c016d7
use any for value type
ahrav e6685c7
fix test
ahrav 6a4eabd
fix test
ahrav 5dc03be
cache results to prevent multiple network calls using the same creds
ahrav 82348f9
use custom value
ahrav 39b1642
rename field
ahrav 4af9af3
use ptr for the cacheItem
ahrav e4b465d
use constructor
ahrav dd9b6ea
remove nil check
ahrav f45bb31
add method to popualte result from the cache item
ahrav f286c33
update comment
ahrav c4162e2
missed a ptr
ahrav 318eebb
add test
ahrav c1a330d
rename
ahrav 8ceef69
address comments
ahrav 5609ca3
address
ahrav 47916e3
make new construct more clear
ahrav 32b75c2
cache results to prevent multiple network calls using the same creds
ahrav 5605ad0
use custom value
ahrav 6c6d58e
rename field
ahrav 72c9722
use ptr for the cacheItem
ahrav eee751f
use constructor
ahrav a08343f
remove nil check
ahrav 0a4155b
add method to popualte result from the cache item
ahrav dac2eb1
update comment
ahrav a8703a2
missed a ptr
ahrav 06f99da
add test
ahrav a586b5f
rename
ahrav 6199dee
Merge branch 'feat-cache-dupe-creds' of github.com:trufflesecurity/tr…
ahrav fae7617
merge main
ahrav 5faf556
merge main
ahrav File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ import ( | |
"strings" | ||
"time" | ||
|
||
"github.com/trufflesecurity/trufflehog/v3/pkg/cache" | ||
"github.com/trufflesecurity/trufflehog/v3/pkg/cache/memory" | ||
"github.com/trufflesecurity/trufflehog/v3/pkg/common" | ||
"github.com/trufflesecurity/trufflehog/v3/pkg/detectors" | ||
"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" | ||
|
@@ -20,6 +22,8 @@ import ( | |
type scanner struct { | ||
verificationClient *http.Client | ||
skipIDs map[string]struct{} | ||
|
||
credsCache cache.Cache | ||
} | ||
|
||
// resourceTypes derived from: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html#identifiers-unique-ids | ||
|
@@ -47,6 +51,11 @@ func New(opts ...func(*scanner)) *scanner { | |
opt(scanner) | ||
} | ||
|
||
scanner.credsCache = memory.New( | ||
memory.WithExpirationInterval(1*time.Hour), | ||
memory.WithPurgeInterval(2*time.Hour), | ||
) | ||
|
||
return scanner | ||
} | ||
|
||
|
@@ -101,6 +110,26 @@ func GetHMAC(key []byte, data []byte) []byte { | |
return hasher.Sum(nil) | ||
} | ||
|
||
// cacheItem represents an item stored in the cache, encompassing the outcome of a verification process. | ||
// It includes the verification result, ExtraData, and VerificationErrors encountered during verification. | ||
// This facilitates the reconstruction of detectors.Result with values for previously verified creds. | ||
type cacheItem struct { | ||
isVerified bool | ||
verificationErr error | ||
extra map[string]string | ||
} | ||
|
||
func newCacheItem(isVerified bool, verificationErr error, extra map[string]string) *cacheItem { | ||
return &cacheItem{isVerified, verificationErr, extra} | ||
} | ||
|
||
// populateResult populates the given detectors.Result with the values from the cacheItem. | ||
func (c *cacheItem) populateResult(result *detectors.Result) { | ||
result.Verified = c.isVerified | ||
result.ExtraData = c.extra | ||
result.SetVerificationError(c.verificationErr) | ||
} | ||
|
||
// FromData will find and optionally verify AWS secrets in a given set of bytes. | ||
func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (results []detectors.Result, err error) { | ||
dataStr := string(data) | ||
|
@@ -126,16 +155,28 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result | |
} | ||
resSecretMatch := strings.TrimSpace(secretMatch[1]) | ||
|
||
rawV2 := resIDMatch + resSecretMatch | ||
|
||
s1 := detectors.Result{ | ||
DetectorType: detectorspb.DetectorType_AWS, | ||
Raw: []byte(resIDMatch), | ||
Redacted: resIDMatch, | ||
RawV2: []byte(resIDMatch + resSecretMatch), | ||
RawV2: []byte(rawV2), | ||
ExtraData: map[string]string{ | ||
"resource_type": resourceTypes[idMatch[2]], | ||
}, | ||
} | ||
|
||
if val, ok := s.credsCache.Get(rawV2); ok { | ||
item, ok := val.(*cacheItem) | ||
if !ok { | ||
continue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is the right behavior, i'd like to maybe log this as an error? Not sure if constructing our custom |
||
} | ||
item.populateResult(&s1) | ||
results = append(results, s1) | ||
continue | ||
} | ||
|
||
if verify { | ||
isVerified, extraData, verificationErr := s.verifyMatch(ctx, resIDMatch, resSecretMatch, true) | ||
s1.Verified = isVerified | ||
|
@@ -153,6 +194,8 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result | |
} | ||
} | ||
|
||
// Cache the result. | ||
s.credsCache.Set(rawV2, newCacheItem(s1.Verified, s1.VerificationError(), s1.ExtraData)) | ||
if !s1.Verified { | ||
// Unverified results that contain common test words are probably not secrets | ||
if detectors.IsKnownFalsePositive(resSecretMatch, detectors.DefaultFalsePositives, true) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just placeholders for now, we could consider passing these in as args to
New
, or use a default value that seems reasonable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How big could the cache conceivably get in a given time frame?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question. It would really depend on what was being scanned. We can run some tests to scan some repos and record some metrics to see what the size of this cache grows to be. Alternatively, we could use a LRU cache where we fix the size if needed.