Skip to content

Commit 1f47847

Browse files
mrsuciumrsuciu
authored
Added RejectCertificateUriMismatch property and explicit ApplicationUri validation after certificates are loaded (#3244)
* Added RejectCertificateUriMismatch property and explicit ApplicationUri validation after certificates are loaded * Perform duplicate detection on thumbprint only * Ported handle multiple ApplictionUri, removed assign ApplictionUri from certificate * Missing letter * Fixed LoadPrivateKeyAsync when no applicationUri is passed. * Add tests for validating certificates with multiple AplicationUris on ApplicationInstance level and server side * Renamed into CheckOrCreateCertificateAsync, logged Certificate, improved tests * Added retry logic for transient connect errors in CreateSessionApplicationUriValidationTests * Added TODO log when telemetry instance available * Removed unused ValidateServerCertificateApplicationUri(X509Certificate2 serverCertificate)
1 parent 8d22863 commit 1f47847

File tree

20 files changed

+1264
-106
lines changed

20 files changed

+1264
-106
lines changed

‎Libraries/Opc.Ua.Client/ReverseConnectManager.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,21 @@ public Registration(
153153
}
154154

155155
/// <summary>
156-
/// Register with the server certificate.
156+
/// Register with the server certificate to extract the application Uri.
157157
/// </summary>
158+
/// <remarks>
159+
/// The first Uri in the subject alternate name field is considered the application Uri.
160+
/// </remarks>
161+
/// <param name="serverCertificate">The server certificate with the application Uri.</param>
162+
/// <param name="endpointUrl">The endpoint Url of the server.</param>
163+
/// <param name="onConnectionWaiting">The connection to use.</param>
158164
public Registration(
159165
X509Certificate2 serverCertificate,
160166
Uri endpointUrl,
161167
EventHandler<ConnectionWaitingEventArgs> onConnectionWaiting)
162168
: this(endpointUrl, onConnectionWaiting)
163169
{
164-
ServerUri = X509Utils.GetApplicationUriFromCertificate(serverCertificate);
170+
ServerUri = X509Utils.GetApplicationUrisFromCertificate(serverCertificate).FirstOrDefault();
165171
}
166172

167173
private Registration(

‎Libraries/Opc.Ua.Client/Session/Session.cs‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,8 +1707,6 @@ public async Task OpenAsync(
17071707

17081708
if (requireEncryption)
17091709
{
1710-
// validation skipped until IOP isses are resolved.
1711-
// ValidateServerCertificateApplicationUri(serverCertificate);
17121710
if (checkDomain)
17131711
{
17141712
await m_configuration
@@ -1836,6 +1834,8 @@ await m_configuration
18361834

18371835
ValidateServerEndpoints(serverEndpoints);
18381836

1837+
ValidateServerCertificateApplicationUri(serverCertificate, m_endpoint);
1838+
18391839
ValidateServerSignature(
18401840
serverCertificate,
18411841
serverSignature,
@@ -6351,31 +6351,6 @@ private void OpenValidateIdentity(
63516351
}
63526352
}
63536353

6354-
#if UNUSED
6355-
/// <summary>
6356-
/// Validates the ServerCertificate ApplicationUri to match the ApplicationUri of the Endpoint
6357-
/// for an open call (Spec Part 4 5.4.1)
6358-
/// </summary>
6359-
private void ValidateServerCertificateApplicationUri(X509Certificate2 serverCertificate)
6360-
{
6361-
string applicationUri = m_endpoint?.Description?.Server?.ApplicationUri;
6362-
//check is only neccessary if the ApplicatioUri is specified for the Endpoint
6363-
if (string.IsNullOrEmpty(applicationUri))
6364-
{
6365-
throw ServiceResultException.Create(
6366-
StatusCodes.BadSecurityChecksFailed,
6367-
"No ApplicationUri is specified for the server in the EndpointDescription.");
6368-
}
6369-
string certificateApplicationUri = X509Utils.GetApplicationUriFromCertificate(serverCertificate);
6370-
if (!string.Equals(certificateApplicationUri, applicationUri, StringComparison.Ordinal))
6371-
{
6372-
throw ServiceResultException.Create(
6373-
StatusCodes.BadSecurityChecksFailed,
6374-
"Server did not return a Certificate matching the ApplicationUri specified in the EndpointDescription.");
6375-
}
6376-
}
6377-
#endif
6378-
63796354
private void BuildCertificateData(
63806355
out byte[] clientCertificateData,
63816356
out byte[] clientCertificateChainData)
@@ -6488,6 +6463,20 @@ private void ValidateServerSignature(
64886463
}
64896464
}
64906465

6466+
/// <summary>
6467+
/// Validates the ServerCertificate ApplicationUri to match the ApplicationUri
6468+
/// of the Endpoint (Spec Part 4 5.4.1) returned by the CreateSessionResponse.
6469+
/// Ensure the endpoint was matched in <see cref="ValidateServerEndpoints"/>
6470+
/// with the applicationUri of the server description before the validation.
6471+
/// </summary>
6472+
private void ValidateServerCertificateApplicationUri(X509Certificate2 serverCertificate, ConfiguredEndpoint endpoint)
6473+
{
6474+
if (serverCertificate != null)
6475+
{
6476+
m_configuration.CertificateValidator.ValidateApplicationUri(serverCertificate, endpoint);
6477+
}
6478+
}
6479+
64916480
/// <summary>
64926481
/// Validates the server endpoints returned.
64936482
/// </summary>

‎Libraries/Opc.Ua.Configuration/ApplicationInstance.cs‎

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,16 @@ public async ValueTask<bool> CheckApplicationInstanceCertificatesAsync(
358358
"Need at least one Application Certificate.");
359359
}
360360

361+
// Note: The FindAsync method searches certificates in this order: thumbprint, subjectName, then applicationUri.
362+
// When SubjectName or Thumbprint is specified, certificates may be loaded even if their ApplicationUri
363+
// doesn't match ApplicationConfiguration.ApplicationUri, however each certificate is validated individually
364+
// in CheckApplicationInstanceCertificateAsync (called via CheckOrCreateCertificateAsync) to ensure it contains
365+
// the configuration's ApplicationUri.
361366
bool result = true;
362367
foreach (CertificateIdentifier certId in securityConfiguration.ApplicationCertificates)
363368
{
364369
ushort minimumKeySize = certId.GetMinKeySize(securityConfiguration);
365-
bool nextResult = await CheckCertificateTypeAsync(
370+
bool nextResult = await CheckOrCreateCertificateAsync(
366371
certId,
367372
silent,
368373
minimumKeySize,
@@ -376,10 +381,14 @@ public async ValueTask<bool> CheckApplicationInstanceCertificatesAsync(
376381
}
377382

378383
/// <summary>
379-
/// Check certificate type
384+
/// Checks, validates, and optionally creates an application certificate.
385+
/// Loads the certificate, validates it against configured requirements (ApplicationUri, key size, domains),
386+
/// and creates a new certificate if none exists and auto-creation is enabled.
387+
/// Note: FindAsync searches certificates in order: thumbprint, subjectName, applicationUri.
388+
/// The applicationUri parameter is only used if thumbprint and subjectName don't find a match.
380389
/// </summary>
381390
/// <exception cref="ServiceResultException"></exception>
382-
private async Task<bool> CheckCertificateTypeAsync(
391+
private async Task<bool> CheckOrCreateCertificateAsync(
383392
CertificateIdentifier id,
384393
bool silent,
385394
ushort minimumKeySize,
@@ -402,7 +411,7 @@ private async Task<bool> CheckCertificateTypeAsync(
402411
await id.LoadPrivateKeyExAsync(passwordProvider, configuration.ApplicationUri, m_telemetry, ct)
403412
.ConfigureAwait(false);
404413

405-
// load the certificate
414+
// load the certificate
406415
X509Certificate2 certificate = await id.FindAsync(
407416
true,
408417
configuration.ApplicationUri,
@@ -738,28 +747,37 @@ await configuration
738747
return false;
739748
}
740749

741-
// check uri.
742-
string applicationUri = X509Utils.GetApplicationUriFromCertificate(certificate);
743-
744-
if (string.IsNullOrEmpty(applicationUri))
750+
// Validate that the certificate contains the configuration's ApplicationUri
751+
if (!X509Utils.CompareApplicationUriWithCertificate(certificate, configuration.ApplicationUri, out var certificateUris))
745752
{
746-
const string message =
747-
"The Application URI could not be read from the certificate. Use certificate anyway?";
748-
if (!await ApproveMessageAsync(message, silent).ConfigureAwait(false))
753+
if (certificateUris.Count == 0)
749754
{
750-
return false;
755+
const string message =
756+
"The Application URI could not be found in the certificate. Use certificate anyway?";
757+
if (!await ApproveMessageAsync(message, silent).ConfigureAwait(false))
758+
{
759+
return false;
760+
}
761+
}
762+
else
763+
{
764+
string message = Utils.Format(
765+
"The certificate with subject '{0}' does not contain the ApplicationUri '{1}' from the configuration. Certificate contains: {2}. Use certificate anyway?",
766+
certificate.Subject,
767+
configuration.ApplicationUri,
768+
string.Join(", ", certificateUris));
769+
770+
if (!await ApproveMessageAsync(message, silent).ConfigureAwait(false))
771+
{
772+
return false;
773+
}
751774
}
752-
}
753-
else if (!configuration.ApplicationUri.Equals(applicationUri, StringComparison.Ordinal))
754-
{
755-
m_logger.LogInformation(
756-
"Updated the ApplicationUri: {PreviousApplicationUri} --> {NewApplicationUri}",
757-
configuration.ApplicationUri,
758-
applicationUri);
759-
configuration.ApplicationUri = applicationUri;
760775
}
761776

762-
m_logger.LogInformation("Using the ApplicationUri: {ApplicationUri}", applicationUri);
777+
m_logger.LogInformation(
778+
"Certificate {Certificate} validated for ApplicationUri: {ApplicationUri}",
779+
certificate.AsLogSafeString(),
780+
configuration.ApplicationUri);
763781

764782
// update configuration.
765783
id.Certificate = certificate;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
using System;
3131
using System.Collections.Generic;
32+
using System.Linq;
3233
using System.Runtime.Serialization;
3334
using System.Security.Cryptography.X509Certificates;
3435

@@ -202,7 +203,7 @@ public string ApplicationUri
202203
{
203204
try
204205
{
205-
return X509Utils.GetApplicationUriFromCertificate(Certificate);
206+
return X509Utils.GetApplicationUrisFromCertificate(Certificate).FirstOrDefault();
206207
}
207208
catch (Exception e)
208209
{

‎Libraries/Opc.Ua.Security.Certificates/Extensions/X509SubjectAltNameExtension.cs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,23 @@ public X509SubjectAltNameExtension(string applicationUri, IEnumerable<string> do
116116
{
117117
Oid = new Oid(SubjectAltName2Oid, kFriendlyName);
118118
Critical = false;
119-
Initialize(applicationUri, domainNames);
119+
Initialize(new string[] { applicationUri }, domainNames);
120+
RawData = Encode();
121+
m_decoded = true;
122+
}
123+
124+
/// <summary>
125+
/// Build the Subject Alternative name extension (for OPC UA application certs).
126+
/// </summary>
127+
/// <param name="applicationUris">The application Uri</param>
128+
/// <param name="domainNames">The domain names. DNS Hostnames, IPv4 or IPv6 addresses</param>
129+
public X509SubjectAltNameExtension(
130+
IEnumerable<string> applicationUris,
131+
IEnumerable<string> domainNames)
132+
{
133+
Oid = new Oid(SubjectAltName2Oid, kFriendlyName);
134+
Critical = false;
135+
Initialize(applicationUris, domainNames);
120136
RawData = Encode();
121137
m_decoded = true;
122138
}
@@ -408,14 +424,17 @@ private void Decode(byte[] data)
408424
/// <summary>
409425
/// Initialize the Subject Alternative name extension.
410426
/// </summary>
411-
/// <param name="applicationUri">The application Uri</param>
427+
/// <param name="applicationUris">The application Uris</param>
412428
/// <param name="generalNames">The general names. DNS Hostnames, IPv4 or IPv6 addresses</param>
413-
private void Initialize(string applicationUri, IEnumerable<string> generalNames)
429+
private void Initialize(IEnumerable<string> applicationUris, IEnumerable<string> generalNames)
414430
{
415431
var uris = new List<string>();
416432
var domainNames = new List<string>();
417433
var ipAddresses = new List<string>();
418-
uris.Add(applicationUri);
434+
foreach (string applicationUri in applicationUris)
435+
{
436+
uris.Add(applicationUri);
437+
}
419438
foreach (string generalName in generalNames)
420439
{
421440
switch (Uri.CheckHostName(generalName))

‎Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,8 +529,8 @@ private async ValueTask<UpdateCertificateMethodStateResult> UpdateCertificateAsy
529529
cert.CertificateType == certificateTypeId)
530530
?? certificateGroup.ApplicationCertificates.FirstOrDefault(cert =>
531531
cert.Certificate != null &&
532-
m_configuration.ApplicationUri ==
533-
X509Utils.GetApplicationUriFromCertificate(cert.Certificate) &&
532+
X509Utils.GetApplicationUrisFromCertificate(cert.Certificate)
533+
.Any(uri => uri.Equals(m_configuration.ApplicationUri, StringComparison.Ordinal)) &&
534534
cert.CertificateType == certificateTypeId))
535535
?? throw new ServiceResultException(
536536
StatusCodes.BadInvalidArgument,

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

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -380,31 +380,27 @@ X509Certificate2Collection clientCertificateChain
380380

381381
if (context.SecurityPolicyUri != SecurityPolicies.None)
382382
{
383-
string certificateApplicationUri = X509Utils
384-
.GetApplicationUriFromCertificate(
385-
parsedClientCertificate);
386-
387-
// verify if applicationUri from ApplicationDescription matches the applicationUri in the client certificate.
388-
if (!string.IsNullOrEmpty(certificateApplicationUri) &&
389-
!string.IsNullOrEmpty(clientDescription.ApplicationUri) &&
390-
certificateApplicationUri != clientDescription.ApplicationUri)
383+
// verify if applicationUri from ApplicationDescription matches the applicationUris in the client certificate.
384+
if (!string.IsNullOrEmpty(clientDescription.ApplicationUri))
391385
{
392-
// report the AuditCertificateDataMismatch event for invalid uri
393-
ServerInternal?.ReportAuditCertificateDataMismatchEvent(
394-
parsedClientCertificate,
395-
null,
396-
clientDescription.ApplicationUri,
397-
StatusCodes.BadCertificateUriInvalid,
398-
m_logger);
386+
if (!X509Utils.CompareApplicationUriWithCertificate(parsedClientCertificate, clientDescription.ApplicationUri))
387+
{
388+
// report the AuditCertificateDataMismatch event for invalid uri
389+
ServerInternal?.ReportAuditCertificateDataMismatchEvent(
390+
parsedClientCertificate,
391+
null,
392+
clientDescription.ApplicationUri,
393+
StatusCodes.BadCertificateUriInvalid,
394+
m_logger);
395+
396+
throw ServiceResultException.Create(
397+
StatusCodes.BadCertificateUriInvalid,
398+
"The URI specified in the ApplicationDescription {0} does not match the URIs in the Certificate.",
399+
clientDescription.ApplicationUri);
400+
}
399401

400-
throw ServiceResultException.Create(
401-
StatusCodes.BadCertificateUriInvalid,
402-
"The URI specified in the ApplicationDescription {0} does not match the URI in the Certificate: {1}.",
403-
clientDescription.ApplicationUri,
404-
certificateApplicationUri);
402+
CertificateValidator.Validate(clientCertificateChain);
405403
}
406-
407-
CertificateValidator.Validate(clientCertificateChain);
408404
}
409405
}
410406
catch (Exception e)

‎Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -722,33 +722,58 @@ public CertificateIdentifierCollection ApplicationCertificates
722722
get => m_applicationCertificates;
723723
set
724724
{
725-
m_applicationCertificates = value ?? [];
725+
if (value == null || value.Count == 0)
726+
{
727+
m_applicationCertificates = new CertificateIdentifierCollection();
728+
return;
729+
}
726730

727-
IsDeprecatedConfiguration = false;
731+
var newCertificates = new CertificateIdentifierCollection(value);
728732

729-
// Remove any unsupported certificate types.
730-
for (int i = m_applicationCertificates.Count - 1; i >= 0; i--)
733+
// Remove unsupported certificate types
734+
for (int i = newCertificates.Count - 1; i >= 0; i--)
731735
{
732-
if (!Utils.IsSupportedCertificateType(
733-
m_applicationCertificates[i].CertificateType))
736+
if (!Utils.IsSupportedCertificateType(newCertificates[i].CertificateType))
734737
{
735-
m_applicationCertificates.RemoveAt(i);
738+
// TODO: Log when ITelemetry instance is available
739+
newCertificates.RemoveAt(i);
736740
}
737741
}
738742

739-
// Remove any duplicates
740-
for (int i = 0; i < m_applicationCertificates.Count; i++)
743+
// Remove any duplicates based on thumbprint
744+
// Only perform duplicate detection if we have actual loaded certificates
745+
for (int i = 0; i < newCertificates.Count; i++)
741746
{
742-
for (int j = m_applicationCertificates.Count - 1; j > i; j--)
747+
for (int j = newCertificates.Count - 1; j > i; j--)
743748
{
744-
if (m_applicationCertificates[i]
745-
.CertificateType == m_applicationCertificates[j].CertificateType)
749+
bool isDuplicate = false;
750+
751+
// Only check for duplicates if both certificates are actually loaded
752+
if (newCertificates[i].Certificate != null && newCertificates[j].Certificate != null)
753+
{
754+
// Compare by actual certificate thumbprint
755+
isDuplicate = newCertificates[i].Certificate.Thumbprint.Equals(
756+
newCertificates[j].Certificate.Thumbprint,
757+
StringComparison.OrdinalIgnoreCase);
758+
}
759+
// If certificates aren't loaded yet, compare by explicit thumbprint configuration
760+
else if (!string.IsNullOrEmpty(newCertificates[i].Thumbprint) &&
761+
!string.IsNullOrEmpty(newCertificates[j].Thumbprint))
762+
{
763+
isDuplicate = newCertificates[i].Thumbprint.Equals(
764+
newCertificates[j].Thumbprint,
765+
StringComparison.OrdinalIgnoreCase);
766+
}
767+
768+
if (isDuplicate)
746769
{
747-
m_applicationCertificates.RemoveAt(j);
770+
newCertificates.RemoveAt(j);
748771
}
749772
}
750773
}
751774

775+
m_applicationCertificates = newCertificates;
776+
752777
SupportedSecurityPolicies = BuildSupportedSecurityPolicies();
753778
}
754779
}

0 commit comments

Comments
 (0)