Skip to content
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.

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

[Server] Allow Subject Name Change of Application Certificate in GDS Push scenario #2833

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions Libraries/Opc.Ua.Configuration/ApplicationInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,10 @@

// reload the certificate from disk in the cache.
var passwordProvider = configuration.SecurityConfiguration.CertificatePasswordProvider;
await id.LoadPrivateKeyEx(passwordProvider).ConfigureAwait(false);
await id.LoadPrivateKeyEx(passwordProvider, configuration.ApplicationUri).ConfigureAwait(false);

// load the certificate
X509Certificate2 certificate = await id.Find(true).ConfigureAwait(false);
X509Certificate2 certificate = await id.Find(true, configuration.ApplicationUri).ConfigureAwait(false);

// check that it is ok.
if (certificate != null)
Expand All @@ -550,7 +550,7 @@
else
{
// check for missing private key.
certificate = await id.Find(false).ConfigureAwait(false);
certificate = await id.Find(false, configuration.ApplicationUri).ConfigureAwait(false);

if (certificate != null)
{
Expand All @@ -568,7 +568,7 @@
StorePath = id.StorePath,
SubjectName = id.SubjectName
};
certificate = await id2.Find(true).ConfigureAwait(false);
certificate = await id2.Find(true, configuration.ApplicationUri).ConfigureAwait(false);

Check warning on line 571 in Libraries/Opc.Ua.Configuration/ApplicationInstance.cs

View check run for this annotation

Codecov / codecov/patch

Libraries/Opc.Ua.Configuration/ApplicationInstance.cs#L571

Added line #L571 was not covered by tests
}

if (certificate != null)
Expand Down Expand Up @@ -965,7 +965,7 @@
}

// reload the certificate from disk.
id.Certificate = await id.LoadPrivateKeyEx(passwordProvider).ConfigureAwait(false);
id.Certificate = await id.LoadPrivateKeyEx(passwordProvider, configuration.ApplicationUri).ConfigureAwait(false);

await configuration.CertificateValidator.UpdateAsync(configuration.SecurityConfiguration).ConfigureAwait(false);

Expand All @@ -990,7 +990,7 @@
}

