Skip to content

Added RejectCertificateUriMismatch property and explicit ApplicationUri validation after certificates are loaded #3244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+452 −17
Draft

Added RejectCertificateUriMismatch property and explicit ApplicationUri validation after certificates are loaded #3244

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
292bc1a
Added RejectCertificateUriMismatch property and explicit ApplicationU…
mrsuciu Oct 8, 2025
3ca4c07
Perform duplicate detection on thumbprint only
mrsuciu Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,15 @@ public IApplicationConfigurationBuilderSecurityOptions SetSuppressNonceValidatio
return this;
}

/// <inheritdoc/>
public IApplicationConfigurationBuilderSecurityOptions SetRejectCertificateUriMismatch(
bool rejectCertificateUriMismatch)
{
ApplicationConfiguration.SecurityConfiguration.RejectCertificateUriMismatch =
rejectCertificateUriMismatch;
return this;
}

/// <inheritdoc/>
public IApplicationConfigurationBuilderSecurityOptions SetSendCertificateChain(
bool sendCertificateChain)
Expand Down
53 changes: 51 additions & 2 deletions Libraries/Opc.Ua.Configuration/ApplicationInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ public async ValueTask<bool> CheckApplicationInstanceCertificatesAsync(
"Need at least one Application Certificate.");
}

