Skip to content

Commit cea6283

Browse files
romanettromanett
committed
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.
1 parent e731b32 commit cea6283

File tree

15 files changed

+245
-42
lines changed

15 files changed

+245
-42
lines changed

‎Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ await CoreClientUtils.SelectEndpointAsync(
376376
}
377377
catch (ServiceResultException e) when ((e.StatusCode is
378378
StatusCodes.BadServerHalted or
379+
StatusCodes.BadConnectionClosed or
379380
StatusCodes.BadSecureChannelClosed or
380381
StatusCodes.BadNoCommunication) &&
381382
attempt < maxAttempts)
@@ -432,6 +433,7 @@ public async Task ConnectAsync(ConfiguredEndpoint endpoint, CancellationToken ct
432433
catch (ServiceResultException e) when ((e.StatusCode is
433434
StatusCodes.BadServerHalted or
434435
StatusCodes.BadSecureChannelClosed or
436+
StatusCodes.BadConnectionClosed or
435437
StatusCodes.BadNoCommunication) &&
436438
attempt < maxAttempts)
437439
{

‎Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ await CoreClientUtils.SelectEndpointAsync(
290290
catch (ServiceResultException e) when ((e.StatusCode is
291291
StatusCodes.BadServerHalted or
292292
StatusCodes.BadSecureChannelClosed or
293+
StatusCodes.BadConnectionClosed or
293294
StatusCodes.BadNoCommunication) &&
294295
attempt < maxAttempts)
295296
{
@@ -345,6 +346,7 @@ public async Task ConnectAsync(ConfiguredEndpoint endpoint, CancellationToken ct
345346
catch (ServiceResultException e) when ((e.StatusCode is
346347
StatusCodes.BadServerHalted or
347348
StatusCodes.BadSecureChannelClosed or
349+
StatusCodes.BadConnectionClosed or
348350
StatusCodes.BadNoCommunication) &&
349351
attempt < maxAttempts)
350352
{

‎Libraries/Opc.Ua.Server/Server/StandardServer.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3340,6 +3340,7 @@ await subscriptionManager.StartupAsync(cancellationToken)
33403340
}
33413341

33423342
CertificateValidator.CertificateUpdate += OnCertificateUpdateAsync;
3343+
CertificateValidator.CertificateUpdateStarted += OnCertificateUpdateStartedAsync;
33433344
}
33443345

33453346
/// <summary>

‎Stack/Opc.Ua.Bindings.Https/Stack/Https/HttpsTransportListener.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,9 @@ public void Stop()
354354
/// </summary>
355355
public async Task SendAsync(HttpContext context)
356356
{
357+
// wait for certificate update to complete before processing requests.
358+
m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne();
359+
357360
string message = string.Empty;
358361
CancellationToken ct = context.RequestAborted;
359362
try
@@ -608,6 +611,13 @@ private bool ValidateClientCertificate(
608611
return true;
609612
}
610613

614+
615+
/// <inheritdoc/>
616+
public void CloseAllChannels(string reason)
617+
{
618+
// nothing to do
619+
}
620+
611621
private EndpointDescriptionCollection m_descriptions;
612622
private ChannelQuotas m_quotas;
613623
private ITransportListenerCallback m_callback;

‎Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs‎

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,15 @@ public event CertificateUpdateEventHandler CertificateUpdate
9797
remove => m_CertificateUpdate -= value;
9898
}
9999

100+
/// <summary>
101+
/// Raised before an application certificate update occurs.
102+
/// </summary>
103+
public event CertificateUpdateEventHandler CertificateUpdateStarted
104+
{
105+
add => m_CertificateUpdateStarted += value;
106+
remove => m_CertificateUpdateStarted -= value;
107+
}
108+
100109
/// <summary>
101110
/// Updates the validator with the current state of the configuration.
102111
/// </summary>
@@ -284,51 +293,69 @@ public virtual async Task UpdateCertificateAsync(
284293
string applicationUri = null,
285294
CancellationToken ct = default)
286295
{
287-
await m_semaphore.WaitAsync(ct).ConfigureAwait(false);
296+
m_updateEvent.Reset();
288297

289298
try
290299
{
291-
m_applicationCertificates.Clear();
292-
//
293-
// crash occurs if the cert is in use still and this has not run yet.
294-
// This might be the intended design but this runs on a free task that
295-
// might not be scheduled right away.
296-
//
297-
// TODO: We need a better way to disconnect all sessions when the cert is
298-
// updated. (See caller of this method)
299-
//
300-
// foreach (CertificateIdentifier applicationCertificate in securityConfiguration
301-
// .ApplicationCertificates)
302-
// {
303-
// applicationCertificate.DisposeCertificate();
304-
// }
305-
306-
foreach (CertificateIdentifier applicationCertificate in securityConfiguration
307-
.ApplicationCertificates)
308-
{
309-
await applicationCertificate
310-
.LoadPrivateKeyExAsync(
311-
securityConfiguration.CertificatePasswordProvider,
312-
applicationUri,
313-
m_telemetry,
314-
ct)
315-
.ConfigureAwait(false);
300+
CertificateUpdateEventHandler started_callback = m_CertificateUpdateStarted;
301+
if (started_callback != null)
302+
{
303+
var args = new CertificateUpdateEventArgs(
304+
securityConfiguration,
305+
GetChannelValidator());
306+
await started_callback(this, args).ConfigureAwait(false);
307+
}
308+
309+
await m_semaphore.WaitAsync(ct).ConfigureAwait(false);
310+
311+
try
312+
{
313+
m_applicationCertificates.Clear();
314+
//
315+
// crash occurs if the cert is in use still and this has not run yet.
316+
// This might be the intended design but this runs on a free task that
317+
// might not be scheduled right away.
318+
//
319+
// TODO: We need a better way to disconnect all sessions when the cert is
320+
// updated. (See caller of this method)
321+
//
322+
// foreach (CertificateIdentifier applicationCertificate in securityConfiguration
323+
// .ApplicationCertificates)
324+
// {
325+
// applicationCertificate.DisposeCertificate();
326+
// }
327+
328+
foreach (CertificateIdentifier applicationCertificate in securityConfiguration
329+
.ApplicationCertificates)
330+
{
331+
await applicationCertificate
332+
.LoadPrivateKeyExAsync(
333+
securityConfiguration.CertificatePasswordProvider,
334+
applicationUri,
335+
m_telemetry,
336+
ct)
337+
.ConfigureAwait(false);
338+
}
339+
}
340+
finally
341+
{
342+
m_semaphore.Release();
316343
}
317-
}
318-
finally
319-
{
320-
m_semaphore.Release();
321-
}
322344

323-
await UpdateAsync(securityConfiguration, applicationUri, ct).ConfigureAwait(false);
345+
await UpdateAsync(securityConfiguration, applicationUri, ct).ConfigureAwait(false);
324346

325-
CertificateUpdateEventHandler callback = m_CertificateUpdate;
326-
if (callback != null)
347+
CertificateUpdateEventHandler callback = m_CertificateUpdate;
348+
if (callback != null)
349+
{
350+
var args = new CertificateUpdateEventArgs(
351+
securityConfiguration,
352+
GetChannelValidator());
353+
await callback(this, args).ConfigureAwait(false);
354+
}
355+
}
356+
finally
327357
{
328-
var args = new CertificateUpdateEventArgs(
329-
securityConfiguration,
330-
GetChannelValidator());
331-
callback(this, args);
358+
m_updateEvent.Set();
332359
}
333360
}
334361

@@ -2234,6 +2261,9 @@ public static bool IsECSecureForProfile(
22342261
throw new NotSupportedException("Unsupported curve type.");
22352262
}
22362263

2264+
/// <inheritdoc/>
2265+
public WaitHandle CertificateUpdateInProgress => m_updateEvent.WaitHandle;
2266+
22372267
/// <summary>
22382268
/// Flag to protect setting by application
22392269
/// from a modification by a SecurityConfiguration.
@@ -2254,13 +2284,15 @@ private enum ProtectFlags
22542284
private readonly ILogger m_logger;
22552285
private readonly ITelemetryContext m_telemetry;
22562286
private readonly Dictionary<string, X509Certificate2> m_validatedCertificates;
2287+
private readonly ManualResetEventSlim m_updateEvent = new(true);
22572288
private CertificateStoreIdentifier m_trustedCertificateStore;
22582289
private CertificateIdentifierCollection m_trustedCertificateList;
22592290
private CertificateStoreIdentifier m_issuerCertificateStore;
22602291
private CertificateIdentifierCollection m_issuerCertificateList;
22612292
private CertificateStoreIdentifier m_rejectedCertificateStore;
22622293
private event CertificateValidationEventHandler m_CertificateValidation;
22632294
private event CertificateUpdateEventHandler m_CertificateUpdate;
2295+
private event CertificateUpdateEventHandler m_CertificateUpdateStarted;
22642296
private readonly List<X509Certificate2> m_applicationCertificates;
22652297
private ProtectFlags m_protectFlags;
22662298
private bool m_autoAcceptUntrustedCertificates;
@@ -2350,7 +2382,7 @@ public CertificateUpdateEventArgs(
23502382
/// <summary>
23512383
/// Used to handle certificate update events.
23522384
/// </summary>
2353-
public delegate void CertificateUpdateEventHandler(
2385+
public delegate Task CertificateUpdateEventHandler(
23542386
CertificateValidator sender,
23552387
CertificateUpdateEventArgs e);
23562388
}

‎Stack/Opc.Ua.Core/Security/Certificates/ICertificateValidator.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,29 @@ namespace Opc.Ua
3838
/// </summary>
3939
public interface ICertificateValidator
4040
{
41+
/// <summary>
42+
/// Raised when an application certificate update occurs.
43+
/// </summary>
44+
event CertificateUpdateEventHandler CertificateUpdate;
45+
46+
/// <summary>
47+
/// Raised before an application certificate update occurs.
48+
/// </summary>
49+
event CertificateUpdateEventHandler CertificateUpdateStarted;
50+
51+
/// <summary>
52+
/// An event that signals that a certificate update is in progress.
53+
/// </summary>
54+
WaitHandle CertificateUpdateInProgress { get; }
55+
56+
/// <summary>
57+
/// Updates the validator with a new application certificate.
58+
/// </summary>
59+
Task UpdateCertificateAsync(
60+
SecurityConfiguration securityConfiguration,
61+
string applicationUri = null,
62+
CancellationToken ct = default);
63+
4164
/// <summary>
4265
/// Validates a certificate.
4366
/// </summary>

‎Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ protected virtual EndpointBase GetEndpointInstance(ServerBase server)
771771
/// <summary>
772772
/// Called after the application certificate update.
773773
/// </summary>
774-
protected virtual async void OnCertificateUpdateAsync(object sender, CertificateUpdateEventArgs e)
774+
protected virtual async Task OnCertificateUpdateAsync(object sender, CertificateUpdateEventArgs e)
775775
{
776776
try
777777
{
@@ -810,6 +810,25 @@ await InstanceCertificateTypesProvider.LoadCertificateChainAsync(certificate)
810810
}
811811
}
812812

813+
/// <summary>
814+
/// Called before the application certificate update.
815+
/// </summary>
816+
protected virtual Task OnCertificateUpdateStartedAsync(object sender, CertificateUpdateEventArgs e)
817+
{
818+
try
819+
{
820+
foreach (ITransportListener listener in TransportListeners)
821+
{
822+
listener.CloseAllChannels("Update of ApplicationCertificate");
823+
}
824+
}
825+
catch (Exception ex)
826+
{
827+
m_logger.LogError(ex, "Failed to close all channels on certificate update: {EventArgs}", e);
828+
}
829+
return Task.CompletedTask;
830+
}
831+
813832
/// <summary>
814833
/// Create the transport listener for the service host endpoint.
815834
/// </summary>

‎Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,28 @@ public void IdleCleanup()
189189
}
190190
}
191191

192+
/// <summary>
193+
/// Force the channel to close immediately, e.g. due to certificate update.
194+
/// </summary>
195+
public void ForceClose(string reason)
196+
{
197+
TcpChannelState state;
198+
199+
lock (DataLock)
200+
{
201+
state = State;
202+
if (state is TcpChannelState.Open or TcpChannelState.Connecting)
203+
{
204+
state = State = TcpChannelState.Closing;
205+
}
206+
}
207+
208+
if (state is TcpChannelState.Closing or TcpChannelState.Opening or TcpChannelState.Faulted)
209+
{
210+
OnForceClosed(reason);
211+
}
212+
}
213+
192214
/// <summary>
193215
/// The time in milliseconds elapsed since the channel received or sent messages
194216
/// or received a keep alive.
@@ -349,6 +371,32 @@ private void OnCleanup(object state)
349371
}
350372
}
351373

374+
/// <summary>
375+
/// Called when the channel is force closed.
376+
/// </summary>
377+
private void OnForceClosed(string reason)
378+
{
379+
lock (DataLock)
380+
{
381+
// nothing to do if the channel is now open or closed.
382+
if (State is TcpChannelState.Closed or TcpChannelState.Open)
383+
{
384+
return;
385+
}
386+
387+
m_logger.LogInformation(
388+
"{Channel} Force Close Socket={SocketHandle:X8}, ChannelId={ChannelId}, TokenId={TokenId}, Reason={Reason}",
389+
ChannelName,
390+
(Socket?.Handle) ?? 0,
391+
CurrentToken != null ? CurrentToken.ChannelId : 0,
392+
CurrentToken != null ? CurrentToken.TokenId : 0,
393+
reason);
394+
395+
// close channel.
396+
ChannelClosed();
397+
}
398+
}
399+
352400
/// <summary>
353401
/// Closes the channel and releases resources.
354402
/// Sets state to Closed and notifies monitors.

‎Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,15 @@ public void Close()
401401
Stop();
402402
}
403403

404+
/// <inheritdoc/>
405+
public void CloseAllChannels(string reason)
406+
{
407+
foreach (TcpListenerChannel channel in m_channels.Values.ToList())
408+
{
409+
channel.ForceClose(reason);
410+
}
411+
}
412+
404413
/// <inheritdoc/>
405414
public void UpdateChannelLastActiveTime(string globalChannelId)
406415
{
@@ -807,6 +816,9 @@ private void OnAccept(object sender, SocketAsyncEventArgs e)
807816
{
808817
bool isBlocked = false;
809818

819+
// wait for certificate update to complete
820+
m_quotas.CertificateValidator.CertificateUpdateInProgress.WaitOne();
821+
810822
// Track potential problematic client behavior only if Basic128Rsa15 security policy is offered
811823
if (m_activeClientTracker != null)
812824
{

‎Stack/Opc.Ua.Core/Stack/Transport/ITransportListener.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ void Open(
7474
/// <exception cref="ServiceResultException">Thrown if any communication error occurs.</exception>
7575
void Close();
7676

77+
/// <summary>
78+
/// Closes all connections of the listener.
79+
/// </summary>
80+
void CloseAllChannels(string reason);
81+
7782
/// <summary>
7883
/// Updates the application certificate for a listener.
7984
/// </summary>

0 commit comments

Comments
 (0)