diff --git a/.claude/rules/data-plane-auth.md b/.claude/rules/data-plane-auth.md index ae4301f88..d59a8ef0a 100644 --- a/.claude/rules/data-plane-auth.md +++ b/.claude/rules/data-plane-auth.md @@ -235,6 +235,17 @@ Two things to know before relying on any of this: the same `RoleCarryingUserIdentity` a key produces. Set it `false` to make keys mandatory. A key may **not** use the `anonymous` principal, and a failure to read anonymous's grants **denies** rather than admitting with no roles. +- ⚠️ **The SDK's failed-auth lockout is OFF; ours is UserName-only (#833).** SDK 2.0.1 `SessionManager` refuses + EVERY activation (Anonymous included, before the token is read) from a client with 5 failures in 60 s, for 5 min, + keyed cert-thumbprint-else-peer-IP. `OpcUaApplicationHost.DisableSdkAuthenticationLockout` zeroes + `MaxFailedAuthenticationAttempts` on built AND loaded configs; `FailedCredentialLockout` (`OpcUa:CredentialLockout`, + 5 / 60 s / 300 s) counts and refuses UserName activations only, keyed thumbprint+peer together. Do not re-enable the + SDK setting. The check takes a verification SLOT (`BeginAsync`/`Complete`/`Abandon`) — a check and a + separate record let a parallel burst through. At most `max − failures` verify at once; the rest WAIT (never refused + for parallelism — ScadaBridge opens many keyed sessions from one IP), refused only on lockout or, uncounted, after + `SlotWaitSeconds`. An expired lockout resets the count. On + `None` endpoints the key is effectively IP-only (NAT neighbours share it, UserName only). `docs/security.md` + §"Failed-credential lockout". - **Keys reach a driver-only node inside the deployment artifact** (stage 4). `ConfigComposer` snapshots the `ApiKey` rows — the peppered HMAC only, never a secret, and the pepper is node config that is deliberately *not* in the artifact — and `DeploymentArtifact.ParseDataPlaneAuthority` decodes them diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index 38c9b7086..aec25c30f 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -308,7 +308,7 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `InvalidDataException` — before #804 the 400 was a transport fault and the `OUT_OF_RANGE` re-baseline never ran live. The restart (`instanceId`) test also runs on every re-baseline snapshot (on 2.x that is where a restart is first seen). 3 re-baselines with no `/sample` chunk ⇒ Degraded with `LastSuccessfulRead` still advancing (no - watchdog reconnect loop over a `SampleCount` ≥ `bufferSize`). Every parse prohibits DTDs; the streams walk is + watchdog reconnect loop over a `SampleCount` > `bufferSize` — live: `count == bufferSize` is accepted; Test Connect + init warn, #825). Every parse prohibits DTDs; the streams walk is depth-bounded (64 — an unbounded recursion is a process-killing `StackOverflowException`). Non-finite floats are never Good; integer-valued decimals serve integral tags losslessly. Matrix: `docs/drivers/MTConnect.md`. - **MTConnect CONDITION alarms (#626):** a CONDITION raw tag is a native Part 9 condition only when its `TagConfig` diff --git a/docs/Configuration.md b/docs/Configuration.md index a9f229797..23f2e0e55 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -74,6 +74,10 @@ Until #678.6 the overlay never loaded on a node carrying a `cluster-*` role (the | `EnabledSecurityProfiles` | list of `OpcUaSecurityProfile` | `[None, Basic256Sha256Sign, Basic256Sha256SignAndEncrypt]` | Transport-security profiles, one endpoint per entry. Must contain ≥1. Profile detail in [`security.md`](security.md). | | `AutoAcceptUntrustedClientCertificates` | bool | `false` | Auto-trust unknown client certs on first connect (dev convenience). Not validated. See [`security.md`](security.md). | | `PeerApplicationUris` | list of string | `[]` (empty) | Partner node `ApplicationUri`s published in `Server.ServerArray` for redundancy discovery. See [`Redundancy.md`](Redundancy.md). | +| `CredentialLockout:MaxFailedAttempts` | int | `5` | Failed UserName (API-key) activations from one client before its UserName activations are refused `BadUserAccessDenied` (#833). `≤ 0` disables. **Never refuses Anonymous.** See [`security.md`](security.md#failed-credential-lockout-opcuacredentiallockout). | +| `CredentialLockout:FailureWindowSeconds` | int | `60` | A failure more than this long after the client's previous one starts a fresh count. Must be `> 0` while the lockout is enabled (validated). | +| `CredentialLockout:SlotWaitSeconds` | int | `10` | How long a UserName activation waits for one of its client's verification slots (at most `MaxFailedAttempts − failures` verify at once; the rest queue) before that one attempt fails `BadTimeout`, uncounted. Must be `> 0` while enabled (validated). | +| `CredentialLockout:LockoutSeconds` | int | `300` | How long a locked-out client's UserName activations are refused; an expired lockout starts a fresh count. Must be `> 0` while the lockout is enabled (validated). | > **Transport security profiles** (the values in `EnabledSecurityProfiles` — `None`, `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`) and the PKI trust flow are documented in full in [`security.md`](security.md). This page does not duplicate them. diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 3731bbb23..222216b11 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -362,7 +362,7 @@ what the driver's cursor expects) is detected **two ways**, both triggering a **Re-baselines without progress are bounded *and visible* (#804).** The reconnect backoff already paced a stream that only ever re-baselines (`A_rebaseline_that_delivers_nothing…`), but every `/current` also marked the stream delivered, so the driver read **Healthy** -for ever while no `/sample` chunk arrived — the live case is a `SampleCount` at or +for ever while no `/sample` chunk arrived — the live case is a `SampleCount` above the Agent's `bufferSize`, refused `OUT_OF_RANGE` (`'count' must be less than N`) on every open. After **3** consecutive re-baselines with no chunk in between (`MaxRebaselinesWithoutProgress`, the Java driver's `MAX_REBASELINES_WITHOUT_PROGRESS`) @@ -371,7 +371,13 @@ observations in between"` and one Warning naming the `bufferSize` cause; it stay Degraded across further re-baselines until a chunk is delivered. Values keep flowing off the re-baselines and `LastSuccessfulRead` keeps advancing — each `/current` is the Agent answering — so the comms-loss watchdog does not reconnect-loop a configuration -problem. Size `SampleCount` below the Agent's `bufferSize` (in the `/probe` header). +problem. Size `SampleCount` at most the Agent's `bufferSize` (in the `/probe` header). +**It is caught before it happens (#825):** the `/probe` header's `bufferSize` is parsed +(`MTConnectProbeModel.BufferSize`, advisory — missing/malformed is null, never a parse failure); **Test +Connect** reads `sampleCount` (default 1000 when absent) and answers Ok **with a warning** (an amber +"OK with warning" chip showing the text) when it is **above** the buffer (live, cppagent 2.7: `count == bufferSize` is +accepted, `bufferSize + 1` refused `'count' must be less than {bufferSize+1}`), and the driver's **initialize** +(every deploy) logs the same Warning and puts `agentBufferSize` on the `/hosts` diagnostics bag. **An Agent `instanceId` change means the Agent restarted**, and is checked **before** the sequence-gap check (a restart also usually trips the gap, so @@ -716,6 +722,23 @@ deliberately different, kept. - **Deferred live checks:** these legs ran through the driver, **not** through a deployed OtOpcUa server and an OPC UA client (the docker-dev rig was not used); the no-progress cap was observed via a `SampleCount` above the Agent's `bufferSize`, not a genuinely lapped ring buffer; no real machine tool or non-cppagent Agent was exercised. + **Closed by the #823 gate below**, except the real-machine / non-cppagent caveat. + +### End-to-end gate through a deployed server (#823, 2026-11-11, docker-dev) + +An MTConnect driver (`SampleCount` 1000, `HeartbeatMs` 10000, `RequestTimeoutMs` 5000) against the shared fixture, +two raw tags (`fixture_x_pos` Double, `fixture_execution` String) referenced into a UNS equipment, deployed to the +central pair, and two Client.CLI subscriptions on central-1 (`ns=3` equipment folder, `ns=2` raw device). Raw and UNS +NodeIds moved together in every leg. Fixture containers were restored healthy afterwards. + +| Leg | What happened on the wire | +|---|---| +| Adapter stop | Both NodeIds `BadNoCommunication` within 1 s of the adapter dying (Agent marks items UNAVAILABLE); driver stayed Healthy. Adapter start ⇒ Good 1–2 s later. | +| Agent restart (`docker restart`, ~12 s) | Stream lost and reconnected ~1 s; the stale cursor was refused `OUT_OF_RANGE`, the re-baseline saw the new `instanceId`, **rediscovery raised** — `/hosts` shows the **RE-BROWSE** chip. Values stayed Good (outage shorter than the comms-loss window). | +| Agent stopped ~2 min | Values held Good-but-stale; **Bad at 118 s** (comms-loss watchdog, connection refused). Agent start ⇒ re-init on the 10 s retry, Good ~8 s after the Agent was back. | +| Agent paused (`docker pause`) | Stream idle watchdog at 24 s (2 × 10000 + 5000); **Bad at 94 s** (watchdog); unpause ⇒ Good within ~3 s. | +| Genuine buffer overrun | A stalling TCP proxy froze the driver's transport while 32 000 observations were injected (`AllowPut`) — the Agent's `firstSequence` passed the driver's cursor (75 856 → 77 083). On release the resume was refused `OUT_OF_RANGE`, the driver re-baselined from `/current` within 1 s and resumed; values Good, no restart misdiagnosed. A raised-count stream (count 16, interval 5 s) was **not** lapped by a 266 obs/s burst: chunks are sent back-to-back while data is pending, so `interval` does not cap throughput. | +| `SampleCount` vs `bufferSize` (#825) | Test Connect rendered the amber "OK with warning" chip and a deploy logged the Warning (`/hosts` shows `agentBufferSize`). The same gate measured the Agent's real edge — `count = 16384` streams, `16385` is `OUT_OF_RANGE` — so the first build's "at or above" threshold was corrected to "above" (pinned live by `Test_Connect_warns_when_SampleCount_exceeds_the_live_agents_buffer_size`). | ## Known limitations diff --git a/docs/security.md b/docs/security.md index 83854915b..b2fa3e8e4 100644 --- a/docs/security.md +++ b/docs/security.md @@ -313,6 +313,56 @@ charged process-wide only — never handed a fresh budget. Clients behind one NA guard applies to a driver-only node: its `ArtifactApiKeyStore` is two in-memory dictionary probes and is never wrapped. +### Failed-credential lockout (`OpcUa:CredentialLockout`) + +**A failed sign-in never locks a client out of anonymous access (#833).** OPC Foundation SDK 2.0.1's +`SessionManager` has its own lockout: after `ServerConfiguration.MaxFailedAuthenticationAttempts` (SDK +default **5**) failed identity validations — each within 60 s of the previous — it refuses **every** +`ActivateSession` from that client for **5 minutes** with `BadUserAccessDenied` ("Too many failed +authentication attempts"). Its check runs before the identity token is looked at, so it refused +**Anonymous** too (even a namespace-array read needs an activated session), a successful anonymous +activation did not clear it, and the authenticator was never called (no "anonymous admitted" line). Its key +is the client certificate thumbprint when one is presented, else the peer IP — so certificate-less clients +behind one NAT share a key, and on a `None` endpoint a presented certificate is not proven. That is what +the docker-dev live gate hit: four failed `multi-role` UserName activations against central-1, then every +anonymous session refused for five minutes while central-2 (which never saw the failures) kept admitting. + +The host now sets `MaxFailedAuthenticationAttempts = 0` on every configuration it builds or loads +(`OpcUaApplicationHost.DisableSdkAuthenticationLockout`) and enforces a narrower replacement, +`FailedCredentialLockout`, in its identity-registry adapter: + +- **UserName activations only.** A failed one counts; at `MaxFailedAttempts` (5) within + `FailureWindowSeconds` (60) of each other, that client's **UserName** activations are refused + `BadUserAccessDenied` for `LockoutSeconds` (300) without the key store being consulted, and a Warning is + logged once. A refusal during the lockout is not counted (it never extends itself); an **expired lockout + always starts a fresh count**, whatever `FailureWindowSeconds` is; a successful UserName activation clears the + count. **Anonymous activations never consult it**, from any client. The first refusal of each lockout episode + is logged at Information, the rest at Debug. +- **Parallel activations wait — they are never refused for parallelism.** A client gets at most + `MaxFailedAttempts − recordedFailures` verification **slots**; a UserName activation beyond that **queues** (FIFO) + until one frees, so no more than `MaxFailedAttempts` unverified guesses are ever in progress-or-failed per window, + while a legitimate consumer opening many keyed sessions at once (e.g. a ScadaBridge site node behind one NAT IP on a + `None` endpoint) just waits milliseconds and is admitted. A waiter is refused only when the key actually **locks + out** (`BadUserAccessDenied`, without reaching the authenticator), or after `SlotWaitSeconds` (default 10) + with `BadTimeout` — **not** counted as a credential failure. A cancelled activation leaves the queue and frees + any slot it held. +- **Keyed on certificate thumbprint *and* peer address together.** That separates clients only when they present + a certificate. ⚠️ On a `None` endpoint clients commonly present none, so the key is **effectively the IP alone**: + NAT neighbours there **do** share one count (for UserName activations only — never for anonymous ones). When the + transport reports no peer address at all, every certificate-less client shares the single key `-@-`. (Observed on + docker-dev: every host client arrives as `-@192.168.97.1`.) +- **Validated at host start:** with `MaxFailedAttempts > 0`, `LockoutSeconds`, `FailureWindowSeconds` and + `SlotWaitSeconds` must all be `> 0` (`OpcUaApplicationHostOptionsValidator`); `MaxFailedAttempts <= 0` disables + the lockout. +- Every denial counts, including a key-store timeout or miss-budget refusal (as the SDK's did) — a ConfigDb + outage can therefore lock a keyed client out for up to `LockoutSeconds` after recovery. +- Bounded at 4096 tracked clients (expired entries are pruned; a client arriving at a full table is not + tracked). API-key secrets are not guessable, so this is defence in depth, not the brute-force control. + +Pinned over the wire by `CredentialLockoutWireTests` (OpcUaServer.IntegrationTests — anonymous admitted after +six failures; a valid key refused while locked out; `MaxFailedAttempts = 0` admits after seven failures, +proving the SDK lockout is off) and unit-tested by `FailedCredentialLockoutTests`. + ### LDAP bind flow (`OtOpcUaLdapAuthService`) LDAP is configured under the `Security:Ldap` section (bound to `LdapOptions`, `src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs`, `SectionName = "Security:Ldap"`). The app authenticator is `OtOpcUaLdapAuthService` — a thin wrapper around the shared `ZB.MOM.WW.Auth.Ldap` directory client that adds two app-only concerns the shared library deliberately does not model: the `Enabled` master switch and `DevStubMode`. Since #520 it serves the **Admin UI cookie login (`/auth/login`) only** — `LdapOpcUaUserAuthenticator` is deleted and the OPC UA UserName path authenticates API keys instead (see [API-key identity](#api-key-identity-opc-ua-data-plane)). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.SampleLag.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.SampleLag.cs index 210bf153e..1d130fbd1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.SampleLag.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.SampleLag.cs @@ -55,6 +55,28 @@ public sealed partial class MTConnectDriver : IDriverDiagnostics /// Times a pump raised its count. Diagnostics only. private long _sampleCountRaises; + /// The Agent's /probe bufferSize as of the last initialize, or 0 when unknown. Diagnostics only. + private long _agentBufferSize; + + /// + /// lmxopcua#825 — the configuration warning for a the Agent + /// will refuse. An Agent answers /sample?count=N with OUT_OF_RANGE ("'count' must be less than + /// bufferSize + 1") when N > bufferSize — live-measured on cppagent 2.7 (bufferSize 16384: 16384 is + /// accepted, 16385 refused) — so such a driver never streams: it re-baselines from + /// /current and reports Degraded after three of them. Shared by + /// (Test Connect) and the driver's initialize (deploy), so both say the same thing. + /// + /// The configured count. + /// The Agent's buffer size, or null when its header did not say. + /// The warning text, or when the count fits (or the buffer size is unknown). + internal static string? SampleCountBufferWarning(int sampleCount, long? bufferSize) => + bufferSize is { } size && sampleCount > size + ? $"SampleCount {sampleCount.ToString(CultureInfo.InvariantCulture)} is above the Agent's bufferSize " + + $"{size.ToString(CultureInfo.InvariantCulture)}: the Agent refuses every /sample stream (OUT_OF_RANGE), so " + + "values will only re-baseline from /current and the driver reports Degraded. Set SampleCount to at most " + + $"{size.ToString(CultureInfo.InvariantCulture)}." + : null; + /// The ceiling a pump started with may raise its count to. /// The configured . /// The ceiling — never below . @@ -77,6 +99,12 @@ public sealed partial class MTConnectDriver : IDriverDiagnostics bag["sampleLag"] = lag.ToString(CultureInfo.InvariantCulture); } + var bufferSize = Interlocked.Read(ref _agentBufferSize); + if (bufferSize > 0) + { + bag["agentBufferSize"] = bufferSize.ToString(CultureInfo.InvariantCulture); + } + bag["sampleSaturatedChunks"] = Interlocked.Read(ref _saturatedChunks).ToString(CultureInfo.InvariantCulture); bag["sampleCountRaises"] = Interlocked.Read(ref _sampleCountRaises).ToString(CultureInfo.InvariantCulture); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index dd6b53940..8cc6566a1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -2350,7 +2350,7 @@ public sealed partial class MTConnectDriver // #804 — the backoff above PACES a stream that only ever re-baselines, but each /current // also marked the stream delivered, so it read Healthy for ever while no /sample chunk - // arrived (a SampleCount at or above the Agent's bufferSize is refused OUT_OF_RANGE on + // arrived (a SampleCount above the Agent's bufferSize is refused OUT_OF_RANGE on // EVERY open). After MaxRebaselinesWithoutProgress in a row it is reported Degraded, and // stays so (holdStalled) until a chunk is delivered. if (!outcome.DeliveredAny && @@ -2363,7 +2363,7 @@ public sealed partial class MTConnectDriver _logger.LogWarning( outcome.Error, - "MTConnect driver {DriverInstanceId}: {Count} consecutive /current re-baselines of {AgentUri} without a single /sample chunk in between; reporting the stream Degraded until one is delivered. Values are re-baselined, not streamed. An Agent answering every /sample with OUT_OF_RANGE usually means SampleCount is at or above its bufferSize.", + "MTConnect driver {DriverInstanceId}: {Count} consecutive /current re-baselines of {AgentUri} without a single /sample chunk in between; reporting the stream Degraded until one is delivered. Values are re-baselined, not streamed. An Agent answering every /sample with OUT_OF_RANGE usually means SampleCount is above its bufferSize.", _driverInstanceId, rebaselinesWithoutProgress, AgentDisplay(options)); } @@ -3333,6 +3333,15 @@ public sealed partial class MTConnectDriver // ---- commit point: everything below is non-throwing bookkeeping ---- Volatile.Write(ref _options, options); + // lmxopcua#825 — a SampleCount the Agent will refuse is a configuration error; say so at deploy rather than + // leaving it to surface as three progress-free re-baselines and a Degraded stream. + Interlocked.Exchange(ref _agentBufferSize, probe.BufferSize ?? 0); + if (SampleCountBufferWarning(options.SampleCount, probe.BufferSize) is { } bufferWarning) + { + _logger.LogWarning("MTConnect driver {DriverInstanceId} ({AgentUri}): {Warning}", + _driverInstanceId, AgentDisplay(options), bufferWarning); + } + // Derived from the options AND the /probe model just fetched: a raw tag whose TagConfig // blob declares no type takes the type the Agent itself declares for that DataItem, so a // browse-committed tag (whose blob carries only the address) publishes as the number it diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverProbe.cs index 0e668b404..62cd96dc0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriverProbe.cs @@ -132,8 +132,18 @@ public sealed class MTConnectDriverProbe : IDriverProbe /// driver's tag/timing configuration — see the "reachable ≠ deployable" remarks on the type. /// public string? DeviceName { get; set; } + + /// + /// The /sample count (lmxopcua#825). Not needed to reach the Agent, but read so Test Connect + /// can warn when it is above the Agent's bufferSize — a config the Agent will refuse on every + /// stream. Absent ⇒ the driver's own default. + /// + public int? SampleCount { get; set; } } + /// The driver's default SampleCount (MTConnectDriver.ParseOptions) when the config omits it. + private const int DefaultSampleCount = 1_000; + /// public async Task ProbeAsync(string configJson, TimeSpan timeout, CancellationToken ct) { @@ -195,10 +205,17 @@ public sealed class MTConnectDriverProbe : IDriverProbe try { using var client = new MTConnectAgentClient(options, _handler); - _ = await client.ProbeAsync(cts.Token).ConfigureAwait(false); + var model = await client.ProbeAsync(cts.Token).ConfigureAwait(false); sw.Stop(); - return new(true, $"MTConnect /probe OK ({safeUri})", sw.Elapsed); + // Reachable but misconfigured stays Ok (the endpoint answered — Test Connect's question), with the + // warning in the message so the operator sees it before deploying (lmxopcua#825). + var warning = MTConnectDriver.SampleCountBufferWarning(dto.SampleCount ?? DefaultSampleCount, model.BufferSize); + return new(true, + warning is null + ? $"MTConnect /probe OK ({safeUri})" + : $"MTConnect /probe OK ({safeUri}) — WARNING: {warning}", + sw.Elapsed); } catch (OperationCanceledException) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs index d9992bf01..3a1b48c4c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDtos.cs @@ -8,7 +8,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// XElement. /// /// Every device the Agent's probe response declares. -public sealed record MTConnectProbeModel(IReadOnlyList Devices); +/// +/// The Agent's observation ring-buffer size (the /probe Header's bufferSize), or +/// when the header omits it or it is not a positive integer. A /sample +/// count above it is refused OUT_OF_RANGE by the Agent (lmxopcua#825; live: count == bufferSize is accepted). +/// +public sealed record MTConnectProbeModel(IReadOnlyList Devices, long? BufferSize = null); /// /// Common shape shared by and : diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs index d50d66bfa..e9561a22b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectProbeParser.cs @@ -132,7 +132,21 @@ internal static class MTConnectProbeParser "MTConnect /probe response declares no devices; refusing to report an empty device model as a successful probe."); } - return new MTConnectProbeModel(devices); + return new MTConnectProbeModel(devices, ReadBufferSize(root)); + } + + /// + /// The Header's bufferSize (lmxopcua#825). Advisory only — a missing, malformed or non-positive + /// value is , never a parse failure, because nothing but a configuration warning reads it. + /// + private static long? ReadBufferSize(XElement root) + { + var header = MTConnectXml.ChildrenNamed(root, "Header").FirstOrDefault(); + var raw = header is null ? null : MTConnectXml.OptionalAttribute(header, "bufferSize"); + return long.TryParse(raw, System.Globalization.NumberStyles.None, System.Globalization.CultureInfo.InvariantCulture, out var size) + && size > 0 + ? size + : null; } private static MTConnectDevice ReadDevice(XElement element) => diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs index c9237a7fd..8285b25dd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -76,7 +76,7 @@ internal static class MTConnectXml /// /// Why this exists (#804). A modern Agent (cppagent 2.x, the fixture's /// mtconnect/agent:2.7) does not serve its error documents under HTTP 200 the way older - /// ones did: it answers an out-of-range /sample?from= — and a count at or above its + /// ones did: it answers an out-of-range /sample?from= — and a count above its /// buffer — with HTTP 400, and an unknown device with 404, each carrying a /// well-formed MTConnectError body. A status check alone turns those into an /// , which the pump treats as a transient transport loss and diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor index 105fa06b1..e3adbff5b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/EquipmentPage.razor @@ -311,12 +311,14 @@ else - - @* One confirm for every destructive row action on this page (R04 H1) — the same RawConfirmDialog - /raw routes its deletes through. A mis-click used to delete immediately. *@ - } + + @* One confirm for every destructive row action on this page (R04 H1) — the same RawConfirmDialog + /raw routes its deletes through. A mis-click used to delete immediately. It must render OUTSIDE the + tab branches (#832): inside the Alarms branch it existed only on that tab, so the Tags tab's Remove + and the Virtual Tags tab's Delete opened a dialog that was never on the page. *@ + } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor index 06ec04ec1..356b8ef7f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor @@ -12,7 +12,16 @@ @if (_result is not null) { - @if (_result.Ok) + @if (_result.Ok && WarningOf(_result.Message) is { } warning) + { + @* Reachable but misconfigured (e.g. MTConnect SampleCount > the Agent's bufferSize, #825): a probe + that answers Ok with a "WARNING:" message is shown amber with the warning, never a plain green OK. *@ + + OK with warning · @_result.LatencyMs?.ToString("F0") ms + +
@warning
+ } + else if (_result.Ok) { OK · @_result.LatencyMs?.ToString("F0") ms @@ -28,6 +37,16 @@ @code { + /// The marker a probe puts in an Ok message to flag a warning (see MTConnectDriverProbe). + internal const string WarningMarker = "WARNING:"; + + /// The warning text after , or null when the message carries none. + internal static string? WarningOf(string? message) + { + var at = message?.IndexOf(WarningMarker, StringComparison.Ordinal) ?? -1; + return at < 0 ? null : message![(at + WarningMarker.Length)..].Trim(); + } + /// Driver type key — must match an installed IDriverProbe.DriverType. [Parameter, EditorRequired] public string DriverType { get; set; } = ""; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/OpcUaApplicationHostOptionsValidator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/OpcUaApplicationHostOptionsValidator.cs index 789afca9b..c51ed0d40 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/OpcUaApplicationHostOptionsValidator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Configuration/OpcUaApplicationHostOptionsValidator.cs @@ -29,5 +29,15 @@ public sealed class OpcUaApplicationHostOptionsValidator : OptionsValidatorBase< // IReadOnlyCollection, so it can't bind to MinCount's IReadOnlyCollection parameter // directly. ToList() bridges to the shared primitive while preserving the count (and message). builder.MinCount(o.EnabledSecurityProfiles?.ToList(), 1, "OpcUa:EnabledSecurityProfiles"); + + // #833 review: an enabled credential lockout needs positive durations — a zero LockoutSeconds would lock + // nothing, a zero FailureWindowSeconds would never accumulate. MaxFailedAttempts <= 0 disables it. + var lockout = o.CredentialLockout; + builder.RequireThat(lockout is null || lockout.MaxFailedAttempts <= 0 + || (lockout.LockoutSeconds > 0 && lockout.FailureWindowSeconds > 0 && lockout.SlotWaitSeconds > 0), + "OpcUa:CredentialLockout:LockoutSeconds, OpcUa:CredentialLockout:FailureWindowSeconds and " + + "OpcUa:CredentialLockout:SlotWaitSeconds must all be " + + "greater than 0 while OpcUa:CredentialLockout:MaxFailedAttempts is greater than 0 (set " + + "MaxFailedAttempts to 0 to disable the lockout)"); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs index cfaecdca3..61f8db9ab 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs @@ -113,6 +113,12 @@ public sealed class OpcUaApplicationHostOptions /// own transport defaults, so an absent section changes nothing. /// public OpcUaQuotaOptions Quotas { get; set; } = new(); + + /// + /// Failed-credential lockout for UserName activations (OpcUa:CredentialLockout:*, lmxopcua#833). + /// Replaces the SDK's own lockout, which also refused Anonymous; see . + /// + public OpcUaCredentialLockoutOptions CredentialLockout { get; set; } = new(); } /// @@ -175,6 +181,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable private StandardServer? _server; private readonly ITelemetryContext _telemetry = DefaultTelemetry.Create(_ => { }); private readonly List _identityAuthenticators = []; + private readonly FailedCredentialLockout _credentialLockout; private readonly object _lifetimeLock = new(); private readonly TaskCompletionSource _disposeCompletion = new(TaskCreationOptions.RunContinuationsAsynchronously); private int _disposeStarted; @@ -191,6 +198,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable _options = options; _logger = logger; _userAuthenticator = userAuthenticator ?? NullOpcUaUserAuthenticator.Instance; + _credentialLockout = new FailedCredentialLockout(options.CredentialLockout ?? new OpcUaCredentialLockoutOptions()); } /// Gets the OPC Foundation application instance, or null if not yet started. @@ -260,14 +268,73 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable ct.ThrowIfCancellationRequested(); var args = new ImpersonateEventArgs(context.TokenHandler, context.UserTokenPolicy, context.EndpointDescription); var peer = (host._server as OtOpcUaSdkServer)?.ActivationPeerAddress; - // Port zero is only an adapter for the existing IP-budget seam: the SDK exposes no peer port. - await HandleImpersonationAsync(host._userAuthenticator, args, host._logger, host._options.AllowAnonymous, - peer is null ? null : new System.Net.IPEndPoint(peer, 0), ct).ConfigureAwait(false); - return ServiceResult.IsBad(args.IdentityValidationError) - ? AuthenticationResult.Reject(args.IdentityValidationError) - : args.Identity is { } identity - ? AuthenticationResult.Accept(identity) - : AuthenticationResult.Reject(new ServiceResult(StatusCodes.BadIdentityTokenRejected)); + + // lmxopcua#833 — only credential-bearing (UserName) activations are counted and refused. Anonymous + // never consults the lockout, so failed sign-ins cannot lock a client (or its NAT neighbours) out of + // anonymous access the way the SDK's own lockout did. + var lockoutKey = tokenType == UserTokenType.UserName + ? FailedCredentialLockout.KeyFor(context.ChannelCertificate?.Thumbprint, peer) + : null; + if (lockoutKey is not null) + { + // Waits (bounded) for one of this client's verification slots — parallel activations queue, they are + // never refused for parallelism. Refused only when locked out, or (uncounted) when no slot freed in time. + var decision = await host._credentialLockout.BeginAsync(lockoutKey, ct).ConfigureAwait(false); + if (decision.Verdict == CredentialLockoutVerdict.LockedOut) + { + // Logged once per lockout episode at Information; every further refusal at Debug, so a client + // hammering a lockout cannot flood the log (#833 review). + var seconds = (long)Math.Ceiling(decision.Remaining.TotalSeconds); + host._logger.Log(decision.FirstRefusal ? LogLevel.Information : LogLevel.Debug, + "OpcUaApplicationHost: UserName activation refused — client {Client} is locked out for {Seconds}s more", + lockoutKey, seconds); + return AuthenticationResult.Reject(ServiceResult.Create(StatusCodes.BadUserAccessDenied, + $"Too many failed authentication attempts. Try again in {seconds} seconds.")); + } + + if (decision.Verdict == CredentialLockoutVerdict.SlotWaitTimedOut) + { + host._logger.LogWarning( + "OpcUaApplicationHost: UserName activation for client {Client} timed out waiting for a " + + "verification slot (OpcUa:CredentialLockout:SlotWaitSeconds); not counted as a failure", + lockoutKey); + return AuthenticationResult.Reject(ServiceResult.Create(StatusCodes.BadTimeout, + "Timed out waiting to verify the credential. Try again.")); + } + } + + // The slot BeginAsync granted is released on every path — completed with the outcome, or abandoned + // (uncounted) when the activation was cancelled or threw. + var completed = false; + try + { + // Port zero is only an adapter for the existing IP-budget seam: the SDK exposes no peer port. + await HandleImpersonationAsync(host._userAuthenticator, args, host._logger, host._options.AllowAnonymous, + peer is null ? null : new System.Net.IPEndPoint(peer, 0), ct).ConfigureAwait(false); + var result = ServiceResult.IsBad(args.IdentityValidationError) + ? AuthenticationResult.Reject(args.IdentityValidationError) + : args.Identity is { } identity + ? AuthenticationResult.Accept(identity) + : AuthenticationResult.Reject(new ServiceResult(StatusCodes.BadIdentityTokenRejected)); + + if (lockoutKey is not null) + { + var admitted = args.Identity is not null && !ServiceResult.IsBad(args.IdentityValidationError); + completed = true; + if (host._credentialLockout.Complete(lockoutKey, admitted)) + { + host._logger.LogWarning( + "OpcUaApplicationHost: client {Client} locked out of UserName activation for {Seconds}s after " + + "{Attempts} failed attempts (OpcUa:CredentialLockout); anonymous activation is unaffected", + lockoutKey, host._options.CredentialLockout.LockoutSeconds, host._options.CredentialLockout.MaxFailedAttempts); + } + } + return result; + } + finally + { + if (lockoutKey is not null && !completed) host._credentialLockout.Abandon(lockoutKey); + } } } @@ -488,7 +555,9 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable { if (!string.IsNullOrWhiteSpace(_options.ApplicationConfigPath)) { - return await _application!.LoadApplicationConfigurationAsync(_options.ApplicationConfigPath, true, ct); + var loaded = await _application!.LoadApplicationConfigurationAsync(_options.ApplicationConfigPath, true, ct); + if (loaded.ServerConfiguration is { } loadedServer) DisableSdkAuthenticationLockout(loadedServer); + return loaded; } var serverConfig = new ServerConfiguration @@ -496,6 +565,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable BaseAddresses = [$"opc.tcp://{_options.PublicHostname}:{_options.OpcUaPort}/OtOpcUa"], }; ApplyRequestQuotas(serverConfig, _options.Quotas); + DisableSdkAuthenticationLockout(serverConfig); serverConfig.SecurityPolicies = BuildSecurityPolicies(_options.EnabledSecurityProfiles).ToArray(); serverConfig.UserTokenPolicies = BuildUserTokenPolicies(_options.AllowAnonymous).ToArray(); @@ -531,6 +601,15 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable return config; } + /// + /// Switches off the SDK's per-client failed-authentication lockout (lmxopcua#833). It refuses EVERY + /// activation from a locked client — Anonymous included — for 5 minutes; + /// enforces the UserName-only replacement. + /// + /// The configuration to set. + internal static void DisableSdkAuthenticationLockout(ServerConfiguration serverConfig) => + serverConfig.MaxFailedAuthenticationAttempts = 0; + /// /// Applies the request-processing quotas (OpcUa:Quotas:*, review 01 P6 — they were hard-coded at /// 5 / 100 / 200) to the SDK server configuration. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/FailedCredentialLockout.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/FailedCredentialLockout.cs new file mode 100644 index 000000000..24fffa4dc --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/FailedCredentialLockout.cs @@ -0,0 +1,414 @@ +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +/// +/// The OpcUa:CredentialLockout section: how many failed UserName activations one client may make +/// before its UserName activations are refused for a while (lmxopcua#833). +/// +/// +/// The defaults are the OPC Foundation SDK 2.0.1 SessionManager lockout this replaces: 5 failures, +/// each inside 60 s of the previous one, lock the client out for 5 minutes. When enabled +/// ( > 0) every duration must be positive — enforced at host start by +/// OpcUaApplicationHostOptionsValidator. +/// +public sealed class OpcUaCredentialLockoutOptions +{ + /// Failed UserName activations that lock a client out; 0 or less disables the lockout. + public int MaxFailedAttempts { get; set; } = 5; + + /// A failure more than this many seconds after the previous one starts a fresh count. + public int FailureWindowSeconds { get; set; } = 60; + + /// How long a locked-out client's UserName activations are refused, in seconds. + public int LockoutSeconds { get; set; } = 300; + + /// + /// How long a UserName activation waits for one of its client's verification slots before that one attempt + /// is failed (BadTimeout, NOT counted as a credential failure). Only reached when a client has more + /// verifications in progress than minus its recorded failures; a verification + /// is milliseconds, so in practice this bounds a pathological store stall. Kept well under the SDK's + /// operation timeout so the client gets this answer rather than a transport timeout. + /// + public int SlotWaitSeconds { get; set; } = 10; +} + +/// The outcome of . +public enum CredentialLockoutVerdict +{ + /// A verification slot is held — authenticate, then call . + Proceed, + + /// The client is locked out; refuse without authenticating. + LockedOut, + + /// No slot freed within the wait; fail this one attempt, uncounted. + SlotWaitTimedOut, +} + +/// What decided. +/// Proceed, locked out, or timed out waiting. +/// Time left on the lockout when . +/// True for the first refusal of a lockout episode (log it at Information, the rest at Debug). +public readonly record struct CredentialLockoutDecision( + CredentialLockoutVerdict Verdict, TimeSpan Remaining, bool FirstRefusal); + +/// +/// Brute-force lockout for UserName (API-key) activations, scoped so it can never refuse an +/// anonymous activation (lmxopcua#833). +/// +/// +/// +/// Why this exists instead of the SDK's own lockout. OPC Foundation SDK 2.0.1's +/// SessionManager.ActivateSessionAsync counts every failed identity validation per client and, +/// at ServerConfiguration.MaxFailedAuthenticationAttempts (default 5), refuses every +/// activation from that client for 5 minutes with BadUserAccessDenied — the lockout check runs +/// before the token is even looked at, so it refuses Anonymous too, and a successful anonymous +/// activation does not clear it. Its key is the client certificate thumbprint when one is presented, +/// else the peer IP: on a None endpoint the presented certificate is not proven, so a client +/// could lock out another client's sessions by presenting its certificate, and behind a NAT every +/// certificate-less client shares one key. The host therefore sets +/// MaxFailedAuthenticationAttempts = 0 and enforces this narrower lockout instead. +/// +/// +/// What it does. Only UserName activations are counted and refused; Anonymous never consults it. +/// The key is the presented client certificate thumbprint and the observed peer address together. +/// ⚠️ That only separates clients that present a certificate: on a None endpoint clients commonly +/// present none, so the key is effectively the IP alone and NAT neighbours there share one count +/// (for UserName activations only — never for anonymous ones); and when the transport reports no peer +/// address, every certificate-less client shares the single key -@-. Timing: a failure within +/// of the previous one adds to the +/// count; at the client is refused for +/// ; a refusal during the lockout is not +/// counted (the lockout never extends itself); an expired lockout starts a fresh count (whatever the +/// window); a successful UserName activation clears the client's count. +/// +/// +/// Concurrency — parallelism waits, it is never refused. The authenticate call is awaited between the +/// check and the outcome, so a check and a separate record would let a parallel burst of guesses all pass +/// before the first failure lands. therefore hands out at most +/// MaxFailedAttempts − recordedFailures verification slots per key; an activation beyond that +/// waits (FIFO) for a slot, and / release it and wake the +/// next waiter. So no more than MaxFailedAttempts unverified guesses are ever in progress-or-failed +/// per window, while a legitimate client opening many keyed sessions at once (a ScadaBridge site node behind +/// one NAT IP) just queues for milliseconds. Waiters are refused only when the key actually locks out (they +/// never reach the authenticator) or after +/// (uncounted); a cancelled waiter is removed from the queue. +/// +/// +/// Memory is bounded at : entries with no failures, slots or waiters are +/// pruned when it fills, and a client that still finds it full is not tracked (it can only evade by +/// rotating certificates or addresses, which it could do against the SDK lockout too; API-key secrets are +/// not guessable). +/// +/// +public sealed class FailedCredentialLockout +{ + /// The most clients tracked at once. + public const int MaxTrackedClients = 4096; + + private readonly OpcUaCredentialLockoutOptions _options; + private readonly TimeProvider _time; + private readonly Dictionary _entries = new(StringComparer.Ordinal); + private readonly object _gate = new(); + + /// Initializes a new instance of the class. + /// The configured thresholds. + /// The clock; when null. + public FailedCredentialLockout(OpcUaCredentialLockoutOptions options, TimeProvider? time = null) + { + _options = options ?? throw new ArgumentNullException(nameof(options)); + _time = time ?? TimeProvider.System; + } + + /// Whether the lockout is switched on. + public bool Enabled => _options.MaxFailedAttempts > 0; + + /// Builds the lockout key for a client. + /// The presented client certificate's thumbprint, if any. + /// The transport-observed peer address, if known. + /// The key; never null. + public static string KeyFor(string? certificateThumbprint, System.Net.IPAddress? peer) => + $"{(string.IsNullOrEmpty(certificateThumbprint) ? "-" : certificateThumbprint)}@{peer?.ToString() ?? "-"}"; + + /// + /// Acquires a verification slot for , waiting for one when the client already has as + /// many in progress as it may. On the caller MUST release the + /// slot with or . + /// + /// The client key from . + /// Cancels the wait; a cancelled waiter is dequeued and the exception propagates. + /// The decision. + public async ValueTask BeginAsync(string key, CancellationToken ct = default) + { + if (!Enabled) return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false); + + Waiter waiter; + Entry entry; + lock (_gate) + { + var now = _time.GetUtcNow(); + if (!_entries.TryGetValue(key, out var existing)) + { + if (_entries.Count >= MaxTrackedClients) Prune(now); + // Untracked when still full: it gets no slot accounting, as before (see remarks). + if (_entries.Count >= MaxTrackedClients) return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false); + existing = new Entry(); + _entries[key] = existing; + } + + entry = existing; + Refresh(entry, now); + if (TryLockedOut(entry, now, out var refused)) return refused; + if (entry.Waiters.Count == 0 && HasFreeSlot(entry)) + { + entry.InFlight++; + return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false); + } + + waiter = new Waiter(); + entry.Waiters.AddLast(waiter.Node = new LinkedListNode(waiter)); + } + + var wait = TimeSpan.FromSeconds(Math.Max(1, _options.SlotWaitSeconds)); + try + { + return await waiter.Completion.Task.WaitAsync(wait, _time, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is TimeoutException or OperationCanceledException) + { + lock (_gate) + { + if (waiter.Completion.Task.IsCompletedSuccessfully) + { + // Decided in the same instant the wait gave up. A timeout takes the decision as made; a + // cancellation hands a granted slot back (waking the next waiter) so nothing leaks. + var decided = waiter.Completion.Task.Result; + if (ex is not OperationCanceledException) return decided; + if (decided.Verdict == CredentialLockoutVerdict.Proceed) ReleaseCore(key, entry, outcome: null); + } + else + { + entry.Waiters.Remove(waiter.Node!); + waiter.Completion.TrySetCanceled(); + RemoveIfIdle(key, entry); + } + } + + if (ex is OperationCanceledException) throw; + return new(CredentialLockoutVerdict.SlotWaitTimedOut, TimeSpan.Zero, false); + } + } + + /// Releases a slot from and records the activation's outcome. + /// The client key. + /// Whether the activation was admitted. + /// when this failure locked the client out. + public bool Complete(string key, bool succeeded) + { + if (!Enabled) return false; + lock (_gate) + { + return _entries.TryGetValue(key, out var entry) && ReleaseCore(key, entry, succeeded); + } + } + + /// Releases a slot without recording an outcome (the activation was cancelled or threw). + /// The client key. + public void Abandon(string key) + { + if (!Enabled) return; + lock (_gate) + { + if (_entries.TryGetValue(key, out var entry)) ReleaseCore(key, entry, outcome: null); + } + } + + /// Whether is locked out right now (observation; reserves nothing). + /// The client key from . + /// The time left on the lockout when this returns true. + /// when the client's UserName activations are refused. + public bool IsLockedOut(string key, out TimeSpan remaining) + { + remaining = TimeSpan.Zero; + if (!Enabled) return false; + var now = _time.GetUtcNow(); + lock (_gate) + { + if (!_entries.TryGetValue(key, out var entry)) return false; + Refresh(entry, now); + if (entry.LockedUntil <= now) return false; + remaining = entry.LockedUntil - now; + return true; + } + } + + /// Records one failed UserName activation with no slot (test seam / direct callers). + /// The client key. + /// when this failure locked the client out. + public bool RecordFailure(string key) + { + if (!Enabled) return false; + var now = _time.GetUtcNow(); + lock (_gate) + { + if (!_entries.TryGetValue(key, out var entry)) + { + if (_entries.Count >= MaxTrackedClients) Prune(now); + if (_entries.Count >= MaxTrackedClients) return false; + entry = new Entry(); + _entries[key] = entry; + } + var locked = RecordFailureCore(entry, now); + Pump(entry, now); + return locked; + } + } + + /// Clears a client's count after a successful UserName activation (no slot). + /// The client key. + public void RecordSuccess(string key) + { + lock (_gate) + { + if (!_entries.TryGetValue(key, out var entry)) return; + ClearCount(entry); + Pump(entry, _time.GetUtcNow()); + RemoveIfIdle(key, entry); + } + } + + /// How many clients are tracked (test observation). + internal int TrackedCount + { + get { lock (_gate) return _entries.Count; } + } + + /// How many activations are waiting for a slot on (test observation). + internal int WaitingCount(string key) + { + lock (_gate) return _entries.TryGetValue(key, out var e) ? e.Waiters.Count : 0; + } + + private bool HasFreeSlot(Entry entry) => entry.Failures + entry.InFlight < _options.MaxFailedAttempts; + + private bool TryLockedOut(Entry entry, DateTimeOffset now, out CredentialLockoutDecision refused) + { + refused = default; + if (entry.LockedUntil <= now) return false; + refused = new(CredentialLockoutVerdict.LockedOut, entry.LockedUntil - now, !entry.RefusalLogged); + entry.RefusalLogged = true; + return true; + } + + /// Releases one slot, records (null = uncounted) and wakes waiters. + private bool ReleaseCore(string key, Entry entry, bool? outcome) + { + var now = _time.GetUtcNow(); + if (entry.InFlight > 0) entry.InFlight--; + var lockedNow = false; + if (outcome == true) ClearCount(entry); + else if (outcome == false) lockedNow = RecordFailureCore(entry, now); + Pump(entry, now); + RemoveIfIdle(key, entry); + return lockedNow; + } + + /// + /// Hands freed slots to waiters in order — or, when the key is locked out, refuses every waiter (they never + /// reach the authenticator). Called under the gate after anything that can free a slot or lock the key. + /// + private void Pump(Entry entry, DateTimeOffset now) + { + Refresh(entry, now); + while (entry.Waiters.First is { } first) + { + if (TryLockedOut(entry, now, out var refused)) + { + entry.Waiters.RemoveFirst(); + first.Value.Completion.TrySetResult(refused); + continue; + } + + if (!HasFreeSlot(entry)) return; + entry.Waiters.RemoveFirst(); + entry.InFlight++; + if (!first.Value.Completion.TrySetResult(new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false))) + entry.InFlight--; // a waiter that already gave up never takes the slot + } + } + + private static void ClearCount(Entry entry) + { + entry.Failures = 0; + entry.LockedUntil = default; + entry.RefusalLogged = false; + } + + private bool RecordFailureCore(Entry entry, DateTimeOffset now) + { + Refresh(entry, now); + if (entry.LockedUntil > now) return false; // a refusal (or a late in-flight failure) never extends it + entry.Failures++; + entry.LastFailure = now; + if (entry.Failures < _options.MaxFailedAttempts) return false; + entry.LockedUntil = now + TimeSpan.FromSeconds(Math.Max(0, _options.LockoutSeconds)); + entry.RefusalLogged = false; + return entry.LockedUntil > now; + } + + /// + /// Forgets a run of failures that has gone stale: an expired lockout always starts a fresh count (else a + /// LockoutSeconds shorter than FailureWindowSeconds relocked on the first failure after it), + /// and so does a failure older than the window. + /// + private void Refresh(Entry entry, DateTimeOffset now) + { + if (entry.LockedUntil != default && entry.LockedUntil <= now) + { + ClearCount(entry); + } + else if (entry.LockedUntil == default && entry.Failures > 0 + && now - entry.LastFailure > TimeSpan.FromSeconds(Math.Max(0, _options.FailureWindowSeconds))) + { + entry.Failures = 0; + entry.RefusalLogged = false; + } + } + + private void RemoveIfIdle(string key, Entry entry) + { + if (IsIdle(entry, _time.GetUtcNow())) _entries.Remove(key); + } + + private void Prune(DateTimeOffset now) + { + foreach (var key in _entries.Where(e => IsIdle(e.Value, now)).Select(e => e.Key).ToList()) + _entries.Remove(key); + } + + /// Idle = nothing to remember: no failures, no lockout, no slot held and nobody waiting. + private bool IsIdle(Entry entry, DateTimeOffset now) + { + Refresh(entry, now); + return entry.InFlight == 0 && entry.Waiters.Count == 0 && entry.Failures == 0 && entry.LockedUntil == default; + } + + /// One activation waiting for a slot. + private sealed class Waiter + { + public readonly TaskCompletionSource Completion = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + public LinkedListNode? Node; + } + + /// One client's failure count, slots, waiters and lockout. Mutated only under the gate. + private sealed class Entry + { + public int Failures; + public int InFlight; + public DateTimeOffset LastFailure; + public DateTimeOffset LockedUntil; + public bool RefusalLogged; + public readonly LinkedList Waiters = new(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs index 47a57aa6d..7df610d57 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.IntegrationTests/MTConnectAgentIntegrationTests.cs @@ -868,7 +868,7 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) } /// - /// #804 — a SampleCount at or above the Agent's bufferSize is refused OUT_OF_RANGE on + /// #804 — a SampleCount above the Agent's bufferSize is refused OUT_OF_RANGE on /// EVERY /sample open (live: 'count' must be less than N). The driver keeps serving values off /// /current re-baselines, and — after three in a row with no /sample chunk — reports the stream /// Degraded with LastSuccessfulRead still advancing, rather than Healthy for ever (before #804 the @@ -947,6 +947,44 @@ public sealed class MTConnectAgentIntegrationTests(MTConnectAgentFixture agent) seen.Skip(before).ShouldContain(e => e.Snapshot.StatusCode == Good); } + /// + /// #825 — Test Connect reads the live Agent's /probe bufferSize and warns (still Ok) when the + /// configured SampleCount would be refused. The live Agent accepts count == bufferSize and refuses + /// bufferSize + 1, so both sides of that edge are checked against the Agent itself as well as the probe. + /// + [Fact(Timeout = TestTimeoutMs)] + public async Task Test_Connect_warns_when_SampleCount_exceeds_the_live_agents_buffer_size() + { + SkipIfDown(); + + var bufferSize = (int?)agent.ProbeDocument?.Root?.Elements().FirstOrDefault(e => e.Name.LocalName == "Header") + ?.Attribute("bufferSize"); + if (bufferSize is not > 1) + { + Assert.Skip($"The Agent at {agent.AgentUri} declares no usable bufferSize in its /probe header."); + } + + // The Agent's own edge, so the warning threshold is pinned to what the Agent actually does. + using (var http = new HttpClient { Timeout = TimeSpan.FromSeconds(15) }) + { + (await http.GetStringAsync($"{agent.AgentUri}/sample?count={bufferSize.Value}", Ct)).ShouldNotContain("OUT_OF_RANGE"); + var over = await http.GetAsync($"{agent.AgentUri}/sample?count={bufferSize.Value + 1}", Ct); + (await over.Content.ReadAsStringAsync(Ct)).ShouldContain("OUT_OF_RANGE"); + } + + var refused = await new MTConnectDriverProbe().ProbeAsync( + JsonSerializer.Serialize(new { agentUri = agent.AgentUri, sampleCount = bufferSize.Value + 1 }), + TimeSpan.FromSeconds(15), Ct); + refused.Ok.ShouldBeTrue(refused.Message); + refused.Message!.ShouldContain($"WARNING: SampleCount {bufferSize.Value + 1} is above the Agent's bufferSize {bufferSize.Value}"); + + var fits = await new MTConnectDriverProbe().ProbeAsync( + JsonSerializer.Serialize(new { agentUri = agent.AgentUri, sampleCount = bufferSize.Value }), + TimeSpan.FromSeconds(15), Ct); + fits.Ok.ShouldBeTrue(fits.Message); + fits.Message!.ShouldNotContain("WARNING"); + } + private static bool Is(string? value, string expected) => string.Equals(value?.Trim(), expected, StringComparison.OrdinalIgnoreCase); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBufferSizeWarningTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBufferSizeWarningTests.cs new file mode 100644 index 000000000..dbe00e969 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectBufferSizeWarningTests.cs @@ -0,0 +1,129 @@ +using System.Net; +using System.Net.Http; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests; + +/// +/// lmxopcua#825 — a SampleCount above the Agent's bufferSize is refused OUT_OF_RANGE on +/// every /sample stream. The /probe header carries bufferSize, so Test Connect and the +/// driver's initialize (deploy) both warn instead of leaving it to surface as a Degraded stream. +/// +[Trait("Category", "Unit")] +public sealed class MTConnectBufferSizeWarningTests +{ + private static string ProbeXml(string? bufferSizeAttribute) => + $""" + + +
+ + + + + + + """; + + [Theory] + [InlineData("16384", 16384L)] + [InlineData(null, null)] + [InlineData("0", null)] + [InlineData("-5", null)] + [InlineData("lots", null)] + public void The_probe_parser_reads_the_header_buffer_size_and_never_fails_on_it(string? attribute, long? expected) => + MTConnectProbeParser.Parse(ProbeXml(attribute)).BufferSize.ShouldBe(expected); + + [Theory] + [InlineData(16384, 16384L, false)] // live, cppagent 2.7: count == bufferSize is accepted + [InlineData(16385, 16384L, true)] // ... and bufferSize + 1 refused OUT_OF_RANGE + [InlineData(20000, 16384L, true)] + [InlineData(1_000_000, null, false)] + public void The_warning_fires_only_above_the_buffer_size(int sampleCount, long? bufferSize, bool warns) + { + var warning = MTConnectDriver.SampleCountBufferWarning(sampleCount, bufferSize); + (warning is not null).ShouldBe(warns); + if (warns) warning!.ShouldContain($"at most {bufferSize}"); + } + + [Fact] + public async Task Test_Connect_stays_ok_but_carries_the_warning_when_SampleCount_does_not_fit() + { + var probe = new MTConnectDriverProbe(new StubHandler(ProbeXml("16384"))); + var r = await probe.ProbeAsync( + "{\"agentUri\":\"http://stub-agent:5000\",\"sampleCount\":16385}", TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + + r.Ok.ShouldBeTrue("the Agent answered — reachability is not the problem"); + r.Message!.ShouldContain("WARNING:"); + r.Message!.ShouldContain("bufferSize 16384"); + } + + [Fact] + public async Task Test_Connect_checks_the_driver_default_when_SampleCount_is_absent() + { + // The default SampleCount is 1000 — an Agent with a tiny buffer refuses it too. + var probe = new MTConnectDriverProbe(new StubHandler(ProbeXml("512"))); + var r = await probe.ProbeAsync( + "{\"agentUri\":\"http://stub-agent:5000\"}", TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + r.Ok.ShouldBeTrue(); + r.Message!.ShouldContain("SampleCount 1000 is above the Agent's bufferSize 512"); + } + + [Fact] + public async Task Test_Connect_message_is_unchanged_when_SampleCount_fits() + { + var probe = new MTConnectDriverProbe(new StubHandler(ProbeXml("16384"))); + var r = await probe.ProbeAsync( + "{\"agentUri\":\"http://stub-agent:5000\",\"sampleCount\":16384}", TimeSpan.FromSeconds(3), + TestContext.Current.CancellationToken); + + r.Ok.ShouldBeTrue(); + r.Message!.ShouldNotContain("WARNING"); + } + + [Theory] + [InlineData(131073, true)] + [InlineData(131072, false)] + [InlineData(1000, false)] + public async Task Initialize_warns_and_surfaces_the_buffer_size_on_the_diagnostics_bag(int sampleCount, bool warns) + { + var client = CannedAgentClient.FromFixtures(); // Fixtures/probe.xml: bufferSize="131072" + client.Probe.BufferSize.ShouldBe(131072L); + var logger = new RecordingDriverLogger(); + var driver = new MTConnectDriver( + new MTConnectDriverOptions + { + AgentUri = "http://fixture-agent:5000", + RequestTimeoutMs = 5000, + SampleCount = sampleCount, + Tags = [new MTConnectTagDefinition("dev1_pos", DriverDataType.Float64)], + }, + "mt-825", + _ => client, + logger); + try + { + await driver.InitializeAsync("{}", TestContext.Current.CancellationToken); + + logger.Warnings.Any(w => w.Contains("bufferSize", StringComparison.Ordinal)).ShouldBe(warns); + driver.GetDiagnostics()["agentBufferSize"].ShouldBe("131072"); + } + finally + { + await driver.ShutdownAsync(TestContext.Current.CancellationToken); + } + } + + private sealed class StubHandler(string probeXml) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken ct) => + Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(probeXml, System.Text.Encoding.UTF8, "application/xml"), + }); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs index ccaccf26a..b60d1683e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectSubscribeTests.cs @@ -1168,7 +1168,7 @@ public sealed class MTConnectSubscribeTests /// /// #804 (Java-driver parity: MAX_REBASELINES_WITHOUT_PROGRESS). An Agent that answers EVERY - /// /sample open with OUT_OF_RANGE (live: a count at or above its bufferSize) is + /// /sample open with OUT_OF_RANGE (live: a count above its bufferSize) is /// re-baselined from /current each time, and the backoff paces that — but every re-baseline also marked /// the stream delivered, so the driver read Healthy for ever while not one /sample chunk arrived. /// After in a row it is Degraded and stays so diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DeleteConfirmationWiringTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DeleteConfirmationWiringTests.cs index 3505c267b..767554a60 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DeleteConfirmationWiringTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/DeleteConfirmationWiringTests.cs @@ -28,6 +28,31 @@ public sealed class DeleteConfirmationWiringTests Regex.Matches(src, "btn-outline-danger").Count.ShouldBe(3); } + /// + /// #832: the shared confirm used to sit INSIDE the Alarms tab branch, so it was on the page only while that + /// tab was showing — the Tags tab's Remove and the Virtual Tags tab's Delete set the dialog visible and + /// nothing appeared. Pinned by brace depth: the dialog must be at the depth of the tab @if chain, + /// not inside any branch of it. + /// + [Fact] + public void Equipment_page_confirm_renders_outside_every_tab_branch() + { + var src = AdminUiSource.Read("Components", "Pages", "Uns", "EquipmentPage.razor"); + var markup = src[..src.IndexOf("@code", StringComparison.Ordinal)]; + + int DepthAt(int index) => + markup[..index].Count(c => c == '{') - markup[..index].Count(c => c == '}'); + + var tabChain = markup.IndexOf("@if (_activeTab == \"details\")", StringComparison.Ordinal); + var dialog = markup.IndexOf("#833 review: an enabled lockout with a non-positive duration fails; a disabled one is not checked. + [Theory] + [InlineData(5, 0, 60, false)] + [InlineData(5, 300, 0, false)] + [InlineData(5, -1, -1, false)] + [InlineData(5, 30, 600, true)] + [InlineData(0, 0, 0, true)] + [InlineData(-1, 0, 0, true)] + public void Credential_lockout_durations_must_be_positive_while_enabled( + int maxFailedAttempts, int lockoutSeconds, int windowSeconds, bool valid) + { + var result = Sut.Validate(null, new OpcUaApplicationHostOptions + { + CredentialLockout = new ZB.MOM.WW.OtOpcUa.OpcUaServer.Security.OpcUaCredentialLockoutOptions + { + MaxFailedAttempts = maxFailedAttempts, LockoutSeconds = lockoutSeconds, FailureWindowSeconds = windowSeconds, + }, + }); + + result.Succeeded.ShouldBe(valid); + if (!valid) result.Failures!.ShouldContain(f => f.Contains("OpcUa:CredentialLockout:LockoutSeconds")); + } + + /// #833 review: the slot wait must be positive while the lockout is enabled. + [Fact] + public void Credential_lockout_slot_wait_must_be_positive_while_enabled() + { + var result = Sut.Validate(null, new OpcUaApplicationHostOptions + { + CredentialLockout = new ZB.MOM.WW.OtOpcUa.OpcUaServer.Security.OpcUaCredentialLockoutOptions { SlotWaitSeconds = 0 }, + }); + + result.Failed.ShouldBeTrue(); + result.Failures!.ShouldContain(f => f.Contains("OpcUa:CredentialLockout:SlotWaitSeconds")); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/CredentialLockoutWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/CredentialLockoutWireTests.cs new file mode 100644 index 000000000..342c67c33 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/CredentialLockoutWireTests.cs @@ -0,0 +1,208 @@ +using System.Net; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// lmxopcua#833 over the wire. A few failed UserName activations used to trip the OPC Foundation SDK's +/// per-client lockout, which then refused EVERY activation from that client — Anonymous included — with +/// BadUserAccessDenied for 5 minutes. The host now disables the SDK lockout and enforces a +/// UserName-only replacement (). +/// +public sealed class CredentialLockoutWireTests +{ + private const string GoodKey = "otua_k-good_secret"; + + [Fact] + public async Task Failed_credentials_never_lock_the_client_out_of_anonymous_activation() + { + var ct = TestContext.Current.CancellationToken; + var auth = new CountingAuthenticator(); + await using var rig = await Rig.StartAsync(auth, new OpcUaCredentialLockoutOptions(), ct); + + for (var i = 0; i < 6; i++) + { + var rejected = await Should.ThrowAsync(() => rig.ConnectAsync($"otua_k-bad{i}_x", ct)); + rejected.StatusCode.ShouldBeOneOf(StatusCodes.BadIdentityTokenRejected, StatusCodes.BadUserAccessDenied); + } + + using var anonymous = await rig.ConnectAsync(credential: null, ct); + var ns = await anonymous.ReadValueAsync(VariableIds.Server_NamespaceArray, ct); + StatusCode.IsGood(ns.StatusCode).ShouldBeTrue(); + auth.AnonymousCalls.ShouldBe(1, "the anonymous activation must reach the authenticator, not be refused before it"); + } + + [Fact] + public async Task Failed_credentials_still_lock_out_further_UserName_activations_from_that_client() + { + var ct = TestContext.Current.CancellationToken; + var auth = new CountingAuthenticator(); + await using var rig = await Rig.StartAsync( + auth, new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 3 }, ct); + + for (var i = 0; i < 3; i++) + { + var rejected = await Should.ThrowAsync(() => rig.ConnectAsync($"otua_k-bad{i}_x", ct)); + rejected.StatusCode.ShouldBe(StatusCodes.BadIdentityTokenRejected); + } + + var callsBefore = auth.UserNameCalls; + var locked = await Should.ThrowAsync(() => rig.ConnectAsync(GoodKey, ct)); + locked.StatusCode.ShouldBe(StatusCodes.BadUserAccessDenied); + auth.UserNameCalls.ShouldBe(callsBefore, "a locked-out client's credential is refused without being checked"); + } + + /// + /// #833 review — one client opening more keyed sessions at once than MaxFailedAttempts (a ScadaBridge site + /// node behind one NAT IP) is never refused for parallelism: the extra activations wait for a slot. + /// + [Fact] + public async Task Parallel_valid_activations_beyond_the_threshold_all_succeed() + { + var ct = TestContext.Current.CancellationToken; + var auth = new CountingAuthenticator(); + await using var rig = await Rig.StartAsync(auth, new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 2 }, ct); + + var sessions = await Task.WhenAll(Enumerable.Range(0, 8).Select(_ => rig.ConnectAsync(GoodKey, ct))); + try + { + sessions.ShouldAllBe(s => s.Connected); + auth.UserNameCalls.ShouldBe(8); + } + finally + { + foreach (var s in sessions) s.Dispose(); + } + } + + [Fact] + public async Task A_disabled_lockout_admits_a_valid_key_after_any_number_of_failures() + { + // Also proves the SDK's own lockout (default 5) is switched off: with ours disabled nothing may refuse. + var ct = TestContext.Current.CancellationToken; + var auth = new CountingAuthenticator(); + await using var rig = await Rig.StartAsync( + auth, new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 0 }, ct); + + for (var i = 0; i < 7; i++) + { + var rejected = await Should.ThrowAsync(() => rig.ConnectAsync($"otua_k-bad{i}_x", ct)); + rejected.StatusCode.ShouldBe(StatusCodes.BadIdentityTokenRejected); + } + + using var session = await rig.ConnectAsync(GoodKey, ct); + session.Connected.ShouldBeTrue(); + } + + private sealed class Rig : IAsyncDisposable + { + private readonly string _pkiRoot; + private OpcUaApplicationHost _host = null!; + private int _port; + + private Rig(string pkiRoot) => _pkiRoot = pkiRoot; + + public static async Task StartAsync( + IOpcUaUserAuthenticator authenticator, OpcUaCredentialLockoutOptions lockout, CancellationToken ct) + { + var rig = new Rig(Path.Combine(Path.GetTempPath(), $"otopcua-lockout-{Guid.NewGuid():N}")); + var applicationUri = $"urn:OtOpcUa.Lockout.{Guid.NewGuid():N}"; + (rig._host, _) = await OpcUaHostStart.StartAsync( + port => + { + rig._port = port; + return new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.Lockout", + ApplicationUri = applicationUri, + OpcUaPort = port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = rig._pkiRoot, + EnabledSecurityProfiles = [OpcUaSecurityProfile.None], + AutoAcceptUntrustedClientCertificates = true, + CredentialLockout = lockout, + }, + NullLogger.Instance, + authenticator); + }, + () => new OtOpcUaSdkServer(), ct); + return rig; + } + + /// Opens a session; a null credential activates anonymously. + public async Task ConnectAsync(string? credential, CancellationToken ct) + { + var appConfig = new ApplicationConfiguration(DefaultTelemetry.Create(_ => { })) + { + ApplicationName = "OtOpcUa.LockoutClient", + ApplicationUri = "urn:OtOpcUa.LockoutClient", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = TestClientSecurity.Build(TestClientSecurity.AllocatePkiRoot()), + ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 }, + }; + await appConfig.ValidateAsync(ApplicationType.Client, ct); + + var endpoint = await CoreClientUtils.SelectEndpointAsync( + appConfig, $"opc.tcp://127.0.0.1:{_port}/OtOpcUa", false, DefaultTelemetry.Create(_ => { }), ct); + + return await new DefaultSessionFactory(DefaultTelemetry.Create(_ => { })).CreateAsync( + appConfig, + new ConfiguredEndpoint(null, endpoint ?? throw new InvalidOperationException("no endpoint"), EndpointConfiguration.Create(appConfig)), + updateBeforeConnect: false, + checkDomain: false, + sessionName: "CredentialLockoutWireTests", + sessionTimeout: 60_000, + identity: credential is null + ? new UserIdentity() + : new UserIdentity("apikey", Encoding.UTF8.GetBytes(credential)), + preferredLocales: default, + ct: ct); + } + + public async ValueTask DisposeAsync() + { + await _host.DisposeAsync(); + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch (IOException) { /* best effort */ } + } + } + } + + /// Admits and anonymous; denies everything else; counts calls. + private sealed class CountingAuthenticator : IOpcUaUserAuthenticator + { + private int _userNameCalls; + private int _anonymousCalls; + + public int UserNameCalls => Volatile.Read(ref _userNameCalls); + public int AnonymousCalls => Volatile.Read(ref _anonymousCalls); + + public Task AuthenticateUserNameAsync(string username, string password, CancellationToken ct) + { + Interlocked.Increment(ref _userNameCalls); + return Task.FromResult(password == GoodKey + ? OpcUaUserAuthResult.AllowKey("good", [], "k-good") + : OpcUaUserAuthResult.Deny("Invalid API key.")); + } + + public Task AuthenticateUserNameAsync( + string username, string password, IPEndPoint? remoteEndpoint, CancellationToken ct) => + AuthenticateUserNameAsync(username, password, ct); + + public Task AuthenticateAnonymousAsync(CancellationToken ct) + { + Interlocked.Increment(ref _anonymousCalls); + return Task.FromResult(OpcUaUserAuthResult.Allow(OpcUaPrincipals.Anonymous, [])); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/FailedCredentialLockoutTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/FailedCredentialLockoutTests.cs new file mode 100644 index 000000000..1bd619e8f --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/FailedCredentialLockoutTests.cs @@ -0,0 +1,302 @@ +using System.Net; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// lmxopcua#833 — the UserName-only lockout that replaces the SDK's all-token lockout. +public sealed class FailedCredentialLockoutTests +{ + private static readonly string Client = FailedCredentialLockout.KeyFor("THUMB", IPAddress.Parse("10.0.0.5")); + + [Fact] + public void Locks_at_the_threshold_and_releases_after_the_lockout() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions(), clock); + + for (var i = 0; i < 4; i++) lockout.RecordFailure(Client).ShouldBeFalse(); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + lockout.RecordFailure(Client).ShouldBeTrue("the fifth failure locks"); + lockout.IsLockedOut(Client, out var remaining).ShouldBeTrue(); + remaining.ShouldBe(TimeSpan.FromMinutes(5)); + + clock.Advance(TimeSpan.FromSeconds(299)); + lockout.IsLockedOut(Client, out _).ShouldBeTrue(); + clock.Advance(TimeSpan.FromSeconds(1)); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + lockout.RecordFailure(Client).ShouldBeFalse("a lockout that ran out starts a fresh count"); + } + + [Fact] + public void A_refusal_during_the_lockout_does_not_extend_it() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1 }, clock); + lockout.RecordFailure(Client).ShouldBeTrue(); + clock.Advance(TimeSpan.FromSeconds(200)); + lockout.RecordFailure(Client).ShouldBeFalse(); + clock.Advance(TimeSpan.FromSeconds(100)); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + } + + [Fact] + public void Failures_spaced_wider_than_the_window_never_accumulate() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions(), clock); + for (var i = 0; i < 10; i++) + { + lockout.RecordFailure(Client).ShouldBeFalse(); + clock.Advance(TimeSpan.FromSeconds(61)); + } + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + } + + [Fact] + public void A_success_clears_the_count() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions(), new ManualClock()); + for (var i = 0; i < 4; i++) lockout.RecordFailure(Client); + lockout.RecordSuccess(Client); + for (var i = 0; i < 4; i++) lockout.RecordFailure(Client).ShouldBeFalse(); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + } + + [Fact] + public void The_key_needs_both_certificate_and_address_so_one_client_cannot_lock_out_another() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1 }, new ManualClock()); + var attacker = FailedCredentialLockout.KeyFor("VICTIMTHUMB", IPAddress.Parse("10.0.0.99")); + var victim = FailedCredentialLockout.KeyFor("VICTIMTHUMB", IPAddress.Parse("10.0.0.5")); + var natNeighbour = FailedCredentialLockout.KeyFor("OTHERTHUMB", IPAddress.Parse("10.0.0.99")); + + lockout.RecordFailure(attacker).ShouldBeTrue(); + lockout.IsLockedOut(victim, out _).ShouldBeFalse("same certificate, different address"); + lockout.IsLockedOut(natNeighbour, out _).ShouldBeFalse("same address, different certificate"); + } + + [Fact] + public void Zero_attempts_disables_it() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 0 }, new ManualClock()); + for (var i = 0; i < 50; i++) lockout.RecordFailure(Client).ShouldBeFalse(); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + lockout.TrackedCount.ShouldBe(0); + } + + [Fact] + public void Tracking_is_bounded_and_prunes_expired_clients_first() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions(), clock); + for (var i = 0; i < FailedCredentialLockout.MaxTrackedClients; i++) lockout.RecordFailure($"c{i}"); + lockout.TrackedCount.ShouldBe(FailedCredentialLockout.MaxTrackedClients); + + lockout.RecordFailure("overflow"); + lockout.TrackedCount.ShouldBe(FailedCredentialLockout.MaxTrackedClients, "full and nothing expired ⇒ not tracked"); + + clock.Advance(TimeSpan.FromSeconds(61)); + lockout.RecordFailure("overflow"); + lockout.TrackedCount.ShouldBe(1, "expired entries are pruned to make room"); + } + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + /// + /// #833 review — a legitimate client opening many keyed sessions at once (a ScadaBridge site node behind one + /// NAT IP) must never be refused for parallelism: at most MaxFailedAttempts verify concurrently, the rest + /// WAIT for a slot, and all 20 succeed. + /// + [Fact] + public async Task An_all_valid_parallel_burst_queues_for_slots_and_every_activation_succeeds() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 5 }, new ManualClock()); + var concurrent = 0; + var maxConcurrent = 0; + var proceeded = 0; + + await Task.WhenAll(Enumerable.Range(0, 20).Select(attempt => Task.Run(async () => + { + var decision = await lockout.BeginAsync(Client, Ct); + decision.Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + Interlocked.Increment(ref proceeded); + var now = Interlocked.Increment(ref concurrent); + InterlockedMax(ref maxConcurrent, now); + await Task.Delay(20, Ct); // the authenticate call + Interlocked.Decrement(ref concurrent); + lockout.Complete(Client, succeeded: true); + }, Ct))); + + proceeded.ShouldBe(20); + maxConcurrent.ShouldBe(5, "exactly MaxFailedAttempts verify at once — no more, and the cap is actually reached"); + lockout.TrackedCount.ShouldBe(0, "nothing left behind once the burst drains"); + } + + /// + /// #833 review — the security half: a parallel burst of guesses that all arrive before any failure is + /// recorded still reaches the authenticator only MaxFailedAttempts times; once those fail, every waiter is + /// refused as locked out without being authenticated. + /// + [Fact] + public async Task An_all_bad_parallel_burst_authenticates_only_max_attempts_and_refuses_the_waiters_as_locked_out() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 5 }, new ManualClock()); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var authenticated = 0; + var lockedOut = 0; + + var attempts = Enumerable.Range(0, 20).Select(attempt => Task.Run(async () => + { + var decision = await lockout.BeginAsync(Client, Ct); + if (decision.Verdict == CredentialLockoutVerdict.LockedOut) + { + Interlocked.Increment(ref lockedOut); + return; + } + + decision.Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + Interlocked.Increment(ref authenticated); + await release.Task; // hold the slots until all 15 others are queued behind them + lockout.Complete(Client, succeeded: false); + }, Ct)).ToArray(); + + await WaitUntilAsync(() => lockout.WaitingCount(Client) == 15); + authenticated.ShouldBe(5); + release.SetResult(); + await Task.WhenAll(attempts); + + authenticated.ShouldBe(5, "no waiter may reach the authenticator once the key locks out"); + lockedOut.ShouldBe(15); + lockout.IsLockedOut(Client, out _).ShouldBeTrue(); + lockout.WaitingCount(Client).ShouldBe(0); + } + + /// After a recorded failure the client has one slot fewer: capacity is MaxFailedAttempts − failures. + [Fact] + public async Task Recorded_failures_shrink_the_number_of_concurrent_verifications() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 3 }, new ManualClock()); + lockout.RecordFailure(Client); + lockout.RecordFailure(Client); + + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + var second = lockout.BeginAsync(Client, Ct).AsTask(); + await WaitUntilAsync(() => lockout.WaitingCount(Client) == 1); + second.IsCompleted.ShouldBeFalse("one failure short of the threshold leaves exactly one slot"); + + lockout.Complete(Client, succeeded: true); // a success clears the count and hands the slot on + (await second).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + lockout.Complete(Client, succeeded: true); + lockout.TrackedCount.ShouldBe(0); + } + + /// A cancelled waiter is dequeued and leaves nothing behind (no leaked slot, entry prunable). + [Fact] + public async Task A_cancelled_waiter_is_removed_and_leaks_nothing() + { + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1 }, new ManualClock()); + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(Ct); + var waiting = lockout.BeginAsync(Client, cts.Token).AsTask(); + await WaitUntilAsync(() => lockout.WaitingCount(Client) == 1); + cts.Cancel(); + await Should.ThrowAsync(waiting); + lockout.WaitingCount(Client).ShouldBe(0); + + lockout.Complete(Client, succeeded: true); + lockout.TrackedCount.ShouldBe(0, "the held slot and the cancelled waiter are both gone"); + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + lockout.Abandon(Client); + lockout.TrackedCount.ShouldBe(0); + } + + /// A waiter that never gets a slot times out, uncounted — the client is not one failure closer. + [Fact] + public async Task A_slot_wait_timeout_fails_that_attempt_without_counting_it() + { + var lockout = new FailedCredentialLockout( + new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1, SlotWaitSeconds = 1 }, new ManualClock()); + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.SlotWaitTimedOut); + lockout.WaitingCount(Client).ShouldBe(0); + + lockout.Complete(Client, succeeded: true); + lockout.TrackedCount.ShouldBe(0, "the timed-out attempt recorded no failure"); + } + + [Fact] + public async Task Only_the_first_refusal_of_a_lockout_episode_is_flagged_for_Information_logging() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1, LockoutSeconds = 10 }, clock); + lockout.RecordFailure(Client).ShouldBeTrue(); + + var r1 = await lockout.BeginAsync(Client, Ct); + r1.Verdict.ShouldBe(CredentialLockoutVerdict.LockedOut); + r1.FirstRefusal.ShouldBeTrue(); + (await lockout.BeginAsync(Client, Ct)).FirstRefusal.ShouldBeFalse(); + + clock.Advance(TimeSpan.FromSeconds(11)); + (await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed); + lockout.Complete(Client, succeeded: false).ShouldBeTrue("a new episode"); + (await lockout.BeginAsync(Client, Ct)).FirstRefusal.ShouldBeTrue("each lockout episode logs its first refusal once"); + } + + private static void InterlockedMax(ref int target, int value) + { + int seen; + while ((seen = Volatile.Read(ref target)) < value && Interlocked.CompareExchange(ref target, value, seen) != seen) + { + } + } + + private static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (!condition()) + { + DateTime.UtcNow.ShouldBeLessThan(deadline, "condition not reached in time"); + await Task.Delay(5, Ct); + } + } + + /// + /// #833 review — with LockoutSeconds shorter than FailureWindowSeconds, the count survived the expired lockout + /// (the last failure was still inside the window), so the very next failure relocked the client. + /// + [Fact] + public void An_expired_lockout_starts_a_fresh_count_even_inside_the_failure_window() + { + var clock = new ManualClock(); + var lockout = new FailedCredentialLockout( + new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 2, FailureWindowSeconds = 600, LockoutSeconds = 10 }, clock); + lockout.RecordFailure(Client).ShouldBeFalse(); + lockout.RecordFailure(Client).ShouldBeTrue(); + + clock.Advance(TimeSpan.FromSeconds(11)); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + lockout.RecordFailure(Client).ShouldBeFalse("one failure after the lockout is the first of a new run"); + lockout.IsLockedOut(Client, out _).ShouldBeFalse(); + lockout.RecordFailure(Client).ShouldBeTrue(); + } + + [Fact] + public void The_host_switches_the_sdk_lockout_off() + { + var config = new Opc.Ua.ServerConfiguration(); + config.MaxFailedAuthenticationAttempts.ShouldBe(5, "the SDK default this guards against"); + OpcUaApplicationHost.DisableSdkAuthenticationLockout(config); + config.MaxFailedAuthenticationAttempts.ShouldBe(0); + } + + private sealed class ManualClock : TimeProvider + { + private DateTimeOffset _now = new(2026, 11, 11, 0, 0, 0, TimeSpan.Zero); + public override DateTimeOffset GetUtcNow() => _now; + public void Advance(TimeSpan by) => _now += by; + } +}