Three pre-existing gRPC auth interceptors accept a whitespace-only configured key as SET (IsNullOrEmpty vs IsNullOrWhiteSpace) #543

New Issue

2026-08-06T04:33:14-04:00

dohertj2 commented

2026-08-06 04:33:14 -04:00

Summary

The three pre-existing gRPC auth interceptors — TelemetryStreamAuthInterceptor,
ConfigServeAuthInterceptor, LocalDbSyncAuthInterceptor (all in
src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/) — guard against an unset configured key
with string.IsNullOrEmpty(expected). A whitespace-only configured key (e.g. " ") is therefore
treated as set, and the interceptor proceeds to FixedTimeEquals(presented, expected) — so a
whitespace-only configured key opens the endpoint to a whitespace-only bearer token, rather than
failing closed the way an actually-unset key does.

Code references (as of feat/grpc-phase0-contract-auth, commit c2bfad4a):

TelemetryStreamAuthInterceptor.cs:120
ConfigServeAuthInterceptor.cs:113
LocalDbSyncAuthInterceptor.cs:118

The two new mesh-transport interceptors added in the ClusterClient→gRPC migration's Phase 0
(MeshCommandAuthInterceptor, node-side, and CentralCommandAuthInterceptor, central-side) use
string.IsNullOrWhiteSpace for the equivalent check instead, closing this specific gap in the new
code. This issue tracks whether the same hardening should be backported to the three existing,
shipped, live-gated endpoints.

Why this is an issue and not a drive-by fix

Backporting IsNullOrWhiteSpace to the three existing interceptors is a behaviour change on
endpoints that are already shipped and live-gated (telemetry stream, config-serve fetch-and-cache,
LocalDb sync) — a deployment currently relying on a whitespace-only key (however unlikely) would
start failing closed instead of accepting a whitespace bearer. That warrants its own decision and
its own live gate, rather than a silent drive-by change bundled into the gRPC migration's Phase 0.

Suggested scope

Decide whether to align all three (and any future auth interceptors) on
string.IsNullOrWhiteSpace, then live-gate each affected endpoint (telemetry stream, config
fetch, LocalDb sync) to confirm a real whitespace-only-key deployment (if any exists) does not
silently break, before merging.

## Summary The three pre-existing gRPC auth interceptors — `TelemetryStreamAuthInterceptor`, `ConfigServeAuthInterceptor`, `LocalDbSyncAuthInterceptor` (all in `src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/`) — guard against an **unset** configured key with `string.IsNullOrEmpty(expected)`. A whitespace-only configured key (e.g. `" "`) is therefore treated as *set*, and the interceptor proceeds to `FixedTimeEquals(presented, expected)` — so a whitespace-only configured key opens the endpoint to a whitespace-only bearer token, rather than failing closed the way an actually-unset key does. Code references (as of `feat/grpc-phase0-contract-auth`, commit `c2bfad4a`): - `TelemetryStreamAuthInterceptor.cs:120` - `ConfigServeAuthInterceptor.cs:113` - `LocalDbSyncAuthInterceptor.cs:118` The two **new** mesh-transport interceptors added in the ClusterClient→gRPC migration's Phase 0 (`MeshCommandAuthInterceptor`, node-side, and `CentralCommandAuthInterceptor`, central-side) use `string.IsNullOrWhiteSpace` for the equivalent check instead, closing this specific gap in the new code. This issue tracks whether the same hardening should be backported to the three existing, shipped, live-gated endpoints. ## Why this is an issue and not a drive-by fix Backporting `IsNullOrWhiteSpace` to the three existing interceptors is a **behaviour change** on endpoints that are already shipped and live-gated (telemetry stream, config-serve fetch-and-cache, LocalDb sync) — a deployment currently relying on a whitespace-only key (however unlikely) would start failing closed instead of accepting a whitespace bearer. That warrants its own decision and its own live gate, rather than a silent drive-by change bundled into the gRPC migration's Phase 0. ## Suggested scope Decide whether to align all three (and any future auth interceptors) on `string.IsNullOrWhiteSpace`, then live-gate each affected endpoint (telemetry stream, config fetch, LocalDb sync) to confirm a real whitespace-only-key deployment (if any exists) does not silently break, before merging.

dohertj2 referenced this issue from a commit

2026-08-06 05:04:43 -04:00

chore(mesh): rig PSK config + docs stub

dohertj2 referenced this issue from a commit

2026-08-06 19:36:00 -04:00

fix(auth): #543 whitespace-only configured key fails closed in the three pre-existing gRPC auth interceptors

dohertj2 referenced this issue from a commit

2026-08-06 19:36:00 -04:00

docs(auth): #543 add the one-public-ctor remarks paragraph to the two newly-pinned interceptors

dohertj2 referenced this issue from a commit

