From ba64c251f220e253c01e07e0690fb2f731901a09 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 23 Sep 2026 09:54:59 +0200 Subject: [PATCH] Init AppSec before the run context, keep credentials out of its logged URL, and parse its concurrency cap at int size --- proxy/cmd/proxy/cmd/root.go | 19 ++++++++++++++++-- proxy/internal/appsec/client_test.go | 8 +++++++- proxy/internal/restrict/restrict.go | 10 ---------- proxy/server.go | 29 ++++++++++++++++++++-------- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/proxy/cmd/proxy/cmd/root.go b/proxy/cmd/proxy/cmd/root.go index 28fef84ad..67a669e8d 100644 --- a/proxy/cmd/proxy/cmd/root.go +++ b/proxy/cmd/proxy/cmd/root.go @@ -132,8 +132,8 @@ func init() { rootCmd.Flags().StringVar(&crowdsecAPIKey, "crowdsec-api-key", envStringOrDefault("NB_PROXY_CROWDSEC_API_KEY", ""), "CrowdSec bouncer API key") rootCmd.Flags().StringVar(&appsecURL, "crowdsec-appsec-url", envStringOrDefault("NB_PROXY_CROWDSEC_APPSEC_URL", ""), "CrowdSec AppSec (WAF) endpoint for HTTP request inspection, e.g. http://127.0.0.1:7422/ (reuses the bouncer API key)") rootCmd.Flags().DurationVar(&appsecTimeout, "crowdsec-appsec-timeout", envDurationOrDefault("NB_PROXY_CROWDSEC_APPSEC_TIMEOUT", 0), "Timeout for a single AppSec inspection call (0 = 200ms)") - rootCmd.Flags().IntVar(&appsecMaxConcurrent, "crowdsec-appsec-max-concurrent", int(envInt64OrDefault("NB_PROXY_CROWDSEC_APPSEC_MAX_CONCURRENT", 0)), "Cap on AppSec inspections in flight; further requests are denied in enforce mode rather than queued (0 = 256, negative = no cap)") - rootCmd.Flags().Int64Var(&captureBudgetBytes, "capture-budget-bytes", envInt64OrDefault("NB_PROXY_CAPTURE_BUDGET_BYTES", 0), "Total in-flight request-body buffering across the proxy, shared by AppSec inspection and agent-network capture (0 = 256MiB)") + rootCmd.Flags().IntVar(&appsecMaxConcurrent, "crowdsec-appsec-max-concurrent", envIntOrDefault("NB_PROXY_CROWDSEC_APPSEC_MAX_CONCURRENT", 0), "Cap on AppSec inspections in flight; further requests are denied in enforce mode rather than queued (0 = 256, negative = no cap)") + rootCmd.Flags().Int64Var(&captureBudgetBytes, "capture-budget-bytes", envInt64OrDefault("NB_PROXY_CAPTURE_BUDGET_BYTES", 0), "Total in-flight request-body buffering across the proxy, shared by AppSec inspection and agent-network capture (0 or negative = 256MiB)") rootCmd.Flags().Int64Var(&appsecMaxBodyBytes, "crowdsec-appsec-max-body-bytes", envInt64OrDefault("NB_PROXY_CROWDSEC_APPSEC_MAX_BODY_BYTES", 0), "Cap on the request body mirrored to AppSec (0 = 64KiB, negative = headers and URI only)") } @@ -328,6 +328,21 @@ func envInt64OrDefault(key string, def int64) int64 { return parsed } +// envIntOrDefault parses at the platform int size, so a value that does not fit +// falls back to the default instead of wrapping on 32-bit builds. +func envIntOrDefault(key string, def int) int { + v, exists := os.LookupEnv(key) + if !exists { + return def + } + parsed, err := strconv.Atoi(v) + if err != nil { + log.Warnf("parse %s=%q: %v, using default %d", key, v, err, def) + return def + } + return parsed +} + func envDurationOrDefault(key string, def time.Duration) time.Duration { v, exists := os.LookupEnv(key) if !exists { diff --git a/proxy/internal/appsec/client_test.go b/proxy/internal/appsec/client_test.go index e804f831f..59f5452cf 100644 --- a/proxy/internal/appsec/client_test.go +++ b/proxy/internal/appsec/client_test.go @@ -37,7 +37,13 @@ func (e *engine) start(t *testing.T) *httptest.Server { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) - require.NoError(t, err) + if err != nil { + // FailNow is only valid on the test goroutine, and this is the + // server's; report and answer 500 so Inspect sees the failure. + t.Errorf("read mirrored body: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } e.requests++ e.gotMethod = r.Method e.gotHeader = r.Header.Clone() diff --git a/proxy/internal/restrict/restrict.go b/proxy/internal/restrict/restrict.go index 2af384803..1870e59dd 100644 --- a/proxy/internal/restrict/restrict.go +++ b/proxy/internal/restrict/restrict.go @@ -253,16 +253,6 @@ func (v Verdict) IsCrowdSec() bool { } } -// IsAppSec returns true when the verdict originates from an AppSec inspection. -func (v Verdict) IsAppSec() bool { - switch v { - case DenyAppSecBan, DenyAppSecCaptcha, DenyAppSecUnavailable: - return true - default: - return false - } -} - // IsObserveOnly returns true when v is a CrowdSec verdict and the filter is in // observe mode. Callers should log the verdict but not block the request. func (f *Filter) IsObserveOnly(v Verdict) bool { diff --git a/proxy/server.go b/proxy/server.go index f223f39df..d5e9cc9ba 100644 --- a/proxy/server.go +++ b/proxy/server.go @@ -390,6 +390,16 @@ func (s *Server) Start(ctx context.Context) error { return fmt.Errorf("init middleware manager: %w", err) } + // Must precede the mapping worker: the worker opens the management stream + // and reports proxyCapabilities, which reads appsecClient. Building it + // afterwards would both race the read and, when the worker won, advertise + // the proxy as AppSec-incapable for the lifetime of that stream. It also + // runs before the run context and the NetBird client exist, so a config + // error here leaves nothing running behind it. + if err := s.initAppSec(); err != nil { + return err + } + runCtx, runCancel := context.WithCancel(ctx) s.runCancel = runCancel @@ -400,13 +410,6 @@ func (s *Server) Start(ctx context.Context) error { s.crowdsecRegistry = crowdsec.NewRegistry(s.CrowdSecAPIURL, s.CrowdSecAPIKey, log.NewEntry(s.Logger)) s.crowdsecServices = make(map[types.ServiceID]bool) - // Must precede the mapping worker: the worker opens the management stream - // and reports proxyCapabilities, which reads appsecClient. Building it - // afterwards would both race the read and, when the worker won, advertise - // the proxy as AppSec-incapable for the lifetime of that stream. - if err := s.initAppSec(); err != nil { - return err - } go s.newManagementMappingWorker(runCtx, s.mgmtClient) @@ -1972,10 +1975,20 @@ func (s *Server) initAppSec() error { } s.appsecClient = client - s.Logger.Infof("CrowdSec AppSec inspection available at %s", s.CrowdSecAppSecURL) + s.Logger.Infof("CrowdSec AppSec inspection available at %s", appSecEndpointForLog(s.CrowdSecAppSecURL)) return nil } +// appSecEndpointForLog reduces the AppSec URL to scheme, host and path, so +// credentials in userinfo or the query never reach the log. +func appSecEndpointForLog(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return "" + } + return (&url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path}).String() +} + // appSecMode resolves the per-service AppSec mode. A service asking for // inspection on a proxy with no AppSec endpoint keeps its mode so the auth // middleware fails closed for enforce, mirroring the CrowdSec behavior.