[client] Clarify that metrics ingest X-Peer-ID is not a credential (#7363)

* [client] Clarify that metrics ingest X-Peer-ID is not a credential

The ingest endpoint is intentionally unauthenticated: it accepts telemetry
from peers of both cloud and self-hosted deployments, and for a self-hosted
peer there is no shared trust anchor to authenticate against. The X-Peer-ID
header is a correlation tag whose format check exists to bound InfluxDB tag
cardinality.

Both the function name (validateAuth) and the 401 response implied an
authentication control that was never there, which invites the reading that
the check can be bypassed. Rename it to validatePeerIDFormat and return 400,
matching the other input validation failures in the same handler. Document
the intent in the godoc and the infra README.

No behavioural change for clients: push.go classifies responses by 2xx range
rather than by status code, so 400 and 401 are handled identically.

* [client] Reject metrics ingest bodies whose peer_id tag disagrees with the header

validateTag checked tag names against the per-measurement allowlist but only
bounded the value length, so the peer_id tag was free-form text up to 64 bytes
and could differ from the X-Peer-ID header the request was accepted with.

Tie the two together: the tag value must equal the header value. Since the
header is already checked to be 16 hex characters, this transitively constrains
the tag to the same shape. Every client sends the same value in both places
(metrics.go feeds agentInfo.peerID to both push.SetPeerID and the body tags),
so well-behaved clients are unaffected.

The mismatch is rejected rather than silently overwritten: rewriting the value
would re-serialize caller-controlled text back into line protocol and would
hide misbehaving senders instead of surfacing them. Rejection also matches the
other input validation failures in the same handler, which all return 400.

This narrows the value space of the peer_id tag but does not by itself bound
InfluxDB series cardinality: a sender that puts the same arbitrary 16 hex
characters in both the header and the body still passes. Limiting that needs a
per-source rate limit in front of the service.

* [client] Document what the metrics ingest peer_id check does and does not bound

The README described X-Peer-ID as the correlation tag, but grouping is done by
the peer_id tag in the submitted line protocol: that is what is forwarded to
InfluxDB, while the header only serves as the value each tag is checked against.

It also claimed the format check bounds tag cardinality. It bounds the value
space of the tag, not the number of distinct series, so state that explicitly
and point out that series cardinality has to be limited outside this service.

* [client] Set timeouts on the metrics ingest HTTP server

The server ran on http.ListenAndServe with no timeouts, silenced with a
nolint:gosec for G114. Without ReadHeaderTimeout a client can hold a connection
open by sending headers slowly, and without ReadTimeout or IdleTimeout
connections accumulate on an endpoint that takes unauthenticated requests.

Construct an http.Server with explicit limits instead, which also drops the
nolint. Handler stays nil so the existing DefaultServeMux registrations are
unaffected.

WriteTimeout is deliberately larger than the 10s upstream client timeout: the
response is only written after the forward to InfluxDB completes, so a tighter
value would cut off the server's own valid response.

* Revert "[client] Document what the metrics ingest peer_id check does and does not bound"

This reverts commit 837a5d8dda.

* Revert "[client] Reject metrics ingest bodies whose peer_id tag disagrees with the header"

This reverts commit 91d4f6128.

Tying the body peer_id tag to the X-Peer-ID header assumed the two always agree,
but a profile switch breaks that. UpdateAgentInfo swaps agentInfo.peerID and
calls push.SetPeerID with the new value while leaving the sample buffer alone,
and the peer_id is baked into each buffered line at record time, so samples from
the previous profile ship under the new header.

The consequences compound: validateLineProtocol rejects the whole batch on the
first bad line, so fresh samples are dropped along with the stale ones, and
push.go only resets the buffer after a successful push, so the batch is retried
and fails again. Metrics from that client stay stuck until the old samples age
out of the buffer, up to maxSampleAge (5 days).

Deciding whether the previous profile's unsent samples may be discarded, or
whether the push has to be partitioned per identity, is a product call, so
restore the previous behaviour for now. The header keeps its format check;
the body peer_id tag goes back to being bounded only by maxTagValueLength.

* [client] Stop claiming the metrics ingest peer ID format check bounds tag cardinality

The X-Peer-ID header is never forwarded to InfluxDB; the stored peer_id
tag comes from the request body and is constrained only by the tag
allowlist and the maximum tag value length. Align the README and the
validatePeerIDFormat godoc with the actual behavior after the
header/body match check was reverted.

client/internal/metrics/infra/README.md

@@ -32,13 +32,24 @@ Clients do not talk to InfluxDB directly. An ingest server sits between clients
 ```text
 Client ──POST──▶ Ingest Server (:8087) ──▶ InfluxDB (internal)
                   │
+                  ├─ Checks the X-Peer-ID header format
                   ├─ Validates line protocol
                   ├─ Allowlists measurements, fields, and tags
                   ├─ Rejects out-of-bound values
                   └─ Serves remote config at /config
 ```
 
-- **No secret/token-based client auth** — the ingest server holds the InfluxDB token server-side. Clients must send a hashed peer ID via `X-Peer-ID` header.
+- **Intentionally unauthenticated** — the endpoint receives obfuscated telemetry from
+  the peers of both cloud and self-hosted deployments. For a self-hosted peer there is
+  no shared trust anchor with this server, so there is nothing to authenticate against.
+- **`X-Peer-ID` is a correlation tag, not a credential** — it carries the obfuscated
+  peer identifier so samples from one peer can be grouped. The server only checks that
+  the header is well-formed (16 hex chars); a malformed value is rejected with
+  `400 Bad Request`, not `401`. Any well-formed value is accepted by design, and the
+  header must not be relied on for access control. The header itself is not forwarded
+  to InfluxDB — the stored `peer_id` tag comes from the request body and is constrained
+  only by the tag allowlist and the maximum tag value length.
+- **The InfluxDB token stays server-side** — clients never hold a write credential.
 - **InfluxDB is not exposed** — only accessible within the docker network
 - Source: `ingest/main.go`
 
@@ -61,7 +72,7 @@ Tags:
 - `version`: NetBird version string
 - `os`: Operating system (linux, darwin, windows, android, ios, etc.)
 - `arch`: CPU architecture (amd64, arm64, etc.)
-- `peer_id`: anonymised peer identifier (truncated SHA-256 of the WireGuard public key)
+- `peer_id`: obfuscated peer identifier (truncated SHA-256 of the WireGuard public key)
 - `connection_pair_id`: deterministic identifier for the peer pair, identical on both sides
 
 **Note:** `SignalingReceived` is set when the first offer or answer arrives from the remote peer (in both initial and reconnection paths). It excludes the potentially unbounded wait for the remote peer to come online.
@@ -195,7 +206,7 @@ docker compose up -d
 ```
 
 This starts:
-- **Ingest server** on http://localhost:8087 — accepts client metrics (requires `X-Peer-ID` header, no secret/token auth)
+- **Ingest server** on http://localhost:8087 — accepts client metrics (unauthenticated by design; expects a well-formed `X-Peer-ID` correlation tag)
 - **InfluxDB** — internal only, not exposed to host
 - **Grafana** on http://localhost:3001
 

client/internal/metrics/infra/ingest/main.go

@@ -22,6 +22,11 @@ const (
 	maxDurationSeconds = 86400.0          // reject any duration field > 24 hours
 	peerIDLength       = 16               // truncated SHA-256: 8 bytes = 16 hex chars
 	maxTagValueLength  = 64               // reject tag values longer than this
+	readTimeout        = 30 * time.Second // must fit reading a compressed body up to maxBodySize
+	writeTimeout       = 60 * time.Second // must exceed the upstream client timeout below
+	idleTimeout        = 120 * time.Second
+	readHeaderTimeout  = 10 * time.Second
+	maxHeaderBytes     = 1 << 20 // 1 MB
 )
 
 type measurementSpec struct {
@@ -124,8 +129,17 @@ func main() {
 		fmt.Fprint(w, "ok") //nolint:errcheck
 	})
 
+	srv := &http.Server{
+		Addr:              listenAddr,
+		ReadTimeout:       readTimeout,
+		ReadHeaderTimeout: readHeaderTimeout,
+		WriteTimeout:      writeTimeout,
+		IdleTimeout:       idleTimeout,
+		MaxHeaderBytes:    maxHeaderBytes,
+	}
+
 	log.Printf("ingest server listening on %s, forwarding to %s", listenAddr, influxURL)
-	if err := http.ListenAndServe(listenAddr, nil); err != nil { //nolint:gosec
+	if err := srv.ListenAndServe(); err != nil {
 		log.Fatal(err)
 	}
 }
@@ -137,8 +151,8 @@ func handleIngest(client *http.Client, influxURL, influxToken string) http.Handl
 			return
 		}
 
-		if err := validateAuth(r); err != nil {
-			http.Error(w, err.Error(), http.StatusUnauthorized)
+		if err := validatePeerIDFormat(r); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 
@@ -187,8 +201,13 @@ func forwardToInflux(w http.ResponseWriter, r *http.Request, client *http.Client
 	io.Copy(w, resp.Body) //nolint:errcheck
 }
 
-// validateAuth checks that the X-Peer-ID header contains a valid hashed peer ID.
-func validateAuth(r *http.Request) error {
+// validatePeerIDFormat checks the shape of the X-Peer-ID header. The header is a
+// correlation tag, not a credential: this endpoint is intentionally
+// unauthenticated so that peers of self-hosted deployments, for which no shared
+// trust anchor exists, can report obfuscated telemetry. The header is not forwarded
+// to InfluxDB, so this check does not bound the stored peer_id tag; it only rejects
+// a malformed header as a bad request rather than an auth failure.
+func validatePeerIDFormat(r *http.Request) error {
 	peerID := r.Header.Get("X-Peer-ID")
 	if peerID == "" {
 		return fmt.Errorf("missing X-Peer-ID header")

client/internal/metrics/infra/ingest/main_test.go

@@ -94,7 +94,7 @@ func TestValidateLineProtocol_RejectsOnBadLine(t *testing.T) {
 	require.Error(t, err)
 }
 
-func TestValidateAuth(t *testing.T) {
+func TestValidatePeerIDFormat(t *testing.T) {
 	tests := []struct {
 		name    string
 		peerID  string
@@ -113,7 +113,7 @@ func TestValidateAuth(t *testing.T) {
 			if tt.peerID != "" {
 				r.Header.Set("X-Peer-ID", tt.peerID)
 			}
-			err := validateAuth(r)
+			err := validatePeerIDFormat(r)
 			if tt.wantErr {
 				require.Error(t, err)
 			} else {