diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index 6911b42..afb5605 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -104,7 +104,7 @@ Full design + implementation for each row lives in the linked domain doc under i | SEC-33 | Low | P1 | M | old SEC-23 (co-locate) | Done | Host-meaningful path rooting; drop Windows literals from appsettings; validate Galaxy `SnapshotCachePath` | | SEC-34 | Low | P2 | S | — | Done | Verification cache: expiry outlives TTL; `Invalidate` races in-flight repopulation | | SEC-35 | Info | — | S | — | N/A (doc note discharged 2026-08-07) | Production hard-stops key on exact `Production` environment name (doc-only) | -| SEC-36 | Low | P1 | M | cross-repo `scadaproj/infra/glauth` | Not started | Committed dev LDAP service-account password: rotate, remove, move dev channel to user-secrets | +| SEC-36 | Low | P1 | M | cross-repo `scadaproj/infra/glauth` | Done (repo-side; live rotation is operator-pending per runbook) | Committed dev LDAP service-account password: rotate, remove, move dev channel to user-secrets | ### Clients — [50-clients.md](50-clients.md) @@ -174,4 +174,5 @@ Sequence these together rather than piecemeal — several are one change set spa | 2026-08-07 | **GWC-28, GWC-29, GWC-30, TST-28 → `Done`** (branch `fix/gwc-28-29-30-polish`). GWC-28: `WorkerClient.WriteLoopAsync` now stamps `envelope.Sequence = unchecked(++_nextSequence)` immediately before `_writer.WriteAsync`, and `CreateEnvelope` leaves it unset; `_nextSequence` dropped from `long` + `Interlocked` to a plain `ulong` touched only by the write loop (the channel's single consumer, `SingleReader = true`), so wire order and sequence order are the same thing by construction. Mirrors the worker's WRK-04 stamping, which the gateway half had never received; `gateway.md`'s envelope-sequence rule now states that both sides stamp at write inside their single write path and that inbound enforcement (still open, old **GWC-10**) would rely on it. New `WorkerClientTests.ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire` (32 parallel invokes, sequences asserted strictly increasing in wire order) failed 3/3 pre-fix. GWC-29: added `MxAccessGrpcMapper.MapCommand(MxCommand)`; `Invoke` no longer deep-clones the whole `MxCommandRequest` just to overwrite and discard its command. The one clone inside `MapCommand` stays and is documented as required — `commandToInvoke` may be the gRPC-owned `request.Command` and is read again after dispatch by `TrackCommandReply`, so it is what keeps `CreateCommandEnvelope`'s no-aliasing invariant true. New `MxAccessGrpcMapperTests.MapCommandFromCommandClonesPayload` (isolation + both overloads equal under a `FakeTimeProvider`). GWC-30: `WorkerFrameReader` reuses a per-instance `_lengthPrefix` scratch buffer instead of allocating 4 bytes per frame, with a class remark that `ReadAsync` is not reentrant (single read loop per `WorkerClient`; handshake reads complete before the loop starts); guarded by new `WorkerFrameProtocolTests.ReadAsync_WithMultipleFramesOnOneReader_ParsesEveryFrame` (5 frames, varying payload lengths, one reader). TST-28: new `[Theory] WorkerClientTests.StartAsync_SendsGatewayHelloWithConfiguredMaxFrameBytes` over the default and a 2 MiB override via `FakeWorkerHarness.CreateConnectedPairAsync(maxMessageBytes:)` — test-only, and the mutation check (hard-code `MaxFrameBytes = 0`) failed both cases before being reverted. Verification: `NonWindows.slnx` 0 warnings/0 errors; `WorkerClientTests` 25 passed, `WorkerFrameProtocolTests` 11 passed, `MxAccessGrpcMapperTests` 6 passed, `MxAccessGatewayService*` 29 passed, full gateway suite 844 passed / 0 failed (`TMPDIR=/tmp` on macOS). | | 2026-08-07 | **WRK-21 + WRK-28 + WRK-23 + IPC-30 → `Done`** (branch `fix/wrk-21-drain-cluster`, commits `33ba612` + test-fixture follow-ups `7c2eaf0`/`a256560`). WRK-21: `MxAccessEventQueue` gains a byte-budgeted `Drain(maxEvents, maxTotalBytes)` returning the new `WorkerEventDrainResult`, sizing inside the queue lock so an event that will not fit is never dequeued; `CreateDrainEventsReply` budgets against the negotiated frame max less a 64 KiB wrapper reserve and reports truncation through the existing `DiagnosticMessage` (no proto change), satisfying IPC-23 R1–R3; both reply-write seams (`HandleControlCommandAsync`, `ProcessCommandAsync`) now catch `MessageTooLarge` and answer the correlation with an `InvalidRequest` reply instead of unwinding/faulting the session. WRK-28: the 10,000 ceiling moved to `GatewayContractInfo.MaxDrainEventsPerCommand`, referenced by the gateway validator and the worker clamp (C# const, no `.proto` change). WRK-23: `WorkerFrameWriter` peek-stamps then commits `Sequence` only immediately before the stream write, so rejections leave no wire gap. IPC-30: an oversized event frame stays session-fatal but writes a `PROTOCOL_VIOLATION` `WorkerFault` with `command_method = EventDrain` naming family/handles/sequence/sizes (never the value) before exiting. Docs same commit: `MxAccessWorkerInstanceDesign.md`, `WorkerFrameProtocol.md`, `gateway.md`. **IPC-23 → `In progress`** — mechanics landed here; the proto-comment/doc wave (and its regen fan-out) is still pending and must not be folded into this branch. **Evidence** — macOS: `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` 0 warnings/0 errors, `dotnet test …MxGateway.Tests --filter FullyQualifiedName~MxAccessGrpcRequestValidator` 4/4 passed. windev (`scripts/ci/windev-worker-ci.ps1 -Sha a2565604 -Mode test`, 2026-08-07 06:47): x86 Worker build 0 warnings/0 errors, `Worker.Tests` **367 passed / 0 failed / 11 skipped** (skips are the live-MXAccess/dev-rig opt-ins), script exit 0. **Harness note:** `PipePair` runs both pipe ends in one process with blocking `FlushFileBuffers` per frame, so it wedges on multi-MB frames or after ~85 large round trips; the pipe tests therefore negotiate a 128 KiB frame maximum and walk 1,000 events to empty, while the full 10,000-event drain-to-empty no-loss proof runs at the queue layer (`MxAccessEventQueueTests`). | | 2026-08-07 | Code-review follow-ups on the same branch (commit `6bc3f9b`). (1) **Important** — `ResolveDrainReplyByteBudget` was a step, not a floor: just above the 64 KiB reserve the budget collapsed to a few bytes (exactly 1024 at the validator floor `MaxMessageBytes = 1024 + 64 KiB`), so a byte-heavy `DrainEvents` truncated on every call and the drain-until-empty loop never terminated. Now `Math.Max(frameMax - reserve, frameMax / 2)` — monotonic, never below half the frame max. New test `WorkerPipeSessionTests.DrainEvents_AtValidatorFloorFrameMax_MakesProgressAndTerminates` drives a byte-heavy queue at the exact validator floor and asserts drain-to-empty with no head reported oversized. (2) **Hardening** — the reply-too-large fallback write is now itself size-guarded (`WriteReplyTooLargeFallbackAsync`, shared by the control and STA reply seams) so a pathologically tiny negotiated max below the gateway floor (the WRK-24 gap) cannot make even the backstop session-fatal; log-and-swallow, comment points at WRK-24. (3) **Comment** — corrected the `RepeatedFieldOverheadBytes` docs: `WorkerEvent.CalculateSize()` already includes the event's tag+length, so the 8 bytes is pure slack, not wrapper compensation. **Evidence** — macOS build 0/0, validator filter 4/4. windev (`windev-worker-ci.ps1 -Sha 6bc3f9b -Mode test`, 07:07): x86 Worker build 0/0, `Worker.Tests` **368 passed / 0 failed / 11 skipped**, script exit 0. (An earlier run of the same SHA flaked on the pre-existing `RunAsync_WhenStaActivityIsStale_WritesWatchdogFault` — a 5 s CTS timeout under first-run load, untouched by this change; it passed on the clean re-run and in both prior full runs.) | +| 2026-08-07 | **SEC-36 → `Done` (repo-side); live rotation operator-pending** (branch `fix/sec-36-ldap-secret`). **State reconciliation:** the plaintext-in-`appsettings.json` half was already discharged before this branch — HEAD ships `"ServiceAccountPassword": "${secret:ldap/mxgateway/bind}"` (a fail-closed encrypted-store reference, documented at `GatewayConfiguration.md:252`, tested by `PreHostSecretExpansionTests`), not the literal the spec's stale baseline assumed. **Deviation from spec step 2:** the `${secret:}` reference was **kept, not deleted** — deleting it would regress the shipped/documented/tested secret-store channel and the finding (committed plaintext) is already resolved for `appsettings.json`. The live residual leak was the literal value in `glauth.md`, `docs/GatewayTesting.md`, and the historical `archreview/*` SEC-06 evidence — all scrubbed to `` placeholders with a source-of-truth pointer (`scadaproj/infra/glauth/`). Added `mxaccessgw-server` to the server csproj (dev channel) and extended the `ValidateLdap` blank-password message to name both channels (dev user-secrets, deployed `MxGateway__Ldap__ServiceAccountPassword`) — asserted by the updated `Validate_Fails_WhenLdapEnabledAndServiceAccountPasswordBlank`. Docs same commit: `GatewayConfiguration.md` (three-channel + rotation note), `glauth.md` (placeholders + rotation-required note + runbook pointer), `docs/GatewayTesting.md` (live-LDAP env-var example de-secreted). New operator runbook `docs/runbooks/SEC-36-ldap-credential-rotation.md` carries the cutover order (generate in glauth → pre-stage NEW env var on `10.100.0.48` and on `wonder-app-vd03` only if `Ldap.Enabled` → rotate GLAuth on `10.100.0.35` → verify dashboard login → devs set user-secrets). **The live GLAuth rotation + NSSM staging remain the operator's to execute** and are tracked in that runbook. Verification (macOS): `dotnet build …Server` 0 warnings/0 errors; `--filter ~GatewayOptionsValidator` green; `git grep -i` for the old value returns nothing across tracked files. | | 2026-08-07 | **P1 doc-drift batch → `Done`: TST-27, WRK-26 (discharges IPC-29), CLI-42, CLI-43, IPC-28** (branch `fix/doc-drift-batch`). Doc-only; no source, proto, or test changes — cross-checked against HEAD in this worktree. **TST-27**: `docs/GatewayConfiguration.md`'s `ShowTagValues` row no longer says "Reserved" — it now states what `false` (default) does (`DashboardEventBroadcaster` blanks tag values from a deep-cloned `MxEvent` before the SignalR events-hub mirror, metadata still renders), the security relevance (the per-session hub ACL, SEC-25 roadmap item 12, still does not exist, so this redaction is the only thing between a low-trust Viewer and other sessions' tag values), and the honest scope limit (the flag does **not** cover `/browse`). **WRK-26** (discharges **IPC-29**): `docs/MxAccessWorkerInstanceDesign.md`'s "Outbound Queues" section rewritten from the stale five-level priority list to the two-class `Control`/`Event` scheduler actually shipped (`WorkerFrameWriter`/`WorkerFrameWritePriority.cs`), with the collapsed-decision rationale recorded, and the overflow paragraph rewritten to the implemented fail-fast (`WorkerFault` category `QueueOverflow` → fault frame written → `RunAsync` unwinds → generic `WorkerExitCode.UnexpectedFailure`, dedicated code still open). `docs/WorkerFrameProtocol.md` gained a new "Write Scheduling And Sequencing" section: the two priority classes, enqueue-then-contend/single-lock-holder-drains-all, write-time peek-stamp-commit sequencing, per-frame-rejection vs. stream-failure semantics, and flush coalescing — stated truthfully as landed (WRK-23's peek-stamp-commit is live at HEAD) or not (the drain loop still awaits each event `WriteAsync` individually, so WRK-25's N-events-one-flush batching has **not** landed and the section says so explicitly). Cancellation is deliberately **not** documented as a firm contract — a one-paragraph placeholder notes it is pending WRK-22, which has not landed (confirmed by reading `WorkerFrameWriter.cs`: no `Claimed`/tombstone machinery exists yet). `gateway.md:328-330` was cross-checked and left unchanged — its sequence prose (both sides stamp at write, per GWC-28) already reads true. **CLI-42**: `clients/rust/README.md` and `docs/ClientPackaging.md`'s Rust section now document the vendored proto layout matching `clients/rust/build.rs` exactly — repo-path-first resolution (`../../src/ZB.MOM.WW.MxGateway.Contracts/Protos`) falling back to `clients/rust/protos/` when the canonical path is absent (published-tarball case), the same-commit refresh rule enforced by `scripts/check-codegen.ps1` Check 3, and why `cargo package`/`cargo publish` run without `--no-verify` (matches `scripts/pack-clients.ps1:190-192`). **CLI-43**: `docs/style-guides/JavaStyleGuide.md` line 8 now says "Target Java 17 (the Ignition 8.3 baseline...)" mirroring the CLI-12 wording, matching the shipped `clients/java/build.gradle` toolchain-17 build. **IPC-28**: `docs/Grpc.md`'s exception-mapping prose gained `CommandTooLarge` → `ResourceExhausted` (verified against the live `switch` in `Grpc/MxAccessGatewayService.cs:950-960`), and the `Invoke` section gained one sentence on the oversized-payload path (`WorkerClient.InvokeAsync` rejects at the enqueue boundary per-correlation, session not faulted — verified against `WorkerClient.cs:220-234`), cross-referencing the headroom rule already documented in `docs/GatewayConfiguration.md:120-129`. Did not touch the DrainEvents-truncation row or the proto/`Generated/` trees — those belong to a parallel codegen task per the handoff note. **Source files cross-read for accuracy** (no edits): `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs`, `.../WorkerFrameWritePriority.cs`, `.../WorkerPipeSession.cs` (confirmed two-class scheduler, WRK-21/23/28/30 landed, WRK-25/WRK-22 not landed), `src/ZB.MOM.WW.MxGateway.Worker/WorkerApplication.cs` (exit-code mapping), `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs` (overflow fault path), `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs` + `Configuration/DashboardOptions.cs` + `docs/GatewayDashboardDesign.md:170` (ShowTagValues), `src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs:940-963` + `Workers/WorkerClient.cs:205-244` + `Workers/WorkerClientErrorCode.cs` (CommandTooLarge mapping), `clients/rust/build.rs`, `clients/rust/Cargo.toml`, `scripts/check-codegen.ps1`, `scripts/pack-clients.ps1` (Rust vendoring), `gateway.md:326-360` (sequence-prose cross-check). Verification (greps, doc-only — no build required): `grep -n 'Reserved' docs/GatewayConfiguration.md` no longer matches the `ShowTagValues` row; `grep -n 'faults' docs/MxAccessWorkerInstanceDesign.md` shows no remaining five-level list; `grep -n 'scheduling' docs/WorkerFrameProtocol.md` finds the new section; `grep -rn 'Java 21' docs/style-guides/` empty; `grep -i vendored docs/ClientPackaging.md clients/rust/README.md` non-empty in both; `grep -n 'CommandTooLarge' docs/Grpc.md` shows the mapping. | diff --git a/archreview/2026-07-12/remediation/40-security-dashboard.md b/archreview/2026-07-12/remediation/40-security-dashboard.md index 909ac8b..438caf2 100644 --- a/archreview/2026-07-12/remediation/40-security-dashboard.md +++ b/archreview/2026-07-12/remediation/40-security-dashboard.md @@ -15,7 +15,7 @@ Repo rules that bind every entry: docs change in the same commit as the source ( | SEC-33 | Low | P1 | M | — (co-locate SEC-23) | Done | Any-platform path-rooting acceptance re-opens SEC-01 on Unix; Galaxy `SnapshotCachePath` unvalidated | | SEC-34 | Low | P2 | S | — | Done | Verification cache: expiry outlives TTL; `Invalidate` races in-flight repopulation | | SEC-35 | Info | — | S | — | N/A (doc-only note discharged 2026-08-07) | Production hard-stops key on the exact `Production` environment name | -| SEC-36 | Low | P1 | M | cross-repo (`scadaproj/infra/glauth`) | Not started | Committed dev LDAP service-account password: remove from repo and rotate | +| SEC-36 | Low | P1 | M | cross-repo (`scadaproj/infra/glauth`) | Done (repo-side; live rotation operator-pending per runbook) | Committed dev LDAP service-account password: remove from repo and rotate | --- @@ -218,3 +218,5 @@ dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --fil dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~GatewayOptionsValidator" ``` (asserts the blank-password validation still fires with the updated message). Manual: with user-secrets set on the dev box, `dotnet run --project src/ZB.MOM.WW.MxGateway.Server/...` and a dashboard `/login` as `multi-role` succeeds against the rotated GLAuth; the deployed-host login re-check from step 1 counts as the production verification. Live-LDAP integration tests (`MXGATEWAY_RUN_LIVE_LDAP_TESTS=1`) only where the GLAuth instance is reachable; otherwise document skipped per the testing matrix. + +**Outcome (2026-08-07 — Done, repo-side; live rotation operator-pending).** Landed on `fix/sec-36-ldap-secret`. **The design's baseline had already shifted:** at HEAD `appsettings.json` no longer commits the literal — it ships `"ServiceAccountPassword": "${secret:ldap/mxgateway/bind}"`, a fail-closed encrypted-store reference (documented `GatewayConfiguration.md:252`, tested by `PreHostSecretExpansionTests`) introduced by the Secrets-store adoption after this remediation was written. **Deviation from Implementation step 2:** the `${secret:}` reference was **kept, not deleted** — deleting it regresses the shipped/documented/tested store channel and the committed-plaintext finding is already resolved for `appsettings.json`. The load-bearing residual — the literal value still present in `glauth.md`'s samples (`:33,65,103,136,245`), `docs/GatewayTesting.md`, and the historical `archreview/*` SEC-06 evidence — was scrubbed to `` placeholders, each with a pointer to the source of truth `scadaproj/infra/glauth/` and a rotation-required note. Steps 3–6 implemented as designed: `mxaccessgw-server` added (step 3); the `ValidateLdap` blank-password message now names both channels — dev `dotnet user-secrets set "MxGateway:Ldap:ServiceAccountPassword" ` and deployed `MxGateway__Ldap__ServiceAccountPassword` — plus a note on the `${secret:}` store default (step 4), asserted by the extended `Validate_Fails_WhenLdapEnabledAndServiceAccountPasswordBlank`; docs updated same commit (step 5); `git grep -i` for the old value is empty across tracked files (step 6). The cross-repo **step 1 (rotate GLAuth on `10.100.0.35`, pre-stage the NSSM env var on `10.100.0.48` and on `wonder-app-vd03` only if `Ldap.Enabled`, verify dashboard login)** is the operator's to execute, captured in the new runbook `docs/runbooks/SEC-36-ldap-credential-rotation.md`. Verification (macOS): `dotnet build …Server` 0 warnings/0 errors; `dotnet test --filter ~GatewayOptionsValidator` green. diff --git a/archreview/40-security-dashboard.md b/archreview/40-security-dashboard.md index 44bb408..c80b122 100644 --- a/archreview/40-security-dashboard.md +++ b/archreview/40-security-dashboard.md @@ -49,7 +49,7 @@ Impact: logout (`Dashboard/DashboardEndpointRouteBuilderExtensions.cs:136-155`) Recommendation: keep the lifetime short (or shorten to ~5 minutes given the factory refreshes per reconnect, `docs/GatewayDashboardDesign.md:497-499`), and confirm no request-path logging captures query strings (Serilog request logging is not currently enabled; keep it that way or scrub `access_token`). **SEC-6 · Medium — LDAP is plaintext-by-default with a committed service-account password.** -Evidence: `src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs:49-61` (defaults `Transport=None`, `AllowInsecure=true`, `ServiceAccountPassword = "serviceaccount123"`), `appsettings.json:21-33` (same values checked into the repo), `glauth.md:30,327` (dev LDAPS disabled; "binding sends passwords cleartext on the wire"). +Evidence: `src/ZB.MOM.WW.MxGateway.Server/Configuration/LdapOptions.cs:49-61` (defaults `Transport=None`, `AllowInsecure=true`, `ServiceAccountPassword = ""` — value redacted per SEC-36), `appsettings.json:21-33` (same values checked into the repo), `glauth.md:30,327` (dev LDAPS disabled; "binding sends passwords cleartext on the wire"). Impact: every dashboard login sends the operator's password in cleartext to `10.100.0.35:3893`, and the LDAP service-account credential is in source control. This is a documented dev posture (the shadow-options rationale at `LdapOptions.cs:20-28` is explicit that the shared library is secure-by-default), and the validator does enforce the `Transport=None ⇒ AllowInsecure` consistency rule (`GatewayOptionsValidator.cs:82-85`) — but nothing distinguishes dev from prod at runtime. Recommendation: for production deployment docs, require `Transport=Ldaps`/`StartTls` + `AllowInsecure=false` and move `ServiceAccountPassword` to env-var/secret configuration; consider an `IsProduction` startup check mirroring SEC-4. LDAP injection risk is delegated to the shared `ZB.MOM.WW.Auth.Ldap` provider (bind-then-search per `Dashboard/DashboardAuthenticator.cs:41-47`); its escaping cannot be verified from this repo — flag for review in the donor repo. diff --git a/archreview/remediation/40-security-dashboard.md b/archreview/remediation/40-security-dashboard.md index 79ccf00..90af3a1 100644 --- a/archreview/remediation/40-security-dashboard.md +++ b/archreview/remediation/40-security-dashboard.md @@ -132,7 +132,7 @@ This document turns every finding in the Security/Dashboard/Observability review ## SEC-06 — LDAP plaintext-by-default with a committed service password `Medium` · `P1` -**Finding.** *(review SEC-6)* `Configuration/LdapOptions.cs:49-61` defaults `Transport=None`, `AllowInsecure=true`, `ServiceAccountPassword="serviceaccount123"`; `appsettings.json:21-33` ships the same. `glauth.md:30,327` confirms dev LDAPS is disabled and binds send cleartext. The validator enforces the `Transport=None ⇒ AllowInsecure` consistency rule (`GatewayOptionsValidator.cs:82-85`) but nothing distinguishes dev from prod. +**Finding.** *(review SEC-6)* `Configuration/LdapOptions.cs:49-61` defaults `Transport=None`, `AllowInsecure=true`, `ServiceAccountPassword=""` (value redacted per SEC-36); `appsettings.json:21-33` ships the same. `glauth.md:30,327` confirms dev LDAPS is disabled and binds send cleartext. The validator enforces the `Transport=None ⇒ AllowInsecure` consistency rule (`GatewayOptionsValidator.cs:82-85`) but nothing distinguishes dev from prod. **Impact.** Every dashboard login sends the operator's password cleartext to `10.100.0.35:3893`, and a service-account credential is in source control. @@ -140,7 +140,7 @@ This document turns every finding in the Security/Dashboard/Observability review **Implementation.** - `Configuration/GatewayOptionsValidator.cs`: in `ValidateLdap`, when Production and `Transport == None`, emit an error (co-locate with SEC-04's env plumbing). -- Deployment: keep `serviceaccount123` only for local GLAuth dev; document env-var override (`MxGateway__Ldap__ServiceAccountPassword`) for the NSSM-wrapped hosts; rotate the dev credential's reuse. +- Deployment: keep the dev service-account password only for local GLAuth dev; document env-var override (`MxGateway__Ldap__ServiceAccountPassword`) for the NSSM-wrapped hosts; rotate the dev credential's reuse. - Docs: `docs/GatewayConfiguration.md` Ldap section and a production hardening note referencing `glauth.md`. - Tests: `GatewayOptionsValidatorTests` — `Transport=None` + Production → invalid. diff --git a/docs/GatewayConfiguration.md b/docs/GatewayConfiguration.md index dbb7f2b..2790940 100644 --- a/docs/GatewayConfiguration.md +++ b/docs/GatewayConfiguration.md @@ -249,7 +249,7 @@ dev/test GLAuth posture (`glauth.md`), not a production posture. | `MxGateway:Ldap:AllowInsecure` | `true` | Permits a plaintext bind. Must be `true` when `Transport` is `None`; set `false` (with `Ldaps`/`StartTls`) in production. | | `MxGateway:Ldap:SearchBase` | `dc=zb,dc=local` | Search base DN. | | `MxGateway:Ldap:ServiceAccountDn` | `cn=serviceaccount,dc=zb,dc=local` | Bind DN for the search account. | -| `MxGateway:Ldap:ServiceAccountPassword` | `${secret:ldap/mxgateway/bind}` | Search-account password. **No longer a committed plaintext value:** `appsettings.json` ships the reference `${secret:ldap/mxgateway/bind}`, which the pre-host `${secret:}` expander resolves at startup from the encrypted secrets store (the code-side design default is now blank, so a missing/unresolved value fails closed rather than falling back to a leaked credential). Seed the value once with `secret set ldap/mxgateway/bind ` (the store's master key must be present via `ZB_SECRETS_MASTER_KEY`); startup aborts with `SecretNotFoundException` if the secret is absent. An operator may instead override it directly with the env var `MxGateway__Ldap__ServiceAccountPassword` (double-underscore form) — a plain literal there is used as-is and the secret lookup is skipped. | +| `MxGateway:Ldap:ServiceAccountPassword` | `${secret:ldap/mxgateway/bind}` | Search-account password. **Never a committed plaintext value (SEC-36):** the shared GLAuth bind credential is supplied out-of-band through one of three channels, all binding to this key. **(1) Encrypted secrets store (shipped default):** `appsettings.json` ships the reference `${secret:ldap/mxgateway/bind}`, which the pre-host `${secret:}` expander resolves at startup from the encrypted secrets store (the code-side design default is blank, so a missing/unresolved value fails closed rather than falling back to a leaked credential). Seed it once with `secret set ldap/mxgateway/bind ` (the store's master key must be present via `ZB_SECRETS_MASTER_KEY`); startup aborts with `SecretNotFoundException` if the secret is absent. **(2) Deployed hosts — env var:** override directly with `MxGateway__Ldap__ServiceAccountPassword` (double-underscore form) in the NSSM service environment — a plain literal there is used as-is and the store lookup is skipped. **(3) Dev boxes — user-secrets:** `dotnet user-secrets set "MxGateway:Ldap:ServiceAccountPassword" ` (the server carries `mxaccessgw-server`; user-secrets load automatically in the Development environment and live under the user profile, outside the tree). The value comes from the GLAuth source of truth `scadaproj/infra/glauth/`, never from a repo file. **Rotation:** because the credential was historically committed, rotating it in `scadaproj/infra/glauth/` (and redeploying the shared GLAuth on `10.100.0.35`) is required — see `docs/runbooks/SEC-36-ldap-credential-rotation.md` for the cutover order. A blank/unresolved value fails startup validation with a message naming the two supported channels. | | `MxGateway:Ldap:UserNameAttribute` | `cn` | LDAP attribute holding the login user name. | | `MxGateway:Ldap:DisplayNameAttribute` | `cn` | LDAP attribute holding the display name. | | `MxGateway:Ldap:GroupAttribute` | `memberOf` | LDAP attribute enumerating group membership (mapped to dashboard roles via `MxGateway:Dashboard:GroupToRole`). | diff --git a/docs/GatewayTesting.md b/docs/GatewayTesting.md index 074a20f..82df0bf 100644 --- a/docs/GatewayTesting.md +++ b/docs/GatewayTesting.md @@ -241,14 +241,15 @@ an unreachable LDAP server is absorbed into a failed result rather than throwing pre-host secrets expander, which this suite's bare `ConfigurationBuilder` does not run). Before running the live LDAP suite, set `MxGateway__Ldap__ServiceAccountPassword` to the real GLAuth service-account -password (dev value `serviceaccount123` for the shared GLAuth) so the suite -binds with the real password instead of the literal token. +password so the suite binds with the real password instead of the literal +token. Obtain the current value from the GLAuth source of truth +`scadaproj/infra/glauth/` (per `glauth.md`); it is not committed here. Run the LDAP live tests explicitly: ```bash $env:MXGATEWAY_RUN_LIVE_LDAP_TESTS = "1" -$env:MxGateway__Ldap__ServiceAccountPassword = "serviceaccount123" +$env:MxGateway__Ldap__ServiceAccountPassword = "" dotnet test src/ZB.MOM.WW.MxGateway.IntegrationTests/ZB.MOM.WW.MxGateway.IntegrationTests.csproj --filter FullyQualifiedName~DashboardLdapLiveTests ``` diff --git a/docs/runbooks/SEC-36-ldap-credential-rotation.md b/docs/runbooks/SEC-36-ldap-credential-rotation.md new file mode 100644 index 0000000..cba3021 --- /dev/null +++ b/docs/runbooks/SEC-36-ldap-credential-rotation.md @@ -0,0 +1,119 @@ +# SEC-36 — LDAP Service-Account Credential Rotation (Operator Runbook) + +Operator steps to rotate the shared GLAuth service-account password after the repo-side +removal landed (SEC-36). The repo change (removal of the committed value, the two supported +secret channels, and this runbook) is already merged; the live rotation below is the +load-bearing half and is yours to execute. + +> **Never put the old or new password in this repo, in a commit, in a chat, or in this file.** +> The value lives only in the GLAuth source of truth and in each host's out-of-band channel. + +## Why + +The dev GLAuth service-account password (`cn=serviceaccount,dc=zb,dc=local`) was historically +committed to this repo. Removal alone is insufficient — the old value is permanently recoverable +from git history — so **rotation is required**. Until the shared GLAuth on `10.100.0.35:3893` +stops honoring the old value, the repo history discloses a live directory account with LDAP +search capability over `dc=zb,dc=local`. + +## Where the credential lives now (three channels, all bind `MxGateway:Ldap:ServiceAccountPassword`) + +- **Source of truth:** `scadaproj/infra/glauth/config.toml` on host `10.100.0.35` (the `serviceaccount` + user's `passsha256`). `scadaproj` is a shared monorepo — stage only the explicit glauth paths. +- **Encrypted secrets store (gateway default):** `appsettings.json` ships `${secret:ldap/mxgateway/bind}`, + resolved from the local encrypted store (seed with `secret set ldap/mxgateway/bind `). +- **Deployed hosts:** env var `MxGateway__Ldap__ServiceAccountPassword` in the NSSM service environment. +- **Dev boxes:** `dotnet user-secrets set "MxGateway:Ldap:ServiceAccountPassword" ` + (the server carries `mxaccessgw-server`). + +See `docs/GatewayConfiguration.md` (the `ServiceAccountPassword` row) and `glauth.md`. + +## Preconditions + +- SSH access to the GLAuth docker host `10.100.0.35` and to the deployed gateway host(s). +- Write access to `scadaproj/infra/glauth/`. +- Know which deployed hosts run LDAP-backed dashboard login: + - **`10.100.0.48`** (`windev`) — primary; verify here. + - **`wonder-app-vd03`** — its dashboard is disabled. **Check `MxGateway:Ldap:Enabled` there first.** + If LDAP is disabled (`Enabled=false`), it has nothing to bind and needs no env var — skip it. +- A generated replacement secret (see step 1). Generate the `passsha256` per `glauth.md` + ("Generate `passsha256` from a plaintext password"). + +## Cutover order + +Follow this order so no window opens where the deployed dashboard cannot bind. **Do not rotate +GLAuth before the deployed hosts already carry the new value.** + +1. **Generate the new secret in `scadaproj/infra/glauth/`.** Pick a new password, compute its + `passsha256`, and stage the change to the `serviceaccount` user in `config.toml` (do not + `docker compose up` yet — the directory must keep honoring the OLD value until the deployed + hosts carry the NEW one). + +2. **Pre-stage the NEW value on every LDAP-enabled deployed host** via the env-var channel, so the + host is ready the instant GLAuth flips: + ```powershell + nssm get MxAccessGw AppEnvironmentExtra + nssm set MxAccessGw AppEnvironmentExtra MxGateway__Ldap__ServiceAccountPassword= + # restart the service so the new environment is picked up + nssm restart MxAccessGw + ``` + Do this on `10.100.0.48`, and on `wonder-app-vd03` **only if** `MxGateway:Ldap:Enabled=true` there. + (Alternatively seed the encrypted store with `secret set ldap/mxgateway/bind `; the + env var overrides the store and is the simplest per-host mechanism.) + At this moment the deployed host holds the NEW value but GLAuth still honors the OLD one — binds + still fail closed against the old directory, which is expected and brief; proceed immediately. + +3. **Rotate GLAuth on `10.100.0.35`** to honor the new value: + ```bash + ssh 10.100.0.35 + cd ~/Desktop/scadaproj/infra/glauth + docker compose up -d --force-recreate + docker compose logs -f # confirm clean startup, no TOML parse error + ``` + +4. **Verify dashboard login on the deployed host(s).** Browse to the gateway dashboard on + `10.100.0.48` and log in as `multi-role` / `password` (Administrator) — a successful login proves + the search bind used the new service-account credential end-to-end. If `wonder-app-vd03` runs + LDAP, verify it too; if its dashboard/LDAP is disabled, no check is needed. + +5. **The repo change is already landed** (removal of the committed value, ``, the + validator message naming the two channels, and doc/scrub updates). Nothing more to commit for + the cutover. + +6. **Developers set user-secrets on next pull.** After pulling, a dev box with no secret configured + will fail startup with a validation message naming the exact command. One-time per machine: + ```bash + dotnet user-secrets set "MxGateway:Ldap:ServiceAccountPassword" + ``` + (value from `scadaproj/infra/glauth/`, never from a repo file). + +## Verifying the rotation + +- **Primary:** dashboard `/login` as `multi-role` on `10.100.0.48` succeeds (step 4). +- **`wonder-app-vd03`:** only if `MxGateway:Ldap:Enabled=true`; otherwise no action. +- **Live-LDAP integration tests** (opt-in, only where the GLAuth instance is reachable): + ```bash + $env:MXGATEWAY_RUN_LIVE_LDAP_TESTS = "1" + $env:MxGateway__Ldap__ServiceAccountPassword = "" # shell env only, never committed + dotnet test src/ZB.MOM.WW.MxGateway.IntegrationTests/ZB.MOM.WW.MxGateway.IntegrationTests.csproj ` + --filter FullyQualifiedName~DashboardLdapLiveTests + ``` + A green `DashboardLdapLiveTests` run confirms the new credential binds and searches. Where GLAuth + is unreachable, document the suite as skipped per the `docs/GatewayTesting.md` opt-in matrix. +- **Old value is dead:** after step 3, a bind with the old password must fail. Do not test this from + a shared-NAT box — GLAuth's 3-fail / 10-minute per-IP lockout can lock the whole office. + +## Rollback + +If dashboard login breaks after step 3, restore the previous `passsha256` in +`scadaproj/infra/glauth/config.toml`, `docker compose up -d --force-recreate`, and re-point the +deployed hosts' env var / store back to the previous value. Because the deployed hosts were +pre-staged in step 2, the exposure window is only steps 2→4. + +## Done criteria + +- GLAuth on `10.100.0.35` honors only the new value. +- Every LDAP-enabled deployed host binds with the new value (dashboard login verified). +- The source of truth `scadaproj/infra/glauth/config.toml` carries the new `passsha256`. +- No repo file (this one included) contains the old or new value. +- The SEC-36 tracker rows are `Done` with this runbook cited for the operator action. diff --git a/glauth.md b/glauth.md index 32dd3c7..9dcfc78 100644 --- a/glauth.md +++ b/glauth.md @@ -30,10 +30,19 @@ gw-specific role. | LDAPS | disabled in dev (`Transport=None`, `AllowInsecure=true`) | | Base DN | `dc=zb,dc=local` | | Bind DN format | `cn={username},dc=zb,dc=local` | -| Service account DN | `cn=serviceaccount,dc=zb,dc=local` / `serviceaccount123` | +| Service account DN | `cn=serviceaccount,dc=zb,dc=local` (password: ``) | | Group OU | `ou=,ou=groups,dc=zb,dc=local` | | Failed-bind throttle | 3 fails → 10-minute IP lockout (per `[behaviors]`) | +> **Service-account password is not committed (SEC-36).** The samples below show +> `` as a placeholder, not the real value. The single source of +> truth is **`scadaproj/infra/glauth/config.toml`** on host `10.100.0.35`; the gateway consumes +> it out-of-band (encrypted secrets store reference `${secret:ldap/mxgateway/bind}`, the +> `MxGateway__Ldap__ServiceAccountPassword` env var on deployed hosts, or `dotnet user-secrets` +> on dev boxes — see `docs/GatewayConfiguration.md`). The credential was historically committed +> to this repo (and remains recoverable from git history), so **rotation is required**; the +> operator runbook is `docs/runbooks/SEC-36-ldap-credential-rotation.md`. + ## Pre-existing groups (LmxOpcUa role taxonomy) These map cleanly onto MxAccess capability boundaries — mxaccessgw @@ -62,7 +71,7 @@ group below). | `writeconfig` | `writeconfig123` | 5006 | WriteConfigure | — | + WriteSecured (Configure) | | `alarmack` | `alarmack123` | 5003 | AlarmAck | — | Alarm acknowledgment | | `admin` | `admin123` | 5004 | ReadOnly | WriteOperate, AlarmAck, WriteTune, WriteConfigure | All roles | -| `serviceaccount` | `serviceaccount123` | 5999 | ReadOnly | — | LDAP search capability (for bind-then-search) | +| `serviceaccount` | `` | 5999 | ReadOnly | — | LDAP search capability (for bind-then-search) | For mxaccessgw dev, `admin` covers every gw-side capability test; `readonly` is the right "negative" case for proving Browse-OK / @@ -100,7 +109,7 @@ by `sAMAccountName`, not `cn`. Use this only for dev convenience. ``` 1. Bind as the service account (cn=serviceaccount,dc=zb,dc=local - / serviceaccount123). + / ). 2. Search under dc=zb,dc=local with filter (uid=) — or any attribute the deployment identifies users by. GLAuth populates uid + cn. @@ -133,7 +142,7 @@ ldap: allowInsecureLdap: true # dev only searchBase: "dc=zb,dc=local" serviceAccountDn: "cn=serviceaccount,dc=zb,dc=local" - serviceAccountPassword: "serviceaccount123" + serviceAccountPassword: "" # not committed; see source-of-truth note userNameAttribute: "uid" # GLAuth populates this; AD uses sAMAccountName displayNameAttribute: "cn" groupAttribute: "memberOf" @@ -242,7 +251,7 @@ Or via `ldapsearch` if you have OpenLDAP CLI tools: ```bash ldapsearch -x -H ldap://10.100.0.35:3893 \ - -D "cn=serviceaccount,dc=zb,dc=local" -w serviceaccount123 \ + -D "cn=serviceaccount,dc=zb,dc=local" -w '' \ -b "dc=zb,dc=local" "(uid=multi-role)" ``` diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs index 468b8e6..05a6967 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs @@ -145,7 +145,12 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase); " + + "on deployed hosts set the environment variable " + + "MxGateway__Ldap__ServiceAccountPassword. " + + "(appsettings.json ships the ${secret:ldap/mxgateway/bind} store reference as the default.)", builder); AddIfBlank( options.UserNameAttribute, diff --git a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj index 57712ad..a402153 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj +++ b/src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj @@ -2,6 +2,10 @@ net10.0 + + mxaccessgw-server diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs index 87c62e3..c84aa3b 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs @@ -757,9 +757,15 @@ public sealed class GatewayOptionsValidatorTests new LdapOptions { Enabled = true, ServiceAccountPassword = string.Empty }); ValidateOptionsResult result = new GatewayOptionsValidator().Validate(null, options); Assert.True(result.Failed); - Assert.Contains( + string failure = Assert.Single( result.Failures!, f => f.Contains("MxGateway:Ldap:ServiceAccountPassword is required when LDAP login is enabled.")); + + // SEC-36: the message must steer the operator to the two supported out-of-band channels + // (dev user-secrets, deployed env var) so a blanked/unresolved credential never gets + // "fixed" by re-committing a value. + Assert.Contains("dotnet user-secrets set", failure); + Assert.Contains("MxGateway__Ldap__ServiceAccountPassword", failure); } private static GatewayOptions WithSecurity(SecurityOptions security)