Skip to content

Commit

Permalink
Expose detector-specific false positive logic (#2743)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rosecodym authored Apr 30, 2024
1 parent dc930f9 commit 2f7029b
Show file tree
Hide file tree
Showing 21 changed files with 135 additions and 71 deletions.
5 changes: 5 additions & 0 deletions pkg/custom_detectors/custom_detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions pkg/detectors/azurebatch/azurebatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
50 changes: 23 additions & 27 deletions pkg/detectors/falsepositives.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@ import (
"unicode/utf8"

ahocorasick "github.com/BobuSumisu/aho-corasick"
"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb"

"github.com/trufflesecurity/trufflehog/v3/pkg/context"
)

var DefaultFalsePositives = []FalsePositive{"example", "xxxxxx", "aaaaaa", "abcde", "00000", "sample", "www"}

type FalsePositive string

type CustomFalsePositiveChecker interface {
IsFalsePositive(result Result) bool
}

//go:embed "badlist.txt"
var badList []byte

Expand All @@ -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
Expand Down Expand Up @@ -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:
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:
if !result.Verified && result.Raw != nil {
if !isFalsePositive(result) {
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)
Expand Down
53 changes: 53 additions & 0 deletions pkg/detectors/falsepositives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions pkg/detectors/ftp/ftp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/detectors/gcp/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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[^}]+\}`)
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/detectors/generic/generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/detectors/github/v1/github_old.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions pkg/detectors/github/v2/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/detectors/jdbc/jdbc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions pkg/detectors/jupiterone/jupiterone.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/detectors/mongodb/mongodb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 0 additions & 5 deletions pkg/detectors/onfleet/onfleet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/detectors/pagarme/pagarme.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
7 changes: 7 additions & 0 deletions pkg/detectors/postgres/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
}
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 2f7029b

Please sign in to comment.