// delete certificate and private key.
X509Certificate2 certificate = await id.Find().ConfigureAwait(false);
X509Certificate2 certificate = await id.Find(configuration.ApplicationUri).ConfigureAwait(false);
if (certificate != null)
{
Utils.LogCertificate(TraceMasks.Security, "Deleting application instance certificate and private key.", certificate);
Expand Down
32 changes: 16 additions & 16 deletions Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,18 @@ private ServiceResult UpdateCertificate(
X509Utils.CompareDistinguishedName(cert.Certificate.Subject, newCert.Subject) &&
cert.CertificateType == certificateTypeId);

// if no cert was found search by ApplicationUri
if (existingCertIdentifier == null)
{
existingCertIdentifier = certificateGroup.ApplicationCertificates.FirstOrDefault(cert =>
m_configuration.ApplicationUri == X509Utils.GetApplicationUriFromCertificate(cert.Certificate) &&
cert.CertificateType == certificateTypeId);
}

// if there is no such existing certificate then this is an error
if (existingCertIdentifier == null)
{

throw new ServiceResultException(StatusCodes.BadInvalidArgument, "No existing certificate found for the specified certificate type and subject name.");
}

Expand All @@ -428,16 +437,6 @@ private ServiceResult UpdateCertificate(
throw new ServiceResultException(StatusCodes.BadCertificateInvalid, "Certificate data is invalid.");
}

// validate new subject matches the previous subject,
// otherwise application may not be able to find it after restart
// TODO: An issuer may modify the subject of an issued certificate,
// but then the configuration must be updated too!
// NOTE: not a strict requirement here for ASN.1 byte compare
if (!X509Utils.CompareDistinguishedName(existingCertIdentifier.Certificate.Subject, newCert.Subject))
{
throw new ServiceResultException(StatusCodes.BadSecurityChecksFailed, "Subject Name of new certificate doesn't match the application.");
}

// self signed
bool selfSigned = X509Utils.IsSelfSigned(newCert);
if (selfSigned && newIssuerCollection.Count != 0)
Expand Down Expand Up @@ -484,7 +483,7 @@ private ServiceResult UpdateCertificate(
}
else
{
X509Certificate2 certWithPrivateKey = existingCertIdentifier.LoadPrivateKeyEx(passwordProvider).Result;
X509Certificate2 certWithPrivateKey = existingCertIdentifier.LoadPrivateKeyEx(passwordProvider, m_configuration.ApplicationUri).Result;
exportableKey = X509Utils.CreateCopyWithPrivateKey(certWithPrivateKey, false);
}

Expand Down Expand Up @@ -592,17 +591,18 @@ private ServiceResult CreateSigningRequest(

ServerCertificateGroup certificateGroup = VerifyGroupAndTypeId(certificateGroupId, certificateTypeId);



// identify the existing certificate for which to CreateSigningRequest
// it should be of the same type
CertificateIdentifier existingCertIdentifier = certificateGroup.ApplicationCertificates.FirstOrDefault(cert =>
cert.CertificateType == certificateTypeId);

if (!String.IsNullOrEmpty(subjectName))
if (string.IsNullOrEmpty(subjectName))
{
throw new ArgumentNullException(nameof(subjectName));
subjectName = existingCertIdentifier.Certificate.Subject;
}


certificateGroup.TemporaryApplicationCertificate?.Dispose();
certificateGroup.TemporaryApplicationCertificate = null;

Expand All @@ -619,7 +619,7 @@ private ServiceResult CreateSigningRequest(
certWithPrivateKey = CertificateFactory.CreateCertificate(
m_configuration.ApplicationUri,
null,
existingCertIdentifier.Certificate.Subject,
subjectName,
null)
.SetNotBefore(DateTime.Today.AddDays(-1))
.SetNotAfter(DateTime.Today.AddDays(14))
Expand Down Expand Up @@ -675,7 +675,7 @@ private ServiceResult ApplyChanges(
// give the client some time to receive the response
// before the certificate update may disconnect all sessions
await Task.Delay(1000).ConfigureAwait(false);
await m_configuration.CertificateValidator.UpdateCertificateAsync(m_configuration.SecurityConfiguration).ConfigureAwait(false);
await m_configuration.CertificateValidator.UpdateCertificateAsync(m_configuration.SecurityConfiguration, m_configuration.ApplicationUri).ConfigureAwait(false);
}
);
}
Expand Down
48 changes: 36 additions & 12 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,21 +153,21 @@
/// <summary>
/// Finds a certificate in a store.
/// </summary>
public Task<X509Certificate2> Find()
public Task<X509Certificate2> Find(string applicationUri = null)
{
return Find(false);
return Find(false, applicationUri);
}

/// <summary>
/// Loads the private key for the certificate with an optional password.
/// </summary>
public Task<X509Certificate2> LoadPrivateKey(string password)
=> LoadPrivateKeyEx(password != null ? new CertificatePasswordProvider(password) : null);
public Task<X509Certificate2> LoadPrivateKey(string password, string applicationUri = null)
=> LoadPrivateKeyEx(password != null ? new CertificatePasswordProvider(password) : null, applicationUri);

/// <summary>
/// Loads the private key for the certificate with an optional password provider.
/// </summary>
public async Task<X509Certificate2> LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider)
public async Task<X509Certificate2> LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider, string applicationUri = null)
{
if (this.StoreType != CertificateStoreType.X509Store)
{
Expand All @@ -177,7 +177,14 @@
if (store?.SupportsLoadPrivateKey == true)
{
string password = passwordProvider?.GetPassword(this);
m_certificate = await store.LoadPrivateKey(this.Thumbprint, this.SubjectName, this.CertificateType, password).ConfigureAwait(false);
m_certificate = await store.LoadPrivateKey(this.Thumbprint, this.SubjectName, null, this.CertificateType, password).ConfigureAwait(false);

//find certificate by applicationUri instead of subjectName, as the subjectName could have changed after a certificate update
if (m_certificate == null && !string.IsNullOrEmpty(applicationUri))
{
m_certificate = await store.LoadPrivateKey(this.Thumbprint, null, applicationUri, this.CertificateType, password).ConfigureAwait(false);
}

return m_certificate;
}
}
Expand All @@ -191,9 +198,10 @@
/// </summary>
/// <remarks>The certificate type is used to match the signature and public key type.</remarks>
/// <param name="needPrivateKey">if set to <c>true</c> the returned certificate must contain the private key.</param>
/// <param name="applicationUri">the application uri in the extensions of the certificate.</param>
/// <returns>An instance of the <see cref="X509Certificate2"/> that is embedded by this instance or find it in
/// the selected store pointed out by the <see cref="StorePath"/> using selected <see cref="SubjectName"/>.</returns>
public async Task<X509Certificate2> Find(bool needPrivateKey)
/// the selected store pointed out by the <see cref="StorePath"/> using selected <see cref="SubjectName"/> or if specified applicationUri.</returns>
public async Task<X509Certificate2> Find(bool needPrivateKey, string applicationUri = null)
{
X509Certificate2 certificate = null;

Expand All @@ -215,7 +223,7 @@

X509Certificate2Collection collection = await store.Enumerate().ConfigureAwait(false);

certificate = Find(collection, m_thumbprint, m_subjectName, m_certificateType, needPrivateKey);
certificate = Find(collection, m_thumbprint, m_subjectName, applicationUri, m_certificateType, needPrivateKey);

if (certificate != null)
{
Expand Down Expand Up @@ -308,13 +316,15 @@
/// <param name="collection">The collection.</param>
/// <param name="thumbprint">The thumbprint of the certificate.</param>
/// <param name="subjectName">Subject name of the certificate.</param>
/// <param name="applicationUri">ApplicationUri in the SubjectAltNameExtension of the certificate.</param>
/// <param name="certificateType">The certificate type.</param>
/// <param name="needPrivateKey">if set to <c>true</c> [need private key].</param>
/// <returns></returns>
public static X509Certificate2 Find(
X509Certificate2Collection collection,
string thumbprint,
string subjectName,
string applicationUri,
NodeId certificateType,
bool needPrivateKey)
{
Expand Down Expand Up @@ -372,6 +382,20 @@
}
}

//find by application uri
if (!string.IsNullOrEmpty(applicationUri))
{
foreach (X509Certificate2 certificate in collection)
{
if (applicationUri == X509Utils.GetApplicationUriFromCertificate(certificate) &&
ValidateCertificateType(certificate, certificateType) &&
(!needPrivateKey || certificate.HasPrivateKey))

Check warning on line 392 in Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs

View check run for this annotation

Codecov / codecov/patch

Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs#L391-L392

Added lines #L391 - L392 were not covered by tests
{
return certificate;

Check warning on line 394 in Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs

View check run for this annotation

Codecov / codecov/patch

Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs#L394

Added line #L394 was not covered by tests
}
}
}

// certificate not found.
return null;
}
Expand Down Expand Up @@ -574,7 +598,7 @@
certificateType == ObjectTypeIds.EccBrainpoolP256r1ApplicationCertificateType)
{
return true;
}
}

