From 4a44238b8c6940a9869c5cc4a8807ebaff4f1d2f Mon Sep 17 00:00:00 2001 From: romanett Date: Tue, 21 Jan 2025 11:23:14 +0100 Subject: [PATCH] [Server] Allow Subject Name Change of Application Certificate in GDS Push scenario (#2833) * allow Subject Name change * modifiy subject names in push test --- .../ApplicationInstance.cs | 12 ++--- .../Configuration/ConfigurationNodeManager.cs | 32 ++++++------- .../Certificates/CertificateIdentifier.cs | 48 ++++++++++++++----- .../Certificates/CertificateValidator.cs | 10 ++-- .../Certificates/DirectoryCertificateStore.cs | 14 ++++-- .../Certificates/ICertificateStore.cs | 3 +- .../X509CertificateStore.cs | 2 +- .../Configuration/ApplicationConfiguration.cs | 2 +- .../CertificateStoreTypeTest.cs | 4 +- .../Certificates/CertificateStoreTypeTest.cs | 4 +- Tests/Opc.Ua.Gds.Tests/PushTest.cs | 6 +-- 11 files changed, 85 insertions(+), 52 deletions(-) diff --git a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs b/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs index ce9526fe89..bd4487c10f 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs +++ b/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs @@ -525,10 +525,10 @@ private async Task CheckCertificateTypeAsync( // 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) @@ -550,7 +550,7 @@ private async Task CheckCertificateTypeAsync( 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) { @@ -568,7 +568,7 @@ private async Task CheckCertificateTypeAsync( StorePath = id.StorePath, SubjectName = id.SubjectName }; - certificate = await id2.Find(true).ConfigureAwait(false); + certificate = await id2.Find(true, configuration.ApplicationUri).ConfigureAwait(false); } if (certificate != null) @@ -965,7 +965,7 @@ await id.Certificate.AddToStoreAsync( } // 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); @@ -990,7 +990,7 @@ private static async Task DeleteApplicationInstanceCertificateAsync(ApplicationC } // 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); diff --git a/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs b/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs index a57b5ddc9c..658cf94617 100644 --- a/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs +++ b/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs @@ -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."); } @@ -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) @@ -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); } @@ -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; @@ -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)) @@ -677,7 +677,7 @@ private ServiceResult ApplyChanges( await Task.Delay(1000).ConfigureAwait(false); try { - await m_configuration.CertificateValidator.UpdateCertificateAsync(m_configuration.SecurityConfiguration).ConfigureAwait(false); + await m_configuration.CertificateValidator.UpdateCertificateAsync(m_configuration.SecurityConfiguration, m_configuration.ApplicationUri).ConfigureAwait(false); } catch (Exception ex) { diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs index 40df8a66b7..539f95ae3e 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs @@ -160,21 +160,21 @@ public X509Certificate2 Certificate /// /// Finds a certificate in a store. /// - public Task Find() + public Task Find(string applicationUri = null) { - return Find(false); + return Find(false, applicationUri); } /// /// Loads the private key for the certificate with an optional password. /// - public Task LoadPrivateKey(string password) - => LoadPrivateKeyEx(password != null ? new CertificatePasswordProvider(password) : null); + public Task LoadPrivateKey(string password, string applicationUri = null) + => LoadPrivateKeyEx(password != null ? new CertificatePasswordProvider(password) : null, applicationUri); /// /// Loads the private key for the certificate with an optional password provider. /// - public async Task LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider) + public async Task LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider, string applicationUri = null) { if (this.StoreType != CertificateStoreType.X509Store) { @@ -184,7 +184,14 @@ public async Task LoadPrivateKeyEx(ICertificatePasswordProvide 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; } } @@ -198,9 +205,10 @@ public async Task LoadPrivateKeyEx(ICertificatePasswordProvide /// /// The certificate type is used to match the signature and public key type. /// if set to true the returned certificate must contain the private key. + /// the application uri in the extensions of the certificate. /// An instance of the that is embedded by this instance or find it in - /// the selected store pointed out by the using selected . - public async Task Find(bool needPrivateKey) + /// the selected store pointed out by the using selected or if specified applicationUri. + public async Task Find(bool needPrivateKey, string applicationUri = null) { X509Certificate2 certificate = null; @@ -222,7 +230,7 @@ public async Task Find(bool needPrivateKey) 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) { @@ -315,6 +323,7 @@ private static string GetDisplayName(X509Certificate2 certificate) /// The collection. /// The thumbprint of the certificate. /// Subject name of the certificate. + /// ApplicationUri in the SubjectAltNameExtension of the certificate. /// The certificate type. /// if set to true [need private key]. /// @@ -322,6 +331,7 @@ public static X509Certificate2 Find( X509Certificate2Collection collection, string thumbprint, string subjectName, + string applicationUri, NodeId certificateType, bool needPrivateKey) { @@ -379,6 +389,20 @@ public static X509Certificate2 Find( } } + //find by application uri + if (!string.IsNullOrEmpty(applicationUri)) + { + foreach (X509Certificate2 certificate in collection) + { + if (applicationUri == X509Utils.GetApplicationUriFromCertificate(certificate) && + ValidateCertificateType(certificate, certificateType) && + (!needPrivateKey || certificate.HasPrivateKey)) + { + return certificate; + } + } + } + // certificate not found. return null; } @@ -581,7 +605,7 @@ public static bool ValidateCertificateType(X509Certificate2 certificate, NodeId certificateType == ObjectTypeIds.EccBrainpoolP256r1ApplicationCertificateType) { return true; - } + } break; @@ -671,7 +695,7 @@ public void DisposeCertificate() { ObjectTypes.RsaMinApplicationCertificateType, "RsaMin"}, { ObjectTypes.ApplicationCertificateType, "Rsa"}, }; -#endregion + #endregion #region Private Methods /// @@ -963,7 +987,7 @@ public Task LoadPrivateKey(string thumbprint, string subjectNa } /// - public Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password) + public Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password) { return Task.FromResult(null); } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs index 22509eba46..8be7461eeb 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs @@ -180,7 +180,7 @@ private void InternalUpdate( /// /// Updates the validator with the current state of the configuration. /// - public virtual async Task UpdateAsync(SecurityConfiguration configuration) + public virtual async Task UpdateAsync(SecurityConfiguration configuration, string applicationUri = null) { if (configuration == null) { @@ -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); @@ -251,7 +251,7 @@ public virtual async Task UpdateAsync(SecurityConfiguration configuration) /// /// Updates the validator with a new application certificate. /// - public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityConfiguration) + public virtual async Task UpdateCertificateAsync(SecurityConfiguration securityConfiguration, string applicationUri = null) { await m_semaphore.WaitAsync().ConfigureAwait(false); @@ -267,7 +267,7 @@ 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 @@ -275,7 +275,7 @@ await applicationCertificate.LoadPrivateKeyEx( m_semaphore.Release(); } - await UpdateAsync(securityConfiguration).ConfigureAwait(false); + await UpdateAsync(securityConfiguration, applicationUri).ConfigureAwait(false); lock (m_callbackLock) { diff --git a/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs index f5059530a5..9b9d3422b7 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs @@ -427,13 +427,13 @@ 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 LoadPrivateKey(string thumbprint, string subjectName, string password) { - return LoadPrivateKey(thumbprint, subjectName, null, password); + return LoadPrivateKey(thumbprint, subjectName, null, null, password); } /// /// Loads the private key from a PFX file in the certificate store. /// - public async Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password) + public async Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password) { if (NoPrivateKeys || m_privateKeySubdir == null || m_certificateSubdir == null || !m_certificateSubdir.Exists) @@ -441,7 +441,7 @@ public async Task LoadPrivateKey(string thumbprint, string sub return null; } - if (string.IsNullOrEmpty(thumbprint) && string.IsNullOrEmpty(subjectName)) + if (string.IsNullOrEmpty(thumbprint) && string.IsNullOrEmpty(subjectName) && string.IsNullOrEmpty(applicationUri)) { return null; } @@ -484,6 +484,14 @@ public async Task 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; diff --git a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs index 3061c167d3..c0fba02d49 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs @@ -110,11 +110,12 @@ public interface ICertificateStore : IDisposable /// /// The thumbprint. /// The certificate subject. + /// The application uri in the cert extension. /// The certificate type to load. /// The certificate password. /// Returns always null if SupportsLoadPrivateKey returns false. /// The matching certificate with private key - Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password); + Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password); /// /// Checks if issuer has revoked the certificate. diff --git a/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs index 7e66e0d8e5..0494863271 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs @@ -223,7 +223,7 @@ public Task LoadPrivateKey(string thumbprint, string subjectNa /// /// The LoadPrivateKey special handling is not necessary in this store. - public Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password) + public Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password) { return Task.FromResult(null); } diff --git a/Stack/Opc.Ua.Core/Stack/Configuration/ApplicationConfiguration.cs b/Stack/Opc.Ua.Core/Stack/Configuration/ApplicationConfiguration.cs index a38b399ab7..5ea3724354 100644 --- a/Stack/Opc.Ua.Core/Stack/Configuration/ApplicationConfiguration.cs +++ b/Stack/Opc.Ua.Core/Stack/Configuration/ApplicationConfiguration.cs @@ -442,7 +442,7 @@ public virtual async Task Validate(ApplicationType applicationType) // load private keys foreach (var applicationCertificate in SecurityConfiguration.ApplicationCertificates) { - await applicationCertificate.LoadPrivateKeyEx(SecurityConfiguration.CertificatePasswordProvider).ConfigureAwait(false); + await applicationCertificate.LoadPrivateKeyEx(SecurityConfiguration.CertificatePasswordProvider, ApplicationUri).ConfigureAwait(false); } Func generateDefaultUri = () => { diff --git a/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs b/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs index 2cbd3949dd..2862506093 100644 --- a/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs +++ b/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs @@ -244,8 +244,8 @@ public Task IsRevoked(X509Certificate2 issuer, X509Certificate2 cert public bool SupportsLoadPrivateKey => m_innerStore.SupportsLoadPrivateKey; /// - public Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password) - => m_innerStore.LoadPrivateKey(thumbprint, subjectName, certificateType, password); + public Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password) + => m_innerStore.LoadPrivateKey(thumbprint, subjectName, applicationUri, certificateType, password); [Obsolete("Method is deprecated. Use only for RSA certificates, the replacing LoadPrivateKey with certificateType parameter should be used.")] public Task LoadPrivateKey(string thumbprint, string subjectName, string password) diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs index 48b237cc3c..5ae072ba57 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs @@ -161,8 +161,8 @@ public Task LoadPrivateKey(string thumbprint, string subjectNa => m_innerStore.LoadPrivateKey(thumbprint, subjectName, password); /// - public Task LoadPrivateKey(string thumbprint, string subjectName, NodeId certificateType, string password) - => m_innerStore.LoadPrivateKey(thumbprint, subjectName, certificateType, password); + public Task LoadPrivateKey(string thumbprint, string subjectName, string applicationUri, NodeId certificateType, string password) + => m_innerStore.LoadPrivateKey(thumbprint, subjectName, applicationUri, certificateType, password); /// public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) diff --git a/Tests/Opc.Ua.Gds.Tests/PushTest.cs b/Tests/Opc.Ua.Gds.Tests/PushTest.cs index 880623d65d..0c067713ee 100644 --- a/Tests/Opc.Ua.Gds.Tests/PushTest.cs +++ b/Tests/Opc.Ua.Gds.Tests/PushTest.cs @@ -399,7 +399,7 @@ public void UpdateCertificateCASigned(bool regeneratePrivateKey) byte[] csr = m_pushClient.PushClient.CreateSigningRequest( null, m_pushClient.PushClient.ApplicationCertificateType, - null, + m_selfSignedServerCert.Subject + "2", regeneratePrivateKey, null); Assert.IsNotNull(csr); @@ -487,7 +487,7 @@ public void UpdateCertificateSelfSigned(string keyFormat) X509Certificate2 newCert = CertificateFactory.CreateCertificate( m_applicationRecord.ApplicationUri, m_applicationRecord.ApplicationNames[0].Text, - m_selfSignedServerCert.Subject, + m_selfSignedServerCert.Subject + "1", null).CreateForRSA(); byte[] privateKey = null; @@ -546,7 +546,7 @@ public void UpdateCertificateWithNewKeyPair(string keyFormat) m_applicationRecord.ApplicationId, null, null, - m_selfSignedServerCert.Subject, + m_selfSignedServerCert.Subject + "3", m_domainNames, keyFormat, null);