docs(security): #835/#836 review — unsaved-endpoint rule, credential-use audit, accurate residuals, AuthType-gated path rule, directory resolution

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-11-11 04:12:40 -05:00
parent 271b96a644
commit 9072777422
3 changed files with 37 additions and 7 deletions
+4
View File
@@ -367,6 +367,10 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa
deploy (`CredentialReferenceOtherInstance`), Test Connect (`TestDriverConnect.DriverInstanceId`) and browse
(`OpenAsync(…, driverInstanceId, …)`; no ConfigDb ⇒ fail closed). One rule: `DriverCredentialPolicy.DescribeOtherInstanceProblems`
+ `DriverInstanceIdentities`. The runtime adds no check (drivers don't know the fleet); `env:` is not instance-scoped.
⚠️ Test Connect/browse configs can carry UNSAVED edits: the claimed owner counts only when every endpoint value
(`DriverCredentialRules.EndpointFields`) is one that driver stores (`EffectiveOwnerAsync`, server-side) — a new
credential-sending driver must declare its endpoint keys there. Credential-carrying Test Connect/browse is audited
(`CredentialUseAudit`).
- **Key-material file paths (#836):** OpcUaClient `UserCertificatePath` is a RELATIVE path inside
`Drivers:OpcUaClient:UserCertificateDirectory` (`DriverCredentialPolicy` path field + `CredentialFilePaths`, all seams;
load-time `ResolveWithin` = lexical + link-resolved containment, before the PFX password resolves). `LoadCertificate`
+1 -1
View File
@@ -551,7 +551,7 @@ counterpart**: an AbLegacy driver with no key always runs libplctag (Native is o
| Key | Type | Default | Meaning |
|---|---|---|---|
| `UserCertificateDirectory` | string | `<LocalAppData>/OtOpcUa/pki/user-certificates` | Directory (made absolute at startup). Env form: `Drivers__OpcUaClient__UserCertificateDirectory`. ACL it to the service account + administrators. |
| `UserCertificateDirectory` | string | `<LocalAppData>/OtOpcUa/pki/user-certificates` | Directory; a relative value is resolved against the install directory (`AppContext.BaseDirectory`), never the working directory. Env form: `Drivers__OpcUaClient__UserCertificateDirectory`. ACL it to the service account + administrators. |
### `Drivers:Galaxy:AckIdentityCache` (cached API-key → Galaxy ack identity set, #765)
+32 -6
View File
@@ -1347,7 +1347,25 @@ through `DriverInstanceIdentities` (only when the config carries an in-scope `se
3. **Test Connect** — `AdminOperationsActor`, before the probe; `TestDriverConnect.DriverInstanceId` names the owner
(the device modal passes it; omitted ⇒ treated as a new driver, i.e. stricter).
4. **Live browse** — `BrowserSessionService.OpenAsync(…, driverInstanceId, …)`, before any browser; `/raw` browse
resolves the owner from the device. Without a ConfigDb it fails closed.
resolves the owner from the device. Without a ConfigDb, or when the ConfigDb read faults, it is refused (a message on
the modal, never a circuit error).
**An unsaved endpoint never gets the named owner's own secrets (review I-1).** Test Connect and browse take a config
that may carry UNSAVED edits — the device modal merges the stored DriverConfig with the device config being typed — so
naming the owner would otherwise let a Designer type their own host into a victim's device and Test Connect it with the
victim's `secret:galaxy/<victim-id>/…`. The server (`DriverInstanceIdentities.EffectiveOwnerAsync`, called by the actor
and by `BrowserSessionService` — the client's claim is not trusted) honours the named owner only when it exists, has the
same type, and every **endpoint value** of the config (`DriverCredentialRules.EndpointFields`: Galaxy `Endpoint`,
OpcUaClient `EndpointUrl`/`EndpointUrls`/`DiscoveryUrl`, Heidenhain `Host`/`Port`) is one that driver already stores
(its DriverConfig or any DeviceConfig). Otherwise the owner is treated as null — no instance-named secret at all — and
the refusal says to save the change first. Saving is the deliberate, persisted act; a saved endpoint change then
behaves like any other saved config.
**Every credential-carrying Test Connect / browse is audited.** When the config carries an in-scope `secret:` or `env:`
reference, `CredentialUseAudit` records an audit event (category `CredentialUse`, action `TestConnect` / `Browse`,
actor = the signed-in user, target = the named driver or `(unsaved driver)`, outcome, and the endpoint host(s) — URI
user-info dropped, never the reference or a value) through the shared `IAuditWriter`, plus a structured Information
log line (`Credential use: …`).
Already-running drivers: a deployed driver resolves its reference exactly as before (the driver itself does not know
the other instances, so the runtime adds no check) — the same deprecated window as #815: the deploy gate refuses the
@@ -1356,10 +1374,11 @@ the driver's own value under `galaxy/<its DriverInstanceId>/…` (or a shared na
at it.
Residuals (accepted): `env:` references are not instance-scoped (a variable is node-wide, and there is no naming
convention to match — a deployment that needs per-instance isolation must use the secret store); and the rule cannot
stop a Designer from editing an **existing** driver's endpoint and Test-Connecting with that driver's own reference —
that is the general "anyone who may author config may redirect it" property described above (the lever is the page
policy). A gateway-side complement — mxaccessgw binding each API key to the client identity / Galaxy it was issued for
convention to match — a deployment that needs per-instance isolation must use the secret store); a reference to a
**shared** name (one no instance is called) is not tied to any owner, so an unsaved endpoint can still be Test-Connected
with it (audited); and a Designer who **saves** a new endpoint on an existing driver can then Test Connect / browse /
deploy it with that driver's own reference — the general "anyone who may author config may redirect it" property
described above (the save is persisted, the use is audited; the lever is the page policy). A gateway-side complement — mxaccessgw binding each API key to the client identity / Galaxy it was issued for
— would make a captured key useless elsewhere; that is cross-repo (`mxaccessgw`) and not implemented here.
#### Key-material file paths are confined to a node directory (#836)
@@ -1371,9 +1390,16 @@ service account can read (another product's client certificate, a host certifica
- The field is a **relative path** (`plant1/user.pfx`) inside the node's user-certificate directory,
**`Drivers:OpcUaClient:UserCertificateDirectory`** (env `Drivers__OpcUaClient__UserCertificateDirectory`; default
`<LocalAppData>/OtOpcUa/pki/user-certificates`, beside the driver's PKI stores — for a service running as LocalSystem
that is under `C:\Windows\System32\config\systemprofile\AppData\Local`). Segments are `[A-Za-z0-9._-]`; an absolute,
that is under `C:\Windows\System32\config\systemprofile\AppData\Local`). A **relative** directory setting is resolved
against the install directory (`AppContext.BaseDirectory`), never the working directory (a Windows service starts in
`System32`). Segments are `[A-Za-z0-9._-]`; an absolute,
rooted, drive-qualified or UNC path, `.`/`..`/empty segments and anything else are refused
(`CredentialFilePaths`, rule row `DriverCredentialPolicy.OpcUaClientUserCertificatePathField`).
- The rule applies unless the **same JSON object** explicitly sets `AuthType` to something other than `Certificate`
(string or enum ordinal) — the driver never loads the file then, and the AdminUI form no longer persists the hidden
field. An object without `AuthType` stays checked, and the deploy gate also checks the driver's config **as the
runtime merges it with its devices**, so splitting `AuthType: Certificate` (device) from the path (driver) cannot slip
a path past the gate.
- **Refused at the same five seams as #815** (save, deploy gate `CredentialReferenceOutOfScope`, Test Connect, browse —
messages never carry the path) and **at load time**: `OpcUaUserIdentityFactory.ResolveSecretRefsAsync` (called by the
running driver and the AdminUI browse before the identity is built) joins it to the directory, canonicalises it, and