diff --git a/pkg/detectors/account_filter.go b/pkg/detectors/account_filter.go new file mode 100644 index 000000000..832bf1f76 --- /dev/null +++ b/pkg/detectors/account_filter.go @@ -0,0 +1,79 @@ +package detectors + +// AccountFilter implements account-based filtering functionality that detectors can embed +// to gain allow and deny list capabilities for account IDs. +type AccountFilter struct { + allowedAccounts map[string]struct{} + deniedAccounts map[string]struct{} +} + +// SetAllowedAccounts configures the allowed account IDs. +// If set, only accounts in this list will be verified. +func (a *AccountFilter) SetAllowedAccounts(accountIDs []string) { + if len(accountIDs) == 0 { + a.allowedAccounts = nil + return + } + + accounts := make(map[string]struct{}, len(accountIDs)) + for _, accountID := range accountIDs { + accounts[accountID] = struct{}{} + } + a.allowedAccounts = accounts +} + +// SetDeniedAccounts configures the denied account IDs. +// Accounts in this list will never be verified. +func (a *AccountFilter) SetDeniedAccounts(accountIDs []string) { + if len(accountIDs) == 0 { + a.deniedAccounts = nil + return + } + + accounts := make(map[string]struct{}, len(accountIDs)) + for _, accountID := range accountIDs { + accounts[accountID] = struct{}{} + } + a.deniedAccounts = accounts +} + +// ShouldSkipAccount checks if an account ID should be skipped for verification +// based on allow and deny lists. +// +// Precedence: deny list > allow list (if account is in both, it's denied) +func (a *AccountFilter) ShouldSkipAccount(accountID string) bool { + // Check deny list first - takes precedence + if len(a.deniedAccounts) > 0 { + if _, isDenied := a.deniedAccounts[accountID]; isDenied { + return true + } + } + + // Check allow list - if populated, account must be in it + if len(a.allowedAccounts) > 0 { + if _, isAllowed := a.allowedAccounts[accountID]; !isAllowed { + return true + } + } + + // Account is allowed for verification + return false +} + +// IsInDenyList checks if an account ID is in the deny list +func (a *AccountFilter) IsInDenyList(accountID string) bool { + if len(a.deniedAccounts) == 0 { + return false + } + _, isDenied := a.deniedAccounts[accountID] + return isDenied +} + +// IsInAllowList checks if an account ID is in the allow list +func (a *AccountFilter) IsInAllowList(accountID string) bool { + if len(a.allowedAccounts) == 0 { + return false + } + _, isAllowed := a.allowedAccounts[accountID] + return isAllowed +} diff --git a/pkg/detectors/account_filter_test.go b/pkg/detectors/account_filter_test.go new file mode 100644 index 000000000..a5a6a4c19 --- /dev/null +++ b/pkg/detectors/account_filter_test.go @@ -0,0 +1,112 @@ +package detectors + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEmbeddedAccountFilter(t *testing.T) { + type Scanner struct{ AccountFilter } + + t.Run("no filtering configured - should not skip", func(t *testing.T) { + var s Scanner // Fresh instance for this test + + shouldSkip := s.ShouldSkipAccount("test-account") + assert.False(t, shouldSkip) + assert.False(t, s.IsInDenyList("test-account")) + assert.False(t, s.IsInAllowList("test-account")) + }) + + t.Run("allowed accounts only", func(t *testing.T) { + var s Scanner // Fresh instance for this test + s.SetAllowedAccounts([]string{"allowed-account-1", "allowed-account-2"}) + + // Account in allow list - should not skip + shouldSkip := s.ShouldSkipAccount("allowed-account-1") + assert.False(t, shouldSkip) + assert.True(t, s.IsInAllowList("allowed-account-1")) + + // Account not in allow list - should skip + shouldSkip = s.ShouldSkipAccount("other-account") + assert.True(t, shouldSkip) + assert.False(t, s.IsInAllowList("other-account")) + }) + + t.Run("denied accounts only", func(t *testing.T) { + var s Scanner // Fresh instance for this test + s.SetDeniedAccounts([]string{"denied-account-1", "denied-account-2"}) + + // Account in deny list - should skip + shouldSkip := s.ShouldSkipAccount("denied-account-1") + assert.True(t, shouldSkip) + assert.True(t, s.IsInDenyList("denied-account-1")) + + // Account not in deny list - should not skip (no allow list restrictions) + shouldSkip = s.ShouldSkipAccount("other-account") + assert.False(t, shouldSkip) + assert.False(t, s.IsInDenyList("other-account")) + }) + + t.Run("deny list takes precedence over allow list", func(t *testing.T) { + var s Scanner // Fresh instance for this test + s.SetAllowedAccounts([]string{"conflicted-account", "allowed-only-account"}) + s.SetDeniedAccounts([]string{"conflicted-account"}) // Same account in both lists + + // Account is in both allow and deny lists - deny takes precedence + shouldSkip := s.ShouldSkipAccount("conflicted-account") + assert.True(t, shouldSkip) + assert.True(t, s.IsInDenyList("conflicted-account")) + assert.True(t, s.IsInAllowList("conflicted-account")) + + // Account only in allow list - should not skip + shouldSkip = s.ShouldSkipAccount("allowed-only-account") + assert.False(t, shouldSkip) + assert.False(t, s.IsInDenyList("allowed-only-account")) + assert.True(t, s.IsInAllowList("allowed-only-account")) + }) + + t.Run("allow list with denied account not in allow list", func(t *testing.T) { + var s Scanner // Fresh instance for this test + s.SetAllowedAccounts([]string{"trusted-account"}) // Allow one account + s.SetDeniedAccounts([]string{"blocked-account"}) // Deny different account + + // Account in deny list (not in allow list) - should skip due to deny list + shouldSkip := s.ShouldSkipAccount("blocked-account") + assert.True(t, shouldSkip) + assert.True(t, s.IsInDenyList("blocked-account")) + assert.False(t, s.IsInAllowList("blocked-account")) + + // Account in allow list (not in deny list) - should not skip + shouldSkip = s.ShouldSkipAccount("trusted-account") + assert.False(t, shouldSkip) + assert.False(t, s.IsInDenyList("trusted-account")) + assert.True(t, s.IsInAllowList("trusted-account")) + + // Account in neither list - should skip due to allow list restriction + shouldSkip = s.ShouldSkipAccount("unknown-account") + assert.True(t, shouldSkip) + assert.False(t, s.IsInDenyList("unknown-account")) + assert.False(t, s.IsInAllowList("unknown-account")) + }) + + t.Run("clearing lists", func(t *testing.T) { + var s Scanner // Fresh instance for this test + s.SetAllowedAccounts([]string{"initial-allowed"}) + s.SetDeniedAccounts([]string{"initial-denied"}) + + // Verify initial state + assert.True(t, s.ShouldSkipAccount("random-account")) // Not in allow list + assert.True(t, s.ShouldSkipAccount("initial-denied")) // In deny list + + // Clear allowed accounts with nil + s.SetAllowedAccounts(nil) + assert.False(t, s.ShouldSkipAccount("random-account")) // No allow list restriction + assert.True(t, s.ShouldSkipAccount("initial-denied")) // Still in deny list + + // Clear denied accounts with empty slice + s.SetDeniedAccounts([]string{}) + assert.False(t, s.ShouldSkipAccount("initial-denied")) // No longer denied + assert.False(t, s.ShouldSkipAccount("initial-allowed")) // No restrictions + }) +} diff --git a/pkg/detectors/aws/access_keys/accesskey.go b/pkg/detectors/aws/access_keys/accesskey.go index eb48a4efe..99eb2fe73 100644 --- a/pkg/detectors/aws/access_keys/accesskey.go +++ b/pkg/detectors/aws/access_keys/accesskey.go @@ -26,6 +26,7 @@ import ( type scanner struct { verificationClient config.HTTPClient skipIDs map[string]struct{} + detectors.AccountFilter detectors.DefaultMultiPartCredentialProvider } @@ -51,6 +52,18 @@ func WithSkipIDs(skipIDs []string) func(*scanner) { } } +func WithAllowedAccounts(accounts []string) func(*scanner) { + return func(s *scanner) { + s.SetAllowedAccounts(accounts) + } +} + +func WithDeniedAccounts(accounts []string) func(*scanner) { + return func(s *scanner) { + s.SetDeniedAccounts(accounts) + } +} + // Ensure the scanner satisfies the interface at compile time. var _ interface { detectors.Detector @@ -143,38 +156,22 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result }, } - // Decode the account ID. - account, err := aws.GetAccountNumFromID(idMatch) + // Decode the AWS Account ID. + accountID, err := aws.GetAccountNumFromID(idMatch) isCanary := false if err != nil { - logger.V(3).Info("Failed to decode account number", "err", err) + logger.V(3).Info("Failed to decode AWS Account ID", "err", err) } else { - s1.ExtraData["account"] = account + s1.ExtraData["account"] = accountID - // Handle canary IDs. - if _, ok := thinkstCanaryList[account]; ok { + // Check if this is a canary token + if _, ok := thinkstCanaryList[accountID]; ok { isCanary = true s1.ExtraData["message"] = thinkstMessage - if verify { - verified, arn, err := s.verifyCanary(ctx, idMatch, secretMatch) - s1.Verified = verified - if arn != "" { - s1.ExtraData["arn"] = arn - } - s1.SetVerificationError(err, secretMatch) - } } - if _, ok := thinkstKnockoffsCanaryList[account]; ok { + if _, ok := thinkstKnockoffsCanaryList[accountID]; ok { isCanary = true s1.ExtraData["message"] = thinkstKnockoffsMessage - if verify { - verified, arn, err := s.verifyCanary(ctx, idMatch, secretMatch) - s1.Verified = verified - if arn != "" { - s1.ExtraData["arn"] = arn - } - s1.SetVerificationError(err, secretMatch) - } } if isCanary { @@ -182,21 +179,48 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result } } - if verify && !isCanary { - isVerified, extraData, verificationErr := s.verifyMatch(ctx, idMatch, secretMatch, len(secretMatches) > 1) - s1.Verified = isVerified - - // Log if the calculated ID does not match the ID value from verification. - // Should only be edge cases at most. - if account != "" && extraData["account"] != "" && extraData["account"] != s1.ExtraData["account"] { - logger.V(2).Info("Calculated account ID does not match actual account ID", "calculated", account, "actual", extraData["account"]) + if verify { + // Check account filtering before verification for ALL secrets (including canaries) + if accountID != "" { + if s.ShouldSkipAccount(accountID) { + var skipReason string + if s.IsInDenyList(accountID) { + skipReason = aws.VerificationErrAccountIDInDenyList + } else { + skipReason = aws.VerificationErrAccountIDNotInAllowList + } + s1.SetVerificationError(fmt.Errorf("%s", skipReason), secretMatch) + results = append(results, s1) + continue + } } - // Append the extraData to the existing ExtraData map. - for k, v := range extraData { - s1.ExtraData[k] = v + // Perform verification based on token type + if isCanary { + // Canary verification logic + verified, arn, err := s.verifyCanary(ctx, idMatch, secretMatch) + s1.Verified = verified + if arn != "" { + s1.ExtraData["arn"] = arn + } + s1.SetVerificationError(err, secretMatch) + } else { + // Normal verification logic + isVerified, extraData, verificationErr := s.verifyMatch(ctx, idMatch, secretMatch, len(secretMatches) > 1) + s1.Verified = isVerified + + // Log if the calculated ID does not match the ID value from verification. + // Should only be edge cases at most. + if accountID != "" && extraData["account"] != "" && extraData["account"] != s1.ExtraData["account"] { + logger.V(2).Info("Calculated account ID does not match actual account ID", "calculated", accountID, "actual", extraData["account"]) + } + + // Append the extraData to the existing ExtraData map. + for k, v := range extraData { + s1.ExtraData[k] = v + } + s1.SetVerificationError(verificationErr, secretMatch) } - s1.SetVerificationError(verificationErr, secretMatch) } if !s1.Verified && aws.FalsePositiveSecretPat.MatchString(secretMatch) { diff --git a/pkg/detectors/aws/access_keys/accesskey_test.go b/pkg/detectors/aws/access_keys/accesskey_test.go index 2ecaae3e1..bf6cb4774 100644 --- a/pkg/detectors/aws/access_keys/accesskey_test.go +++ b/pkg/detectors/aws/access_keys/accesskey_test.go @@ -2,6 +2,7 @@ package access_keys import ( "context" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -92,3 +93,113 @@ func TestAWS_Pattern(t *testing.T) { }) } } + +func TestAWS_WithAllowedAccounts(t *testing.T) { + accounts := []string{"123456789012", "999888777666"} + s := New(WithAllowedAccounts(accounts)) + + // Test that allowed accounts are properly configured + shouldSkip := s.ShouldSkipAccount("123456789012") + require.False(t, shouldSkip) + require.True(t, s.IsInAllowList("123456789012")) + + // Test that non-allowed accounts are skipped + shouldSkip = s.ShouldSkipAccount("111222333444") + require.True(t, shouldSkip) + require.False(t, s.IsInAllowList("111222333444")) +} + +func TestAWS_WithDeniedAccounts(t *testing.T) { + accounts := []string{"123456789012", "999888777666"} + s := New(WithDeniedAccounts(accounts)) + + // Test that denied accounts are properly skipped + shouldSkip := s.ShouldSkipAccount("123456789012") + require.True(t, shouldSkip) + require.True(t, s.IsInDenyList("123456789012")) + + // Test that non-denied accounts are not skipped + shouldSkip = s.ShouldSkipAccount("111222333444") + require.False(t, shouldSkip) + require.False(t, s.IsInDenyList("111222333444")) +} + +func TestAWS_CanaryTokenFiltering(t *testing.T) { + // Using known canary token from integration tests + canaryAccessKeyID := "AKIASP2TPHJSQH3FJRUX" // Account ID: 171436882533 + canarySecret := "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + testData := []byte(fmt.Sprintf("%s:%s", canaryAccessKeyID, canarySecret)) + + t.Run("debug canary detection", func(t *testing.T) { + // First, let's test basic canary detection without verification + s := New() + + results, err := s.FromData(context.Background(), false, testData) // verify = false + require.NoError(t, err) + require.Len(t, results, 1) + + result := results[0] + t.Logf("Result without verification - Verified: %v, Account: %s, IsCanary: %s, Message: %s", + result.Verified, result.ExtraData["account"], result.ExtraData["is_canary"], result.ExtraData["message"]) + + // Should detect as canary but not verify (since verify=false) + require.False(t, result.Verified) + require.Equal(t, "171436882533", result.ExtraData["account"]) + require.Equal(t, "true", result.ExtraData["is_canary"]) + require.Contains(t, result.ExtraData["message"], "canarytokens.org") + }) + + t.Run("canary token with allow list - account not allowed", func(t *testing.T) { + // Configure scanner with allow list that excludes the canary account + s := New(WithAllowedAccounts([]string{"123456789012", "999888777666"})) + + results, err := s.FromData(context.Background(), true, testData) + require.NoError(t, err) + require.Len(t, results, 1) + + result := results[0] + // Should detect the canary token but not verify it due to filtering + require.False(t, result.Verified) + require.NotNil(t, result.VerificationError()) + require.Contains(t, result.VerificationError().Error(), "not in the allow list") + require.Equal(t, "171436882533", result.ExtraData["account"]) + require.Equal(t, "true", result.ExtraData["is_canary"]) + }) + + t.Run("canary token with deny list - account denied", func(t *testing.T) { + // Configure scanner with deny list that includes the canary account + s := New(WithDeniedAccounts([]string{"171436882533", "123456789012"})) + + results, err := s.FromData(context.Background(), true, testData) + require.NoError(t, err) + require.Len(t, results, 1) + + result := results[0] + // Should detect the canary token but not verify it due to filtering + require.False(t, result.Verified) + require.NotNil(t, result.VerificationError()) + require.Contains(t, result.VerificationError().Error(), "in the deny list") + require.Equal(t, "171436882533", result.ExtraData["account"]) + require.Equal(t, "true", result.ExtraData["is_canary"]) + }) + + t.Run("precedence test - deny list takes precedence over allow list", func(t *testing.T) { + // Configure scanner where canary account is in both allow and deny lists + s := New( + WithAllowedAccounts([]string{"171436882533", "123456789012"}), + WithDeniedAccounts([]string{"171436882533"}), + ) + + results, err := s.FromData(context.Background(), true, testData) + require.NoError(t, err) + require.Len(t, results, 1) + + result := results[0] + // Should detect the canary token but not verify it since deny takes precedence + require.False(t, result.Verified) + require.NotNil(t, result.VerificationError()) + require.Contains(t, result.VerificationError().Error(), "in the deny list") + require.Equal(t, "171436882533", result.ExtraData["account"]) + require.Equal(t, "true", result.ExtraData["is_canary"]) + }) +} diff --git a/pkg/detectors/aws/common.go b/pkg/detectors/aws/common.go index c18addc0e..02bf5a713 100644 --- a/pkg/detectors/aws/common.go +++ b/pkg/detectors/aws/common.go @@ -7,6 +7,12 @@ const ( RequiredSecretEntropy = 4.25 ) +// Verification error messages +const ( + VerificationErrAccountIDInDenyList = "Account ID is in the deny list for verification" + VerificationErrAccountIDNotInAllowList = "Account ID is not in the allow list for verification" +) + var SecretPat = regexp.MustCompile(`(?:[^A-Za-z0-9+/]|\A)([A-Za-z0-9+/]{40})(?:[^A-Za-z0-9+/]|\z)`) type IdentityResponse struct { diff --git a/pkg/detectors/aws/session_keys/sessionkey.go b/pkg/detectors/aws/session_keys/sessionkey.go index debb002a2..701747837 100644 --- a/pkg/detectors/aws/session_keys/sessionkey.go +++ b/pkg/detectors/aws/session_keys/sessionkey.go @@ -23,6 +23,7 @@ type scanner struct { *detectors.CustomMultiPartCredentialProvider verificationClient *http.Client skipIDs map[string]struct{} + detectors.AccountFilter } func New(opts ...func(*scanner)) *scanner { @@ -33,7 +34,7 @@ func New(opts ...func(*scanner)) *scanner { opt(scanner) } - scanner.CustomMultiPartCredentialProvider = detectors.NewCustomMultiPartCredentialProvider(2048) // ???? + scanner.CustomMultiPartCredentialProvider = detectors.NewCustomMultiPartCredentialProvider(2048) return scanner } @@ -48,6 +49,18 @@ func WithSkipIDs(skipIDs []string) func(*scanner) { } } +func WithAllowedAccounts(accounts []string) func(*scanner) { + return func(s *scanner) { + s.SetAllowedAccounts(accounts) + } +} + +func WithDeniedAccounts(accounts []string) func(*scanner) { + return func(s *scanner) { + s.SetDeniedAccounts(accounts) + } +} + // Ensure the scanner satisfies the interface at compile time. var _ interface { detectors.Detector @@ -121,6 +134,35 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result } if verify { + // If we haven't already found an AWS Account ID for this ID (via API), calculate one for filtering. + var accountIDForFiltering string + if accountID, err := aws.GetAccountNumFromID(idMatch); err == nil { + accountIDForFiltering = accountID + } + + // Check account filtering before verification + if accountIDForFiltering != "" { + if s.ShouldSkipAccount(accountIDForFiltering) { + var skipReason string + if s.IsInDenyList(accountIDForFiltering) { + skipReason = aws.VerificationErrAccountIDInDenyList + } else { + skipReason = aws.VerificationErrAccountIDNotInAllowList + } + s1.SetVerificationError(fmt.Errorf("%s", skipReason), secretMatch) + // If we haven't already found an AWS Account ID for this ID (via API), calculate one. + if _, ok := s1.ExtraData["account"]; !ok { + if accountID, err := aws.GetAccountNumFromID(idMatch); err != nil { + logger.V(3).Info("Failed to decode AWS Account ID", "err", err) + } else { + s1.ExtraData["account"] = accountID + } + } + results = append(results, s1) + continue + } + } + isVerified, extraData, verificationErr := s.verifyMatch(ctx, idMatch, secretMatch, sessionMatch, true) s1.Verified = isVerified if extraData != nil { @@ -134,12 +176,12 @@ func (s scanner) FromData(ctx context.Context, verify bool, data []byte) (result continue } - // If we haven't already found an account number for this ID (via API), calculate one. + // If we haven't already found an AWS Account ID for this ID (via API), calculate one. if _, ok := s1.ExtraData["account"]; !ok { - if account, err := aws.GetAccountNumFromID(idMatch); err != nil { - logger.V(3).Info("Failed to decode account number", "err", err) + if accountID, err := aws.GetAccountNumFromID(idMatch); err != nil { + logger.V(3).Info("Failed to decode AWS Account ID", "err", err) } else { - s1.ExtraData["account"] = account + s1.ExtraData["account"] = accountID } } diff --git a/pkg/detectors/aws/session_keys/sessionkeys_test.go b/pkg/detectors/aws/session_keys/sessionkeys_test.go index c1f0690cc..b884dcb3e 100644 --- a/pkg/detectors/aws/session_keys/sessionkeys_test.go +++ b/pkg/detectors/aws/session_keys/sessionkeys_test.go @@ -94,3 +94,33 @@ func TestAWSSessionKey_Pattern(t *testing.T) { }) } } + +func TestAWSSessionKey_WithAllowedAccounts(t *testing.T) { + accounts := []string{"123456789012", "999888777666"} + s := New(WithAllowedAccounts(accounts)) + + // Test that allowed accounts are properly configured + shouldSkip := s.ShouldSkipAccount("123456789012") + require.False(t, shouldSkip) + require.True(t, s.IsInAllowList("123456789012")) + + // Test that non-allowed accounts are skipped + shouldSkip = s.ShouldSkipAccount("111222333444") + require.True(t, shouldSkip) + require.False(t, s.IsInAllowList("111222333444")) +} + +func TestAWSSessionKey_WithDeniedAccounts(t *testing.T) { + accounts := []string{"123456789012", "999888777666"} + s := New(WithDeniedAccounts(accounts)) + + // Test that denied accounts are properly skipped + shouldSkip := s.ShouldSkipAccount("123456789012") + require.True(t, shouldSkip) + require.True(t, s.IsInDenyList("123456789012")) + + // Test that non-denied accounts are not skipped + shouldSkip = s.ShouldSkipAccount("111222333444") + require.False(t, shouldSkip) + require.False(t, s.IsInDenyList("111222333444")) +}