From 90210e7c0c50df850524163fbc4bfbdc86826562 Mon Sep 17 00:00:00 2001 From: CyberMind-FR Date: Sun, 19 Jul 2026 05:49:41 +0200 Subject: [PATCH] feat(sbxwaf): run detect/escalate on static-asset paths (0.1.37) Co-Authored-By: Gerald KERMA --- .../cmd/sbxwaf/inspect_test.go | 87 +++++++++++++++++ .../secubox-toolbox-ng/cmd/sbxwaf/main.go | 95 ++++++++++--------- packages/secubox-toolbox-ng/debian/changelog | 11 +++ 3 files changed, 147 insertions(+), 46 deletions(-) diff --git a/packages/secubox-toolbox-ng/cmd/sbxwaf/inspect_test.go b/packages/secubox-toolbox-ng/cmd/sbxwaf/inspect_test.go index e229e234..ab15bc8c 100644 --- a/packages/secubox-toolbox-ng/cmd/sbxwaf/inspect_test.go +++ b/packages/secubox-toolbox-ng/cmd/sbxwaf/inspect_test.go @@ -14,6 +14,8 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" + "strings" "testing" ) @@ -144,6 +146,91 @@ func TestInspectStaticAssetSkip(t *testing.T) { } } +// buildDetectStaticRulesFile writes a rules file with a detect category whose +// pattern matches a static-asset probe path. +func buildDetectStaticRulesFile(t *testing.T) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "waf-rules*.json") + if err != nil { + t.Fatal(err) + } + _, _ = f.WriteString(`{"categories":{"probe":{"mode":"detect","severity":"high",` + + `"patterns":[{"id":"panos","pattern":"/global-protect/portal/css/bootstrap\\.min\\.css"}]}}}`) + f.Close() + return f.Name() +} + +// A detect rule on a static-asset path now fires (logged) and the request still +// passes through — the core of the fix. Before Option B the .css path skipped +// inspection entirely. +func TestInspectStaticAssetDetectFires(t *testing.T) { + const wantBody = "css ok" + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, wantBody) + })) + defer backend.Close() + + rulesPath := buildDetectStaticRulesFile(t) + srv := newInspectServer(t, rulesPath, backend.URL[len("http://"):]) + // Route a threat log to a temp file so we can assert the detect record. + logPath := filepath.Join(t.TempDir(), "threats.log") + srv.threatLog = NewThreatLog(logPath) + + handler := srv.handler() + req := httptest.NewRequest(http.MethodGet, + "http://app.example.com/global-protect/portal/css/bootstrap.min.css", nil) + req.Host = "app.example.com" + req.RemoteAddr = "1.2.3.4:12345" // public IP + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + // detect = observe: request passes through to backend, NOT blocked. + if rec.Code == http.StatusForbidden { + t.Fatalf("detect must not block; got 403") + } + body, _ := io.ReadAll(rec.Result().Body) + if string(body) != wantBody { + t.Fatalf("expected backend body %q, got %q", wantBody, string(body)) + } + // And it was logged as action=detect. + logged, _ := os.ReadFile(logPath) + if !strings.Contains(string(logged), `"action":"detect"`) || + !strings.Contains(string(logged), `"category":"probe"`) { + t.Fatalf("expected a detect record for category probe; log=%s", logged) + } +} + +// A legit static asset that matches NO rule still passes with no detect log. +func TestInspectStaticAssetNoMatchPassthrough(t *testing.T) { + const wantBody = "css ok" + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, wantBody) + })) + defer backend.Close() + + rulesPath := buildDetectStaticRulesFile(t) + srv := newInspectServer(t, rulesPath, backend.URL[len("http://"):]) + logPath := filepath.Join(t.TempDir(), "threats.log") + srv.threatLog = NewThreatLog(logPath) + + handler := srv.handler() + req := httptest.NewRequest(http.MethodGet, + "http://app.example.com/static/css/site.min.css", nil) // different path + req.Host = "app.example.com" + req.RemoteAddr = "1.2.3.4:12345" + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("legit static asset should pass; got %d", rec.Code) + } + logged, _ := os.ReadFile(logPath) + if strings.Contains(string(logged), `"action":"detect"`) { + t.Fatalf("no detect record expected for a non-matching static asset; log=%s", logged) + } +} + // TestInspectNCBypass: NC mobile auth path with payload → not blocked. func TestInspectNCBypass(t *testing.T) { const wantBody = "nc login ok" diff --git a/packages/secubox-toolbox-ng/cmd/sbxwaf/main.go b/packages/secubox-toolbox-ng/cmd/sbxwaf/main.go index 009ec9bc..01f80247 100644 --- a/packages/secubox-toolbox-ng/cmd/sbxwaf/main.go +++ b/packages/secubox-toolbox-ng/cmd/sbxwaf/main.go @@ -331,67 +331,70 @@ func (s *Server) handler() http.Handler { rawPath = r.URL.Path } - skip := privateCIDR(ip) || staticAsset(rawPath) || ncBypass(rawPath) + // Static-asset PATHS are NOT fully skipped: a scanner probing a + // static-extension path (e.g. /global-protect/.../bootstrap.min.css on + // a box with no PAN-OS) must still be fingerprinted. On a static asset + // we run only detect/escalate categories on path+query+ua (no body); + // block categories stay skipped so a legit asset is never newly blocked + // (preserves the old fast-path's zero-FP property — see + // TestInspectStaticAssetSkip). Private-IP / NC-bypass / trusted-host + // remain FULL skips (LAN + NC mobile tokens are never fingerprinted). + skip := privateCIDR(ip) || ncBypass(rawPath) - // Trusted-host skip: bypass WAF inspection for known internal hosts - // (matches Python check_request whitelist in secubox_waf.py:761-763). - // Checked AFTER privateCIDR/static/NC so that the cheap skips run first. if !skip && s.isTrustedHost(r.Host) { skip = true } - if !skip { - // Read up to s.maxBodyInspect bytes for WAF inspection, then - // restore the FULL body (prefix + remaining stream) so the - // upstream proxy receives every byte intact. - // - // Streaming approach: we buffer at most maxBodyInspect bytes (the - // inspection window), then forward a MultiReader of that buffer + - // the unconsumed tail of r.Body. This keeps memory bounded even - // for multi-GB uploads (PeerTube / Nextcloud file uploads). - // - // PARITY GAP: only the first maxBodyInspect bytes are inspected. - // A payload appended after that offset is NOT detected. When a body - // exceeds the cap, an AUDIT log line is emitted so truncation is - // operator-visible (action="body-inspect-truncated"). See - // docs/CUTOVER.md for the documented detection gap. - cap := s.maxBodyInspect - if cap <= 0 { - cap = defaultMaxBodyInspect - } - var bodyBytes []byte - if r.Body != nil { - prefix, _ := io.ReadAll(io.LimitReader(r.Body, cap)) - bodyBytes = prefix - // Restore: prefix already read + remaining stream not yet consumed. - r.Body = io.NopCloser(io.MultiReader(bytes.NewReader(prefix), r.Body)) + isStatic := staticAsset(rawPath) - // Emit audit log when inspection was truncated (Content-Length known - // or body read returned exactly cap bytes → likely more data follows). - if int64(len(prefix)) == cap { - if s.threatLog != nil { - s.threatLog.Record(ThreatRecord{ - ClientIP: ip, - Host: r.Host, - Method: r.Method, - Path: rawPath, - Category: "body-inspect-truncated", - Severity: "audit", - Action: "body-inspect-truncated", - UA: r.Header.Get("User-Agent"), - }) + if !skip { + var bodyBytes []byte + includeBlock := true + if isStatic { + // Fingerprint-only pass: detect/escalate on path+query+ua, no + // body read, no block evaluation. + includeBlock = false + } else { + // Read up to s.maxBodyInspect bytes for WAF inspection, then + // restore the FULL body (prefix + remaining stream) so the + // upstream proxy receives every byte intact. PARITY GAP: only + // the first maxBodyInspect bytes are inspected; a payload + // appended after that offset is NOT detected (see docs/CUTOVER.md). + cap := s.maxBodyInspect + if cap <= 0 { + cap = defaultMaxBodyInspect + } + if r.Body != nil { + prefix, _ := io.ReadAll(io.LimitReader(r.Body, cap)) + bodyBytes = prefix + r.Body = io.NopCloser(io.MultiReader(bytes.NewReader(prefix), r.Body)) + + if int64(len(prefix)) == cap { + if s.threatLog != nil { + s.threatLog.Record(ThreatRecord{ + ClientIP: ip, + Host: r.Host, + Method: r.Method, + Path: rawPath, + Category: "body-inspect-truncated", + Severity: "audit", + Action: "body-inspect-truncated", + UA: r.Header.Get("User-Agent"), + }) + } + log.Printf("sbxwaf: AUDIT body-inspect-truncated host=%s path=%s ip=%s cap=%d", + r.Host, rawPath, ip, cap) } - log.Printf("sbxwaf: AUDIT body-inspect-truncated host=%s path=%s ip=%s cap=%d", - r.Host, rawPath, ip, cap) } } - cat, sev, mode, hit := s.rules.Match( + cat, sev, mode, hit := s.rules.MatchModes( r.Method, rawPath, r.URL.RawQuery, string(bodyBytes), r.Header.Get("User-Agent"), + includeBlock, ) if hit && mode == modeDetect { // Observe only: log it, let it through. A detect category diff --git a/packages/secubox-toolbox-ng/debian/changelog b/packages/secubox-toolbox-ng/debian/changelog index 7f7fe0bb..8d3f9276 100644 --- a/packages/secubox-toolbox-ng/debian/changelog +++ b/packages/secubox-toolbox-ng/debian/changelog @@ -1,3 +1,14 @@ +secubox-toolbox-ng (0.1.37-1~bookworm1) bookworm; urgency=medium + + * sbxwaf: fingerprint scanners on static-asset paths. Previously any path + ending in .js/.css/.png/... fully skipped WAF inspection, so a probe for an + absent product served on a static-asset path (e.g. the PAN-OS + /global-protect/.../bootstrap.min.css) evaded detection entirely. Now + static-asset paths run detect/escalate categories (path+query+ua, no body); + block categories stay skipped so a legitimate asset is never newly blocked. + + -- Gerald KERMA Sat, 19 Jul 2026 10:00:00 +0200 + secubox-toolbox-ng (0.1.36-1~bookworm1) bookworm; urgency=medium * #826 move browser-ja4.txt to /etc/secubox/sentinel (a conffile, like