From 2f7029bc4d373263e3d74e3beaf60279c8b3dd7a Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Tue, 30 Apr 2024 16:10:26 -0400 Subject: [PATCH] Expose detector-specific false positive logic (#2743) This PR: Creates an optional interface that detectors can use to customize their false positive detection Implements this interface on detectors that have custom logic In most cases this "custom logic" is simply a no-op because the detector does not participate in false positive detection Eliminates inline (old-style) false positive exclusion in a few detectors that #2643 missed --- pkg/custom_detectors/custom_detectors.go | 5 ++ pkg/detectors/azurebatch/azurebatch.go | 5 ++ .../azurecontainerregistry.go | 5 ++ pkg/detectors/falsepositives.go | 50 ++++++++--------- pkg/detectors/falsepositives_test.go | 53 +++++++++++++++++++ pkg/detectors/ftp/ftp.go | 9 ++-- pkg/detectors/gcp/gcp.go | 5 ++ .../gcpapplicationdefaultcredentials.go | 5 -- pkg/detectors/generic/generic.go | 4 -- pkg/detectors/github/v1/github_old.go | 2 + pkg/detectors/github/v2/github.go | 3 -- pkg/detectors/jdbc/jdbc.go | 7 ++- pkg/detectors/jupiterone/jupiterone.go | 5 -- pkg/detectors/mongodb/mongodb.go | 5 ++ pkg/detectors/onfleet/onfleet.go | 5 -- pkg/detectors/pagarme/pagarme.go | 5 -- pkg/detectors/postgres/postgres.go | 7 +++ pkg/detectors/shopify/shopify.go | 8 ++- pkg/detectors/uri/uri.go | 8 ++- pkg/detectors/wiz/wiz.go | 8 --- pkg/engine/engine.go | 2 +- 21 files changed, 135 insertions(+), 71 deletions(-) diff --git a/pkg/custom_detectors/custom_detectors.go b/pkg/custom_detectors/custom_detectors.go index 4e6b8feea..c914caa26 100644 --- a/pkg/custom_detectors/custom_detectors.go +++ b/pkg/custom_detectors/custom_detectors.go @@ -29,6 +29,7 @@ type CustomRegexWebhook struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*CustomRegexWebhook)(nil) +var _ detectors.CustomFalsePositiveChecker = (*CustomRegexWebhook)(nil) // NewWebhookCustomRegex initializes and validates a CustomRegexWebhook. An // unexported type is intentionally returned here to ensure the values have @@ -109,6 +110,10 @@ func (c *CustomRegexWebhook) FromData(ctx context.Context, verify bool, data []b return results, nil } +func (c *CustomRegexWebhook) IsFalsePositive(_ detectors.Result) bool { + return false +} + func (c *CustomRegexWebhook) createResults(ctx context.Context, match map[string][]string, verify bool, results chan<- detectors.Result) error { if common.IsDone(ctx) { // TODO: Log we're possibly leaving out results. diff --git a/pkg/detectors/azurebatch/azurebatch.go b/pkg/detectors/azurebatch/azurebatch.go index f04b907e3..bd9e1936f 100644 --- a/pkg/detectors/azurebatch/azurebatch.go +++ b/pkg/detectors/azurebatch/azurebatch.go @@ -23,6 +23,7 @@ type Scanner struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( defaultClient = common.SaneHttpClient() @@ -105,6 +106,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return results, nil } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_AzureBatch } diff --git a/pkg/detectors/azurecontainerregistry/azurecontainerregistry.go b/pkg/detectors/azurecontainerregistry/azurecontainerregistry.go index b0651ab92..55c0f4b3f 100644 --- a/pkg/detectors/azurecontainerregistry/azurecontainerregistry.go +++ b/pkg/detectors/azurecontainerregistry/azurecontainerregistry.go @@ -19,6 +19,7 @@ type Scanner struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( defaultClient = common.SaneHttpClient() @@ -94,6 +95,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return results, nil } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_AzureContainerRegistry } diff --git a/pkg/detectors/falsepositives.go b/pkg/detectors/falsepositives.go index 29ce2e7c8..1b3594a6d 100644 --- a/pkg/detectors/falsepositives.go +++ b/pkg/detectors/falsepositives.go @@ -8,8 +8,6 @@ import ( "unicode/utf8" ahocorasick "github.com/BobuSumisu/aho-corasick" - "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" - "github.com/trufflesecurity/trufflehog/v3/pkg/context" ) @@ -17,6 +15,10 @@ var DefaultFalsePositives = []FalsePositive{"example", "xxxxxx", "aaaaaa", "abcd type FalsePositive string +type CustomFalsePositiveChecker interface { + IsFalsePositive(result Result) bool +} + //go:embed "badlist.txt" var badList []byte @@ -43,6 +45,17 @@ func init() { filter = builder.Build() } +func GetFalsePositiveCheck(detector Detector) func(Result) bool { + checker, ok := detector.(CustomFalsePositiveChecker) + if ok { + return checker.IsFalsePositive + } + + return func(res Result) bool { + return IsKnownFalsePositive(string(res.Raw), DefaultFalsePositives, true) + } +} + // IsKnownFalsePositive will not return a valid secret finding if any of the disqualifying conditions are met // Currently that includes: No number, english word in key, or matches common example pattens. // Only the secret key material should be passed into this function @@ -132,34 +145,17 @@ func FilterResultsWithEntropy(ctx context.Context, results []Result, entropy flo } // FilterKnownFalsePositives filters out known false positives from the results. -func FilterKnownFalsePositives(ctx context.Context, results []Result, falsePositives []FalsePositive, wordCheck bool, shouldLog bool) []Result { +func FilterKnownFalsePositives(ctx context.Context, detector Detector, results []Result, shouldLog bool) []Result { var filteredResults []Result + + isFalsePositive := GetFalsePositiveCheck(detector) + for _, result := range results { - if !result.Verified { - switch result.DetectorType { - case detectorspb.DetectorType_CustomRegex: + if !result.Verified && result.Raw != nil { + if !isFalsePositive(result) { filteredResults = append(filteredResults, result) - case detectorspb.DetectorType_GCP, - detectorspb.DetectorType_URI, - detectorspb.DetectorType_AzureBatch, - detectorspb.DetectorType_AzureContainerRegistry, - detectorspb.DetectorType_Shopify, - detectorspb.DetectorType_Postgres, - detectorspb.DetectorType_MongoDB, - detectorspb.DetectorType_JDBC: - filteredResults = append(filteredResults, result) - default: - if result.Raw != nil { - if !IsKnownFalsePositive(string(result.Raw), falsePositives, wordCheck) { - filteredResults = append(filteredResults, result) - } else { - if shouldLog { - ctx.Logger().Info("Filtered out known false positive", "result", result) - } - } - } else { - filteredResults = append(filteredResults, result) - } + } else if shouldLog { + ctx.Logger().Info("Filtered out known false positive", "result", result) } } else { filteredResults = append(filteredResults, result) diff --git a/pkg/detectors/falsepositives_test.go b/pkg/detectors/falsepositives_test.go index 216ce87b9..e4222373f 100644 --- a/pkg/detectors/falsepositives_test.go +++ b/pkg/detectors/falsepositives_test.go @@ -4,10 +4,63 @@ package detectors import ( + "context" _ "embed" "testing" + + "github.com/stretchr/testify/assert" + logContext "github.com/trufflesecurity/trufflehog/v3/pkg/context" + "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) +type fakeDetector struct{} +type customFalsePositiveChecker struct{ fakeDetector } + +func (d fakeDetector) FromData(ctx context.Context, verify bool, data []byte) ([]Result, error) { + return nil, nil +} + +func (d fakeDetector) Keywords() []string { + return nil +} + +func (d fakeDetector) Type() detectorspb.DetectorType { + return detectorspb.DetectorType(0) +} + +func (d customFalsePositiveChecker) IsFalsePositive(result Result) bool { + return IsKnownFalsePositive(string(result.Raw), []FalsePositive{"a specific magic string"}, false) +} + +func TestFilterKnownFalsePositives_DefaultLogic(t *testing.T) { + results := []Result{ + {Raw: []byte("00000")}, // "default" false positive list + {Raw: []byte("number")}, // from wordlist + {Raw: []byte("hga8adshla3434g")}, // real secret + } + expected := []Result{ + {Raw: []byte("hga8adshla3434g")}, + } + filtered := FilterKnownFalsePositives(logContext.Background(), fakeDetector{}, results, false) + assert.ElementsMatch(t, expected, filtered) +} + +func TestFilterKnownFalsePositives_CustomLogic(t *testing.T) { + results := []Result{ + {Raw: []byte("a specific magic string")}, // specific target + {Raw: []byte("00000")}, // "default" false positive list + {Raw: []byte("number")}, // from wordlist + {Raw: []byte("hga8adshla3434g")}, // real secret + } + expected := []Result{ + {Raw: []byte("00000")}, + {Raw: []byte("number")}, + {Raw: []byte("hga8adshla3434g")}, + } + filtered := FilterKnownFalsePositives(logContext.Background(), customFalsePositiveChecker{}, results, false) + assert.ElementsMatch(t, expected, filtered) +} + func TestIsFalsePositive(t *testing.T) { type args struct { match string diff --git a/pkg/detectors/ftp/ftp.go b/pkg/detectors/ftp/ftp.go index 4a76d2441..8a689f52b 100644 --- a/pkg/detectors/ftp/ftp.go +++ b/pkg/detectors/ftp/ftp.go @@ -30,6 +30,7 @@ type Scanner struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( keyPat = regexp.MustCompile(`\bftp://[\S]{3,50}:([\S]{3,50})@[-.%\w\/:]+\b`) @@ -96,16 +97,16 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } } - if detectors.IsKnownFalsePositive(string(s1.Raw), []detectors.FalsePositive{"@ftp.freebsd.org"}, false) { - continue - } - results = append(results, s1) } return results, nil } +func (s Scanner) IsFalsePositive(result detectors.Result) bool { + return detectors.IsKnownFalsePositive(string(result.Raw), []detectors.FalsePositive{"@ftp.freebsd.org"}, false) +} + func isErrDeterminate(e error) bool { ftpErr := &textproto.Error{} return errors.As(e, &ftpErr) && ftpErr.Code == ftpNotLoggedIn diff --git a/pkg/detectors/gcp/gcp.go b/pkg/detectors/gcp/gcp.go index 6d1993e62..6a2b3e1da 100644 --- a/pkg/detectors/gcp/gcp.go +++ b/pkg/detectors/gcp/gcp.go @@ -18,6 +18,7 @@ type Scanner struct{} // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( keyPat = regexp.MustCompile(`\{[^{]+auth_provider_x509_cert_url[^}]+\}`) @@ -120,6 +121,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_GCP } diff --git a/pkg/detectors/gcpapplicationdefaultcredentials/gcpapplicationdefaultcredentials.go b/pkg/detectors/gcpapplicationdefaultcredentials/gcpapplicationdefaultcredentials.go index eeecab957..debe41fa6 100644 --- a/pkg/detectors/gcpapplicationdefaultcredentials/gcpapplicationdefaultcredentials.go +++ b/pkg/detectors/gcpapplicationdefaultcredentials/gcpapplicationdefaultcredentials.go @@ -82,11 +82,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1.SetVerificationError(verificationErr, match) } - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if !s1.Verified && detectors.IsKnownFalsePositive(string(s1.Raw), detectors.DefaultFalsePositives, true) { - continue - } - results = append(results, s1) } diff --git a/pkg/detectors/generic/generic.go b/pkg/detectors/generic/generic.go index 4037a15aa..327a139d1 100644 --- a/pkg/detectors/generic/generic.go +++ b/pkg/detectors/generic/generic.go @@ -75,10 +75,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result // Least expensive-> most expensive filters. // Substrings, then patterns. - if detectors.IsKnownFalsePositive(token, detectors.DefaultFalsePositives, true) { - continue - } - // toss any that match regexes if hasReMatch(s.excludeMatchers, token) { continue diff --git a/pkg/detectors/github/v1/github_old.go b/pkg/detectors/github/v1/github_old.go index 6f7b56d26..3ade673cf 100644 --- a/pkg/detectors/github/v1/github_old.go +++ b/pkg/detectors/github/v1/github_old.go @@ -69,6 +69,8 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result token := match[1] + // Note that this false positive check happens **before** verification! I don't know why it's written this way + // but that's why this logic wasn't moved into a CustomFalsePositiveChecker implementation. specificFPs := []detectors.FalsePositive{"github commit"} if detectors.IsKnownFalsePositive(token, specificFPs, false) { continue diff --git a/pkg/detectors/github/v2/github.go b/pkg/detectors/github/v2/github.go index 912c17a20..2c21a3ce4 100644 --- a/pkg/detectors/github/v2/github.go +++ b/pkg/detectors/github/v2/github.go @@ -83,9 +83,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } } - if !s1.Verified && detectors.IsKnownFalsePositive(string(s1.Raw), detectors.DefaultFalsePositives, true) { - continue - } results = append(results, s1) } diff --git a/pkg/detectors/jdbc/jdbc.go b/pkg/detectors/jdbc/jdbc.go index cc28db559..4b6cee99f 100644 --- a/pkg/detectors/jdbc/jdbc.go +++ b/pkg/detectors/jdbc/jdbc.go @@ -45,6 +45,7 @@ func WithIgnorePattern(ignoreStrings []string) func(*Scanner) { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( keyPat = regexp.MustCompile(`(?i)jdbc:[\w]{3,10}:[^\s"']{0,512}`) @@ -98,14 +99,16 @@ matchLoop: // TODO: specialized redaction } - - results = append(results, s) } return } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func tryRedactAnonymousJDBC(conn string) string { if s, ok := tryRedactURLParams(conn); ok { return s diff --git a/pkg/detectors/jupiterone/jupiterone.go b/pkg/detectors/jupiterone/jupiterone.go index 2e1d0eaf0..2bf322bb1 100644 --- a/pkg/detectors/jupiterone/jupiterone.go +++ b/pkg/detectors/jupiterone/jupiterone.go @@ -83,11 +83,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } } - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if !s1.Verified && detectors.IsKnownFalsePositive(resMatch, detectors.DefaultFalsePositives, true) { - continue - } - results = append(results, s1) } diff --git a/pkg/detectors/mongodb/mongodb.go b/pkg/detectors/mongodb/mongodb.go index 6e9a8ea65..bb48b1a40 100644 --- a/pkg/detectors/mongodb/mongodb.go +++ b/pkg/detectors/mongodb/mongodb.go @@ -24,6 +24,7 @@ type Scanner struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( defaultTimeout = 2 * time.Second @@ -72,6 +73,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return results, nil } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func isErrDeterminate(err error) bool { switch e := err.(type) { case topology.ConnectionError: diff --git a/pkg/detectors/onfleet/onfleet.go b/pkg/detectors/onfleet/onfleet.go index cb1060b0d..5e348345f 100644 --- a/pkg/detectors/onfleet/onfleet.go +++ b/pkg/detectors/onfleet/onfleet.go @@ -60,11 +60,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1.SetVerificationError(verificationErr, match) } - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if !s1.Verified && detectors.IsKnownFalsePositive(match, detectors.DefaultFalsePositives, true) { - continue - } - results = append(results, s1) } diff --git a/pkg/detectors/pagarme/pagarme.go b/pkg/detectors/pagarme/pagarme.go index 5cc5be351..af1fca76e 100644 --- a/pkg/detectors/pagarme/pagarme.go +++ b/pkg/detectors/pagarme/pagarme.go @@ -60,11 +60,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1.SetVerificationError(verificationErr, match) } - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if !s1.Verified && detectors.IsKnownFalsePositive(match, detectors.DefaultFalsePositives, true) { - continue - } - results = append(results, s1) } diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 09ec98d92..4f1a5b8e3 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -55,6 +55,9 @@ type Scanner struct { detectLoopback bool // Automated tests run against localhost, but we want to ignore those results in the wild } +var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) + func (s Scanner) Keywords() []string { return []string{"postgres"} } @@ -144,6 +147,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete return results, nil } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func findUriMatches(data []byte) []map[string]string { var matches []map[string]string for _, uri := range uriPattern.FindAll(data, -1) { diff --git a/pkg/detectors/shopify/shopify.go b/pkg/detectors/shopify/shopify.go index 187889f17..115d3d4df 100644 --- a/pkg/detectors/shopify/shopify.go +++ b/pkg/detectors/shopify/shopify.go @@ -3,10 +3,11 @@ package shopify import ( "context" "encoding/json" - regexp "github.com/wasilibs/go-re2" "net/http" "strings" + regexp "github.com/wasilibs/go-re2" + "github.com/trufflesecurity/trufflehog/v3/pkg/common" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" @@ -16,6 +17,7 @@ type Scanner struct{} // Ensure the Scanner satisfies the interface at compile time var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( client = common.SaneHttpClient() @@ -87,6 +89,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + type shopifyTokenAccessScopes struct { AccessScopes []struct { Handle string `json:"handle"` diff --git a/pkg/detectors/uri/uri.go b/pkg/detectors/uri/uri.go index 33d63ad77..8015b0f21 100644 --- a/pkg/detectors/uri/uri.go +++ b/pkg/detectors/uri/uri.go @@ -2,12 +2,13 @@ package uri import ( "context" - regexp "github.com/wasilibs/go-re2" "net/http" "net/url" "strings" "time" + regexp "github.com/wasilibs/go-re2" + "github.com/trufflesecurity/trufflehog/v3/pkg/common" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" @@ -20,6 +21,7 @@ type Scanner struct { // Ensure the Scanner satisfies the interface at compile time. var _ detectors.Detector = (*Scanner)(nil) +var _ detectors.CustomFalsePositiveChecker = (*Scanner)(nil) var ( keyPat = regexp.MustCompile(`\b(?:https?:)?\/\/[\S]{3,50}:([\S]{3,50})@[-.%\w\/:]+\b`) @@ -101,6 +103,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result return results, nil } +func (s Scanner) IsFalsePositive(_ detectors.Result) bool { + return false +} + func verifyURL(ctx context.Context, client *http.Client, u *url.URL) (bool, error) { // defuse most SSRF payloads u.Path = strings.TrimSuffix(u.Path, "/") diff --git a/pkg/detectors/wiz/wiz.go b/pkg/detectors/wiz/wiz.go index e5bde3842..c0de255cf 100644 --- a/pkg/detectors/wiz/wiz.go +++ b/pkg/detectors/wiz/wiz.go @@ -69,14 +69,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result s1.SetVerificationError(verificationErr, idMatch, secretMatch) } - // This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key. - if !s1.Verified { - if detectors.IsKnownFalsePositive(idMatch, detectors.DefaultFalsePositives, true) || - detectors.IsKnownFalsePositive(secretMatch, detectors.DefaultFalsePositives, true) { - continue - } - } - results = append(results, s1) // If we've found a verified match with this ID, we don't need to look for anymore. So move on to the next ID. diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 22cc18686..1798a2467 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -831,7 +831,7 @@ func (e *Engine) detectChunk(ctx context.Context, data detectableChunk) { results = detectors.CleanResults(results) } - results = detectors.FilterKnownFalsePositives(ctx, results, detectors.DefaultFalsePositives, true, e.logFilteredUnverified) + results = detectors.FilterKnownFalsePositives(ctx, data.detector, results, e.logFilteredUnverified) if e.filterEntropy != nil { results = detectors.FilterResultsWithEntropy(ctx, results, *e.filterEntropy, e.logFilteredUnverified)