diff --git a/.gitignore b/.gitignore index 79a48876f6d..aa22a4e0035 100644 --- a/.gitignore +++ b/.gitignore @@ -138,9 +138,6 @@ custom-gcl tools/dibble/dibble # OPENFRAME(agent-openframe-mode): ignore fork build artifacts (orbit/osquery binaries) — openframe/docs/agent-openframe-mode.md -# Openframe build artifacts (not the source directories) +# Keep anchored: unanchored `orbit**` matches any basename at any depth, incl. client/orbit_*.go /orbit-* /osquery-* -# Openframe -orbit** -osquery** diff --git a/client/device_client.go b/client/device_client.go index 23930b3f7be..2b839a5c9d9 100644 --- a/client/device_client.go +++ b/client/device_client.go @@ -107,6 +107,11 @@ func (dc *DeviceClient) requestAttempt(verb string, path string, query string, p } dc.SetClientCapabilitiesHeader(request) + // >>> OPENFRAME(agent-json-content-type): declare JSON so a WAF parses the body instead of regexing it whole — openframe/docs/agent-json-content-type.md + if len(bodyBytes) > 0 { + request.Header.Set("Content-Type", "application/json") + } + // <<< OPENFRAME(agent-json-content-type) response, err := dc.DoHTTPRequest(request) if err != nil { return fmt.Errorf("%s %s: %w", verb, path, err) diff --git a/client/orbit_client.go b/client/orbit_client.go index b21f496dcfb..3fdecdefac2 100644 --- a/client/orbit_client.go +++ b/client/orbit_client.go @@ -172,6 +172,11 @@ func (oc *OrbitClient) requestWithExternal(verb string, pathOrURL string, params return err } oc.SetClientCapabilitiesHeader(request) + // >>> OPENFRAME(agent-json-content-type): declare JSON so a WAF parses the body instead of regexing it whole — openframe/docs/agent-json-content-type.md + if len(bodyBytes) > 0 { + request.Header.Set("Content-Type", "application/json") + } + // <<< OPENFRAME(agent-json-content-type) // >>> OPENFRAME(agent-openframe-mode): inject Bearer auth header on every request when in openframe mode — openframe/docs/agent-openframe-mode.md if oc.openFrameMode { authToken := oc.authManager.GetToken() diff --git a/client/orbit_client_content_type_test.go b/client/orbit_client_content_type_test.go new file mode 100644 index 00000000000..26011291a4d --- /dev/null +++ b/client/orbit_client_content_type_test.go @@ -0,0 +1,83 @@ +package client + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/stretchr/testify/require" +) + +// >>> OPENFRAME(agent-json-content-type): guards the header against an upstream refactor of the +// request builders, which would drop it with no git conflict — openframe/docs/agent-json-content-type.md + +func TestOrbitClientJSONContentType(t *testing.T) { + var gotMethod, gotContentType string + var gotBody []byte + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotContentType = r.Method, r.Header.Get("Content-Type") + gotBody, _ = io.ReadAll(r.Body) + + if strings.HasSuffix(r.URL.Path, "/orbit/enroll") { + writeEnrollResponse(t, w, "a-node-key") + return + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + _, nodeKeyPath := newNodeKeyFile(t, "existing-key") + + t.Run("request with a body declares application/json", func(t *testing.T) { + key, err := newReenrollTestClient(t, srv.URL, nodeKeyPath).enroll() + require.NoError(t, err) + require.Equal(t, "a-node-key", key) + + require.Equal(t, http.MethodPost, gotMethod) + require.NotEmpty(t, gotBody) + require.Equal(t, "application/json", gotContentType) + }) + + t.Run("bodyless request declares no content type", func(t *testing.T) { + gotContentType = "sentinel" + require.NoError(t, newReenrollTestClient(t, srv.URL, nodeKeyPath).Ping()) + + require.Equal(t, http.MethodHead, gotMethod) + require.Empty(t, gotContentType) + }) +} + +func TestDeviceClientJSONContentType(t *testing.T) { + var gotContentType string + var gotBody []byte + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotContentType = r.Header.Get("Content-Type") + gotBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + dc, err := NewDeviceClient(srv.URL, true, "", nil, "") + require.NoError(t, err) + + t.Run("request with a body declares application/json", func(t *testing.T) { + require.NoError(t, dc.ReportError("a-token", fleet.FleetdError{ErrorSource: "orbit"})) + + require.NotEmpty(t, gotBody) + require.Equal(t, "application/json", gotContentType) + }) + + t.Run("bodyless request declares no content type", func(t *testing.T) { + gotContentType = "sentinel" + require.NoError(t, dc.CheckToken("a-token")) + + require.Empty(t, gotContentType) + }) +} + +// <<< OPENFRAME(agent-json-content-type) diff --git a/openframe/docs/README.md b/openframe/docs/README.md index 934b9e0937a..74acf3327df 100644 --- a/openframe/docs/README.md +++ b/openframe/docs/README.md @@ -49,6 +49,7 @@ The agent has its own switch, `--openframe-mode` / `ORBIT_OPENFRAME_MODE`. |-----|--------| | [agent-openframe-mode.md](agent-openframe-mode.md) | OpenFrame agent mode: gateway URL prefix, encrypted bearer-token pipeline (extract / decrypt / refresh), custom osqueryd, `orbit uuid` command. | | [node-key-management.md](node-key-management.md) | Node-key enrollment caching, 401 re-enrollment, Windows file-lock resilience. | +| [agent-json-content-type.md](agent-json-content-type.md) | Orbit sets `Content-Type: application/json` on requests with a body (upstream sets none). Without it a WAF cannot JSON-parse the body — Cloud Armor flagged 100% of `/orbit/config` polls as SQLi. Unconditional, not gated on OpenFrame mode. | ## Database migrations diff --git a/openframe/docs/agent-json-content-type.md b/openframe/docs/agent-json-content-type.md new file mode 100644 index 00000000000..0202cb86f92 --- /dev/null +++ b/openframe/docs/agent-json-content-type.md @@ -0,0 +1,94 @@ +# Agent JSON `Content-Type` header + +**Slug:** `OPENFRAME(agent-json-content-type)` · **File:** [`client/orbit_client.go`](../../client/orbit_client.go) + +Orbit sets `Content-Type: application/json` on every request that carries a body. +Three lines, unconditional (not gated on `--openframe-mode`), because it is correct +for any Fleet deployment sitting behind a WAF — not just OpenFrame's. + +## Why the fork carries this + +Upstream `OrbitClient.requestWithExternal()` marshals `params` to JSON and hands the +bytes to `http.NewRequestWithContext`, but never sets `Content-Type`. Go's `net/http` +adds **no default request `Content-Type`** (the `http.DetectContentType` sniffing you +may be thinking of is server-side, for responses). So orbit ships a JSON body with no +content type at all. + +That is invisible to Fleet itself — `makeDecoder` in +[`server/service/endpoint_utils.go`](../../server/service/endpoint_utils.go) just +`json.Decode`s the body and never inspects the request content type — so upstream has +no reason to notice. It is very visible to a WAF. + +OpenFrame terminates agent traffic on a GCP **Cloud Armor** policy (`client-policy`, +`advanced_options_config { json_parsing = "STANDARD" }`). Cloud Armor keys JSON body +parsing **off the `Content-Type` header**. With no header, parsing silently does not +engage and the entire raw body is evaluated as one opaque field, so the JSON +punctuation itself becomes the attack surface: + +``` +matchedFieldType: ARG_NAMES +matchedFieldName: {"orbit_node_key":"QKafSDrSZRqNP6JYc3bH2T33B2gODS+0"} ← whole body = one field +matchedOffset: 1 matchedLength: 18 ← exactly `"orbit_node_key":"` +preconfiguredExprIds: [owasp-crs-v042200-id942340-sqli] +``` + +CRS **942340** ("SQL authentication bypass 3/3") matches the `"key":"` sequence. That is +content-independent — it does not depend on the node key at all — so it fired on +**100%** of `POST /api/fleet/orbit/config` polls, which is **94.5%** of every WAF match +on the agent policy. It blocked promoting the Cloud Armor WAF band out of `preview`: +enforcing would have `deny(502)`'d every config poll for every agent in the fleet. + +Measured in `shared-j62b` (dev), 2026-08-07. + +## Reproduction + +Same body, four content types, against the dev LB: + +| `Content-Type` sent | Cloud Armor result | +|---|---| +| `application/json` | body parsed → **no match** | +| `application/json; charset=utf-8` | body parsed → **no match** | +| *(absent — upstream behaviour)* | raw `ARG_NAMES` → **942340** | +| `text/plain` | raw `ARG_NAMES` → **942340** | + +Note `json_custom_config` on the Armor side **cannot** substitute for this fix: it only +adds content-type *strings* to match against, and the unfixed request has no header to +match. The header has to come from the agent. + +## Scope + +All three agent→Fleet request builders had the defect; all three are fixed: + +| File | Notes | +|---|---| +| [`client/orbit_client.go`](../../client/orbit_client.go) | `requestWithExternal()`. The live one — 100% of the observed false positives. | +| [`client/device_client.go`](../../client/device_client.go) | `requestAttempt()`. Fleet Desktop, latent — zero `/api/fleet/device/` traffic in dev. | +| [`orbit/cmd/fetch_cert/main.go`](../../orbit/cmd/fetch_cert/main.go) | `requestCert()`. One-shot cert CLI. | + +Both client fixes are guarded on `len(bodyBytes) > 0`, so bodyless requests +(`HEAD /api/fleet/orbit/ping`, `CheckToken`) stay header-free. Orbit's `external` branch +(`DownloadSoftwareInstallerFromURL`) is a bodyless `GET` to a third-party URL and is untouched. + +In `fetch_cert` the header is set **before** `signer.Sign(req)`. `content-type` is not a +covered field today — [`pkg/fleethttpsig`](../../pkg/fleethttpsig/fleethttpsig.go) covers +`@method`, `@authority`, `@path`, `@query`, `content-digest` — but signing the final header +set stays correct if that list ever widens. + +Regression tests: +[`client/orbit_client_content_type_test.go`](../../client/orbit_client_content_type_test.go), +covering both the header on body-carrying requests and its absence on bodyless ones. + +Adding that test surfaced a second fork bug, fixed here too: `.gitignore` carried an +unanchored `orbit**` / `osquery**` alongside the anchored `/orbit-*` / `/osquery-*`. Unanchored +patterns match a basename at any depth, so they silently ignored `orbit/`, `client/orbit_*.go` +and any new `orbit_*_test.go` — 63 tracked files matched, surviving only because ignores do not +apply to already-tracked files. The anchored pair (plus `/build`) covers the real artifacts, so +the unanchored lines were dropped. + +## Upstream + +This is a plain bug fix with no OpenFrame-specific behaviour; it is a good candidate to +send upstream, which would let the fork drop the marker entirely. + +See also [agent-openframe-mode.md](agent-openframe-mode.md) — the adjacent marker in the +same function. diff --git a/openframe/docs/fork-file-manifest.md b/openframe/docs/fork-file-manifest.md index bacac844c16..d5a590a6add 100644 --- a/openframe/docs/fork-file-manifest.md +++ b/openframe/docs/fork-file-manifest.md @@ -131,6 +131,7 @@ and the heaviest standing rebase cost. | Query-results TTL cleanup | `server/config/config.go`, `server/fleet/{cron_schedules,datastore}.go`, `server/datastore/mysql/query_results.go`, `cmd/fleet/{cron,serve}.go` | | Redis key prefix | `server/datastore/redis/redis.go`, `server/config/config.go`, `cmd/fleet/serve.go` | | Agent OpenFrame mode | `orbit/cmd/orbit/orbit.go`, `orbit/pkg/osquery/osquery.go`, `server/service/orbit_client.go`, `server/service/base_client.go` | +| Agent JSON content-type | `client/orbit_client.go`, `client/device_client.go`, `orbit/cmd/fetch_cert/main.go`, `client/orbit_client_content_type_test.go` | | Build / meta | `go.mod`, `go.sum`, `.gitignore`, `README.md`, `.github/pull_request_template.md`, `server/archtest/*` | ### Helm chart (~9 files) diff --git a/openframe/scripts/verify.sh b/openframe/scripts/verify.sh index 6f6d22491d2..69b991b7a8f 100755 --- a/openframe/scripts/verify.sh +++ b/openframe/scripts/verify.sh @@ -46,7 +46,7 @@ rm -f vet.err # 3. Marker presence: if a merge silently dropped fork code, its OPENFRAME markers # vanish too. A slug dropping to zero is a red flag worth a human look. step "OPENFRAME marker presence (dropped-fork-code detector)" -for slug in host-assignments redis-key-prefix redis-seed-nodes query-results-ttl osquery-host-id agent-openframe-mode migration-race; do +for slug in host-assignments redis-key-prefix redis-seed-nodes query-results-ttl osquery-host-id agent-openframe-mode agent-json-content-type migration-race; do n=$(grep -rIl "OPENFRAME($slug" --include='*.go' --include='*.yaml' --include='*.tpl' . 2>/dev/null | wc -l | tr -d ' ') if [ "$n" -gt 0 ]; then ok "$slug — present in $n file(s)"; else bad "$slug — NO markers found (fork code may have been dropped in the merge)"; fi done diff --git a/orbit/cmd/fetch_cert/main.go b/orbit/cmd/fetch_cert/main.go index c3e7980ba7f..911f903c80a 100644 --- a/orbit/cmd/fetch_cert/main.go +++ b/orbit/cmd/fetch_cert/main.go @@ -127,6 +127,9 @@ func requestCert(signer *httpsig.Signer, fleetURL string, certificateAuthorityID if err != nil { return "", fmt.Errorf("creating http request: %w", err) } + // >>> OPENFRAME(agent-json-content-type): declare JSON so a WAF parses the body instead of regexing it whole; set before Sign() in case covered fields ever include it — openframe/docs/agent-json-content-type.md + req.Header.Set("Content-Type", "application/json") + // <<< OPENFRAME(agent-json-content-type) if err := signer.Sign(req); err != nil { return "", fmt.Errorf("failed to sign request: %w", err)