// Note: The FindAsync method searches certificates in this order: thumbprint, subjectName, then applicationUri.
// When SubjectName or Thumbprint is specified, certificates may be loaded even if their ApplicationUri
// doesn't match ApplicationConfiguration.ApplicationUri. Therefore, we must explicitly validate that
// all loaded certificates have the same ApplicationUri.
bool result = true;
foreach (CertificateIdentifier certId in securityConfiguration.ApplicationCertificates)
{
Expand All @@ -372,11 +376,47 @@ public async ValueTask<bool> CheckApplicationInstanceCertificatesAsync(
result = result && nextResult;
}

// When there are multiple certificates, validate they all have the same ApplicationUri.
// Note: Individual certificate URI validation against the configuration happens in
// CheckApplicationInstanceCertificateAsync (called via CheckCertificateTypeAsync above).
// This additional check ensures consistency across multiple certificates.
if (securityConfiguration.ApplicationCertificates.Count > 1)
{
string firstApplicationUri = null;
foreach (CertificateIdentifier certId in securityConfiguration.ApplicationCertificates)
{
if (certId.Certificate != null)
{
string certApplicationUri = X509Utils.GetApplicationUriFromCertificate(certId.Certificate);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IfI remember correctly a cert can have multiple application uri, but we always checked / extracted only the first (long known simplification). This needs to be fixed before introducing this check. Best to fix it in all places. i think in Martins repo is a Prototype die this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec mentions one ApplicationURI and multiple dNSNames if the machine has multiple names.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Shows Part of the Implementation we maybe need: mregen#351

I will check next week

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean you are referring to the following method:

public static IReadOnlyList GetApplicationUrisFromCertificate(X509Certificate2 certificate)

    {

        // extract the alternate domains from the subject alternate name extension.
        X509SubjectAltNameExtension alternateName = X509Extensions.FindExtension<X509SubjectAltNameExtension>(certificate);
        // get the application uris.
        if (alternateName != null && alternateName.Uris != null)
        {
            return alternateName.Uris;
        }

        return new List<string>();
    }

`

But here there is a bit of a confusion in my opinion since while X509SubjectAltNameExtension does indeed provide the Uris property, it also provides the DomainNames property.
What adds to the fuzzy interpretation is that the comment explicitly says

" // extract the alternate domains from the subject alternate name extension."

and returns the Uris property.

In my view (which might be wrong) if I follow the spec there is a single ApplicationURI which is
a globaly unique indentifier for an OPC UA Application and the application instance certificate has the ApplicationURI in the subjectAltNameField
The spec sentence "Additional dNSNames may be specified if the machine has multiple names." points to the DomainNames property of the X509SubjectAltNameExtension.
As a simple example of my interpretation, if the application has an ApplicationUri urn:MyCompany:ServerInstanceA but can be reached via servera.local and 192.168.1.12, the app certificate contains the single ApplicationURI but lists both network addresses and clients can connect successfully using either name or address as long as the "connection path" is found in the certificate.
I have to admit my interpretation is "simple" and based on spec only and cannot find any references to multiple ApplicationUri's in the SAN from the spec point of view, since the RFC 5280 allows multiple SAN URI entries but it is more generic than the spec. The spec does mention that the subjectAltName field is completly described in RFC 5280, so this gives leave to interpretation further more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot assume the first URI is the ApplicationURI. I would propose to remove this validation step and only validate against config


if (!string.IsNullOrEmpty(certApplicationUri))
{
if (firstApplicationUri == null)
{
firstApplicationUri = certApplicationUri;
}
else if (!firstApplicationUri.Equals(certApplicationUri, StringComparison.Ordinal))
{
// Multiple certificates with different URIs - always reject
throw ServiceResultException.Create(
StatusCodes.BadCertificateUriInvalid,
"All application certificates must have the same ApplicationUri. Certificate with subject '{0}' has URI '{1}' but expected '{2}'.",
certId.Certificate.Subject,
certApplicationUri,
firstApplicationUri);
}
}
}
}
}

return result;
}

/// <summary>
/// Check certificate type
/// Check certificate type.
/// Note: FindAsync searches certificates in order: thumbprint, subjectName, applicationUri.
/// The applicationUri parameter is only used if thumbprint and subjectName don't find a match.
/// </summary>
/// <exception cref="ServiceResultException"></exception>
private async Task<bool> CheckCertificateTypeAsync(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to rename to CheckCertificateAsync as more thant the type is checked here

Expand All @@ -402,7 +442,7 @@ private async Task<bool> CheckCertificateTypeAsync(
await id.LoadPrivateKeyExAsync(passwordProvider, configuration.ApplicationUri, m_telemetry, ct)
.ConfigureAwait(false);

// load the certificate
// load the certificate
X509Certificate2 certificate = await id.FindAsync(
true,
configuration.ApplicationUri,
Expand Down Expand Up @@ -752,6 +792,15 @@ await configuration
}
else if (!configuration.ApplicationUri.Equals(applicationUri, StringComparison.Ordinal))
{
if (configuration.SecurityConfiguration.RejectCertificateUriMismatch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to make this check mandatory, as with ECC we can have multiple Certificates with different ApplicationUris, we need to be able to verifiy against the config reliably.

{
throw ServiceResultException.Create(
StatusCodes.BadCertificateUriInvalid,
"The URI specified in the ApplicationConfiguration {0} does not match the URI in the Certificate: {1}.",
configuration.ApplicationUri,
applicationUri);
}

m_logger.LogInformation(
"Updated the ApplicationUri: {PreviousApplicationUri} --> {NewApplicationUri}",
configuration.ApplicationUri,
Expand Down
10 changes: 10 additions & 0 deletions Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,16 @@ IApplicationConfigurationBuilderSecurityOptions SetUseValidatedCertificates(
IApplicationConfigurationBuilderSecurityOptions SetSuppressNonceValidationErrors(
bool suppressNonceValidationErrors);

/// <summary>
/// Whether to reject certificates with an ApplicationUri that does not match the configuration.
/// If set to true, an exception will be thrown when the ApplicationUri in the configuration
/// does not match the ApplicationUri in the certificate.
/// If set to false (default), the configuration ApplicationUri will be updated to match the certificate for backward compatibility.
/// </summary>
/// <param name="rejectCertificateUriMismatch"><see langword="true"/> to reject certificates with mismatched ApplicationUri.</param>
IApplicationConfigurationBuilderSecurityOptions SetRejectCertificateUriMismatch(
bool rejectCertificateUriMismatch);

/// <summary>
/// Whether a certificate chain should be sent with the application certificate.
/// Only used if the application certificate is CA signed.
Expand Down
36 changes: 33 additions & 3 deletions Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ private void Initialize()
AddAppCertToTrustedStore = true;
SendCertificateChain = true;
SuppressNonceValidationErrors = false;
RejectCertificateUriMismatch = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose to set this to true and do a Version increase to 1.5.378 we should always have the Default config be the most secure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that replacing the ApplicationUri with the one from the certificate on the clients was convenient "feature" and most users are and have been using it involuntarily for a long time and this should stay so until it is willingly disabled.

IsDeprecatedConfiguration = false;
}

Expand Down Expand Up @@ -736,13 +737,31 @@ public CertificateIdentifierCollection ApplicationCertificates
}
}

// Remove any duplicates
// Remove any duplicates based on thumbprint or subject name
// also since CertificateType can be null (implicitly RSA)
for (int i = 0; i < m_applicationCertificates.Count; i++)
{
for (int j = m_applicationCertificates.Count - 1; j > i; j--)
{
if (m_applicationCertificates[i]
.CertificateType == m_applicationCertificates[j].CertificateType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldnt this change make more sense in the other PR? Or why was this changed here?

// Compare by thumbprint (should always be present), otherwise by subject name and store path
bool isDuplicate = false;
if (!string.IsNullOrEmpty(m_applicationCertificates[i].Thumbprint) &&
!string.IsNullOrEmpty(m_applicationCertificates[j].Thumbprint))
{
isDuplicate = m_applicationCertificates[i].Thumbprint.Equals(
m_applicationCertificates[j].Thumbprint,
StringComparison.OrdinalIgnoreCase);
}
else if (!string.IsNullOrEmpty(m_applicationCertificates[i].SubjectName) &&
!string.IsNullOrEmpty(m_applicationCertificates[j].SubjectName))
{
isDuplicate = m_applicationCertificates[i].SubjectName.Equals(
m_applicationCertificates[j].SubjectName,
StringComparison.OrdinalIgnoreCase) &&
m_applicationCertificates[i].StorePath == m_applicationCertificates[j].StorePath;
}

if (isDuplicate)
{
m_applicationCertificates.RemoveAt(j);
}
Expand Down Expand Up @@ -927,6 +946,17 @@ public CertificateTrustList TrustedHttpsCertificates
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 21)]
public bool SuppressNonceValidationErrors { get; set; }

/// <summary>
/// Gets or sets a value indicating whether certificate ApplicationUri mismatches should be rejected.
/// </summary>
/// <remarks>
/// If set to true, the application will throw an exception when the ApplicationUri in the configuration
/// does not match the ApplicationUri in the certificate, similar to the server behavior for client certificates.
/// If set to false (default), the configuration ApplicationUri will be updated to match the certificate for backward compatibility.
/// </remarks>
[DataMember(IsRequired = false, EmitDefaultValue = false, Order = 22)]
public bool RejectCertificateUriMismatch { get; set; }

/// <summary>
/// The type of Configuration (deprecated or not)
/// </summary>
Expand Down
13 changes: 13 additions & 0 deletions Stack/Opc.Ua.Core/Types/Utils/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2790,6 +2790,19 @@ public static bool FindStringIgnoreCase(IList<string> strings, string target)
/// <param name="certificateType">The certificate type to check.</param>
public static bool IsSupportedCertificateType(NodeId certificateType)
{
// Handle null or NodeId.Null certificate types
// This occurs when:
// 1. CertificateIdentifier.CertificateType was never set (property is null)
// 2. GetCertificateType() returned NodeId.Null for unknown signature algorithms
//
// Return true (don't reject) to allow further certificate validation to proceed.
// The actual certificate signature and validity checks will still happen elsewhere.
// Older code that doesn't set CertificateType should still work.
if (certificateType == null || NodeId.IsNull(certificateType))
{
return true;
}

if (certificateType.Identifier is uint identifier)
{
switch (identifier)
Expand Down
Loading
Loading