diff --git a/internal/security/detector.go b/internal/security/detector.go index e9301b7..f3df69c 100644 --- a/internal/security/detector.go +++ b/internal/security/detector.go @@ -2,6 +2,8 @@ package security import ( "fmt" + "sort" + "strings" "sync" "time" ) @@ -165,49 +167,75 @@ func (d *Detector) trackRequest(ip string, event *IngestEvent) bool { return window.count > d.rateThreshold } -// ShouldAutoBlock determines if an IP should be automatically blocked -func (d *Detector) ShouldAutoBlock(ip string, event *SecurityEvent) bool { - // Always auto-block scanners +// ShouldAutoBlock reports whether an IP should be auto-blocked, and when it +// should, a human-readable reason naming the rule and the counts or paths that +// tripped it, so a blocked IP carries a trace of what led to the block. +func (d *Detector) ShouldAutoBlock(ip string, event *SecurityEvent) (bool, string) { if event.EventType == EventTypeScannerDetected { - return true + return true, event.Message } - - // Auto-block high request rates if event.EventType == EventTypeHighRequestRate { - return true + return true, event.Message } + // The window fields (including pathHits) are mutated by trackRequest under + // the lock, so read them while holding it rather than through a bare pointer. d.mu.RLock() - window, exists := d.ipRequestCount[ip] - d.mu.RUnlock() + defer d.mu.RUnlock() + window, exists := d.ipRequestCount[ip] if !exists { - return false + return false, "" } - // Auto-block after too many 404s (probing for files/paths) if window.notFoundHits >= d.notFoundThreshold { - return true + return true, fmt.Sprintf("%d not-found responses in the detection window; paths: %s", + window.notFoundHits, summarizePaths(window.pathHits)) } - - // Auto-block after too many auth failures if window.authFailures >= d.authFailureThreshold { - return true + return true, fmt.Sprintf("%d authentication failures in the detection window", window.authFailures) } - - // Auto-block if trying too many unique paths (scanning) if len(window.pathHits) >= d.uniquePathsThreshold { - return true + return true, fmt.Sprintf("probed %d distinct paths in the detection window; paths: %s", + len(window.pathHits), summarizePaths(window.pathHits)) } - - // Auto-block if hammering same path repeatedly - for _, hits := range window.pathHits { + for path, hits := range window.pathHits { if hits >= d.repeatedHitsThreshold { - return true + return true, fmt.Sprintf("%d requests to %s in the detection window", hits, path) } } - return false + return false, "" +} + +// summarizePaths renders the busiest few paths in a window for a block reason, +// most-hit first so the output is stable regardless of map order. +func summarizePaths(pathHits map[string]int) string { + type ph struct { + path string + hits int + } + items := make([]ph, 0, len(pathHits)) + for p, h := range pathHits { + items = append(items, ph{p, h}) + } + sort.Slice(items, func(i, j int) bool { + if items[i].hits != items[j].hits { + return items[i].hits > items[j].hits + } + return items[i].path < items[j].path + }) + + const maxPaths = 3 + parts := make([]string, 0, maxPaths) + for i, it := range items { + if i >= maxPaths { + parts = append(parts, fmt.Sprintf("and %d more", len(items)-maxPaths)) + break + } + parts = append(parts, fmt.Sprintf("%s (%d)", it.path, it.hits)) + } + return strings.Join(parts, ", ") } // CleanupOldWindows removes expired rate tracking windows diff --git a/internal/security/manager.go b/internal/security/manager.go index c3540c5..3a24f7a 100644 --- a/internal/security/manager.go +++ b/internal/security/manager.go @@ -92,9 +92,9 @@ func (m *Manager) IngestEvent(event *IngestEvent, autoBlockDuration time.Duratio result.Event = secEvent // Check if we should auto-block the IP - if m.detector.ShouldAutoBlock(event.SourceIP, secEvent) { + if blocked, reason := m.detector.ShouldAutoBlock(event.SourceIP, secEvent); blocked { expiresAt := time.Now().Add(autoBlockDuration) - _, err := m.db.BlockIP(event.SourceIP, "Auto-blocked due to suspicious activity", &expiresAt, true) + _, err := m.db.BlockIP(event.SourceIP, "Auto-blocked: "+reason, &expiresAt, true) if err == nil { result.AutoBlocked = true result.BlockedIP = event.SourceIP diff --git a/internal/security/manager_test.go b/internal/security/manager_test.go index fd3a242..aed5cf7 100644 --- a/internal/security/manager_test.go +++ b/internal/security/manager_test.go @@ -1,6 +1,7 @@ package security import ( + "strings" "testing" "time" ) @@ -51,6 +52,88 @@ func TestIngestEventAutoBlocksOnRepeatedAuthFailures(t *testing.T) { } } +// A legitimate client that identifies as a general-purpose HTTP tool must not be +// blocked on its first request. This reproduces the reported bug where one 404 +// from curl / a Go or Python script locked the caller out immediately. +func TestIngestEventDoesNotBlockGenericHTTPClient(t *testing.T) { + for _, ua := range []string{"curl/8.4.0", "Go-http-client/1.1", "python-requests/2.31.0", "Wget/1.21"} { + m := newTestManager(t) + result, err := m.IngestEvent(&IngestEvent{ + SourceIP: "203.0.113.20", + RequestPath: "/wp-login.php", + RequestMethod: "GET", + StatusCode: 404, + UserAgent: ua, + }, time.Hour) + if err != nil { + t.Fatalf("IngestEvent(%s): %v", ua, err) + } + if result.AutoBlocked { + t.Errorf("UA %q: a single 404 must not auto-block a general-purpose client", ua) + } + } +} + +// A tool that names itself as an attack scanner is still blocked on sight. +func TestIngestEventBlocksNamedScanner(t *testing.T) { + m := newTestManager(t) + result, err := m.IngestEvent(&IngestEvent{ + SourceIP: "203.0.113.21", + RequestPath: "/", + RequestMethod: "GET", + StatusCode: 200, + UserAgent: "sqlmap/1.7.2#stable", + }, time.Hour) + if err != nil { + t.Fatalf("IngestEvent: %v", err) + } + if !result.AutoBlocked { + t.Fatal("a named scanner user agent must still be auto-blocked") + } +} + +// The persisted block records what tripped it: the rule, the count, and the paths. +func TestIngestEventBlockReasonTracesTrigger(t *testing.T) { + m := newTestManager(t) + + var last *IngestResult + for i := 0; i < 10; i++ { + var err error + last, err = m.IngestEvent(&IngestEvent{ + SourceIP: "203.0.113.22", + RequestPath: "/missing.php", + RequestMethod: "GET", + StatusCode: 404, + UserAgent: "Mozilla/5.0", + }, time.Hour) + if err != nil { + t.Fatalf("IngestEvent: %v", err) + } + } + if !last.AutoBlocked { + t.Fatal("expected block after repeated 404 probing") + } + + blocked, err := m.GetBlockedIPs() + if err != nil { + t.Fatalf("GetBlockedIPs: %v", err) + } + var reason string + for _, b := range blocked { + if b.IP == "203.0.113.22" { + reason = b.Reason + } + } + if reason == "" { + t.Fatal("blocked IP not found") + } + for _, want := range []string{"not-found", "/missing.php"} { + if !strings.Contains(reason, want) { + t.Errorf("block reason should trace the trigger, missing %q in %q", want, reason) + } + } +} + func TestIngestEventSkipsWhitelistedIP(t *testing.T) { m := newTestManager(t) diff --git a/internal/security/rules.go b/internal/security/rules.go index 456f445..2878ab4 100644 --- a/internal/security/rules.go +++ b/internal/security/rules.go @@ -122,7 +122,11 @@ var SuspiciousPaths = []string{ "/clientaccesspolicy.xml", } -// Scanner user agent patterns +// ScannerUserAgents match on sight and block immediately, so this list holds +// only attack tools that name themselves. General-purpose HTTP clients (curl, +// wget, python-requests, go-http-client) are deliberately absent: real scripts +// and health checks use them, so matching them here blocked legitimate callers +// on their first request. They are still caught by the volumetric thresholds. var ScannerUserAgents = []string{ "nikto", "nmap", @@ -144,12 +148,6 @@ var ScannerUserAgents = []string{ "joomscan", "droopescan", "zgrab", - "curl/", - "wget/", - "python-requests", - "go-http-client", - "httpx", - "nuclei", } var suspiciousPathPatterns []*regexp.Regexp