From cea62836460567e036fd0fbc516627e6b7fca0e3 Mon Sep 17 00:00:00 2001 From: Roman Ettlinger Date: Mon, 22 Dec 2025 06:59:34 +0100 Subject: [PATCH 1/7] Handle certificate updates safely; close channels on update Introduce async certificate update events (`Started`/`Completed`) in `CertificateValidator` and `ICertificateValidator`. Add `CertificateUpdateInProgress` wait handle for synchronization. Implement `CloseAllChannels` in transport listeners to force-close connections before certificate updates, and add `ForceClose` to `TcpListenerChannel`. Update server logic to close all channels before updating certificates. Refactor and update tests for new async events and certificate update sequencing. Improves security and reliability during certificate rollover. --- .../GlobalDiscoveryServerClient.cs | 2 + .../ServerPushConfigurationClient.cs | 2 + .../Opc.Ua.Server/Server/StandardServer.cs | 1 + .../Stack/Https/HttpsTransportListener.cs | 10 ++ .../Certificates/CertificateValidator.cs | 110 +++++++++++------- .../Certificates/ICertificateValidator.cs | 23 ++++ Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs | 21 +++- .../Stack/Tcp/TcpListenerChannel.cs | 48 ++++++++ .../Stack/Tcp/TcpTransportListener.cs | 12 ++ .../Stack/Transport/ITransportListener.cs | 5 + Tests/Opc.Ua.Client.Tests/ClientFixture.cs | 2 + .../Certificates/CertificateValidatorTest.cs | 3 +- Tests/Opc.Ua.Gds.Tests/PushTest.cs | 43 +++++++ .../Pkcs10CertificationRequestTests.cs | 4 +- ...ateSessionApplicationUriValidationTests.cs | 1 + 15 files changed, 245 insertions(+), 42 deletions(-) diff --git a/Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs b/Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs index 2841c8f5a..d986aba00 100644 --- a/Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs +++ b/Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs @@ -376,6 +376,7 @@ await CoreClientUtils.SelectEndpointAsync( } catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or + StatusCodes.BadConnectionClosed or StatusCodes.BadSecureChannelClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) @@ -432,6 +433,7 @@ public async Task ConnectAsync(ConfiguredEndpoint endpoint, CancellationToken ct catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) { diff --git a/Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs b/Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs index e2927da54..2d02086f4 100644 --- a/Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs +++ b/Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs @@ -290,6 +290,7 @@ await CoreClientUtils.SelectEndpointAsync( catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) { @@ -345,6 +346,7 @@ public async Task ConnectAsync(ConfiguredEndpoint endpoint, CancellationToken ct catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) { diff --git a/Libraries/Opc.Ua.Server/Server/StandardServer.cs b/Libraries/Opc.Ua.Server/Server/StandardServer.cs index 834e47ea4..0e6726c47 100644 --- a/Libraries/Opc.Ua.Server/Server/StandardServer.cs +++ b/Libraries/Opc.Ua.Server/Server/StandardServer.cs @@ -3340,6 +3340,7 @@ await subscriptionManager.StartupAsync(cancellationToken) } CertificateValidator.CertificateUpdate += OnCertificateUpdateAsync; + CertificateValidator.CertificateUpdateStarted += OnCertificateUpdateStartedAsync; } /// diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 60222fb08..c697a6c19 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -354,6 +354,9 @@ public void Stop() /// public async Task SendAsync(HttpContext context) { + // wait for certificate update to complete before processing requests. + m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne(); + string message = string.Empty; CancellationToken ct = context.RequestAborted; try @@ -608,6 +611,13 @@ private bool ValidateClientCertificate( return true; } + + /// + public void CloseAllChannels(string reason) + { + // nothing to do + } + private EndpointDescriptionCollection m_descriptions; private ChannelQuotas m_quotas; private ITransportListenerCallback m_callback; diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs index 565a60fee..136e4eaa1 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs @@ -97,6 +97,15 @@ public event CertificateUpdateEventHandler CertificateUpdate remove => m_CertificateUpdate -= value; } + /// + /// Raised before an application certificate update occurs. + /// + public event CertificateUpdateEventHandler CertificateUpdateStarted + { + add => m_CertificateUpdateStarted += value; + remove => m_CertificateUpdateStarted -= value; + } + /// /// Updates the validator with the current state of the configuration. /// @@ -284,51 +293,69 @@ public virtual async Task UpdateCertificateAsync( string applicationUri = null, CancellationToken ct = default) { - await m_semaphore.WaitAsync(ct).ConfigureAwait(false); + m_updateEvent.Reset(); try { - m_applicationCertificates.Clear(); - // - // crash occurs if the cert is in use still and this has not run yet. - // This might be the intended design but this runs on a free task that - // might not be scheduled right away. - // - // TODO: We need a better way to disconnect all sessions when the cert is - // updated. (See caller of this method) - // - // foreach (CertificateIdentifier applicationCertificate in securityConfiguration - // .ApplicationCertificates) - // { - // applicationCertificate.DisposeCertificate(); - // } - - foreach (CertificateIdentifier applicationCertificate in securityConfiguration - .ApplicationCertificates) - { - await applicationCertificate - .LoadPrivateKeyExAsync( - securityConfiguration.CertificatePasswordProvider, - applicationUri, - m_telemetry, - ct) - .ConfigureAwait(false); + CertificateUpdateEventHandler started_callback = m_CertificateUpdateStarted; + if (started_callback != null) + { + var args = new CertificateUpdateEventArgs( + securityConfiguration, + GetChannelValidator()); + await started_callback(this, args).ConfigureAwait(false); + } + + await m_semaphore.WaitAsync(ct).ConfigureAwait(false); + + try + { + m_applicationCertificates.Clear(); + // + // crash occurs if the cert is in use still and this has not run yet. + // This might be the intended design but this runs on a free task that + // might not be scheduled right away. + // + // TODO: We need a better way to disconnect all sessions when the cert is + // updated. (See caller of this method) + // + // foreach (CertificateIdentifier applicationCertificate in securityConfiguration + // .ApplicationCertificates) + // { + // applicationCertificate.DisposeCertificate(); + // } + + foreach (CertificateIdentifier applicationCertificate in securityConfiguration + .ApplicationCertificates) + { + await applicationCertificate + .LoadPrivateKeyExAsync( + securityConfiguration.CertificatePasswordProvider, + applicationUri, + m_telemetry, + ct) + .ConfigureAwait(false); + } + } + finally + { + m_semaphore.Release(); } - } - finally - { - m_semaphore.Release(); - } - await UpdateAsync(securityConfiguration, applicationUri, ct).ConfigureAwait(false); + await UpdateAsync(securityConfiguration, applicationUri, ct).ConfigureAwait(false); - CertificateUpdateEventHandler callback = m_CertificateUpdate; - if (callback != null) + CertificateUpdateEventHandler callback = m_CertificateUpdate; + if (callback != null) + { + var args = new CertificateUpdateEventArgs( + securityConfiguration, + GetChannelValidator()); + await callback(this, args).ConfigureAwait(false); + } + } + finally { - var args = new CertificateUpdateEventArgs( - securityConfiguration, - GetChannelValidator()); - callback(this, args); + m_updateEvent.Set(); } } @@ -2234,6 +2261,9 @@ public static bool IsECSecureForProfile( throw new NotSupportedException("Unsupported curve type."); } + /// + public WaitHandle CertificateUpdateInProgress => m_updateEvent.WaitHandle; + /// /// Flag to protect setting by application /// from a modification by a SecurityConfiguration. @@ -2254,6 +2284,7 @@ private enum ProtectFlags private readonly ILogger m_logger; private readonly ITelemetryContext m_telemetry; private readonly Dictionary m_validatedCertificates; + private readonly ManualResetEventSlim m_updateEvent = new(true); private CertificateStoreIdentifier m_trustedCertificateStore; private CertificateIdentifierCollection m_trustedCertificateList; private CertificateStoreIdentifier m_issuerCertificateStore; @@ -2261,6 +2292,7 @@ private enum ProtectFlags private CertificateStoreIdentifier m_rejectedCertificateStore; private event CertificateValidationEventHandler m_CertificateValidation; private event CertificateUpdateEventHandler m_CertificateUpdate; + private event CertificateUpdateEventHandler m_CertificateUpdateStarted; private readonly List m_applicationCertificates; private ProtectFlags m_protectFlags; private bool m_autoAcceptUntrustedCertificates; @@ -2350,7 +2382,7 @@ public CertificateUpdateEventArgs( /// /// Used to handle certificate update events. /// - public delegate void CertificateUpdateEventHandler( + public delegate Task CertificateUpdateEventHandler( CertificateValidator sender, CertificateUpdateEventArgs e); } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs index fc4f18cf2..ee43631b8 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs @@ -38,6 +38,29 @@ namespace Opc.Ua /// public interface ICertificateValidator { + /// + /// Raised when an application certificate update occurs. + /// + event CertificateUpdateEventHandler CertificateUpdate; + + /// + /// Raised before an application certificate update occurs. + /// + event CertificateUpdateEventHandler CertificateUpdateStarted; + + /// + /// An event that signals that a certificate update is in progress. + /// + WaitHandle CertificateUpdateInProgress { get; } + + /// + /// Updates the validator with a new application certificate. + /// + Task UpdateCertificateAsync( + SecurityConfiguration securityConfiguration, + string applicationUri = null, + CancellationToken ct = default); + /// /// Validates a certificate. /// diff --git a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs index 761f00901..734d3345f 100644 --- a/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs +++ b/Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs @@ -771,7 +771,7 @@ protected virtual EndpointBase GetEndpointInstance(ServerBase server) /// /// Called after the application certificate update. /// - protected virtual async void OnCertificateUpdateAsync(object sender, CertificateUpdateEventArgs e) + protected virtual async Task OnCertificateUpdateAsync(object sender, CertificateUpdateEventArgs e) { try { @@ -810,6 +810,25 @@ await InstanceCertificateTypesProvider.LoadCertificateChainAsync(certificate) } } + /// + /// Called before the application certificate update. + /// + protected virtual Task OnCertificateUpdateStartedAsync(object sender, CertificateUpdateEventArgs e) + { + try + { + foreach (ITransportListener listener in TransportListeners) + { + listener.CloseAllChannels("Update of ApplicationCertificate"); + } + } + catch (Exception ex) + { + m_logger.LogError(ex, "Failed to close all channels on certificate update: {EventArgs}", e); + } + return Task.CompletedTask; + } + /// /// Create the transport listener for the service host endpoint. /// diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs index e26a52c99..ab70f06f5 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs @@ -189,6 +189,28 @@ public void IdleCleanup() } } + /// + /// Force the channel to close immediately, e.g. due to certificate update. + /// + public void ForceClose(string reason) + { + TcpChannelState state; + + lock (DataLock) + { + state = State; + if (state is TcpChannelState.Open or TcpChannelState.Connecting) + { + state = State = TcpChannelState.Closing; + } + } + + if (state is TcpChannelState.Closing or TcpChannelState.Opening or TcpChannelState.Faulted) + { + OnForceClosed(reason); + } + } + /// /// The time in milliseconds elapsed since the channel received or sent messages /// or received a keep alive. @@ -349,6 +371,32 @@ private void OnCleanup(object state) } } + /// + /// Called when the channel is force closed. + /// + private void OnForceClosed(string reason) + { + lock (DataLock) + { + // nothing to do if the channel is now open or closed. + if (State is TcpChannelState.Closed or TcpChannelState.Open) + { + return; + } + + m_logger.LogInformation( + "{Channel} Force Close Socket={SocketHandle:X8}, ChannelId={ChannelId}, TokenId={TokenId}, Reason={Reason}", + ChannelName, + (Socket?.Handle) ?? 0, + CurrentToken != null ? CurrentToken.ChannelId : 0, + CurrentToken != null ? CurrentToken.TokenId : 0, + reason); + + // close channel. + ChannelClosed(); + } + } + /// /// Closes the channel and releases resources. /// Sets state to Closed and notifies monitors. diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index 712e8cfa5..eb3156241 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -401,6 +401,15 @@ public void Close() Stop(); } + /// + public void CloseAllChannels(string reason) + { + foreach (TcpListenerChannel channel in m_channels.Values.ToList()) + { + channel.ForceClose(reason); + } + } + /// public void UpdateChannelLastActiveTime(string globalChannelId) { @@ -807,6 +816,9 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) { bool isBlocked = false; + // wait for certificate update to complete + m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne(); + // Track potential problematic client behavior only if Basic128Rsa15 security policy is offered if (m_activeClientTracker != null) { diff --git a/Stack/Opc.Ua.Core/Stack/Transport/ITransportListener.cs b/Stack/Opc.Ua.Core/Stack/Transport/ITransportListener.cs index 26117bc24..7b26c837b 100644 --- a/Stack/Opc.Ua.Core/Stack/Transport/ITransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Transport/ITransportListener.cs @@ -74,6 +74,11 @@ void Open( /// Thrown if any communication error occurs. void Close(); + /// + /// Closes all connections of the listener. + /// + void CloseAllChannels(string reason); + /// /// Updates the application certificate for a listener. /// diff --git a/Tests/Opc.Ua.Client.Tests/ClientFixture.cs b/Tests/Opc.Ua.Client.Tests/ClientFixture.cs index e87855847..ba6cc57d1 100644 --- a/Tests/Opc.Ua.Client.Tests/ClientFixture.cs +++ b/Tests/Opc.Ua.Client.Tests/ClientFixture.cs @@ -241,6 +241,7 @@ await CoreClientUtils.SelectEndpointAsync( catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) { @@ -286,6 +287,7 @@ public async Task ConnectAsync( catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication) && attempt < maxAttempts) { diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs index 8f7e5b8d0..1b469c5fb 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs @@ -2073,8 +2073,9 @@ await certValidator.ValidateAsync( } } - private void OnCertificateUpdate(object sender, CertificateUpdateEventArgs e) + private Task OnCertificateUpdate(object sender, CertificateUpdateEventArgs e) { + return Task.CompletedTask; } private void OnCertificateValidation(object sender, CertificateValidationEventArgs e) diff --git a/Tests/Opc.Ua.Gds.Tests/PushTest.cs b/Tests/Opc.Ua.Gds.Tests/PushTest.cs index a4e3ee280..a88e025d2 100644 --- a/Tests/Opc.Ua.Gds.Tests/PushTest.cs +++ b/Tests/Opc.Ua.Gds.Tests/PushTest.cs @@ -1079,6 +1079,48 @@ private async Task VerifyNewPushServerCertAsync(byte[] certificateBlob) await DisconnectPushClientAsync().ConfigureAwait(false); + var certificateUpdate = new ManualResetEvent(false); + var certificateUpdateStarted = new ManualResetEvent(false); + Task OnCertificateUpdateStarted(object sender, EventArgs e) + { + certificateUpdateStarted.Set(); + return Task.CompletedTask; + } + Task OnCertificateUpdated(object sender, EventArgs e) + { + certificateUpdate.Set(); + return Task.CompletedTask; + } + + var validator = m_server.Config.CertificateValidator; + try + { + validator.CertificateUpdateStarted += OnCertificateUpdateStarted; + validator.CertificateUpdate += OnCertificateUpdated; + + if (!certificateUpdateStarted.WaitOne(TimeSpan.FromSeconds(10))) + { + NUnit.Framework.Assert.Fail("Server certificate update did not start."); + } + + if (!certificateUpdate.WaitOne(TimeSpan.FromSeconds(30))) + { + NUnit.Framework.Assert.Fail("Server certificate update did not complete."); + } + } + finally + { + validator.CertificateUpdateStarted -= OnCertificateUpdateStarted; + validator.CertificateUpdate -= OnCertificateUpdated; + certificateUpdate.Dispose(); + certificateUpdateStarted.Dispose(); + } + + if (!m_server.Config.CertificateValidator.CertificateUpdateInProgress.WaitOne(TimeSpan.FromSeconds(30))) + { + NUnit.Framework.Assert.Fail("Server certificate update is still in progress after waiting 30 seconds."); + } + const int maxWaitSeconds = 10; const int retryIntervalMs = 2000; var stopwatch = Stopwatch.StartNew(); @@ -1100,6 +1142,7 @@ private async Task VerifyNewPushServerCertAsync(byte[] certificateBlob) } await DisconnectPushClientAsync().ConfigureAwait(false); + await DisconnectGDSClientAsync().ConfigureAwait(false); await Task.Delay(retryIntervalMs).ConfigureAwait(false); } catch (Exception ex) diff --git a/Tests/Opc.Ua.Security.Certificates.Tests/Pkcs10CertificationRequestTests.cs b/Tests/Opc.Ua.Security.Certificates.Tests/Pkcs10CertificationRequestTests.cs index 8109e9663..843af749c 100644 --- a/Tests/Opc.Ua.Security.Certificates.Tests/Pkcs10CertificationRequestTests.cs +++ b/Tests/Opc.Ua.Security.Certificates.Tests/Pkcs10CertificationRequestTests.cs @@ -293,6 +293,8 @@ public void GetCertificationRequestInfoReturnsValidData() Assert.Greater(requestInfo.Length, 0); } + private static readonly string[] domainNames = new[] { "localhost" }; + /// /// Test parsing multiple CSRs in sequence. /// @@ -310,7 +312,7 @@ public void ParseMultipleCsrsInSequence() using X509Certificate2 certificate = CertificateBuilder.Create(subject) .SetNotBefore(DateTime.UtcNow.AddDays(-1)) .SetLifeTime(TimeSpan.FromDays(30)) - .AddExtension(new X509SubjectAltNameExtension(applicationUri, new[] { "localhost" })) + .AddExtension(new X509SubjectAltNameExtension(applicationUri, domainNames)) .CreateForRSA(); byte[] csrData = CertificateFactory.CreateSigningRequest(certificate); diff --git a/Tests/Opc.Ua.Server.Tests/CreateSessionApplicationUriValidationTests.cs b/Tests/Opc.Ua.Server.Tests/CreateSessionApplicationUriValidationTests.cs index d6a66d014..7fa544f64 100644 --- a/Tests/Opc.Ua.Server.Tests/CreateSessionApplicationUriValidationTests.cs +++ b/Tests/Opc.Ua.Server.Tests/CreateSessionApplicationUriValidationTests.cs @@ -341,6 +341,7 @@ public void CreateSessionWithMultipleUrisNoneMatchThrows(NodeId certificateType) catch (ServiceResultException e) when ((e.StatusCode is StatusCodes.BadServerHalted or StatusCodes.BadSecureChannelClosed or + StatusCodes.BadConnectionClosed or StatusCodes.BadNoCommunication or StatusCodes.BadNotConnected) && attempt < maxAttempts) From a903bd874d2f9e51408ef0323cc6d62803d1d93c Mon Sep 17 00:00:00 2001 From: Roman Ettlinger Date: Fri, 2 Jan 2026 06:14:57 +0100 Subject: [PATCH 2/7] Stop httpsTransportListener on CloseAllChannels --- .../Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index c697a6c19..35958f693 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -615,7 +615,7 @@ private bool ValidateClientCertificate( /// public void CloseAllChannels(string reason) { - // nothing to do + Stop(); } private EndpointDescriptionCollection m_descriptions; From b8f4eeed72ffd74eb747c57f54b3d3c160e88231 Mon Sep 17 00:00:00 2001 From: Roman Ettlinger Date: Sat, 3 Jan 2026 13:00:53 +0100 Subject: [PATCH 3/7] null check cert validator --- Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index eb3156241..e4d4fea37 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -816,8 +816,18 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) { bool isBlocked = false; + // Add null check before accessing CertificateValidator + ICertificateValidator certificateValidator = m_quotas?.CertificateValidator; + if (certificateValidator == null) + { + // Listener is being disposed, don't process this connection + m_logger?.LogDebug("OnAccept: CertificateValidator is null, listener likely disposed."); + e?.Dispose(); + return; + } + // wait for certificate update to complete - m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne(); + certificateValidator.CertificateUpdateInProgress.WaitOne(); // Track potential problematic client behavior only if Basic128Rsa15 security policy is offered if (m_activeClientTracker != null) From 41d8b1945bc0767c2984bc18c5dba88888e92a3b Mon Sep 17 00:00:00 2001 From: Roman Ettlinger Date: Sat, 3 Jan 2026 13:03:44 +0100 Subject: [PATCH 4/7] unsusbcribe event --- Libraries/Opc.Ua.Server/Server/StandardServer.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Libraries/Opc.Ua.Server/Server/StandardServer.cs b/Libraries/Opc.Ua.Server/Server/StandardServer.cs index 0e6726c47..8222fbbfb 100644 --- a/Libraries/Opc.Ua.Server/Server/StandardServer.cs +++ b/Libraries/Opc.Ua.Server/Server/StandardServer.cs @@ -3398,6 +3398,9 @@ protected override async ValueTask OnServerStoppingAsync(CancellationToken cance Utils.SilentDispose(m_serverInternal); m_serverInternal = null; } + + CertificateValidator.CertificateUpdate -= OnCertificateUpdateAsync; + CertificateValidator.CertificateUpdateStarted -= OnCertificateUpdateStartedAsync; } } From 50ced6d23114832846bd676265268b85507a03cc Mon Sep 17 00:00:00 2001 From: romanett Date: Sat, 3 Jan 2026 13:07:12 +0100 Subject: [PATCH 5/7] Update Tests/Opc.Ua.Gds.Tests/PushTest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Tests/Opc.Ua.Gds.Tests/PushTest.cs | 57 +++++++++++++++--------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/Tests/Opc.Ua.Gds.Tests/PushTest.cs b/Tests/Opc.Ua.Gds.Tests/PushTest.cs index a88e025d2..48d68340f 100644 --- a/Tests/Opc.Ua.Gds.Tests/PushTest.cs +++ b/Tests/Opc.Ua.Gds.Tests/PushTest.cs @@ -1079,42 +1079,43 @@ private async Task VerifyNewPushServerCertAsync(byte[] certificateBlob) await DisconnectPushClientAsync().ConfigureAwait(false); - var certificateUpdate = new ManualResetEvent(false); - var certificateUpdateStarted = new ManualResetEvent(false); - Task OnCertificateUpdateStarted(object sender, EventArgs e) + using (var certificateUpdate = new ManualResetEvent(false)) + using (var certificateUpdateStarted = new ManualResetEvent(false)) { - certificateUpdateStarted.Set(); - return Task.CompletedTask; - } - Task OnCertificateUpdated(object sender, EventArgs e) - { - certificateUpdate.Set(); - return Task.CompletedTask; - } - - var validator = m_server.Config.CertificateValidator; - try - { - validator.CertificateUpdateStarted += OnCertificateUpdateStarted; - validator.CertificateUpdate += OnCertificateUpdated; + Task OnCertificateUpdateStarted(object sender, EventArgs e) + { + certificateUpdateStarted.Set(); + return Task.CompletedTask; + } - if (!certificateUpdateStarted.WaitOne(TimeSpan.FromSeconds(10))) + Task OnCertificateUpdated(object sender, EventArgs e) { - NUnit.Framework.Assert.Fail("Server certificate update did not start."); + certificateUpdate.Set(); + return Task.CompletedTask; } - if (!certificateUpdate.WaitOne(TimeSpan.FromSeconds(30))) + var validator = m_server.Config.CertificateValidator; + try + { + validator.CertificateUpdateStarted += OnCertificateUpdateStarted; + validator.CertificateUpdate += OnCertificateUpdated; + + if (!certificateUpdateStarted.WaitOne(TimeSpan.FromSeconds(10))) + { + NUnit.Framework.Assert.Fail("Server certificate update did not start."); + } + + if (!certificateUpdate.WaitOne(TimeSpan.FromSeconds(30))) + { + NUnit.Framework.Assert.Fail("Server certificate update did not complete."); + } + } + finally { - NUnit.Framework.Assert.Fail("Server certificate update did not complete."); + validator.CertificateUpdateStarted -= OnCertificateUpdateStarted; + validator.CertificateUpdate -= OnCertificateUpdated; } } - finally - { - validator.CertificateUpdateStarted -= OnCertificateUpdateStarted; - validator.CertificateUpdate -= OnCertificateUpdated; - certificateUpdate.Dispose(); - certificateUpdateStarted.Dispose(); - } if (!m_server.Config.CertificateValidator.CertificateUpdateInProgress.WaitOne(TimeSpan.FromSeconds(30))) { From 7a1c91e0ca4ba893631fabe77b7401318ca6ccbd Mon Sep 17 00:00:00 2001 From: romanett Date: Sat, 3 Jan 2026 13:07:50 +0100 Subject: [PATCH 6/7] Update Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs index ee43631b8..a5c7efff4 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs @@ -49,7 +49,8 @@ public interface ICertificateValidator event CertificateUpdateEventHandler CertificateUpdateStarted; /// - /// An event that signals that a certificate update is in progress. + /// A wait handle that is signaled when no certificate update is in progress + /// and unsignaled while a certificate update is in progress. /// WaitHandle CertificateUpdateInProgress { get; } From a4861ed5b91760e951d6fb825d59e67b2c6f7886 Mon Sep 17 00:00:00 2001 From: Roman Ettlinger Date: Sun, 4 Jan 2026 12:45:25 +0100 Subject: [PATCH 7/7] null check cert validator --- .../Stack/Https/HttpsTransportListener.cs | 13 ++++++++++--- Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs | 5 ++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs index 35958f693..f200dbb67 100644 --- a/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs +++ b/Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs @@ -354,13 +354,20 @@ public void Stop() /// public async Task SendAsync(HttpContext context) { - // wait for certificate update to complete before processing requests. - m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne(); - string message = string.Empty; CancellationToken ct = context.RequestAborted; try { + ICertificateValidator certificateValidator = m_quotas?.CertificateValidator; + if (certificateValidator == null) + { + // Listener is closed, don't process this connection + throw new InvalidOperationException("CertificateValidator is null, listener is closed."); + } + + // wait for certificate update to complete before processing requests. + certificateValidator.CertificateUpdateInProgress.WaitOne(); + if (m_callback == null) { await WriteResponseAsync( diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index e4d4fea37..32e0fec4f 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -816,12 +816,11 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) { bool isBlocked = false; - // Add null check before accessing CertificateValidator ICertificateValidator certificateValidator = m_quotas?.CertificateValidator; if (certificateValidator == null) { - // Listener is being disposed, don't process this connection - m_logger?.LogDebug("OnAccept: CertificateValidator is null, listener likely disposed."); + // Listener is closed, don't process this connection + m_logger?.LogDebug("OnAccept: CertificateValidator is null, listener is closed."); e?.Dispose(); return; }