Resolves production issue where legitimate ACK messages were triggering
enumeration detection, causing calls to die at ~64s (Timer H expiry).
### Root Cause:
ACK messages lack dialog-aware fast-path in l4handler.go. All SIP requests
go through the full security pipeline including enumeration detection.
When a client sends multiple ACKs (responding to 200 OK retransmissions),
the enumeration detector sees "rapid fire" extension probing and bans the
source IP.
### The Fix:
Exempt ACK from enumeration detection (l4handler.go:254-258):
- ACK is a mid-dialog request, not an extension probe
- ACKs arrive in response to 200 OK retransmissions (RFC 3261)
- False-positive rapid-fire/sequential detection blocked legitimate traffic
### Impact:
- **For Twilio trunk:** Whitelisting is the operational fix
- **For dynamic IP clients:** This architectural fix enables calls to work
(residential users, mobile clients, peer-to-peer scenarios)
### Agent Thread Protocol:
Added docs/agent-threads/ for cross-project debugging:
- 001-flextel-ack-being-dropped.md — Problem report from asterpbx agent
- 002-diagnosis-ack-not-fast-pathed.md — Root cause analysis and fix options
- PROTOCOL.md — Agent communication protocol documentation
See: docs/agent-threads/ack-loss-from-twilio-trunk/ for full diagnosis
### Test Results:
All 196 tests passing ✅ (1.213s)
This commit addresses all critical architectural issues identified in the
Matt Holt code review, transforming the module from using anti-patterns
to following Caddy best practices.
### 🔴 CRITICAL FIXES:
**1. Global Registry → Caddy App System**
- Created SIPGuardianApp implementing caddy.App interface (app.go)
- Eliminates memory/goroutine leaks on config reload
- Before: guardians accumulated in global map, never cleaned up
- After: Caddy calls Stop() on old app before loading new config
- Impact: Prevents OOM in production with frequent config reloads
**2. Feature Flags → Instance Fields**
- Moved enableMetrics/Webhooks/Storage from globals to *bool struct fields
- Allows per-instance configuration (not shared across all guardians)
- Helper methods default to true if not set
- Impact: Thread-safe, configurable per guardian instance
**3. Prometheus Panic Prevention**
- Replaced MustRegister() with Register() + AlreadyRegisteredError handling
- Makes RegisterMetrics() idempotent and safe for multiple calls
- Before: panics on second call (e.g., config reload)
- After: silently ignores already-registered collectors
- Impact: No more crashes on config reload
### 🟠 HIGH PRIORITY FIXES:
**4. Storage Worker Pool**
- Fixed pool of 4 workers + 1000-entry buffered channel
- Replaces unbounded go func() spawns (3 locations)
- Before: 100k goroutines during DDoS → memory exhaustion
- After: bounded resources, drops writes when full (fail-fast)
- Impact: Survives attacks without resource exhaustion
**5. Config Immutability**
- MaxFailures/FindTime/BanTime no longer modified on running instance
- Prevents race with RecordFailure() reading values without lock
- Changed mutations to warning logs
- Additive changes still allowed (whitelists, webhooks)
- Impact: No more race conditions, predictable ban behavior
### Modified Files:
- app.go (NEW): SIPGuardianApp with proper lifecycle management
- sipguardian.go: Removed module registration, added worker pool, feature flags
- l4handler.go: Use ctx.App() instead of global registry
- metrics.go: Use ctx.App() instead of global registry
- registry.go: Config immutability warnings instead of mutations
### Test Results:
All tests pass (1.228s) ✅
### Breaking Changes:
None - backwards compatible, but requires apps {} block in Caddyfile
for proper lifecycle management
### Estimated Impact:
- Memory leak fix: Prevents unbounded growth over time
- Resource usage: 100k goroutines → 4 workers during attack
- Stability: No more panics on config reload
- Performance: O(n log n) sorting (addressed in quick wins)
Support for whitelisting SIP trunks and providers by hostname or SRV
record with automatic IP resolution and periodic refresh.
Features:
- Hostname resolution via A/AAAA records
- SRV record resolution (e.g., _sip._udp.provider.com)
- Configurable refresh interval (default 5m)
- Stale entry handling when DNS fails
- Admin API endpoints for DNS whitelist management
- Caddyfile directives: whitelist_hosts, whitelist_srv, dns_refresh
This allows whitelisting by provider name rather than tracking
constantly-changing IP addresses.
The layer4 matchers and handlers must implement caddyfile.Unmarshaler
to be usable in Caddyfile syntax. This enables proper parsing of:
- @sip sip { methods ... } matchers
- sip_guardian { ... } handlers
