Fix CrowdSec review findings: observe metadata, idempotent Start, HasRestrictions, proto map alias

This commit is contained in:
Viktor Liu
2026-03-30 13:05:46 +02:00
parent ae84272a30
commit 6411136fec
6 changed files with 25 additions and 5 deletions
@@ -1,6 +1,7 @@
package accesslogs package accesslogs
import ( import (
"maps"
"net" "net"
"net/netip" "net/netip"
"time" "time"
@@ -57,7 +58,7 @@ func (a *AccessLogEntry) FromProto(serviceLog *proto.AccessLog) {
a.BytesDownload = serviceLog.GetBytesDownload() a.BytesDownload = serviceLog.GetBytesDownload()
a.Protocol = AccessLogProtocol(serviceLog.GetProtocol()) a.Protocol = AccessLogProtocol(serviceLog.GetProtocol())
if m := serviceLog.GetMetadata(); len(m) > 0 { if m := serviceLog.GetMetadata(); len(m) > 0 {
a.Metadata = m a.Metadata = maps.Clone(m)
} }
if sourceIP := serviceLog.GetSourceIp(); sourceIP != "" { if sourceIP := serviceLog.GetSourceIp(); sourceIP != "" {
@@ -139,8 +139,8 @@ func (m Manager) ClusterRequireSubdomain(ctx context.Context, clusterAddr string
return m.store.GetClusterRequireSubdomain(ctx, clusterAddr) return m.store.GetClusterRequireSubdomain(ctx, clusterAddr)
} }
// ClusterSupportsCrowdSec returns whether any active proxy in the cluster // ClusterSupportsCrowdSec returns whether all active proxies in the cluster
// has CrowdSec configured. Returns nil when no proxy has reported capabilities. // have CrowdSec configured (unanimous). Returns nil when no proxy has reported capabilities.
func (m Manager) ClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool { func (m Manager) ClusterSupportsCrowdSec(ctx context.Context, clusterAddr string) *bool {
return m.store.GetClusterSupportsCrowdSec(ctx, clusterAddr) return m.store.GetClusterSupportsCrowdSec(ctx, clusterAddr)
} }
+3
View File
@@ -170,6 +170,9 @@ func (mw *Middleware) checkIPRestrictions(w http.ResponseWriter, r *http.Request
if verdict.IsCrowdSec() { if verdict.IsCrowdSec() {
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
cd.SetMetadata("crowdsec_verdict", verdict.String()) cd.SetMetadata("crowdsec_verdict", verdict.String())
if config.IPRestrictions.IsObserveOnly(verdict) {
cd.SetMetadata("crowdsec_mode", "observe")
}
} }
} }
+6
View File
@@ -4,6 +4,7 @@ package crowdsec
import ( import (
"context" "context"
"errors"
"net/netip" "net/netip"
"strings" "strings"
"sync" "sync"
@@ -83,6 +84,11 @@ func (b *Bouncer) Start(ctx context.Context) error {
done := make(chan struct{}) done := make(chan struct{})
b.lifeMu.Lock() b.lifeMu.Lock()
if b.cancel != nil {
b.lifeMu.Unlock()
cancel()
return errors.New("bouncer already started")
}
b.cancel = cancel b.cancel = cancel
b.done = done b.done = done
b.lifeMu.Unlock() b.lifeMu.Unlock()
+7 -2
View File
@@ -174,7 +174,12 @@ func (v Verdict) String() string {
// IsCrowdSec returns true when the verdict originates from a CrowdSec check. // IsCrowdSec returns true when the verdict originates from a CrowdSec check.
func (v Verdict) IsCrowdSec() bool { func (v Verdict) IsCrowdSec() bool {
return v >= DenyCrowdSecBan && v <= DenyCrowdSecUnavailable switch v {
case DenyCrowdSecBan, DenyCrowdSecCaptcha, DenyCrowdSecThrottle, DenyCrowdSecUnavailable:
return true
default:
return false
}
} }
// IsObserveOnly returns true when v is a CrowdSec verdict and the filter is in // IsObserveOnly returns true when v is a CrowdSec verdict and the filter is in
@@ -306,5 +311,5 @@ func (f *Filter) HasRestrictions() bool {
} }
return len(f.AllowedCIDRs) > 0 || len(f.BlockedCIDRs) > 0 || return len(f.AllowedCIDRs) > 0 || len(f.BlockedCIDRs) > 0 ||
len(f.AllowedCountries) > 0 || len(f.BlockedCountries) > 0 || len(f.AllowedCountries) > 0 || len(f.BlockedCountries) > 0 ||
(f.CrowdSec != nil && f.CrowdSecMode != CrowdSecOff) f.CrowdSecMode == CrowdSecEnforce || f.CrowdSecMode == CrowdSecObserve
} }
+5
View File
@@ -403,4 +403,9 @@ func TestFilter_HasRestrictions_CrowdSec(t *testing.T) {
cs := &mockCrowdSec{ready: true} cs := &mockCrowdSec{ready: true}
f := ParseFilter(FilterConfig{CrowdSec: cs, CrowdSecMode: CrowdSecEnforce}) f := ParseFilter(FilterConfig{CrowdSec: cs, CrowdSecMode: CrowdSecEnforce})
assert.True(t, f.HasRestrictions()) assert.True(t, f.HasRestrictions())
// Enforce mode without checker (LAPI not configured): still has restrictions
// because Check() will fail-closed with DenyCrowdSecUnavailable.
f2 := ParseFilter(FilterConfig{CrowdSec: nil, CrowdSecMode: CrowdSecEnforce})
assert.True(t, f2.HasRestrictions())
} }