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)