fix(SEC-36): scrub committed dev LDAP service-account password; add user-secrets channel + rotation runbook

Repo-side half of SEC-36. The appsettings.json plaintext was already discharged
before this branch (HEAD ships the fail-closed ${secret:ldap/mxgateway/bind}
store reference), so the residual leak was the literal value in glauth.md,
docs/GatewayTesting.md, and the historical archreview SEC-06 evidence -- all
scrubbed to <service-account-password> placeholders pointing at the source of
truth scadaproj/infra/glauth/.

- csproj: add <UserSecretsId>mxaccessgw-server</UserSecretsId> (dev channel)
- GatewayOptionsValidator: blank-password message now names both channels
  (dev user-secrets, deployed MxGateway__Ldap__ServiceAccountPassword)
- test: assert the message names both channels
- docs: GatewayConfiguration.md (three channels + rotation note), glauth.md
  (placeholders + rotation-required + runbook pointer), GatewayTesting.md
- new operator runbook docs/runbooks/SEC-36-ldap-credential-rotation.md
  (live rotation + NSSM staging remain operator-pending)
- tracking: SEC-36 -> Done (repo-side) in both registers + change-log

Deviation: kept the ${secret:} reference in appsettings.json rather than
deleting it (spec step 2 assumed the stale plaintext baseline); deleting it
would regress the shipped/documented/tested secret-store channel.

git grep -i for the old value is empty across all tracked files.
This commit is contained in:
Joseph Doherty
2026-08-07 07:40:41 -04:00
parent 10534ec906
commit 8c312c717c
11 changed files with 163 additions and 16 deletions
File diff suppressed because one or more lines are too long
@@ -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 `<service-account-password>` placeholders, each with a pointer to the source of truth `scadaproj/infra/glauth/` and a rotation-required note. Steps 36 implemented as designed: `<UserSecretsId>mxaccessgw-server</UserSecretsId>` added (step 3); the `ValidateLdap` blank-password message now names both channels — dev `dotnet user-secrets set "MxGateway:Ldap:ServiceAccountPassword" <value>` 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.
+1 -1
View File
@@ -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 = "<service-account-password>"` — 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.
@@ -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="<service-account-password>"` (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.