2026-08-06 19:36:00 -04:00

Merge fix/issues-543-544-545: whitespace-key fail-closed backport (#543), de-flaked deadline test (#544), h2c accepted-posture record (#545)

dohertj2 commented

2026-08-06 19:37:00 -04:00

Fixed, merged to master @ 1df84703 and pushed (commit f10f4c37, doc follow-up 48707852).

All three interceptors now guard with string.IsNullOrWhiteSpace (Telemetry→PermissionDenied, ConfigServe→Unauthenticated, LocalDbSync→PermissionDenied — status codes and message text unchanged), matching the two mesh interceptors. Red-first verified: with the source change stashed, exactly the 3 new _WithBlankKeyConfigured_IsDenied facts failed — a configured key " " really was satisfied by bearer " ". Also added the two missing HasExactlyOnePublicConstructor pins (ConfigServe/LocalDbSync were unpinned; Telemetry already had one) and the sibling-standard <remarks> paragraph.

On the issue's decision + live-gate asks:

Decision: align all five on IsNullOrWhiteSpace — done. rg IsNullOrEmpty under Host/Configuration now returns only one non-auth filesystem-path check. Code review (security lens) confirmed no trim anywhere in the bearer-extraction chain (a trimmed comparison could have reopened the hole) and no new timing signal (guard ordering unchanged; FixedTimeEquals still the only byte comparison).
No deployment relies on a whitespace-only key: docker-dev composes set real values (telemetry-docker-dev-key etc.) and production delivers via ${secret:...}.
Live gate (docker-dev, image built from merged master @ 9420f8ef):
- Positive, all three planes with real keys: deploy dispatched → sealed 6/6 acks (exercises ConfigServe fetch auth); telemetry streams re-established after node recreate (failure streaks cleared); LocalDb site-a pair fault-free over a 30-minute window with the sealed deploy flowing through its cache path.
- Negative (telemetry plane, site-b-2 recreated with Telemetry__ApiKey: " "): whitespace bearer → PermissionDenied ("no API key is configured on this node") — the exact call that pre-fix would have been ACCEPTED; real key against the whitespace-configured node → PermissionDenied (fail-closed, as an unset key behaves). Override reverted; positive probe with the real key then held an open stream (DeadlineExceeded after the 6 s client window, not a denial), and central's dialer recovered with no new failures past its backoff window.

Full suite on the merged SHA: exit 0, 50/50 suites, 9,333 passed / 0 failed.

Fixed, merged to `master` @ `1df84703` and pushed (commit `f10f4c37`, doc follow-up `48707852`). All three interceptors now guard with `string.IsNullOrWhiteSpace` (Telemetry→PermissionDenied, ConfigServe→Unauthenticated, LocalDbSync→PermissionDenied — status codes and message text unchanged), matching the two mesh interceptors. **Red-first verified:** with the source change stashed, exactly the 3 new `_WithBlankKeyConfigured_IsDenied` facts failed — a configured key `" "` really was satisfied by bearer `" "`. Also added the two missing `HasExactlyOnePublicConstructor` pins (ConfigServe/LocalDbSync were unpinned; Telemetry already had one) and the sibling-standard `<remarks>` paragraph. On the issue's decision + live-gate asks: - **Decision:** align all five on `IsNullOrWhiteSpace` — done. `rg IsNullOrEmpty` under Host/Configuration now returns only one non-auth filesystem-path check. Code review (security lens) confirmed no trim anywhere in the bearer-extraction chain (a trimmed comparison could have reopened the hole) and no new timing signal (guard ordering unchanged; `FixedTimeEquals` still the only byte comparison). - **No deployment relies on a whitespace-only key:** docker-dev composes set real values (`telemetry-docker-dev-key` etc.) and production delivers via `${secret:...}`. - **Live gate (docker-dev, image built from merged `master` @ `9420f8ef`):** - *Positive, all three planes with real keys:* deploy dispatched → sealed **6/6 acks** (exercises ConfigServe fetch auth); telemetry streams re-established after node recreate (failure streaks cleared); LocalDb site-a pair fault-free over a 30-minute window with the sealed deploy flowing through its cache path. - *Negative (telemetry plane, site-b-2 recreated with `Telemetry__ApiKey: " "`):* whitespace bearer → **PermissionDenied** ("no API key is configured on this node") — the exact call that pre-fix would have been ACCEPTED; real key against the whitespace-configured node → **PermissionDenied** (fail-closed, as an unset key behaves). Override reverted; positive probe with the real key then held an open stream (DeadlineExceeded after the 6 s client window, not a denial), and central's dialer recovered with no new failures past its backoff window. Full suite on the merged SHA: exit 0, 50/50 suites, 9,333 passed / 0 failed.

dohertj2 closed this issue

2026-08-06 19:37:04 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#543