break;

Expand Down Expand Up @@ -664,7 +688,7 @@
{ ObjectTypes.RsaMinApplicationCertificateType, "RsaMin"},
{ ObjectTypes.ApplicationCertificateType, "Rsa"},
};
#endregion
#endregion

#region Private Methods
/// <summary>
Expand Down Expand Up @@ -956,7 +980,7 @@
}

/// <inheritdoc/>
public Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password)
public Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password)
{
return Task.FromResult<X509Certificate2>(null);
}
Expand Down
10 changes: 5 additions & 5 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private void InternalUpdate(
/// <summary>
/// Updates the validator with the current state of the configuration.
/// </summary>
public virtual async Task UpdateAsync(SecurityConfiguration configuration)
public virtual async Task UpdateAsync(SecurityConfiguration configuration, string applicationUri = null)
{
if (configuration == null)
{
Expand Down Expand Up @@ -226,7 +226,7 @@ public virtual async Task UpdateAsync(SecurityConfiguration configuration)
{
foreach (var applicationCertificate in configuration.ApplicationCertificates)
{
X509Certificate2 certificate = await applicationCertificate.Find(true).ConfigureAwait(false);
X509Certificate2 certificate = await applicationCertificate.Find(true, applicationUri).ConfigureAwait(false);
if (certificate == null)
{
Utils.Trace(Utils.TraceMasks.Security, "Could not find application certificate: {0}", applicationCertificate);
Expand All @@ -251,7 +251,7 @@ public virtual async Task UpdateAsync(SecurityConfiguration configuration)
/// <summary>
/// Updates the validator with a new application certificate.
/// </summary>
public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityConfiguration)
public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityConfiguration, string applicationUri = null)
{
await m_semaphore.WaitAsync().ConfigureAwait(false);

Expand All @@ -267,15 +267,15 @@ public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityC
foreach (var applicationCertificate in securityConfiguration.ApplicationCertificates)
{
await applicationCertificate.LoadPrivateKeyEx(
securityConfiguration.CertificatePasswordProvider).ConfigureAwait(false);
securityConfiguration.CertificatePasswordProvider, applicationUri).ConfigureAwait(false);
}
}
finally
{
m_semaphore.Release();
}

await UpdateAsync(securityConfiguration).ConfigureAwait(false);
await UpdateAsync(securityConfiguration, applicationUri).ConfigureAwait(false);

lock (m_callbackLock)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,21 +427,21 @@ public string GetPrivateKeyFilePath(string thumbprint)
[Obsolete("Method is deprecated. Use only for RSA certificates, the replacing LoadPrivateKey with certificateType parameter should be used.")]
public Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, string password)
{
return LoadPrivateKey(thumbprint, subjectName, null, password);
return LoadPrivateKey(thumbprint, subjectName, null, null, password);
}

/// <summary>
/// Loads the private key from a PFX file in the certificate store.
/// </summary>
public async Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password)
public async Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password)
{
if (NoPrivateKeys || m_privateKeySubdir == null ||
m_certificateSubdir == null || !m_certificateSubdir.Exists)
{
return null;
}

if (string.IsNullOrEmpty(thumbprint) && string.IsNullOrEmpty(subjectName))
if (string.IsNullOrEmpty(thumbprint) && string.IsNullOrEmpty(subjectName) && string.IsNullOrEmpty(applicationUri))
{
return null;
}
Expand Down Expand Up @@ -484,6 +484,14 @@ public async Task<X509Certificate2> LoadPrivateKey(string thumbprint, string sub
}
}

if (!string.IsNullOrEmpty(applicationUri))
{
if (!string.Equals(X509Utils.GetApplicationUriFromCertificate(certificate), applicationUri, StringComparison.OrdinalIgnoreCase))
{
continue;
}
}

if (!CertificateIdentifier.ValidateCertificateType(certificate, certificateType))
{
continue;
Expand Down
3 changes: 2 additions & 1 deletion Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,12 @@ public interface ICertificateStore : IDisposable
/// </summary>
/// <param name="thumbprint">The thumbprint.</param>
/// <param name="subjectName">The certificate subject.</param>
/// <param name="applicationUri">The application uri in the cert extension.</param>
/// <param name="certificateType">The certificate type to load.</param>
/// <param name="password">The certificate password.</param>
/// <remarks>Returns always null if SupportsLoadPrivateKey returns false.</remarks>
/// <returns>The matching certificate with private key</returns>
Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password);
Task<X509Certificate2> LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password);

/// <summary>
/// Checks if issuer has revoked the certificate.
Expand Down
Loading
Loading