From c746e75401ba4202c0648f07964c76956770337e Mon Sep 17 00:00:00 2001 From: Javier Marcos <1271349+javuto@users.noreply.github.com> Date: Mon, 6 Jul 2026 10:26:43 +0200 Subject: [PATCH] Per-host filtering for the osctrl-tls HTTP debug dump --- cmd/tls/handlers/debug_http_filter_test.go | 63 ++++++++++++++ cmd/tls/handlers/handlers.go | 38 +++++++++ cmd/tls/handlers/post.go | 99 +++++++++++++++++----- deploy/config/tls.yml | 5 ++ pkg/config/flags.go | 7 ++ pkg/config/types.go | 8 ++ pkg/utils/http-utils.go | 19 +++++ pkg/utils/http-utils_test.go | 45 ++++++++++ 8 files changed, 263 insertions(+), 21 deletions(-) create mode 100644 cmd/tls/handlers/debug_http_filter_test.go diff --git a/cmd/tls/handlers/debug_http_filter_test.go b/cmd/tls/handlers/debug_http_filter_test.go new file mode 100644 index 00000000..a0ef818b --- /dev/null +++ b/cmd/tls/handlers/debug_http_filter_test.go @@ -0,0 +1,63 @@ +package handlers + +import ( + "testing" + + "github.com/jmpsec/osctrl/pkg/config" + "github.com/stretchr/testify/assert" +) + +// debugCfg is a shorthand builder for the debug-HTTP config used by the +// filter tests. +func debugCfg(enable bool, target string) *config.YAMLConfigurationDebug { + return &config.YAMLConfigurationDebug{ + EnableHTTP: enable, + TargetHostIdentifier: target, + } +} + +func TestShouldDebugHTTPDisabled(t *testing.T) { + h := &HandlersTLS{DebugHTTPConfig: debugCfg(false, "")} + assert.False(t, h.shouldDebugHTTP("ABC-123")) + assert.False(t, h.shouldDebugHTTP("")) + // The "dump everything" path is also off when debug is disabled. + assert.False(t, h.debugHTTPAll()) +} + +func TestShouldDebugHTTPNoFilterDumpsAll(t *testing.T) { + // Legacy behavior: empty target + EnableHTTP → dump every request, + // regardless of the node UUID (including unknown/failed lookups). + h := &HandlersTLS{DebugHTTPConfig: debugCfg(true, "")} + // shouldDebugHTTP intentionally returns false here; the legacy path is + // driven by debugHTTPAll() so that malformed/pre-lookup requests are + // still dumped at the top of the handler. + assert.False(t, h.shouldDebugHTTP("ABC-123")) + assert.True(t, h.debugHTTPAll()) +} + +func TestShouldDebugHTTPFiltered(t *testing.T) { + h := &HandlersTLS{DebugHTTPConfig: debugCfg(true, "DEADBEEF-1234")} + + // Matching UUID (case-insensitive on both sides). + assert.True(t, h.shouldDebugHTTP("DEADBEEF-1234")) + assert.True(t, h.shouldDebugHTTP("deadbeef-1234")) + assert.True(t, h.shouldDebugHTTP("DeadBeef-1234")) + + // Non-matching UUID never dumps. + assert.False(t, h.shouldDebugHTTP("CAFE-9999")) + + // Unknown node (empty UUID) never matches a set filter — this is what + // excludes invalid node_key / anonymous traffic from the filtered + // dump on a busy server. + assert.False(t, h.shouldDebugHTTP("")) + + // Filter set ⇒ the legacy "dump everything" path is suppressed. + assert.False(t, h.debugHTTPAll()) +} + +func TestShouldDebugHTTPNilConfig(t *testing.T) { + // Defensive: a handler with no debug config wired must never dump. + h := &HandlersTLS{} + assert.False(t, h.shouldDebugHTTP("DEADBEEF-1234")) + assert.False(t, h.debugHTTPAll()) +} diff --git a/cmd/tls/handlers/handlers.go b/cmd/tls/handlers/handlers.go index a5eb4cdb..eabb4417 100644 --- a/cmd/tls/handlers/handlers.go +++ b/cmd/tls/handlers/handlers.go @@ -3,6 +3,7 @@ package handlers import ( "net/http" "strconv" + "strings" "time" "github.com/jmpsec/osctrl/pkg/auditlog" @@ -215,6 +216,43 @@ func CreateHandlersTLS(opts ...Option) *HandlersTLS { return h } +// shouldDebugHTTP reports whether a request whose originating node UUID is +// `uuid` should be dumped to the debug HTTP log. It encodes the per-host +// filter introduced to make debug HTTP usable on busy servers: +// +// - EnableHTTP off → never dump. +// - TargetHostIdentifier empty → this returns false; the legacy "dump +// everything" path is handled inline at the top of each handler +// (gated on `TargetHostIdentifier == ""`), so non-empty-filter mode +// stays byte-for-byte identical to the previous behavior. +// - TargetHostIdentifier set → dump only when uuid matches, case- +// insensitively. A zero uuid (failed node lookup / invalid key) never +// matches, so anonymous or malformed traffic is excluded from the +// filtered dump — which is the intent. +// +// Node UUIDs are stored uppercase and the osquery host_identifier may +// arrive in any case, so EqualFold is used on both sides. +func (h *HandlersTLS) shouldDebugHTTP(uuid string) bool { + if h.DebugHTTPConfig == nil || !h.DebugHTTPConfig.EnableHTTP { + return false + } + if h.DebugHTTPConfig.TargetHostIdentifier == "" { + return false + } + return uuid != "" && strings.EqualFold(uuid, h.DebugHTTPConfig.TargetHostIdentifier) +} + +// debugHTTPAll reports whether the legacy "dump every request" path +// should run at the top of a handler. It is true only when HTTP debug is +// enabled and no per-host filter is configured; in that mode behavior is +// identical to the previous implementation (full dump before the body is +// parsed, including malformed bodies). When a filter is set this returns +// false and the handler instead dumps only matching requests after the +// node has been identified, via shouldDebugHTTP + DebugHTTPDumpWithBody. +func (h *HandlersTLS) debugHTTPAll() bool { + return h.DebugHTTPConfig != nil && h.DebugHTTPConfig.EnableHTTP && h.DebugHTTPConfig.TargetHostIdentifier == "" +} + func (h *HandlersTLS) PrometheusMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { start := time.Now() diff --git a/cmd/tls/handlers/post.go b/cmd/tls/handlers/post.go index 74d39e2b..2805d4a5 100644 --- a/cmd/tls/handlers/post.go +++ b/cmd/tls/handlers/post.go @@ -103,7 +103,7 @@ func (h *HandlersTLS) EnrollHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP for environment - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -121,6 +121,13 @@ func (h *HandlersTLS) EnrollHandler(w http.ResponseWriter, r *http.Request) { utils.HTTPResponse(w, "", http.StatusInternalServerError, []byte("")) return } + // Per-host filtered HTTP debug dump. In legacy (no-filter) mode the + // top-of-handler dump already covered this request. For enroll the + // host identifier is the osquery host_identifier; it becomes the + // node UUID (uppercased) once the node is created. + if h.shouldDebugHTTP(t.HostIdentifier) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } // Check if received secret is valid var nodeKey string var newNode nodes.OsqueryNode @@ -190,7 +197,7 @@ func (h *HandlersTLS) ConfigHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP for environment - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -209,7 +216,17 @@ func (h *HandlersTLS) ConfigHandler(w http.ResponseWriter, r *http.Request) { return } // We need to update the node info in another go routine - if node, err := h.Nodes.GetByKey(t.NodeKey); err == nil { + node, nodeErr := h.Nodes.GetByKey(t.NodeKey) + matchedUUID := "" + if nodeErr == nil { + matchedUUID = node.UUID + } + // Per-host filtered HTTP debug dump. Legacy (no-filter) mode already + // dumped this request at the top of the handler. + if h.shouldDebugHTTP(matchedUUID) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } + if nodeErr == nil { // Check if node belongs to the environment if node.EnvironmentID != env.ID { log.Warn().Msgf("node UUID: %s in %s environment does not belong to the environment", node.UUID, env.Name) @@ -282,7 +299,7 @@ func (h *HandlersTLS) LogHandler(w http.ResponseWriter, r *http.Request) { }() } // Debug HTTP here so the body will be uncompressed - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Extract POST body and decode JSON @@ -310,7 +327,17 @@ func (h *HandlersTLS) LogHandler(w http.ResponseWriter, r *http.Request) { var nodeInvalid bool var response types.LogResponse // Check if provided node_key is valid and if so, update node - if node, err := h.Nodes.GetByKey(t.NodeKey); err == nil { + node, nodeErr := h.Nodes.GetByKey(t.NodeKey) + matchedUUID := "" + if nodeErr == nil { + matchedUUID = node.UUID + } + // Per-host filtered HTTP debug dump. Legacy (no-filter) mode already + // dumped this request at the top of the handler. + if h.shouldDebugHTTP(matchedUUID) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } + if nodeErr == nil { // Check if node belongs to the environment if node.EnvironmentID != env.ID { log.Warn().Msgf("node UUID: %s in %s environment does not belong to the environment", node.UUID, env.Name) @@ -367,7 +394,7 @@ func (h *HandlersTLS) QueryReadHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -389,7 +416,17 @@ func (h *HandlersTLS) QueryReadHandler(w http.ResponseWriter, r *http.Request) { var response interface{} qs := make(queries.QueryReadQueries) // Check if provided node_key is valid and if so, update node - if node, err := h.Nodes.GetByKey(t.NodeKey); err == nil { + node, nodeErr := h.Nodes.GetByKey(t.NodeKey) + matchedUUID := "" + if nodeErr == nil { + matchedUUID = node.UUID + } + // Per-host filtered HTTP debug dump. Legacy (no-filter) mode already + // dumped this request at the top of the handler. + if h.shouldDebugHTTP(matchedUUID) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } + if nodeErr == nil { // Check if node belongs to the environment if node.EnvironmentID != env.ID { log.Warn().Msgf("node UUID: %s in %s environment does not belong to the environment", node.UUID, env.Name) @@ -416,7 +453,7 @@ func (h *HandlersTLS) QueryReadHandler(w http.ResponseWriter, r *http.Request) { log.Debug().Msgf("node-uuid: %s with nodeid %d added to batch writer for query read update", node.UUID, node.ID) h.recordActivity(env.UUID, node.UUID, activity.EventQueryRead) } else { - log.Err(err).Msg("GetByKey") + log.Err(nodeErr).Msg("GetByKey") nodeInvalid = true accelerate = false } @@ -456,7 +493,7 @@ func (h *HandlersTLS) QueryWriteHandler(w http.ResponseWriter, r *http.Request) return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -477,7 +514,17 @@ func (h *HandlersTLS) QueryWriteHandler(w http.ResponseWriter, r *http.Request) var nodeInvalid bool var response types.QueryWriteResponse // Check if provided node_key is valid and if so, update node - if node, err := h.Nodes.GetByKey(t.NodeKey); err == nil { + node, nodeErr := h.Nodes.GetByKey(t.NodeKey) + matchedUUID := "" + if nodeErr == nil { + matchedUUID = node.UUID + } + // Per-host filtered HTTP debug dump. Legacy (no-filter) mode already + // dumped this request at the top of the handler. + if h.shouldDebugHTTP(matchedUUID) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } + if nodeErr == nil { // Check if node belongs to the environment if node.EnvironmentID != env.ID { log.Warn().Msgf("node UUID: %s in %s environment does not belong to the environment", node.UUID, env.Name) @@ -551,7 +598,7 @@ func (h *HandlersTLS) QuickEnrollHandler(w http.ResponseWriter, r *http.Request) return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Retrieve type of script @@ -624,7 +671,7 @@ func (h *HandlersTLS) QuickRemoveHandler(w http.ResponseWriter, r *http.Request) return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Retrieve type of script @@ -699,7 +746,7 @@ func (h *HandlersTLS) CarveInitHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -721,7 +768,17 @@ func (h *HandlersTLS) CarveInitHandler(w http.ResponseWriter, r *http.Request) { var carveSessionID string var response types.CarveInitResponse // Check if provided node_key is valid and if so, update node - if node, err := h.Nodes.GetByKey(t.NodeKey); err == nil { + node, nodeErr := h.Nodes.GetByKey(t.NodeKey) + matchedUUID := "" + if nodeErr == nil { + matchedUUID = node.UUID + } + // Per-host filtered HTTP debug dump. Legacy (no-filter) mode already + // dumped this request at the top of the handler. + if h.shouldDebugHTTP(matchedUUID) { + utils.DebugHTTPDumpWithBody(h.DebugHTTP, r, body, h.DebugHTTPConfig.ShowBody) + } + if nodeErr == nil { // Check if node belongs to the environment if node.EnvironmentID != env.ID { log.Warn().Msgf("node UUID: %s in %s environment does not belong to the environment", node.UUID, env.Name) @@ -779,7 +836,7 @@ func (h *HandlersTLS) CarveBlockHandler(w http.ResponseWriter, r *http.Request) return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -846,7 +903,7 @@ func (h *HandlersTLS) FlagsHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP if enabled - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -907,7 +964,7 @@ func (h *HandlersTLS) CertHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP if enabled - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -962,7 +1019,7 @@ func (h *HandlersTLS) VerifyHandler(w http.ResponseWriter, r *http.Request) { return } // Debug HTTP if enabled - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -1054,7 +1111,7 @@ func (h *HandlersTLS) ScriptHandler(w http.ResponseWriter, r *http.Request) { actionVar += environments.PowershellTarget } // Debug HTTP if enabled - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body @@ -1114,7 +1171,7 @@ func (h *HandlersTLS) EnrollPackageHandler(w http.ResponseWriter, r *http.Reques return } // Debug HTTP if enabled - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Retrieve package @@ -1245,7 +1302,7 @@ func (h *HandlersTLS) OsqueryConfigEndpointHandler(w http.ResponseWriter, r *htt return } // Debug HTTP - if h.DebugHTTPConfig.EnableHTTP { + if h.debugHTTPAll() { utils.DebugHTTPDump(h.DebugHTTP, r, h.DebugHTTPConfig.ShowBody) } // Decode read POST body. Even though we cap the *decompressed* diff --git a/deploy/config/tls.yml b/deploy/config/tls.yml index 35f7df5d..d21b926e 100644 --- a/deploy/config/tls.yml +++ b/deploy/config/tls.yml @@ -160,3 +160,8 @@ debug: enableHttp: false httpFile: ./debug-http-tls.log showBody: false + # When non-empty, only dump requests from the osquery node whose UUID + # (or enroll host_identifier) matches this value (case-insensitive). + # Empty dumps every request when enableHttp is true. Useful to isolate + # one host's traffic on a busy server. + hostIdentifier: "" diff --git a/pkg/config/flags.go b/pkg/config/flags.go index ca4496ae..59461a23 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -959,6 +959,13 @@ func initDebugFlags(params *ServiceParameters, service string) []cli.Flag { Sources: cli.EnvVars("HTTP_DEBUG_SHOW_BODY"), Destination: ¶ms.Debug.ShowBody, }, + &cli.StringFlag{ + Name: "http-debug-host", + Value: "", + Usage: "Only dump HTTP requests from the osquery node with this UUID / host_identifier (case-insensitive). Empty dumps everything when --enable-http-debug is set.", + Sources: cli.EnvVars("HTTP_DEBUG_HOST"), + Destination: ¶ms.Debug.TargetHostIdentifier, + }, } } diff --git a/pkg/config/types.go b/pkg/config/types.go index c154dbfb..de9dabc0 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -231,6 +231,14 @@ type YAMLConfigurationDebug struct { EnableHTTP bool `yaml:"enableHttp"` HTTPFile string `yaml:"httpFile"` ShowBody bool `yaml:"showBody"` + // TargetHostIdentifier, when non-empty, restricts the HTTP debug dump + // to requests coming from the osquery node whose UUID (uppercase) or + // enroll host_identifier matches this value (case-insensitive). When + // empty, every request is dumped as long as EnableHTTP is true — the + // legacy behavior. Endpoints that identify a node (enroll, config, + // log, queryRead, queryWrite, carveInit) can match; pre-enroll / + // no-node endpoints are skipped while a filter is set. + TargetHostIdentifier string `yaml:"hostIdentifier"` } // YAMLConfigurationWriter to hold the DB batch writer configuration values diff --git a/pkg/utils/http-utils.go b/pkg/utils/http-utils.go index 0699d974..32d0052a 100644 --- a/pkg/utils/http-utils.go +++ b/pkg/utils/http-utils.go @@ -1,6 +1,7 @@ package utils import ( + "bytes" "crypto/tls" "crypto/x509" "encoding/json" @@ -157,6 +158,24 @@ func DebugHTTPDump(l *zerolog.Logger, r *http.Request, showBody bool) { l.Log().Msg(DebugHTTP(r, showBody)) } +// DebugHTTPDumpWithBody is the variant to use when the request body has +// already been read into memory (for example by the handler's readBody). +// It re-wraps the already-buffered bytes onto r.Body so the existing +// httputil-based dump (DebugHTTP) can serialize the full request without +// re-reading the original stream. This is the helper to call on hot paths +// where a host filter decides, after the body has been parsed, whether +// paying the dump cost is worth it — the non-matching path never calls +// it, and the matching path only re-serializes the bytes it already has. +// +// r.Body is mutated as a side effect; callers must not read r.Body again +// after this (handlers use the buffered []byte they already have). +func DebugHTTPDumpWithBody(l *zerolog.Logger, r *http.Request, body []byte, showBody bool) { + if body != nil { + r.Body = io.NopCloser(bytes.NewReader(body)) + } + DebugHTTPDump(l, r, showBody) +} + // trustedProxies is the global set of CIDRs whose X-Real-IP / // X-Forwarded-For headers GetIP is allowed to honor. When empty (the // safe default), GetIP returns the connection's RemoteAddr IP verbatim diff --git a/pkg/utils/http-utils_test.go b/pkg/utils/http-utils_test.go index e844077c..f74a3130 100644 --- a/pkg/utils/http-utils_test.go +++ b/pkg/utils/http-utils_test.go @@ -3,8 +3,10 @@ package utils import ( "net/http" "net/http/httptest" + "strings" "testing" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" ) @@ -209,3 +211,46 @@ func TestSetTrustedProxiesIgnoresInvalid(t *testing.T) { t.Errorf("partial CIDR set: got %q, want %q", got, "203.0.113.5") } } + +// TestDebugHTTPDumpWithBody verifies the bytes-based dump helper used on +// the per-host-filtered debug path. It must serialize a request whose body +// has already been read into a []byte, include the body when showBody is +// true, surface the "No Body" marker when false, and never touch the +// original (already-consumed) request stream. +func TestDebugHTTPDumpWithBody(t *testing.T) { + body := []byte(`{"node_key":"abc","host_identifier":"host-1"}`) + + withLogger := func(t *testing.T, showBody bool) string { + var buf bufWriter + logger := newZerologLogger(&buf) + req := httptest.NewRequest("POST", "/env/enroll", nil) + req.Header.Set("Content-Type", "application/json") + DebugHTTPDumpWithBody(&logger, req, body, showBody) + return buf.String() + } + + t.Run("show body", func(t *testing.T) { + out := withLogger(t, true) + assert.Contains(t, out, "POST /env/enroll") + assert.Contains(t, out, "host_identifier") + assert.Contains(t, out, "node_key") + assert.NotContains(t, out, "No Body") + }) + + t.Run("omit body", func(t *testing.T) { + out := withLogger(t, false) + assert.Contains(t, out, "POST /env/enroll") + assert.Contains(t, out, "No Body") + assert.NotContains(t, out, "node_key") + }) +} + +// bufWriter is a minimal io.Writer backing a zerolog logger for tests. +type bufWriter struct{ b strings.Builder } + +func (w *bufWriter) Write(p []byte) (int, error) { return w.b.Write(p) } +func (w *bufWriter) String() string { return w.b.String() } + +func newZerologLogger(w *bufWriter) zerolog.Logger { + return zerolog.New(w) +}