fix(opcua): failed sign-ins no longer lock a client out of anonymous sessions (#833)
OPC Foundation SDK 2.0.1's SessionManager lockout refused EVERY activation (Anonymous included, before the token is read) from a client with 5 failed identity validations in 60 s, for 5 minutes, keyed on cert thumbprint else peer IP. Disable it (MaxFailedAuthenticationAttempts=0 on built and loaded configs) and enforce FailedCredentialLockout in the identity-registry adapter instead: UserName activations only, keyed on thumbprint+peer together, same 5/60s/300s defaults, configurable via OpcUa:CredentialLockout.
This commit is contained in:
Joseph Doherty
+185
@@ -0,0 +1,185 @@
|
||||
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;
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <c>BadUserAccessDenied</c> for 5 minutes. The host now disables the SDK lockout and enforces a
|
||||
/// UserName-only replacement (<see cref="FailedCredentialLockout"/>).
|
||||
/// </summary>
|
||||
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<ServiceResultException>(() => 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<ServiceResultException>(() => rig.ConnectAsync($"otua_k-bad{i}_x", ct));
|
||||
rejected.StatusCode.ShouldBe(StatusCodes.BadIdentityTokenRejected);
|
||||
}
|
||||
|
||||
var callsBefore = auth.UserNameCalls;
|
||||
var locked = await Should.ThrowAsync<ServiceResultException>(() => rig.ConnectAsync(GoodKey, ct));
|
||||
locked.StatusCode.ShouldBe(StatusCodes.BadUserAccessDenied);
|
||||
auth.UserNameCalls.ShouldBe(callsBefore, "a locked-out client's credential is refused without being checked");
|
||||
}
|
||||
|
||||
[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<ServiceResultException>(() => 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<Rig> 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<OpcUaApplicationHost>.Instance,
|
||||
authenticator);
|
||||
},
|
||||
() => new OtOpcUaSdkServer(), ct);
|
||||
return rig;
|
||||
}
|
||||
|
||||
/// <summary>Opens a session; a null credential activates anonymously.</summary>
|
||||
public async Task<ISession> 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 */ }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Admits <see cref="GoodKey"/> and anonymous; denies everything else; counts calls.</summary>
|
||||
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<OpcUaUserAuthResult> 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<OpcUaUserAuthResult> AuthenticateUserNameAsync(
|
||||
string username, string password, IPEndPoint? remoteEndpoint, CancellationToken ct) =>
|
||||
AuthenticateUserNameAsync(username, password, ct);
|
||||
|
||||
public Task<OpcUaUserAuthResult> AuthenticateAnonymousAsync(CancellationToken ct)
|
||||
{
|
||||
Interlocked.Increment(ref _anonymousCalls);
|
||||
return Task.FromResult(OpcUaUserAuthResult.Allow(OpcUaPrincipals.Anonymous, []));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,120 @@
|
||||
using System.Net;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
|
||||
|
||||
/// <summary>lmxopcua#833 — the UserName-only lockout that replaces the SDK's all-token lockout.</summary>
|
||||
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");
|
||||
}
|
||||
|
||||
[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;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user