From 1c6253a729c7cdfc5038365f3264991a020dd9ff Mon Sep 17 00:00:00 2001 From: Martin Regen Date: Tue, 3 Sep 2024 15:23:32 +0200 Subject: [PATCH 1/6] AddRejected method for ICertificateStore (#2720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a light weight AddRejected certificate store method to `ICertificateStore` to ensure rejected cert history remains small and doesn't have a lot of overhead. - Number of history certificates is configurable using a new `MaxRejectedCertificates` entry. - Rejected certs are only saved to directory cert store, others do ignore the attempt - Fix the caching of `CertificateValidator` and flag some `ÒpenStore` methods deprecated which do not support the caching. Fixes across the board to support the methodology. Basically `store.Close` keeps the cache, `store.Dispose` clears the cache, so recommendation is to use `var store = certStoreIdentifier.OpenStore(); try { ... }-- finally { store.Close }` --- .../Quickstarts.ReferenceClient.Config.xml | 3 +- .../Quickstarts.ReferenceServer.Config.xml | 1 + .../ApplicationConfigurationBuilder.cs | 8 + .../ApplicationInstance.cs | 16 +- .../IApplicationConfigurationBuilder.cs | 10 + .../ApplicationsNodeManager.cs | 62 +++--- .../CertificateGroup.cs | 118 +++++++----- .../GlobalDiscoverySampleServer.cs | 8 +- .../ICertificateGroup.cs | 5 +- .../AuthorizationHelper.cs | 12 +- .../Configuration/ConfigurationNodeManager.cs | 46 +++-- .../Opc.Ua.Server/Configuration/TrustList.cs | 73 ++++--- .../Schema/ApplicationConfiguration.cs | 49 +++-- .../Schema/ApplicationConfiguration.xsd | 1 + .../Schema/SecuredApplicationHelpers.cs | 4 +- .../Certificates/CertificateIdentifier.cs | 19 +- .../CertificateStoreIdentifier.cs | 79 +++++++- .../Certificates/CertificateTrustList.cs | 31 --- .../Certificates/CertificateValidator.cs | 160 +++++++++------ .../Certificates/DirectoryCertificateStore.cs | 182 +++++++++++++++--- .../Certificates/ICertificateStore.cs | 16 ++ .../X509CertificateStore.cs | 9 + .../Security/Certificates/X509Utils.cs | 82 +++++++- .../CertificateStoreTypeTest.cs | 14 +- .../Certificates/CertificateStoreTest.cs | 6 +- .../Certificates/CertificateStoreTypeTest.cs | 26 ++- .../Certificates/CertificateValidatorTest.cs | 107 +++++++++- .../Opc.Ua.Gds.Tests/CertificateGroupTests.cs | 19 +- .../GlobalDiscoveryTestClient.cs | 4 +- Tests/Opc.Ua.Gds.Tests/PushTest.cs | 12 +- 30 files changed, 872 insertions(+), 310 deletions(-) diff --git a/Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml b/Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml index 4fc30af53..b82feaf0f 100644 --- a/Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml +++ b/Applications/ConsoleReferenceClient/Quickstarts.ReferenceClient.Config.xml @@ -35,7 +35,8 @@ Directory %LocalApplicationData%/OPC Foundation/pki/rejected - + 5 + false diff --git a/Applications/ConsoleReferenceServer/Quickstarts.ReferenceServer.Config.xml b/Applications/ConsoleReferenceServer/Quickstarts.ReferenceServer.Config.xml index 686ef2839..59d17b590 100644 --- a/Applications/ConsoleReferenceServer/Quickstarts.ReferenceServer.Config.xml +++ b/Applications/ConsoleReferenceServer/Quickstarts.ReferenceServer.Config.xml @@ -35,6 +35,7 @@ Directory %LocalApplicationData%/OPC Foundation/pki/rejected + 5 diff --git a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs index 199fb1a4a..a5240c95b 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs +++ b/Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs @@ -373,6 +373,13 @@ public IApplicationConfigurationBuilderServerSelected AddUserTokenPolicy(UserTok return this; } + /// + public IApplicationConfigurationBuilderSecurityOptions SetMaxRejectedCertificates(int maxRejectedCertificates) + { + ApplicationConfiguration.SecurityConfiguration.MaxRejectedCertificates = maxRejectedCertificates; + return this; + } + /// public IApplicationConfigurationBuilderSecurityOptions SetAutoAcceptUntrustedCertificates(bool autoAccept) { @@ -1017,6 +1024,7 @@ private void SetSecureDefaults(SecurityConfiguration securityConfiguration) securityConfiguration.SuppressNonceValidationErrors = false; securityConfiguration.SendCertificateChain = true; securityConfiguration.MinimumCertificateKeySize = CertificateFactory.DefaultKeySize; + securityConfiguration.MaxRejectedCertificates = 5; } /// diff --git a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs b/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs index 64dba21e1..1734790b1 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs +++ b/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs @@ -892,12 +892,20 @@ private static async Task DeleteApplicationInstanceCertificateAsync(ApplicationC if (!string.IsNullOrEmpty(thumbprint)) { - using (ICertificateStore store = configuration.SecurityConfiguration.TrustedPeerCertificates.OpenStore()) + ICertificateStore store = configuration.SecurityConfiguration.TrustedPeerCertificates.OpenStore(); + if (store != null) { - bool deleted = await store.Delete(thumbprint).ConfigureAwait(false); - if (deleted) + try { - Utils.LogInfo(TraceMasks.Security, "Application Instance Certificate [{0}] deleted from trusted store.", thumbprint); + bool deleted = await store.Delete(thumbprint).ConfigureAwait(false); + if (deleted) + { + Utils.LogInfo(TraceMasks.Security, "Application Instance Certificate [{0}] deleted from trusted store.", thumbprint); + } + } + finally + { + store.Close(); } } } diff --git a/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs b/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs index 37aac555f..753c2666a 100644 --- a/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs +++ b/Libraries/Opc.Ua.Configuration/IApplicationConfigurationBuilder.cs @@ -451,6 +451,16 @@ public interface IApplicationConfigurationBuilderSecurityOptions : IApplicationConfigurationBuilderExtension, IApplicationConfigurationBuilderCreate { + /// + /// The number of rejected certificates to keep in the store. + /// + /// + /// The number of certificates to keep in the rejected store before it is updated. + /// to keep all rejected certificates. + /// A negative number to keep no history. + /// + IApplicationConfigurationBuilderSecurityOptions SetMaxRejectedCertificates(int maxRejectedCertificates); + /// /// Whether an unknown application certificate should be accepted /// once all other security checks passed. diff --git a/Libraries/Opc.Ua.Gds.Server.Common/ApplicationsNodeManager.cs b/Libraries/Opc.Ua.Gds.Server.Common/ApplicationsNodeManager.cs index 0207479b0..bd3ca1332 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/ApplicationsNodeManager.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/ApplicationsNodeManager.cs @@ -291,7 +291,9 @@ protected async Task InitializeCertificateGroup(CertificateGr } ICertificateGroup certificateGroup = m_certificateGroupFactory.Create( - m_globalDiscoveryServerConfiguration.AuthoritiesStorePath, certificateGroupConfiguration, m_configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath); + m_globalDiscoveryServerConfiguration.AuthoritiesStorePath, + certificateGroupConfiguration, + m_configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath); SetCertificateGroupNodes(certificateGroup); await certificateGroup.Init().ConfigureAwait(false); @@ -654,12 +656,12 @@ private ServiceResult OnGetApplication( } private ServiceResult OnCheckRevocationStatus( - ISystemContext context, - MethodState method, - NodeId objectId, - byte[] certificate, - ref StatusCode certificateStatus, - ref DateTime validityTime) + ISystemContext context, + MethodState method, + NodeId objectId, + byte[] certificate, + ref StatusCode certificateStatus, + ref DateTime validityTime) { AuthorizationHelper.HasAuthenticatedSecureChannel(context); @@ -673,11 +675,20 @@ private ServiceResult OnCheckRevocationStatus( chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; - //add GDS Issuer Cert Store Certificates to the Chain validation for consitent behaviour on all Platforms - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(m_configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath)) + //add GDS Issuer Cert Store Certificates to the Chain validation for consistent behaviour on all Platforms + ICertificateStore store = m_configuration.SecurityConfiguration.TrustedIssuerCertificates.OpenStore(); + if (store != null) { - chain.ChainPolicy.ExtraStore.AddRange(store.Enumerate().GetAwaiter().GetResult()); + try + { + chain.ChainPolicy.ExtraStore.AddRange(store.Enumerate().GetAwaiter().GetResult()); + } + finally + { + store.Close(); + } } + using (var x509 = new X509Certificate2(certificate)) { if (chain.Build(x509)) @@ -747,13 +758,13 @@ private ServiceResult OnCheckRevocationStatus( } private ServiceResult OnGetCertificates( - ISystemContext context, - MethodState method, - NodeId objectId, - NodeId applicationId, - NodeId certificateGroupId, - ref NodeId[] certificateTypeIds, - ref byte[][] certificates) + ISystemContext context, + MethodState method, + NodeId objectId, + NodeId applicationId, + NodeId certificateGroupId, + ref NodeId[] certificateTypeIds, + ref byte[][] certificates) { AuthorizationHelper.HasAuthorization(context, AuthorizationHelper.CertificateAuthorityAdminOrSelfAdmin); @@ -797,7 +808,7 @@ private ServiceResult OnGetCertificates( return ServiceResult.Good; } - + private ServiceResult CheckHttpsDomain(ApplicationRecordDataType application, string commonName) { if (application.ApplicationType == ApplicationType.Client) @@ -1302,9 +1313,10 @@ out privateKeyPassword issuerCertificates[0] = certificateGroup.Certificate.RawData; // store new app certificate - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(m_globalDiscoveryServerConfiguration.ApplicationCertificatesStorePath)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(m_globalDiscoveryServerConfiguration.ApplicationCertificatesStorePath); + using (ICertificateStore store = certificateStoreIdentifier.OpenStore()) { - store.Add(certificate).Wait(); + store?.Add(certificate).Wait(); } m_database.SetApplicationCertificate(applicationId, m_certTypeMap[certificateGroup.CertificateType], signedCertificate); @@ -1505,7 +1517,7 @@ protected override NodeState ValidateNode( handle.Validated = true; return handle.Node; } -#endregion + #endregion #region Overridden Methods #endregion @@ -1551,17 +1563,17 @@ protected void SetCertificateGroupNodes(ICertificateGroup certificateGroup) { certificateGroup.DefaultTrustList.Handle = new TrustList( certificateGroup.DefaultTrustList, - certificateGroup.Configuration.TrustedListPath, - certificateGroup.Configuration.IssuerListPath, + new CertificateStoreIdentifier(certificateGroup.Configuration.TrustedListPath), + new CertificateStoreIdentifier(certificateGroup.Configuration.IssuerListPath), new TrustList.SecureAccess(HasTrustListAccess), new TrustList.SecureAccess(HasTrustListAccess)); } } #region AuthorizationHelpers - private void HasTrustListAccess(ISystemContext context, string trustedStorePath) + private void HasTrustListAccess(ISystemContext context, CertificateStoreIdentifier trustedStore) { - AuthorizationHelper.HasTrustListAccess(context, trustedStorePath, m_certTypeMap, m_database); + AuthorizationHelper.HasTrustListAccess(context, trustedStore, m_certTypeMap, m_database); } #endregion diff --git a/Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs b/Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs index 7f4227db8..1f1b6553c 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/CertificateGroup.cs @@ -34,6 +34,7 @@ using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading.Tasks; +using Opc.Ua.Security; using Opc.Ua.Security.Certificates; namespace Opc.Ua.Gds.Server @@ -46,8 +47,9 @@ public class CertificateGroup : ICertificateGroup public CertificateGroupConfiguration Configuration { get; } public X509Certificate2 Certificate { get; set; } public TrustListState DefaultTrustList { get; set; } - public Boolean UpdateRequired { get; set; } - public string TrustedIssuerCertificatesStorePath { get; } = null; + public bool UpdateRequired { get; set; } + public CertificateStoreIdentifier AuthoritiesStore { get; } + public CertificateStoreIdentifier IssuerCertificatesStore { get; } #endregion public CertificateGroup() @@ -61,13 +63,11 @@ protected CertificateGroup( [Optional] string trustedIssuerCertificatesStorePath ) { - AuthoritiesStorePath = authoritiesStorePath; - AuthoritiesStoreType = CertificateStoreIdentifier.DetermineStoreType(AuthoritiesStorePath); + AuthoritiesStore = new CertificateStoreIdentifier(authoritiesStorePath, false); Configuration = certificateGroupConfiguration; if (trustedIssuerCertificatesStorePath != null) { - - TrustedIssuerCertificatesStorePath = trustedIssuerCertificatesStorePath; + IssuerCertificatesStore = new CertificateStoreIdentifier(trustedIssuerCertificatesStorePath); } SubjectName = Configuration.SubjectName.Replace("localhost", Utils.GetHostName()); } @@ -77,7 +77,8 @@ public virtual async Task Init() { Utils.LogInfo("InitializeCertificateGroup: {0}", SubjectName); - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(AuthoritiesStorePath)) + ICertificateStore store = AuthoritiesStore.OpenStore(); + try { X509Certificate2Collection certificates = await store.Enumerate().ConfigureAwait(false); foreach (var certificate in certificates) @@ -103,6 +104,10 @@ public virtual async Task Init() } } } + finally + { + store?.Close(); + } if (Certificate == null) { @@ -176,19 +181,20 @@ public async virtual Task RevokeCertificateAsync( X509Certificate2 certificate) { X509CRL crl = await RevokeCertificateAsync( - AuthoritiesStorePath, + AuthoritiesStore, certificate, null).ConfigureAwait(false); // Also update TrustedList CRL so registerd Applications can get the new CRL if (crl != null) { - await UpdateAuthorityCertInCertificateStore(Configuration.TrustedListPath).ConfigureAwait(false); + var certificateStoreIdentifier = new CertificateStoreIdentifier(Configuration.TrustedListPath); + await UpdateAuthorityCertInCertificateStore(certificateStoreIdentifier).ConfigureAwait(false); //Also update TrustedIssuerCertificates Store - if (!String.IsNullOrEmpty(TrustedIssuerCertificatesStorePath)) + if (IssuerCertificatesStore != null) { - await UpdateAuthorityCertInCertificateStore(TrustedIssuerCertificatesStorePath).ConfigureAwait(false); + await UpdateAuthorityCertInCertificateStore(IssuerCertificatesStore).ConfigureAwait(false); } } @@ -326,26 +332,26 @@ string subjectName .SetCAConstraint() .SetRSAKeySize(Configuration.CACertificateKeySize) .CreateForRSA() - .AddToStoreAsync( - AuthoritiesStoreType, - AuthoritiesStorePath).ConfigureAwait(false)) + .AddToStoreAsync(AuthoritiesStore).ConfigureAwait(false)) { // save only public key Certificate = new X509Certificate2(newCertificate.RawData); // initialize revocation list - X509CRL crl = await RevokeCertificateAsync(AuthoritiesStorePath, newCertificate, null).ConfigureAwait(false); + X509CRL crl = await RevokeCertificateAsync(AuthoritiesStore, newCertificate, null).ConfigureAwait(false); //Update TrustedList Store if (crl != null) { - await UpdateAuthorityCertInCertificateStore(Configuration.TrustedListPath).ConfigureAwait(false); + // TODO: make CA trust selectable + var certificateStoreIdentifier = new CertificateStoreIdentifier(Configuration.TrustedListPath); + await UpdateAuthorityCertInCertificateStore(certificateStoreIdentifier).ConfigureAwait(false); // Update TrustedIssuerCertificates Store - if (!string.IsNullOrEmpty(TrustedIssuerCertificatesStorePath)) + if (IssuerCertificatesStore != null) { - await UpdateAuthorityCertInCertificateStore(TrustedIssuerCertificatesStorePath).ConfigureAwait(false); + await UpdateAuthorityCertInCertificateStore(IssuerCertificatesStore).ConfigureAwait(false); } } return Certificate; @@ -361,8 +367,8 @@ string subjectName public virtual async Task LoadSigningKeyAsync(X509Certificate2 signingCertificate, string signingKeyPassword) { CertificateIdentifier certIdentifier = new CertificateIdentifier(signingCertificate) { - StorePath = AuthoritiesStorePath, - StoreType = AuthoritiesStoreType + StorePath = AuthoritiesStore.StorePath, + StoreType = AuthoritiesStore.StoreType }; return await certIdentifier.LoadPrivateKey(signingKeyPassword).ConfigureAwait(false); } @@ -373,7 +379,7 @@ public virtual async Task LoadSigningKeyAsync(X509Certificate2 /// The CRL number is increased by one and existing CRL for the issuer are deleted from the store. /// public static async Task RevokeCertificateAsync( - string storePath, + CertificateStoreIdentifier storeIdentifier, X509Certificate2 certificate, string issuerKeyFilePassword = null ) @@ -408,7 +414,8 @@ public static async Task RevokeCertificateAsync( } X509Certificate2 certCA = null; - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(storePath)) + ICertificateStore store = storeIdentifier.OpenStore(); + try { if (store == null) { @@ -422,8 +429,8 @@ public static async Task RevokeCertificateAsync( } CertificateIdentifier certCAIdentifier = new CertificateIdentifier(certCA) { - StorePath = storePath, - StoreType = CertificateStoreIdentifier.DetermineStoreType(storePath) + StorePath = store.StorePath, + StoreType = store.StoreType }; X509Certificate2 certCAWithPrivateKey = await certCAIdentifier.LoadPrivateKey(issuerKeyFilePassword).ConfigureAwait(false); @@ -453,6 +460,9 @@ public static async Task RevokeCertificateAsync( { await store.DeleteCRL(caCrl).ConfigureAwait(false); } + } + finally + { store.Close(); } return updatedCRL; @@ -463,47 +473,55 @@ public static async Task RevokeCertificateAsync( /// /// Updates the certificate authority certificate and CRL in the provided CertificateStore /// - /// the path of the CertificateStore + /// The store which contains the authority ceritificate. (trusted or issuer) /// - protected async Task UpdateAuthorityCertInCertificateStore(string path) + protected async Task UpdateAuthorityCertInCertificateStore(CertificateStoreIdentifier trustedOrIssuerStoreIdentifier) { - if (!String.IsNullOrEmpty(path)) + ICertificateStore authorityStore = AuthoritiesStore.OpenStore(); + ICertificateStore trustedOrIssuerStore = trustedOrIssuerStoreIdentifier.OpenStore(); + try { - using (ICertificateStore authorityStore = CertificateStoreIdentifier.OpenStore(AuthoritiesStorePath)) - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(path)) + if (authorityStore == null || trustedOrIssuerStore == null) { - X509Certificate2Collection certificates = await authorityStore.Enumerate().ConfigureAwait(false); - foreach (var certificate in certificates) + throw new ServiceResultException("Unable to update authority certificate in stores"); + } + + X509Certificate2Collection certificates = await authorityStore.Enumerate().ConfigureAwait(false); + foreach (var certificate in certificates) + { + if (X509Utils.CompareDistinguishedName(certificate.Subject, SubjectName)) { - if (X509Utils.CompareDistinguishedName(certificate.Subject, SubjectName)) + X509Certificate2Collection certs = await trustedOrIssuerStore.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); + if (certs.Count == 0) { - X509Certificate2Collection certs = await store.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); - if (certs.Count == 0) + using (var x509 = new X509Certificate2(certificate.RawData)) { - using (var x509 = new X509Certificate2(certificate.RawData)) - { - await store.Add(x509).ConfigureAwait(false); - } + await trustedOrIssuerStore.Add(x509).ConfigureAwait(false); } + } - // delete existing CRL in trusted list - foreach (var crl in await store.EnumerateCRLs(certificate, false).ConfigureAwait(false)) + // delete existing CRL in trusted list + foreach (var crl in await trustedOrIssuerStore.EnumerateCRLs(certificate, false).ConfigureAwait(false)) + { + if (crl.VerifySignature(certificate, false)) { - if (crl.VerifySignature(certificate, false)) - { - await store.DeleteCRL(crl).ConfigureAwait(false); - } + await trustedOrIssuerStore.DeleteCRL(crl).ConfigureAwait(false); } + } - // copy latest CRL to trusted list - foreach (var crl in await authorityStore.EnumerateCRLs(certificate, true).ConfigureAwait(false)) - { - await store.AddCRL(crl).ConfigureAwait(false); - } + // copy latest CRL to trusted list + foreach (var crl in await authorityStore.EnumerateCRLs(certificate, true).ConfigureAwait(false)) + { + await trustedOrIssuerStore.AddCRL(crl).ConfigureAwait(false); } } } } + finally + { + authorityStore?.Close(); + trustedOrIssuerStore?.Close(); + } } protected X509SubjectAltNameExtension GetAltNameExtensionFromCSRInfo(Org.BouncyCastle.Asn1.Pkcs.CertificationRequestInfo info) @@ -536,8 +554,6 @@ protected X509SubjectAltNameExtension GetAltNameExtensionFromCSRInfo(Org.BouncyC #region Protected Properties protected string SubjectName { get; } - protected string AuthoritiesStorePath { get; } - protected string AuthoritiesStoreType { get; } #endregion } diff --git a/Libraries/Opc.Ua.Gds.Server.Common/GlobalDiscoverySampleServer.cs b/Libraries/Opc.Ua.Gds.Server.Common/GlobalDiscoverySampleServer.cs index dc413c427..9b18c0430 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/GlobalDiscoverySampleServer.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/GlobalDiscoverySampleServer.cs @@ -251,15 +251,19 @@ private bool VerifiyApplicationRegistered(Session session) configuration = new GlobalDiscoveryServerConfiguration(); } //check if application certificate is in the Store of the GDS - using (ICertificateStore ApplicationsStore = CertificateStoreIdentifier.OpenStore(configuration.ApplicationCertificatesStorePath)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(configuration.ApplicationCertificatesStorePath); + using (ICertificateStore ApplicationsStore = certificateStoreIdentifier.OpenStore()) { var matchingCerts = ApplicationsStore.FindByThumbprint(applicationInstanceCertificate.Thumbprint).Result; if (matchingCerts.Contains(applicationInstanceCertificate)) + { applicationRegistered = true; + } } //check if application certificate is revoked - using (ICertificateStore AuthoritiesStore = CertificateStoreIdentifier.OpenStore(configuration.AuthoritiesStorePath)) + certificateStoreIdentifier = new CertificateStoreIdentifier(configuration.AuthoritiesStorePath); + using (ICertificateStore AuthoritiesStore = certificateStoreIdentifier.OpenStore()) { var crls = AuthoritiesStore.EnumerateCRLs().Result; foreach (X509CRL crl in crls) diff --git a/Libraries/Opc.Ua.Gds.Server.Common/ICertificateGroup.cs b/Libraries/Opc.Ua.Gds.Server.Common/ICertificateGroup.cs index f8fe6055c..d1eac1926 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/ICertificateGroup.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/ICertificateGroup.cs @@ -60,7 +60,8 @@ public interface ICertificateGroup NodeId Id { get; set; } NodeId CertificateType { get; set; } CertificateGroupConfiguration Configuration { get; } - string TrustedIssuerCertificatesStorePath { get; } + CertificateStoreIdentifier AuthoritiesStore { get; } + CertificateStoreIdentifier IssuerCertificatesStore { get; } X509Certificate2 Certificate { get; set; } TrustListState DefaultTrustList { get; set; } bool UpdateRequired { get; set; } @@ -68,7 +69,7 @@ public interface ICertificateGroup ICertificateGroup Create( string authoritiesStorePath, CertificateGroupConfiguration certificateGroupConfiguration, - [Optional] string trustedIssuerCertificatesStorePath); + [Optional] string issuerCertificatesStorePath); Task Init(); diff --git a/Libraries/Opc.Ua.Gds.Server.Common/RoleBasedUserManagement/AuthorizationHelper.cs b/Libraries/Opc.Ua.Gds.Server.Common/RoleBasedUserManagement/AuthorizationHelper.cs index a89062cc1..bdb896f38 100644 --- a/Libraries/Opc.Ua.Gds.Server.Common/RoleBasedUserManagement/AuthorizationHelper.cs +++ b/Libraries/Opc.Ua.Gds.Server.Common/RoleBasedUserManagement/AuthorizationHelper.cs @@ -76,18 +76,18 @@ public static void HasAuthorization(ISystemContext context, IEnumerable ro /// Checks if the current session (context) is allowed to access the trust List (has roles CertificateAuthorityAdmin, SecurityAdmin or ) /// /// the current - /// path of the trustList, needed to check for Application Self Admin privilege + /// Certificate store Identifier needed to check for Application Self Admin privilege /// all supported cert types, needed to check for Application Self Admin privilege /// all registered applications , needed to check for Application Self Admin privilege /// - public static void HasTrustListAccess(ISystemContext context, string trustedStorePath, Dictionary certTypeMap, IApplicationsDatabase applicationsDatabase) + public static void HasTrustListAccess(ISystemContext context, CertificateStoreIdentifier trustedStore, Dictionary certTypeMap, IApplicationsDatabase applicationsDatabase) { var roles = new List { GdsRole.CertificateAuthorityAdmin, Role.SecurityAdmin }; if (HasRole(context.UserIdentity, roles)) return; - if (!string.IsNullOrEmpty(trustedStorePath) && certTypeMap != null && applicationsDatabase != null && - CheckSelfAdminPrivilege(context.UserIdentity, trustedStorePath, certTypeMap, applicationsDatabase)) + if (trustedStore != null && certTypeMap != null && applicationsDatabase != null && + CheckSelfAdminPrivilege(context.UserIdentity, trustedStore, certTypeMap, applicationsDatabase)) return; throw new ServiceResultException(StatusCodes.BadUserAccessDenied, $"At least one of the Roles {string.Join(", ", roles)} or ApplicationSelfAdminPrivilege is required to use the TrustList"); @@ -140,7 +140,7 @@ private static bool CheckSelfAdminPrivilege(IUserIdentity userIdentity, NodeId a return false; } - private static bool CheckSelfAdminPrivilege(IUserIdentity userIdentity, string trustedStorePath, Dictionary certTypeMap, IApplicationsDatabase applicationsDatabase) + private static bool CheckSelfAdminPrivilege(IUserIdentity userIdentity, CertificateStoreIdentifier trustedStore, Dictionary certTypeMap, IApplicationsDatabase applicationsDatabase) { GdsRoleBasedIdentity identity = userIdentity as GdsRoleBasedIdentity; if (identity != null) @@ -148,7 +148,7 @@ private static bool CheckSelfAdminPrivilege(IUserIdentity userIdentity, string t foreach (var certType in certTypeMap.Values) { applicationsDatabase.GetApplicationTrustLists(identity.ApplicationId, certType, out var trustListId); - if (trustedStorePath == trustListId) + if (trustedStore.StorePath == trustListId) { return true; } diff --git a/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs b/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs index eb2a11a07..55d0f35d7 100644 --- a/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs +++ b/Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs @@ -69,7 +69,11 @@ ApplicationConfiguration configuration : base(server, configuration) { - m_rejectedStorePath = configuration.SecurityConfiguration.RejectedCertificateStore?.StorePath; + string rejectedStorePath = configuration.SecurityConfiguration.RejectedCertificateStore?.StorePath; + if (!string.IsNullOrEmpty(rejectedStorePath)) + { + m_rejectedStore = new CertificateStoreIdentifier(rejectedStorePath); + } m_certificateGroups = new List(); m_configuration = configuration; // TODO: configure cert groups in configuration @@ -78,8 +82,8 @@ ApplicationConfiguration configuration BrowseName = Opc.Ua.BrowseNames.DefaultApplicationGroup, CertificateTypes = new NodeId[] { ObjectTypeIds.RsaSha256ApplicationCertificateType }, ApplicationCertificate = configuration.SecurityConfiguration.ApplicationCertificate, - IssuerStorePath = configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath, - TrustedStorePath = configuration.SecurityConfiguration.TrustedPeerCertificates.StorePath + IssuerStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedIssuerCertificates.StorePath), + TrustedStore = new CertificateStoreIdentifier(configuration.SecurityConfiguration.TrustedPeerCertificates.StorePath) }; m_certificateGroups.Add(defaultApplicationGroup); } @@ -216,8 +220,8 @@ public void CreateServerConfiguration( certGroup.CertificateTypes; certGroup.Node.TrustList.Handle = new TrustList( certGroup.Node.TrustList, - certGroup.TrustedStorePath, - certGroup.IssuerStorePath, + certGroup.TrustedStore, + certGroup.IssuerStore, new TrustList.SecureAccess(HasApplicationSecureAdminAccess), new TrustList.SecureAccess(HasApplicationSecureAdminAccess)); certGroup.Node.ClearChangeMasks(systemContext, true); @@ -306,7 +310,7 @@ public NamespaceMetadataState CreateNamespaceMetadataState(string namespaceUri) /// public void HasApplicationSecureAdminAccess(ISystemContext context) { - HasApplicationSecureAdminAccess(context, ""); + HasApplicationSecureAdminAccess(context, null); } @@ -317,7 +321,7 @@ public void HasApplicationSecureAdminAccess(ISystemContext context) /// /// /// - public void HasApplicationSecureAdminAccess(ISystemContext context, string _) + public void HasApplicationSecureAdminAccess(ISystemContext context, CertificateStoreIdentifier _) { OperationContext operationContext = (context as SystemContext)?.OperationContext as OperationContext; if (operationContext != null) @@ -486,7 +490,9 @@ private ServiceResult UpdateCertificate( updateCertificate.CertificateWithPrivateKey.Dispose(); updateCertificate.CertificateWithPrivateKey = certOnly; } - using (ICertificateStore issuerStore = CertificateStoreIdentifier.OpenStore(certificateGroup.IssuerStorePath)) + + ICertificateStore issuerStore = certificateGroup.IssuerStore.OpenStore(); + try { foreach (var issuer in updateCertificate.IssuerCollection) { @@ -501,6 +507,10 @@ private ServiceResult UpdateCertificate( } } } + finally + { + issuerStore?.Close(); + } Server.ReportCertificateUpdatedAuditEvent(context, objectId, method, inputArguments, certificateGroupId, certificateTypeId); } @@ -603,14 +613,16 @@ private ServiceResult GetRejectedList( ref byte[][] certificates) { HasApplicationSecureAdminAccess(context); - //No rejected store configured - if (m_rejectedStorePath == null) + + // No rejected store configured + if (m_rejectedStore == null) { certificates = Array.Empty(); return StatusCodes.Good; } - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(m_rejectedStorePath)) - { + + ICertificateStore store = m_rejectedStore.OpenStore(); + try { X509Certificate2Collection collection = store.Enumerate().Result; List rawList = new List(); foreach (var cert in collection) @@ -619,6 +631,10 @@ private ServiceResult GetRejectedList( } certificates = rawList.ToArray(); } + finally + { + store?.Close(); + } return StatusCodes.Good; } @@ -800,15 +816,15 @@ private class ServerCertificateGroup public CertificateGroupState Node; public NodeId[] CertificateTypes; public CertificateIdentifier ApplicationCertificate; - public string IssuerStorePath; - public string TrustedStorePath; + public CertificateStoreIdentifier IssuerStore; + public CertificateStoreIdentifier TrustedStore; public UpdateCertificateData UpdateCertificate; } private ServerConfigurationState m_serverConfigurationNode; private ApplicationConfiguration m_configuration; private IList m_certificateGroups; - private readonly string m_rejectedStorePath; + private CertificateStoreIdentifier m_rejectedStore; private Dictionary m_namespaceMetadataStates = new Dictionary(); #endregion } diff --git a/Libraries/Opc.Ua.Server/Configuration/TrustList.cs b/Libraries/Opc.Ua.Server/Configuration/TrustList.cs index 202f014ec..c33e77a21 100644 --- a/Libraries/Opc.Ua.Server/Configuration/TrustList.cs +++ b/Libraries/Opc.Ua.Server/Configuration/TrustList.cs @@ -48,14 +48,14 @@ public class TrustList /// public TrustList( TrustListState node, - string trustedListPath, - string issuerListPath, + CertificateStoreIdentifier trustedListStore, + CertificateStoreIdentifier issuerListStore, SecureAccess readAccess, SecureAccess writeAccess) { m_node = node; - m_trustedStorePath = trustedListPath; - m_issuerStorePath = issuerListPath; + m_trustedStore = trustedListStore; + m_issuerStore = issuerListStore; m_readAccess = readAccess; m_writeAccess = writeAccess; @@ -75,8 +75,8 @@ public TrustList( /// Delegate to validate the access to the trust list. /// /// - /// the path to identify the trustList - public delegate void SecureAccess(ISystemContext context, string trustedStorePath); + /// the path to identify the trustList + public delegate void SecureAccess(ISystemContext context, CertificateStoreIdentifier trustedStore); #endregion #region Private Methods @@ -141,7 +141,8 @@ private ServiceResult Open( SpecifiedLists = (uint)masks }; - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(m_trustedStorePath)) + ICertificateStore store = m_trustedStore.OpenStore(); + try { if ((masks & TrustListMasks.TrustedCertificates) != 0) { @@ -160,8 +161,13 @@ private ServiceResult Open( } } } + finally + { + store?.Close(); + } - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(m_issuerStorePath)) + store = m_issuerStore.OpenStore(); + try { if ((masks & TrustListMasks.IssuerCertificates) != 0) { @@ -180,6 +186,10 @@ private ServiceResult Open( } } } + finally + { + store?.Close(); + } if (m_readMode) { @@ -370,28 +380,28 @@ private ServiceResult CloseAndUpdate( TrustListMasks updateMasks = TrustListMasks.None; if ((masks & TrustListMasks.IssuerCertificates) != 0) { - if (UpdateStoreCertificates(m_issuerStorePath, issuerCertificates).GetAwaiter().GetResult()) + if (UpdateStoreCertificates(m_issuerStore, issuerCertificates).GetAwaiter().GetResult()) { updateMasks |= TrustListMasks.IssuerCertificates; } } if ((masks & TrustListMasks.IssuerCrls) != 0) { - if (UpdateStoreCrls(m_issuerStorePath, issuerCrls).GetAwaiter().GetResult()) + if (UpdateStoreCrls(m_issuerStore, issuerCrls).GetAwaiter().GetResult()) { updateMasks |= TrustListMasks.IssuerCrls; } } if ((masks & TrustListMasks.TrustedCertificates) != 0) { - if (UpdateStoreCertificates(m_trustedStorePath, trustedCertificates).GetAwaiter().GetResult()) + if (UpdateStoreCertificates(m_trustedStore, trustedCertificates).GetAwaiter().GetResult()) { updateMasks |= TrustListMasks.TrustedCertificates; } } if ((masks & TrustListMasks.TrustedCrls) != 0) { - if (UpdateStoreCrls(m_trustedStorePath, trustedCrls).GetAwaiter().GetResult()) + if (UpdateStoreCrls(m_trustedStore, trustedCrls).GetAwaiter().GetResult()) { updateMasks |= TrustListMasks.TrustedCrls; } @@ -461,13 +471,19 @@ private ServiceResult AddCertificate( result = StatusCodes.BadCertificateInvalid; } - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(isTrustedCertificate ? m_trustedStorePath : m_issuerStorePath)) + var storeIdentifier = isTrustedCertificate? m_trustedStore : m_issuerStore; + ICertificateStore store = storeIdentifier.OpenStore(); + try { - if (cert != null) + if (cert != null && store != null) { store.Add(cert).GetAwaiter().GetResult(); } } + finally + { + store?.Close(); + } m_node.LastUpdateTime.Value = DateTime.UtcNow; } @@ -505,7 +521,8 @@ private ServiceResult RemoveCertificate( } else { - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(isTrustedCertificate ? m_trustedStorePath : m_issuerStorePath)) + var storeIdentifier = isTrustedCertificate ? m_trustedStore : m_issuerStore; + using (ICertificateStore store = storeIdentifier.OpenStore()) { var certCollection = store.FindByThumbprint(thumbprint).GetAwaiter().GetResult(); @@ -596,13 +613,14 @@ private TrustListDataType DecodeTrustListData( } private async Task UpdateStoreCrls( - string storePath, + CertificateStoreIdentifier storeIdentifier, X509CRLCollection updatedCrls) { bool result = true; try { - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(storePath)) + ICertificateStore store = storeIdentifier.OpenStore(); + try { var storeCrls = await store.EnumerateCRLs().ConfigureAwait(false); foreach (var crl in storeCrls) @@ -624,6 +642,10 @@ private async Task UpdateStoreCrls( await store.AddCRL(crl).ConfigureAwait(false); } } + finally + { + store?.Close(); + } } catch { @@ -633,13 +655,14 @@ private async Task UpdateStoreCrls( } private async Task UpdateStoreCertificates( - string storePath, + CertificateStoreIdentifier storeIdentifier, X509Certificate2Collection updatedCerts) { bool result = true; try { - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(storePath)) + ICertificateStore store = storeIdentifier.OpenStore(); + try { var storeCerts = await store.Enumerate().ConfigureAwait(false); foreach (var cert in storeCerts) @@ -661,6 +684,10 @@ private async Task UpdateStoreCertificates( await store.Add(cert).ConfigureAwait(false); } } + finally + { + store?.Close(); + } } catch { @@ -673,7 +700,7 @@ private void HasSecureReadAccess(ISystemContext context) { if (m_readAccess != null) { - m_readAccess.Invoke(context, m_trustedStorePath); + m_readAccess.Invoke(context, m_trustedStore); } else { @@ -685,7 +712,7 @@ private void HasSecureWriteAccess(ISystemContext context) { if (m_writeAccess != null) { - m_writeAccess.Invoke(context, m_trustedStorePath); + m_writeAccess.Invoke(context, m_trustedStore); } else { @@ -700,8 +727,8 @@ private void HasSecureWriteAccess(ISystemContext context) private SecureAccess m_writeAccess; private NodeId m_sessionId; private uint m_fileHandle; - private readonly string m_trustedStorePath; - private readonly string m_issuerStorePath; + private readonly CertificateStoreIdentifier m_trustedStore; + private readonly CertificateStoreIdentifier m_issuerStore; private TrustListState m_node; private Stream m_strm; private bool m_readMode; diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs index 1e7270934..43ac29014 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs @@ -776,6 +776,7 @@ private void Initialize() m_trustedIssuerCertificates = new CertificateTrustList(); m_trustedPeerCertificates = new CertificateTrustList(); m_nonceLength = 32; + m_maxRejectedCertificates = 5; m_autoAcceptUntrustedCertificates = false; m_rejectSHA1SignedCertificates = true; m_rejectUnknownRevocationStatus = false; @@ -868,6 +869,23 @@ public CertificateStoreIdentifier RejectedCertificateStore set { m_rejectedCertificateStore = value; } } + /// + /// Gets or sets a value indicating how many certificates are kept + /// in the rejected store before the oldest is removed. + /// + /// + /// This value can be set by applications. + /// The number of certificates to keep in the rejected store before it is updated. + /// to keep all rejected certificates. + /// A negative number to keep no history. + /// + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 8)] + public int MaxRejectedCertificates + { + get { return m_maxRejectedCertificates; } + set { m_maxRejectedCertificates = value; } + } + /// /// Gets or sets a value indicating whether untrusted certificates should be automatically accepted. /// @@ -875,7 +893,7 @@ public CertificateStoreIdentifier RejectedCertificateStore /// This flag can be set to by servers that allow anonymous clients or use user credentials for authentication. /// It can be set by clients that connect to URLs specified in configuration rather than with user entry. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 8)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 9)] public bool AutoAcceptUntrustedCertificates { get { return m_autoAcceptUntrustedCertificates; } @@ -885,7 +903,7 @@ public bool AutoAcceptUntrustedCertificates /// /// Gets or sets a directory which contains files representing users roles. /// - [DataMember(Order = 9)] + [DataMember(Order = 10)] public string UserRoleDirectory { get { return m_userRoleDirectory; } @@ -898,7 +916,7 @@ public string UserRoleDirectory /// /// This flag can be set to false by servers that accept SHA-1 signed certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 10)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 11)] public bool RejectSHA1SignedCertificates { get { return m_rejectSHA1SignedCertificates; } @@ -911,7 +929,7 @@ public bool RejectSHA1SignedCertificates /// /// This flag can be set to true by servers that must have a revocation list for each CA (even if empty). /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 11)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 12)] public bool RejectUnknownRevocationStatus { get { return m_rejectUnknownRevocationStatus; } @@ -924,7 +942,7 @@ public bool RejectUnknownRevocationStatus /// /// This value can be set to 1024, 2048 or 4096 by servers /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 12)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 13)] public ushort MinimumCertificateKeySize { get { return m_minCertificateKeySize; } @@ -938,7 +956,7 @@ public ushort MinimumCertificateKeySize /// /// This flag can be set to true by applications. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 13)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 14)] public bool UseValidatedCertificates { get { return m_useValidatedCertificates; } @@ -951,7 +969,7 @@ public bool UseValidatedCertificates /// /// It is useful for client/server applications running on the same host and sharing the cert store to autotrust. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 14)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 15)] public bool AddAppCertToTrustedStore { get { return m_addAppCertToTrustedStore; } @@ -964,7 +982,7 @@ public bool AddAppCertToTrustedStore /// /// If set to true the complete certificate chain will be sent for CA signed certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 15)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 16)] public bool SendCertificateChain { get { return m_sendCertificateChain; } @@ -974,7 +992,7 @@ public bool SendCertificateChain /// /// The store containing additional user issuer certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 16)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 17)] public CertificateTrustList UserIssuerCertificates { get @@ -996,7 +1014,7 @@ public CertificateTrustList UserIssuerCertificates /// /// The store containing trusted user certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 17)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 18)] public CertificateTrustList TrustedUserCertificates { get @@ -1018,7 +1036,7 @@ public CertificateTrustList TrustedUserCertificates /// /// The store containing additional Https issuer certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 18)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 19)] public CertificateTrustList HttpsIssuerCertificates { get @@ -1040,7 +1058,7 @@ public CertificateTrustList HttpsIssuerCertificates /// /// The store containing trusted Https certificates. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 19)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 20)] public CertificateTrustList TrustedHttpsCertificates { get @@ -1067,7 +1085,7 @@ public CertificateTrustList TrustedHttpsCertificates /// If set to true the server nonce validation errors are suppressed. /// Please set this flag to true only in close and secured networks since it can cause security vulnerabilities. /// - [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 20)] + [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 21)] public bool SuppressNonceValidationErrors { get { return m_suppressNonceValidationErrors; } @@ -1085,6 +1103,7 @@ public bool SuppressNonceValidationErrors private CertificateTrustList m_trustedUserCertificates; private int m_nonceLength; private CertificateStoreIdentifier m_rejectedCertificateStore; + private int m_maxRejectedCertificates; private bool m_autoAcceptUntrustedCertificates; private string m_userRoleDirectory; private bool m_rejectSHA1SignedCertificates; @@ -2862,7 +2881,6 @@ public CertificateTrustList() /// private void Initialize() { - m_lock = new object(); m_trustedCertificates = new CertificateIdentifierCollection(); } @@ -2878,7 +2896,8 @@ private void Initialize() /// The list of trusted certificates. /// /// - /// The list of trusted certificates is set when TrustedCertificates is not a null value, otherwise new CertificateIdentifierCollection is set. + /// The list of trusted certificates is set when TrustedCertificates is not a null value, + /// otherwise new CertificateIdentifierCollection is set. /// [DataMember(IsRequired = false, EmitDefaultValue = false, Order = 3)] public CertificateIdentifierCollection TrustedCertificates diff --git a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd index 3557ebb9a..d3d201f67 100644 --- a/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd +++ b/Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.xsd @@ -46,6 +46,7 @@ + diff --git a/Stack/Opc.Ua.Core/Schema/SecuredApplicationHelpers.cs b/Stack/Opc.Ua.Core/Schema/SecuredApplicationHelpers.cs index edda07362..b395942d4 100644 --- a/Stack/Opc.Ua.Core/Schema/SecuredApplicationHelpers.cs +++ b/Stack/Opc.Ua.Core/Schema/SecuredApplicationHelpers.cs @@ -390,7 +390,7 @@ public static byte CalculateSecurityLevel(MessageSecurityMode mode, string polic { case SecurityPolicies.Basic128Rsa15: { - Utils.LogWarning("Deprecated Security Policy Basic128Rsa15 requested - Not rcommended."); + Utils.LogWarning("Deprecated Security Policy Basic128Rsa15 requested - Not recommended."); result = 2; break; } @@ -398,7 +398,7 @@ public static byte CalculateSecurityLevel(MessageSecurityMode mode, string polic case SecurityPolicies.Basic256Sha256: result = 6; break; case SecurityPolicies.Aes128_Sha256_RsaOaep: result = 8; break; case SecurityPolicies.Aes256_Sha256_RsaPss: result = 10; break; - case SecurityPolicies.None: + case SecurityPolicies.None: result = 0; break; default: Utils.LogWarning("Security level requested for unknown Security Policy {policy}. Returning security level 0", policyUri); return 0; diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs index e5b62e7d5..4d739a75c 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs @@ -78,7 +78,6 @@ public override bool Equals(object obj) return true; } - if (!(obj is CertificateIdentifier id)) { return false; @@ -168,11 +167,11 @@ public async Task LoadPrivateKeyEx(ICertificatePasswordProvide { if (this.StoreType != CertificateStoreType.X509Store) { - using (ICertificateStore store = CertificateStoreIdentifier.CreateStore(StoreType)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(this.StorePath, this.StoreType, false); + using (ICertificateStore store = certificateStoreIdentifier.OpenStore()) { if (store.SupportsLoadPrivateKey) { - store.Open(this.StorePath, false); string password = passwordProvider?.GetPassword(this); m_certificate = await store.LoadPrivateKey(this.Thumbprint, this.SubjectName, password).ConfigureAwait(false); return m_certificate; @@ -200,10 +199,9 @@ public async Task Find(bool needPrivateKey) else { // open store. - using (ICertificateStore store = CertificateStoreIdentifier.CreateStore(StoreType)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(StorePath, false); + using (ICertificateStore store = certificateStoreIdentifier.OpenStore()) { - store.Open(StorePath, false); - X509Certificate2Collection collection = await store.Enumerate().ConfigureAwait(false); certificate = Find(collection, m_thumbprint, m_subjectName, needPrivateKey); @@ -639,6 +637,9 @@ public void Close() /// public string StorePath => string.Empty; + /// + public bool NoPrivateKeys => true; + /// public async Task Enumerate() { @@ -763,6 +764,12 @@ public Task DeleteCRL(X509CRL crl) { throw new ServiceResultException(StatusCodes.BadNotSupported); } + + /// + public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) + { + return Task.CompletedTask; + } #endregion } #endregion diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateStoreIdentifier.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateStoreIdentifier.cs index 403ecc924..5cce8d7b0 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateStoreIdentifier.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateStoreIdentifier.cs @@ -13,6 +13,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. using System; using System.Collections.Generic; using System.IO; +using System.Threading; namespace Opc.Ua { @@ -21,6 +22,41 @@ namespace Opc.Ua /// public partial class CertificateStoreIdentifier : IFormattable, ICloneable { + #region Constructors + /// + /// Ctor of a certificate store. + /// + public CertificateStoreIdentifier() + { + m_noPrivateKeys = true; + } + + /// + /// Ctor of a certificate store. + /// + /// The store path of the store. + /// If the store supports no private keys. + public CertificateStoreIdentifier(string storePath, bool noPrivateKeys = true) + { + StorePath = storePath; + StoreType = DetermineStoreType(storePath); + m_noPrivateKeys = noPrivateKeys; + } + + /// + /// Ctor of a certificate store. + /// + /// The store path of the store. + /// The type of the store. + /// If the store supports no private keys. + public CertificateStoreIdentifier(string storePath, string storeType, bool noPrivateKeys = true) + { + StorePath = storePath; + StoreType = storeType; + m_noPrivateKeys = noPrivateKeys; + } + #endregion + #region ICloneable /// public virtual object Clone() @@ -180,15 +216,44 @@ public static ICertificateStore CreateStore(string storeTypeName) /// Returns an object to access the store containing the certificates. /// /// - /// Opens an instance of the store which contains public keys. + /// Opens a cached instance of the store which contains public and private keys. + /// To take advantage of the certificate cache use . + /// Disposing the store has no functional impact but may + /// enforce unnecessary refresh of the cached certificate store. /// /// A disposable instance of the . public virtual ICertificateStore OpenStore() { - ICertificateStore store = CreateStore(this.StoreType); - store.Open(this.StorePath); + ICertificateStore store = m_store; + + // determine if the store configuration changed + if (store != null && + (store.StoreType != this.StoreType || + store.StorePath != this.StorePath || + store.NoPrivateKeys != this.m_noPrivateKeys)) + { + var previousStore = Interlocked.CompareExchange(ref m_store, null, store); + previousStore?.Dispose(); + store = null; + } + + // create and open the store + if (store == null && !string.IsNullOrEmpty(this.StoreType) && !string.IsNullOrEmpty(this.StorePath)) + { + store = CreateStore(this.StoreType); + var currentStore = Interlocked.CompareExchange(ref m_store, store, null); + if (currentStore != null) + { + Utils.SilentDispose(store); + store = currentStore; + } + } + + store?.Open(this.StorePath, m_noPrivateKeys); + return store; } + /// /// Returns an object to access the store containing the certificates. /// @@ -198,13 +263,19 @@ public virtual ICertificateStore OpenStore() /// location of the store /// Indicates whether NO private keys are found in the store. Default true. /// A disposable instance of the . - public static ICertificateStore OpenStore(string path, bool noPrivateKeys = true) + [Obsolete("Use non static OpenStore method to take advantage of caching.")] + public static ICertificateStore OpenStore(string path, bool noPrivateKeys) { ICertificateStore store = CertificateStoreIdentifier.CreateStore(CertificateStoreIdentifier.DetermineStoreType(path)); store.Open(path, noPrivateKeys); return store; } #endregion + + #region Private Variables + private ICertificateStore m_store; + private bool m_noPrivateKeys; + #endregion } #region CertificateStoreType Class diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs index 36ac98130..160c7d5ba 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs @@ -39,32 +39,6 @@ namespace Opc.Ua public partial class CertificateTrustList : CertificateStoreIdentifier { #region Public Methods - /// - /// Returns an object to access the store containing the certificate of the trustlist. - /// - /// - /// Opens a cached instance of the store which contains public keys. - /// To take advantage of the certificate cache use - /// and let the CertificateTrustList handle the dispose. - /// Disposing the store has no functional impact but may - /// enforce unnecessary refresh of the cached certificate store. - /// - /// A disposable instance of the . - public override ICertificateStore OpenStore() - { - lock (m_lock) - { - if (m_store == null || - m_store.StoreType != this.StoreType || - m_store.StorePath != this.StorePath) - { - m_store = CreateStore(this.StoreType); - } - m_store.Open(this.StorePath, true); - return m_store; - } - } - /// /// Returns the certificates in the trust list. /// @@ -105,11 +79,6 @@ public async Task GetCertificates() return collection; } #endregion - - #region Private Members - private object m_lock = new object(); - private ICertificateStore m_store; - #endregion } #endregion } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs index 03b6e4982..fa20f6b36 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs @@ -30,6 +30,9 @@ namespace Opc.Ua /// public class CertificateValidator : ICertificateValidator { + // default number of rejected certificates for history + const int kDefaultMaxRejectedCertificates = 5; + #region Constructors /// /// The default constructor. @@ -43,6 +46,7 @@ public CertificateValidator() m_rejectUnknownRevocationStatus = false; m_minimumCertificateKeySize = CertificateFactory.DefaultKeySize; m_useValidatedCertificates = false; + m_maxRejectedCertificates = kDefaultMaxRejectedCertificates; } #endregion @@ -137,14 +141,11 @@ private void InternalUpdate( m_trustedCertificateStore = null; m_trustedCertificateList = null; - if (trustedStore != null) { - m_trustedCertificateStore = new CertificateStoreIdentifier(); - - m_trustedCertificateStore.StoreType = trustedStore.StoreType; - m_trustedCertificateStore.StorePath = trustedStore.StorePath; - m_trustedCertificateStore.ValidationOptions = trustedStore.ValidationOptions; + m_trustedCertificateStore = new CertificateStoreIdentifier(trustedStore.StorePath) { + ValidationOptions = trustedStore.ValidationOptions + }; if (trustedStore.TrustedCertificates != null) { @@ -155,14 +156,11 @@ private void InternalUpdate( m_issuerCertificateStore = null; m_issuerCertificateList = null; - if (issuerStore != null) { - m_issuerCertificateStore = new CertificateStoreIdentifier(); - - m_issuerCertificateStore.StoreType = issuerStore.StoreType; - m_issuerCertificateStore.StorePath = issuerStore.StorePath; - m_issuerCertificateStore.ValidationOptions = issuerStore.ValidationOptions; + m_issuerCertificateStore = new CertificateStoreIdentifier(issuerStore.StorePath) { + ValidationOptions = issuerStore.ValidationOptions + }; if (issuerStore.TrustedCertificates != null) { @@ -172,7 +170,6 @@ private void InternalUpdate( } m_rejectedCertificateStore = null; - if (rejectedCertificateStore != null) { m_rejectedCertificateStore = (CertificateStoreIdentifier)rejectedCertificateStore.MemberwiseClone(); @@ -219,6 +216,10 @@ public virtual async Task Update(SecurityConfiguration configuration) { m_useValidatedCertificates = configuration.UseValidatedCertificates; } + if ((m_protectFlags & ProtectFlags.MaxRejectedCertificates) == 0) + { + m_maxRejectedCertificates = configuration.MaxRejectedCertificates; + } } finally { @@ -422,6 +423,41 @@ public bool UseValidatedCertificates } } + /// + /// Limits the number of certificates which are kept + /// in the history before more rejected certificates are added. + /// A negative value means no history is kept. + /// A value of 0 means all history is kept. + /// + public int MaxRejectedCertificates + { + get => m_maxRejectedCertificates; + set + { + m_semaphore.Wait(); + bool updateStore = false; + try + { + m_protectFlags |= ProtectFlags.MaxRejectedCertificates; + if (m_maxRejectedCertificates != value) + { + m_maxRejectedCertificates = value; + updateStore = true; + } + } + finally + { + m_semaphore.Release(); + } + + if (updateStore) + { + // update the rejected store + Task.Run(async () => await SaveCertificatesAsync(new X509Certificate2Collection()).ConfigureAwait(false)); + } + } + } + /// /// Validates the specified certificate against the trust list. /// @@ -562,7 +598,7 @@ private void HandleCertificateValidationException(ServiceResultException se, X50 certificate, se.Result.StatusCode); // save the chain in rejected store to allow to add certs to a trusted or issuer store - SaveCertificates(chain); + Task.Run(async () => await SaveCertificatesAsync(chain).ConfigureAwait(false)); LogInnerServiceResults(LogLevel.Error, se.Result.InnerResult); throw new ServiceResultException(se, StatusCodes.BadCertificateInvalid); @@ -626,7 +662,7 @@ private void HandleCertificateValidationException(ServiceResultException se, X50 LogInnerServiceResults(LogLevel.Error, se.Result.InnerResult); // save the chain in rejected store to allow to add cert to a trusted or issuer store - SaveCertificates(chain); + Task.Run(async () => await SaveCertificatesAsync(chain).ConfigureAwait(false)); throw new ServiceResultException(se, StatusCodes.BadCertificateInvalid); } @@ -669,61 +705,53 @@ private static void LogInnerServiceResults(LogLevel logLevel, ServiceResult resu /// /// Saves the certificate in the rejected certificate store. /// - private void SaveCertificate(X509Certificate2 certificate) + private Task SaveCertificateAsync(X509Certificate2 certificate, CancellationToken ct = default) { - SaveCertificates(new X509Certificate2Collection { certificate }); + return SaveCertificatesAsync(new X509Certificate2Collection { certificate }, ct); } /// /// Saves the certificate chain in the rejected certificate store. + /// Times out after 5 seconds waiting to gracefully reduce high CPU load. /// - private void SaveCertificates(X509Certificate2Collection certificateChain) + private async Task SaveCertificatesAsync(X509Certificate2Collection certificateChain, CancellationToken ct = default) { + // max time to wait for semaphore + const int kSaveCertificatesTimeout = 5000; + + var rejectedCertificateStore = m_rejectedCertificateStore; + if (rejectedCertificateStore == null) + { + return; + } + try { - m_semaphore.Wait(); + await m_semaphore.WaitAsync(kSaveCertificatesTimeout, ct).ConfigureAwait(false); - if (m_rejectedCertificateStore != null) + try { - Utils.LogTrace("Writing rejected certificate chain to: {0}", m_rejectedCertificateStore); + Utils.LogTrace("Writing rejected certificate chain to: {0}", rejectedCertificateStore); + + ICertificateStore store = rejectedCertificateStore.OpenStore(); try { - ICertificateStore store = m_rejectedCertificateStore.OpenStore(); - try - { - bool leafCertificate = true; - foreach (var certificate in certificateChain) - { - try - { - store.Add(certificate).GetAwaiter().GetResult(); - if (!leafCertificate) - { - Utils.LogCertificate("Saved issuer certificate: ", certificate); - } - leafCertificate = false; - } - catch (ArgumentException aex) - { - // just notify why the certificate cannot be added - Utils.LogCertificate(aex.Message, certificate); - } - } - } - finally - { - store.Close(); - } + // number of certs for history + current chain + await store.AddRejected(certificateChain, m_maxRejectedCertificates).ConfigureAwait(false); } - catch (Exception e) + finally { - Utils.LogError(e, "Could not write certificate to directory: {0}", m_rejectedCertificateStore); + store.Close(); } } + finally + { + m_semaphore.Release(); + } } - finally + catch (Exception e) { - m_semaphore.Release(); + Utils.LogTrace("Could not write certificate to directory: {0} Error:{1}", rejectedCertificateStore, e.Message); } } @@ -753,22 +781,24 @@ private async Task GetTrustedCertificateAsync(X509Certifi if (m_trustedCertificateStore != null) { ICertificateStore store = m_trustedCertificateStore.OpenStore(); - - try + if (store != null) { - X509Certificate2Collection trusted = await store.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); - - for (int ii = 0; ii < trusted.Count; ii++) + try { - if (Utils.IsEqual(trusted[ii].RawData, certificate.RawData)) + X509Certificate2Collection trusted = await store.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); + + for (int ii = 0; ii < trusted.Count; ii++) { - return new CertificateIdentifier(trusted[ii], m_trustedCertificateStore.ValidationOptions); + if (Utils.IsEqual(trusted[ii].RawData, certificate.RawData)) + { + return new CertificateIdentifier(trusted[ii], m_trustedCertificateStore.ValidationOptions); + } } } - } - finally - { - store.Close(); + finally + { + store.Close(); + } } } @@ -1488,7 +1518,7 @@ public void ValidateDomains(X509Certificate2 serverCertificate, ConfiguredEndpoi // write the invalid certificate to rejected store if specified. Utils.LogCertificate(LogLevel.Error, "Certificate rejected. Reason={0}.", serverCertificate, Redact.Create(serviceResult)); - SaveCertificate(serverCertificate); + Task.Run(async () => await SaveCertificateAsync(serverCertificate).ConfigureAwait(false)); } throw serviceResult; @@ -1757,7 +1787,8 @@ private enum ProtectFlags RejectSHA1SignedCertificates = 2, RejectUnknownRevocationStatus = 4, MinimumCertificateKeySize = 8, - UseValidatedCertificates = 16 + UseValidatedCertificates = 16, + MaxRejectedCertificates = 32 }; #endregion @@ -1779,6 +1810,7 @@ private enum ProtectFlags private bool m_rejectUnknownRevocationStatus; private ushort m_minimumCertificateKeySize; private bool m_useValidatedCertificates; + private int m_maxRejectedCertificates; #endregion } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs index b897ba1e2..c0fcc4288 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/DirectoryCertificateStore.cs @@ -20,6 +20,7 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. using System.Threading.Tasks; using Opc.Ua.Security.Certificates; using Opc.Ua.Redaction; +using System.Threading; namespace Opc.Ua { @@ -36,6 +37,7 @@ public class DirectoryCertificateStore : ICertificateStore private const string kCrlExtension = ".crl"; private const string kPemExtension = ".pem"; private const string kPfxExtension = ".pfx"; + private const string kCertSearchString = "*.der"; #region Constructors /// @@ -53,8 +55,6 @@ public DirectoryCertificateStore(bool noSubDirs) m_noSubDirs = noSubDirs; m_certificates = new Dictionary(); } - - #endregion #region IDisposable Members @@ -76,11 +76,7 @@ protected virtual void Dispose(bool disposing) { lock (m_lock) { - m_certificates.Clear(); - m_directory = null; - m_certificateSubdir = null; - m_privateKeySubdir = null; - m_crlSubdir = null; + ClearCertificates(); m_lastDirectoryCheck = DateTime.MinValue; } } @@ -105,12 +101,14 @@ public void Open(string location, bool noPrivateKeys = false) lock (m_lock) { string trimmedLocation = Utils.ReplaceSpecialFolderNames(location); - if (m_directory?.FullName.Equals(trimmedLocation, StringComparison.Ordinal) != true || + var directory = !string.IsNullOrEmpty(trimmedLocation) ? new DirectoryInfo(trimmedLocation) : null; + if (directory == null || + m_directory?.FullName.Equals(directory.FullName, StringComparison.Ordinal) != true || NoPrivateKeys != noPrivateKeys) { NoPrivateKeys = noPrivateKeys; StorePath = location; - m_directory = new DirectoryInfo(trimmedLocation); + m_directory = directory; if (m_noSubDirs) { m_certificateSubdir = m_directory; @@ -123,8 +121,43 @@ public void Open(string location, bool noPrivateKeys = false) m_crlSubdir = new DirectoryInfo(Path.Combine(m_directory.FullName, kCrlPath)); m_privateKeySubdir = !noPrivateKeys ? new DirectoryInfo(Path.Combine(m_directory.FullName, kPrivateKeyPath)) : null; } - m_certificates.Clear(); + + // force load + ClearCertificates(); m_lastDirectoryCheck = DateTime.MinValue; + + // create folders if they do not exist + try + { + if (!m_directory.Exists) + { + m_directory.Create(); + } + + if (!m_certificateSubdir.Exists) + { + m_certificateSubdir.Create(); + } + + if (noPrivateKeys) + { + if (!m_crlSubdir.Exists) + { + m_crlSubdir.Create(); + } + } + else + { + if (!m_privateKeySubdir.Exists) + { + m_privateKeySubdir.Create(); + } + } + } + catch (IOException ex) + { + Utils.LogError("Failed to create certificate store: {0}", ex.Message); + } } } } @@ -141,6 +174,9 @@ public void Close() /// public string StorePath { get; private set; } + /// + public bool NoPrivateKeys { get; private set; } + /// public Task Enumerate() { @@ -210,6 +246,88 @@ public Task Add(X509Certificate2 certificate, string password = null) return Task.CompletedTask; } + /// + public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) + { + if (certificates == null) throw new ArgumentNullException(nameof(certificates)); + + var deleteEntryList = new List(); + lock (m_lock) + { + // sync cache if necessary. + Load(null); + + DateTime now = DateTime.UtcNow; + int entries = 0; + foreach (var certificate in certificates) + { + // limit the number of certificates added per call. + if (maxCertificates != 0 && entries >= maxCertificates) + { + break; + } + + if (m_certificates.TryGetValue(certificate.Thumbprint, out Entry entry)) + { + entry.LastWriteTimeUtc = now; + } + else + { + // build file name. + string fileName = GetFileName(certificate); + + // store is created if it does not exist + var fileInfo = WriteFile(certificate.RawData, fileName, false, true); + + // add entry + entry = new Entry { + Certificate = certificate, + CertificateFile = fileInfo, + PrivateKeyFile = null, + CertificateWithPrivateKey = null, + LastWriteTimeUtc = now + }; + + m_certificates[certificate.Thumbprint] = entry; + } + + entries++; + } + + entries = 0; + foreach (Entry entry in m_certificates.Values.OrderByDescending(e => e.LastWriteTimeUtc)) + { + if (++entries > maxCertificates) + { + m_certificates.Remove(entry.Certificate.Thumbprint); + deleteEntryList.Add(entry); + } + } + } + + bool reload = false; + foreach (var entry in deleteEntryList) + { + try + { + // try to delete + entry.CertificateFile.Delete(); + } + catch (IOException) + { + // file to delete may still be in use, force reload + reload = true; + } + } + + lock (m_lock) + { + m_lastDirectoryCheck = reload ? DateTime.MinValue : DateTime.UtcNow; + } + + return Task.CompletedTask; + } + /// public async Task Delete(string thumbprint) { @@ -580,6 +698,7 @@ public Task EnumerateCRLs() var crls = new X509CRLCollection(); // check for CRL. + m_crlSubdir.Refresh(); if (m_crlSubdir.Exists) { foreach (FileInfo file in m_crlSubdir.GetFiles("*" + kCrlExtension)) @@ -673,7 +792,7 @@ public Task DeleteCRL(X509CRL crl) throw new ArgumentNullException(nameof(crl)); } - + m_crlSubdir.Refresh(); if (m_crlSubdir.Exists) { foreach (FileInfo fileInfo in m_crlSubdir.GetFiles("*" + kCrlExtension)) @@ -696,14 +815,6 @@ public Task DeleteCRL(X509CRL crl) #endregion #region Private Methods - /// - /// Gets or sets a value indicating whether any private keys are found in the store. - /// - /// - /// true if [no private keys]; otherwise, false. - /// - private bool NoPrivateKeys { get; set; } - /// /// Reads the current contents of the directory from disk. /// @@ -714,10 +825,7 @@ private IDictionary Load(string thumbprint) DateTime now = DateTime.UtcNow; // refresh the directories. - if (m_certificateSubdir != null) - { - m_certificateSubdir.Refresh(); - } + m_certificateSubdir.Refresh(); if (!NoPrivateKeys) { @@ -727,7 +835,8 @@ private IDictionary Load(string thumbprint) // check if store exists. if (!m_certificateSubdir.Exists) { - m_certificates.Clear(); + m_certificateSubdir.Create(); + ClearCertificates(); return m_certificates; } @@ -739,12 +848,12 @@ private IDictionary Load(string thumbprint) return m_certificates; } - m_certificates.Clear(); + ClearCertificates(); m_lastDirectoryCheck = now; bool incompleteSearch = false; // check for public keys. - foreach (FileInfo file in m_certificateSubdir.GetFiles("*.der")) + foreach (FileInfo file in m_certificateSubdir.GetFiles(kCertSearchString)) { try { @@ -752,7 +861,8 @@ private IDictionary Load(string thumbprint) Certificate = new X509Certificate2(file.FullName), CertificateFile = file, PrivateKeyFile = null, - CertificateWithPrivateKey = null + CertificateWithPrivateKey = null, + LastWriteTimeUtc = file.LastWriteTimeUtc }; if (!NoPrivateKeys) @@ -825,6 +935,14 @@ private Entry Find(string thumbprint) return entry; } + /// + /// Clear the certificate cache. + /// + private void ClearCertificates() + { + m_certificates.Clear(); + } + /// /// Returns the file name to use for the certificate. /// @@ -869,7 +987,7 @@ private static string GetFileName(X509Certificate2 certificate) /// /// Writes the data to a file. /// - private void WriteFile(byte[] data, string fileName, bool includePrivateKey) + private FileInfo WriteFile(byte[] data, string fileName, bool includePrivateKey, bool allowOverride = false) { var filePath = new StringBuilder(); @@ -883,7 +1001,7 @@ private void WriteFile(byte[] data, string fileName, bool includePrivateKey) if (m_privateKeySubdir == null) { // nothing to do - return; + return null; } filePath.Append(m_privateKeySubdir.FullName); } @@ -912,7 +1030,8 @@ private void WriteFile(byte[] data, string fileName, bool includePrivateKey) } // write file. - var writer = new BinaryWriter(fileInfo.Open(FileMode.Create)); + var fileMode = allowOverride ? FileMode.OpenOrCreate : FileMode.Create; + var writer = new BinaryWriter(fileInfo.Open(fileMode, FileAccess.Write)); try { writer.Write(data); @@ -925,6 +1044,8 @@ private void WriteFile(byte[] data, string fileName, bool includePrivateKey) m_certificateSubdir.Refresh(); m_privateKeySubdir?.Refresh(); + + return fileInfo; } #endregion @@ -935,6 +1056,7 @@ private class Entry public X509Certificate2 Certificate; public FileInfo PrivateKeyFile; public X509Certificate2 CertificateWithPrivateKey; + public DateTime LastWriteTimeUtc; } #endregion diff --git a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs index a22d06064..bc514e300 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/ICertificateStore.cs @@ -47,6 +47,14 @@ public interface ICertificateStore : IDisposable /// string StorePath { get; } + /// + /// Gets a value indicating whether any private keys are found in the store. + /// + /// + /// true if [no private keys]; otherwise, false. + /// + bool NoPrivateKeys { get; } + /// /// Enumerates the certificates in the store. /// @@ -59,6 +67,14 @@ public interface ICertificateStore : IDisposable /// The certificate password. Task Add(X509Certificate2 certificate, string password = null); + /// + /// Adds a rejected certificate chain to the store. + /// + /// The certificate collection. + /// The max number of rejected certificates to keep in the store. + /// A negative number keeps no history, 0 is unlimited. + Task AddRejected(X509Certificate2Collection certificates, int maxCertificates); + /// /// Deletes a certificate from the store. /// diff --git a/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs b/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs index 89886df64..8cb54b9aa 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/X509CertificateStore/X509CertificateStore.cs @@ -122,6 +122,9 @@ public void Close() /// public string StorePath => m_storePath; + /// + public bool NoPrivateKeys => m_noPrivateKeys; + /// public Task Enumerate() { @@ -398,6 +401,12 @@ public Task DeleteCRL(X509CRL crl) } } + /// + public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) + { + return Task.CompletedTask; + } + private bool m_noPrivateKeys; private string m_storeName; private string m_storePath; diff --git a/Stack/Opc.Ua.Core/Security/Certificates/X509Utils.cs b/Stack/Opc.Ua.Core/Security/Certificates/X509Utils.cs index e47f1a4c2..e0698029f 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/X509Utils.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/X509Utils.cs @@ -575,7 +575,8 @@ public static X509Certificate2 AddToStore( // add cert to the store. if (!String.IsNullOrEmpty(storePath) && !String.IsNullOrEmpty(storeType)) { - using (ICertificateStore store = Opc.Ua.CertificateStoreIdentifier.CreateStore(storeType)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(storePath, storeType, false); + using (ICertificateStore store = certificateStoreIdentifier.OpenStore()) { if (store == null) { @@ -590,6 +591,43 @@ public static X509Certificate2 AddToStore( return certificate; } + /// + /// Extension to add a certificate to a . + /// + /// + /// Saves also the private key, if available. + /// If written to a Pfx file, the password is used for protection. + /// + /// The certificate to store. + /// The certificate store. + /// The password to use to protect the certificate. + /// + public static X509Certificate2 AddToStore( + this X509Certificate2 certificate, + CertificateStoreIdentifier storeIdentifier, + string password = null) + { + // add cert to the store. + if (storeIdentifier != null) + { + ICertificateStore store = storeIdentifier.OpenStore(); + try + { + if (store == null || store.NoPrivateKeys == true) + { + throw new ArgumentException("Invalid store type"); + } + + store.Add(certificate, password).Wait(); + } + finally + { + store?.Close(); + } + } + return certificate; + } + /// e /// Extension to add a certificate to a . /// @@ -612,14 +650,14 @@ public static async Task AddToStoreAsync( // add cert to the store. if (!String.IsNullOrEmpty(storePath) && !String.IsNullOrEmpty(storeType)) { - using (ICertificateStore store = Opc.Ua.CertificateStoreIdentifier.CreateStore(storeType)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(storePath, storeType, false); + using (ICertificateStore store = certificateStoreIdentifier.OpenStore()) { if (store == null) { throw new ArgumentException("Invalid store type"); } - store.Open(storePath, false); await store.Add(certificate, password).ConfigureAwait(false); store.Close(); } @@ -627,6 +665,44 @@ public static async Task AddToStoreAsync( return certificate; } + /// e + /// Extension to add a certificate to a . + /// + /// + /// Saves also the private key, if available. + /// If written to a Pfx file, the password is used for protection. + /// + /// The certificate to store. + /// Type of certificate store (Directory) . + /// The password to use to protect the certificate. + /// The cancellation token. + public static async Task AddToStoreAsync( + this X509Certificate2 certificate, + CertificateStoreIdentifier storeIdentifier, + string password = null, + CancellationToken ct = default) + { + // add cert to the store. + if (storeIdentifier != null) + { + ICertificateStore store = storeIdentifier.OpenStore(); + try + { + if (store == null) + { + throw new ArgumentException("Invalid store type"); + } + await store.Add(certificate, password).ConfigureAwait(false); + } + finally + { + store?.Close(); + } + } + return certificate; + } + + /// /// Get the hash algorithm from the hash size in bits. /// diff --git a/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs b/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs index ce3591ff8..12dab4a74 100644 --- a/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs +++ b/Tests/Opc.Ua.Configuration.Tests/CertificateStoreTypeTest.cs @@ -109,8 +109,11 @@ public async Task CertificateStoreTypeNoConfigTest() int instancesCreatedWhileOpeningAuthRootStore = TestCertStore.InstancesCreated; Assert.IsTrue(instancesCreatedWhileLoadingConfig < instancesCreatedWhileOpeningAuthRootStore); - CertificateStoreIdentifier.OpenStore(TestCertStore.StoreTypePrefix + trustedUserStorePath); - Assert.IsTrue(instancesCreatedWhileOpeningAuthRootStore < TestCertStore.InstancesCreated); + var certificateStoreIdentifier = new CertificateStoreIdentifier(TestCertStore.StoreTypePrefix + trustedUserStorePath); + using (var store = certificateStoreIdentifier.OpenStore()) + { + Assert.IsTrue(instancesCreatedWhileOpeningAuthRootStore < TestCertStore.InstancesCreated); + } } #endregion Test Methods @@ -186,6 +189,9 @@ public void Close() /// public string StorePath => m_innerStore.StorePath; + /// + public bool NoPrivateKeys => m_innerStore.NoPrivateKeys; + /// public Task Add(X509Certificate2 certificate, string password = null) { @@ -241,6 +247,10 @@ public Task IsRevoked(X509Certificate2 issuer, X509Certificate2 cert public Task LoadPrivateKey(string thumbprint, string subjectName, string password) => m_innerStore.LoadPrivateKey(thumbprint, subjectName, password); + /// + public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) + => m_innerStore.AddRejected(certificates, maxCertificates); + public static int InstancesCreated => s_instancesCreated; #region Private Members diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTest.cs index fdc220e0f..d77170d59 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTest.cs @@ -154,13 +154,13 @@ public async Task VerifyAppCertDirectoryStore() Assert.True(appCertificate.HasPrivateKey); string password = Guid.NewGuid().ToString(); - // pki directory root for app cert var pkiRoot = Path.GetTempPath() + Path.GetRandomFileName() + Path.DirectorySeparatorChar; var storePath = pkiRoot + "own"; + var certificateStoreIdentifier = new CertificateStoreIdentifier(storePath, false); const string storeType = CertificateStoreType.Directory; appCertificate.AddToStore( - storeType, storePath, password + certificateStoreIdentifier, password ); using (var publicKey = new X509Certificate2(appCertificate.RawData)) @@ -201,7 +201,7 @@ public async Task VerifyAppCertDirectoryStore() using (ICertificateStore store = Opc.Ua.CertificateStoreIdentifier.CreateStore(storeType)) { - store.Open(storePath); + store.Open(storePath, false); await store.Delete(publicKey.Thumbprint).ConfigureAwait(false); } } diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs index a53cb5731..02ccf05ae 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateStoreTypeTest.cs @@ -31,12 +31,19 @@ public async Task CertificateStoreTypeConfigTest() int instancesCreatedWhileLoadingConfig = TestCertStore.InstancesCreated; Assert.IsTrue(instancesCreatedWhileLoadingConfig > 0); var trustedIssuers = appConfig.SecurityConfiguration.TrustedIssuerCertificates; - ICertificateStore trustedIssuersStore = trustedIssuers.OpenStore(); - trustedIssuersStore.Close(); - int instancesCreatedWhileOpeningAuthRootStore = TestCertStore.InstancesCreated; - Assert.IsTrue(instancesCreatedWhileLoadingConfig < instancesCreatedWhileOpeningAuthRootStore); - CertificateStoreIdentifier.OpenStore(TestCertStore.StoreTypePrefix + @"CurrentUser\Disallowed"); - Assert.IsTrue(instancesCreatedWhileOpeningAuthRootStore < TestCertStore.InstancesCreated); + using (var trustedIssuersStore = trustedIssuers.OpenStore()) + { + trustedIssuersStore.Close(); + int instancesCreatedWhileOpeningAuthRootStore = TestCertStore.InstancesCreated; + Assert.IsTrue(instancesCreatedWhileLoadingConfig < instancesCreatedWhileOpeningAuthRootStore); + + var certificateStoreIdentifier = new CertificateStoreIdentifier(TestCertStore.StoreTypePrefix + @"CurrentUser\Disallowed"); + using (var store = certificateStoreIdentifier.OpenStore()) + { + + Assert.IsTrue(instancesCreatedWhileOpeningAuthRootStore < TestCertStore.InstancesCreated); + } + } } #endregion Test Methods } @@ -94,6 +101,9 @@ public void Close() /// public string StorePath => m_innerStore.StorePath; + /// + public bool NoPrivateKeys => m_innerStore.NoPrivateKeys; + /// public Task Add(X509Certificate2 certificate, string password = null) { @@ -149,6 +159,10 @@ public Task IsRevoked(X509Certificate2 issuer, X509Certificate2 cert public Task LoadPrivateKey(string thumbprint, string subjectName, string password) => m_innerStore.LoadPrivateKey(thumbprint, subjectName, password); + /// + public Task AddRejected(X509Certificate2Collection certificates, int maxCertificates) + => m_innerStore.AddRejected(certificates, maxCertificates); + public static int InstancesCreated => s_instancesCreated; #region data members diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs index e7d726a30..f7e376b41 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs @@ -227,6 +227,8 @@ public void VerifySelfSignedAppCertsNotTrusted() var serviceResultException = Assert.Throws(() => certValidator.Validate(new X509Certificate2(cert))); Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); } + + Thread.Sleep(1000); Assert.AreEqual(m_appSelfSignedCerts.Count, validator.RejectedStore.Enumerate().GetAwaiter().GetResult().Count); // add auto approver @@ -298,11 +300,114 @@ public async Task VerifySelfSignedAppCertsThrow() var serviceResultException = Assert.Throws(() => certValidator.Validate(new X509Certificate2(cert))); Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); } + + Thread.Sleep(1000); Assert.AreEqual(m_appSelfSignedCerts.Count, validator.RejectedStore.Enumerate().Result.Count); } } } + /// + /// Verify untrusted app certs do not overflow the rejected store. + /// + [Test] + public async Task VerifyRejectedCertsDoNotOverflowStore() + { + // test number of rejected certs + const int kNumberOfRejectCertsHistory = 5; + + // add all certs to issuer store, make sure validation fails. + using (var validator = TemporaryCertValidator.Create(true)) + { + foreach (var cert in m_appSelfSignedCerts) + { + await validator.IssuerStore.Add(cert).ConfigureAwait(false); + } + X509Certificate2Collection certificates = await validator.IssuerStore.Enumerate().ConfigureAwait(false); + Assert.AreEqual(m_appSelfSignedCerts.Count, certificates.Count); + + var certValidator = validator.Update(); + certValidator.MaxRejectedCertificates = kNumberOfRejectCertsHistory; + try + { + await Task.Delay(1000).ConfigureAwait(false); + + foreach (var cert in m_appCerts) + { + var certs = new X509Certificate2Collection(cert); + certs.AddRange(m_caChain); + var serviceResultException = Assert.Throws(() => certValidator.Validate(certs)); + Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); + } + + foreach (var cert in m_notYetValidAppCerts) + { + var certs = new X509Certificate2Collection(cert); + certs.AddRange(m_caChain); + var serviceResultException = Assert.Throws(() => certValidator.Validate(certs)); + Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); + } + + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.GreaterOrEqual(m_caChain.Length + kNumberOfRejectCertsHistory + 1, certificates.Count); + + foreach (var cert in m_appSelfSignedCerts) + { + var serviceResultException = Assert.Throws(() => certValidator.Validate(new X509Certificate2Collection(cert))); + Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); + } + + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.GreaterOrEqual(kNumberOfRejectCertsHistory + 1, certificates.Count); + + // override with the same content + foreach (var cert in m_appSelfSignedCerts) + { + var serviceResultException = Assert.ThrowsAsync(async () => await certValidator.ValidateAsync(new X509Certificate2Collection(cert), CancellationToken.None).ConfigureAwait(false)); + Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); + } + + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.GreaterOrEqual(kNumberOfRejectCertsHistory + 1, certificates.Count); + + // test setter if overflow certs are not deleted + certValidator.MaxRejectedCertificates = 300; + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.GreaterOrEqual(kNumberOfRejectCertsHistory + 1, certificates.Count); + + // test setter if overflow certs are deleted + certValidator.MaxRejectedCertificates = 3; + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.LessOrEqual(3, certificates.Count); + + // test setter if allcerts are deleted + certValidator.MaxRejectedCertificates = -1; + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.LessOrEqual(0, certificates.Count); + + // ensure no certs are added to the rejected store + foreach (var cert in m_appSelfSignedCerts) + { + var serviceResultException = Assert.ThrowsAsync(async () => await certValidator.ValidateAsync(new X509Certificate2Collection(cert), CancellationToken.None).ConfigureAwait(false)); + Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); + } + await Task.Delay(1000).ConfigureAwait(false); + certificates = await validator.RejectedStore.Enumerate().ConfigureAwait(false); + Assert.LessOrEqual(0, certificates.Count); + } + finally + { + certValidator.MaxRejectedCertificates = kNumberOfRejectCertsHistory; + } + } + } + /// /// Verify self signed app certs are trusted. /// @@ -974,7 +1079,7 @@ public void TestEventHandler() public async Task TestSHA1Rejected(bool trusted, bool rejectSHA1) { #if NET472_OR_GREATER || NETCOREAPP3_1_OR_GREATER - Assert.Ignore("Create SHA1 certificates is unsupported on this .NET version"); + Assert.Ignore("To create SHA1 certificates is unsupported on this .NET version"); #endif var cert = CertificateFactory.CreateCertificate(null, null, "CN=SHA1 signed, O=OPC Foundation", null) .SetHashAlgorithm(HashAlgorithmName.SHA1) diff --git a/Tests/Opc.Ua.Gds.Tests/CertificateGroupTests.cs b/Tests/Opc.Ua.Gds.Tests/CertificateGroupTests.cs index 9b84b8351..b356a7d5f 100644 --- a/Tests/Opc.Ua.Gds.Tests/CertificateGroupTests.cs +++ b/Tests/Opc.Ua.Gds.Tests/CertificateGroupTests.cs @@ -54,7 +54,8 @@ public async Task TestCreateCACertificateAsyncCertIsInTrustedStoreAsync() var certificateGroup = new CertificateGroup().Create(_path + "/authorities", configuration); var certificate = await certificateGroup.CreateCACertificateAsync(configuration.SubjectName).ConfigureAwait(false); Assert.NotNull(certificate); - using (ICertificateStore trustedStore = CertificateStoreIdentifier.OpenStore(configuration.TrustedListPath)) + var certificateStoreIdentifier = new CertificateStoreIdentifier(configuration.TrustedListPath); + using (ICertificateStore trustedStore = certificateStoreIdentifier.OpenStore()) { X509Certificate2Collection certs = await trustedStore.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); Assert.IsTrue(certs.Count == 1); @@ -66,26 +67,30 @@ public async Task TestCreateCACertificateAsyncCertIsInTrustedIssuerStoreAsync() { var applicatioConfiguration = new ApplicationConfiguration(); applicatioConfiguration.SecurityConfiguration = new SecurityConfiguration(); - applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath = _path + "/issuers"; - applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StoreType = "Directory"; + applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath = _path + Path.DirectorySeparatorChar + "issuers"; + applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StoreType = CertificateStoreType.Directory; var cgConfiguration = new CertificateGroupConfiguration(); cgConfiguration.SubjectName = "CN=GDS Test CA, O=OPC Foundation"; cgConfiguration.BaseStorePath = _path; - var certificateGroup = new CertificateGroup().Create(_path + "/authorities", cgConfiguration, applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath); + var certificateGroup = new CertificateGroup().Create(_path + Path.DirectorySeparatorChar + "authorities", cgConfiguration, applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath); X509Certificate2 certificate = await certificateGroup.CreateCACertificateAsync(cgConfiguration.SubjectName).ConfigureAwait(false); Assert.NotNull(certificate); - using (ICertificateStore trustedStore = CertificateStoreIdentifier.OpenStore(applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath)) + using (ICertificateStore trustedStore = applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.OpenStore()) { X509Certificate2Collection certs = await trustedStore.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); Assert.IsTrue(certs.Count == 1); + X509CRLCollection crls = await trustedStore.EnumerateCRLs(certificate).ConfigureAwait(false); + Assert.AreEqual(1, crls.Count); } X509Certificate2 certificateUpdated = await certificateGroup.CreateCACertificateAsync(cgConfiguration.SubjectName).ConfigureAwait(false); Assert.NotNull(certificateUpdated); - using (ICertificateStore trustedStore = CertificateStoreIdentifier.OpenStore(applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.StorePath)) + using (ICertificateStore trustedStore = applicatioConfiguration.SecurityConfiguration.TrustedIssuerCertificates.OpenStore()) { + X509Certificate2Collection certs = await trustedStore.FindByThumbprint(certificate.Thumbprint).ConfigureAwait(false); + Assert.IsTrue(certs.Count == 1); X509CRLCollection crls = await trustedStore.EnumerateCRLs(certificateUpdated).ConfigureAwait(false); - Assert.IsTrue(crls.Count == 1); + Assert.AreEqual(1, crls.Count); } } #endregion diff --git a/Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs b/Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs index 28754d6a6..e7ea4a97e 100644 --- a/Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs +++ b/Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs @@ -55,7 +55,8 @@ public GlobalDiscoveryTestClient(bool autoAccept, string storeType = Certificate public IUserIdentity Anonymous { get; private set; } public ApplicationTestData OwnApplicationTestData { get; private set; } public ApplicationConfiguration Configuration { get; private set; } - #region public methods + + #region Public methods public async Task LoadClientConfiguration(int port = -1) { ApplicationInstance.MessageDlg = new ApplicationMessageDlg(); @@ -201,6 +202,7 @@ public string ReadLogFile() return File.ReadAllText(Utils.ReplaceSpecialFolderNames(Configuration.TraceConfiguration.OutputFilePath)); } #endregion + #region Private Methods private async Task ApplyNewApplicationInstanceCertificateAsync(byte[] certificate, byte[] privateKey) { diff --git a/Tests/Opc.Ua.Gds.Tests/PushTest.cs b/Tests/Opc.Ua.Gds.Tests/PushTest.cs index 47080f98a..5ba625797 100644 --- a/Tests/Opc.Ua.Gds.Tests/PushTest.cs +++ b/Tests/Opc.Ua.Gds.Tests/PushTest.cs @@ -890,29 +890,29 @@ private async Task UpdateStoreCertificates( /// private async Task CreateCATestCerts(string tempStorePath) { - Assert.IsTrue(EraseStore(tempStorePath)); - + var certificateStoreIdentifier = new CertificateStoreIdentifier(tempStorePath, false); + Assert.IsTrue(EraseStore(certificateStoreIdentifier)); string subjectName = "CN=CA Test Cert, O=OPC Foundation"; X509Certificate2 newCACert = await CertificateFactory.CreateCertificate( null, null, subjectName, null) .SetCAConstraint() .CreateForRSA() - .AddToStoreAsync(CertificateStoreType.Directory, tempStorePath).ConfigureAwait(false); + .AddToStoreAsync(certificateStoreIdentifier).ConfigureAwait(false); m_caCert = newCACert; // initialize cert revocation list (CRL) - X509CRL newCACrl = await CertificateGroup.RevokeCertificateAsync(tempStorePath, newCACert).ConfigureAwait(false); + X509CRL newCACrl = await CertificateGroup.RevokeCertificateAsync(certificateStoreIdentifier, newCACert).ConfigureAwait(false); m_caCrl = newCACrl; } - private bool EraseStore(string storePath) + private bool EraseStore(CertificateStoreIdentifier storeIdentifier) { bool result = true; try { - using (ICertificateStore store = CertificateStoreIdentifier.OpenStore(storePath)) + using (ICertificateStore store = storeIdentifier.OpenStore()) { var storeCerts = store.Enumerate().Result; foreach (var cert in storeCerts) From ee8205d2486855f0055ac4c968b7acd0a90d8ddc Mon Sep 17 00:00:00 2001 From: Bo Biene <23037659+BoBiene@users.noreply.github.com> Date: Tue, 3 Sep 2024 15:26:20 +0200 Subject: [PATCH 2/6] Add a 1.05 update notice (#2709) * Add a 1.05 update notice Co-authored-by: Martin Regen --- README.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3a4ca6442..9d0bd673c 100644 --- a/README.md +++ b/README.md @@ -29,14 +29,28 @@ More samples based on the official [Nuget](https://www.nuget.org/packages/OPCFou * Sessions and Subscriptions. * A [PubSub](Docs/PubSub.md) library with samples. -#### **New in 1.5.374.70 +### Key Features and Updates in OPC UA 1.05 + +- **Security Enhancements**: Improved encryption and authentication mechanisms. +- **CRL Support**: Added Certificate Revocation List support for X509Store on Windows. +- **Performance Improvements**: Faster binary encoding and decoding, reducing memory usage and latency. +- **Role-Based Management**: Support for WellKnownRoles and RoleBasedUserManagement [WellKnownRoles & RoleBasedUserManagement](Docs/RoleBasedUserManagement.md). +- **Improved Logging**: Enhanced logging with `ILogger` and `EventSource`. + +#### Breaking Changes and Heads-Up when upgrading from 1.04 to 1.05 + +- A few features are still missing to fully comply for 1.05 (e.g. ECC support), but certification for V1.04 is still possible with the 1.05 release. +- **Thread Safety and Locking**: Improved thread safety and reduced locking in secure channel operations. +- **Audit and Redaction**: New interfaces for auditing and redacting sensitive information. + +#### **New in 1.05.374.70** * CRL Support for the X509Store on Windows #### **New in 1.05.373** * 1.05 Nodeset * Support for [WellKnownRoles & RoleBasedUserManagement](Docs/RoleBasedUserManagement.md). -#### **New in 1.4.368** +#### **New in 1.04.368** * Improved support for [Logging](Docs/Logging.md) with `ILogger` and `EventSource`. * Support for custom certificate stores with refactored `ICertificateStore` and `CertificateStoreType` interface. * Client and Server support for [TransferSubscriptions](Docs/TransferSubscription.md). From 4bbb0624571c5310d2016eb3722d19e95999accf Mon Sep 17 00:00:00 2001 From: kristianmo <32481434+kristianmo@users.noreply.github.com> Date: Tue, 3 Sep 2024 20:00:48 +0200 Subject: [PATCH 3/6] Client.ReadNodes: Check that DataValue.Value is not null when verifying the event notifier for an object (#2729) * Check that DataValue and DataValue.Value is null when verifying the event notifier for an Object --- Libraries/Opc.Ua.Client/Session/Session.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Libraries/Opc.Ua.Client/Session/Session.cs b/Libraries/Opc.Ua.Client/Session/Session.cs index 686d54747..f9b8149b2 100644 --- a/Libraries/Opc.Ua.Client/Session/Session.cs +++ b/Libraries/Opc.Ua.Client/Session/Session.cs @@ -1,7 +1,7 @@ /* ======================================================================== * Copyright (c) 2005-2020 The OPC Foundation, Inc. All rights reserved. * - * OPC Foundation MIT License 1.00 + * OPC Foundation MIT License 1.00 * * Permission is hereby granted, free of charge, to any person * obtaining a copy of this software and associated documentation @@ -4404,7 +4404,7 @@ private Node ProcessReadResponse( value = attributes[Attributes.EventNotifier]; - if (value == null) + if (value == null || value.Value is null) { throw ServiceResultException.Create(StatusCodes.BadUnexpectedError, "Object does not support the EventNotifier attribute."); } From 6833b91fd734d48ed1ed6e8b3d50b26c9c5e7d25 Mon Sep 17 00:00:00 2001 From: Martin Regen Date: Thu, 5 Sep 2024 16:00:11 +0200 Subject: [PATCH 4/6] August release testing: Improve cert blob decoding hot path and fix cert validator semaphore regression (#2748) - cert validator semaphore WaitAsync timeout was not correctly handled, also move all semaphores out of try/finally block to follow best practice - ParseCertBlob shows up in the hot path. API is changed to use ReadOnlyMemory. - Enum.ToString() was flagged in perf analyzer, changed logger call --- .../Common/AsnUtils.cs | 4 +- .../Server/OpcUaServerEventSource.cs | 30 ++++++--- .../Opc.Ua.Server/Server/StandardServer.cs | 2 +- .../Certificates/CertificateFactory.cs | 12 ++-- .../Certificates/CertificateValidator.cs | 62 ++++++++++--------- Stack/Opc.Ua.Core/Types/Utils/Utils.cs | 27 ++++---- .../Certificates/CertificateFactoryTest.cs | 2 +- .../Certificates/CertificateValidatorTest.cs | 2 +- 8 files changed, 80 insertions(+), 61 deletions(-) diff --git a/Libraries/Opc.Ua.Security.Certificates/Common/AsnUtils.cs b/Libraries/Opc.Ua.Security.Certificates/Common/AsnUtils.cs index 7b89c27dd..f4e3ff0e0 100644 --- a/Libraries/Opc.Ua.Security.Certificates/Common/AsnUtils.cs +++ b/Libraries/Opc.Ua.Security.Certificates/Common/AsnUtils.cs @@ -169,12 +169,12 @@ internal static void WriteKeyParameterInteger(this AsnWriter writer, ReadOnlySpa /// return the byte array which contains the X509 blob. /// /// The encoded CRL or certificate sequence. - public static byte[] ParseX509Blob(byte[] blob) + public static ReadOnlyMemory ParseX509Blob(ReadOnlyMemory blob) { try { var x509Reader = new AsnReader(blob, AsnEncodingRules.DER); - byte[] peekBlob = blob.AsSpan(0, x509Reader.PeekContentBytes().Length + 4).ToArray(); + ReadOnlyMemory peekBlob = blob.Slice(0, x509Reader.PeekContentBytes().Length + 4); AsnReader seqReader = x509Reader.ReadSequence(Asn1Tag.Sequence); if (seqReader != null) { diff --git a/Libraries/Opc.Ua.Server/Server/OpcUaServerEventSource.cs b/Libraries/Opc.Ua.Server/Server/OpcUaServerEventSource.cs index 676fb2c2a..93879b3c1 100644 --- a/Libraries/Opc.Ua.Server/Server/OpcUaServerEventSource.cs +++ b/Libraries/Opc.Ua.Server/Server/OpcUaServerEventSource.cs @@ -27,6 +27,7 @@ * http://opcfoundation.org/License/MIT/1.00/ * ======================================================================*/ +using System; using System.Diagnostics.Tracing; using Microsoft.Extensions.Logging; using static Opc.Ua.Utils; @@ -90,20 +91,12 @@ public void SendResponse(uint channelId, uint requestId) } /// - /// A server call message. + /// A server call message, called from ServerCallNative. Do not call directly.. /// [Event(kServerCallId, Message = kServerCallMessage, Level = EventLevel.Informational)] public void ServerCall(string requestType, uint requestId) { - if (IsEnabled()) - { - WriteEvent(kServerCallId, requestType, requestId); - } - else if ((TraceMask & TraceMasks.ServiceDetail) != 0 && - Logger.IsEnabled(LogLevel.Trace)) - { - LogTrace(m_serverCallEventId, kServerCallMessage, requestType, requestId); - } + WriteEvent(kServerCallId, requestType, requestId); } /// @@ -141,6 +134,23 @@ public void MonitoredItemReady(uint id, string state) } } + /// + /// A server call message. + /// + [NonEvent] + public void ServerCallNative(RequestType requestType, uint requestId) + { + if (IsEnabled()) + { + ServerCall(Enum.GetName(typeof(RequestType), requestType), requestId); + } + else if ((TraceMask & TraceMasks.ServiceDetail) != 0 && + Logger.IsEnabled(LogLevel.Trace)) + { + LogTrace(m_serverCallEventId, kServerCallMessage, Enum.GetName(typeof(RequestType), requestType), requestId); + } + } + /// /// Log a WriteValue. /// diff --git a/Libraries/Opc.Ua.Server/Server/StandardServer.cs b/Libraries/Opc.Ua.Server/Server/StandardServer.cs index a5d19b836..706f6647b 100644 --- a/Libraries/Opc.Ua.Server/Server/StandardServer.cs +++ b/Libraries/Opc.Ua.Server/Server/StandardServer.cs @@ -2581,7 +2581,7 @@ protected virtual OperationContext ValidateRequest(RequestHeader requestHeader, OperationContext context = ServerInternal.SessionManager.ValidateRequest(requestHeader, requestType); - ServerUtils.EventLog.ServerCall(context.RequestType.ToString(), context.RequestId); + ServerUtils.EventLog.ServerCallNative(context.RequestType, context.RequestId); // notify the request manager. ServerInternal.RequestManager.RequestReceived(context); diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateFactory.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateFactory.cs index 2a2fde212..2f3acaf63 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateFactory.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateFactory.cs @@ -55,13 +55,18 @@ public static class CertificateFactory /// The encoded data. /// if set to true the copy of the certificate in the cache is used. /// The certificate. - public static X509Certificate2 Create(byte[] encodedData, bool useCache) + public static X509Certificate2 Create(ReadOnlyMemory encodedData, bool useCache) { +#if NET6_0_OR_GREATER + var certificate = new X509Certificate2(encodedData.Span); +#else + var certificate = new X509Certificate2(encodedData.ToArray()); +#endif if (useCache) { - return Load(new X509Certificate2(encodedData), false); + return Load(certificate, false); } - return new X509Certificate2(encodedData); + return certificate; } /// @@ -114,7 +119,6 @@ public static X509Certificate2 Load(X509Certificate2 certificate, bool ensurePri { Utils.LogWarning("Certificate cache has {0} certificates in it.", m_certificates.Count); } - } return certificate; } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs index fa20f6b36..4e30f5d1a 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs @@ -117,10 +117,10 @@ public virtual void Update( CertificateTrustList trustedStore, CertificateStoreIdentifier rejectedCertificateStore) { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - InternalUpdate(issuerStore, trustedStore, rejectedCertificateStore); } finally @@ -186,10 +186,10 @@ public virtual async Task Update(SecurityConfiguration configuration) throw new ArgumentNullException(nameof(configuration)); } + await m_semaphore.WaitAsync().ConfigureAwait(false); + try { - await m_semaphore.WaitAsync().ConfigureAwait(false); - InternalUpdate( configuration.TrustedIssuerCertificates, configuration.TrustedPeerCertificates, @@ -237,10 +237,10 @@ public virtual async Task Update(SecurityConfiguration configuration) /// public virtual async Task UpdateCertificate(SecurityConfiguration securityConfiguration) { + await m_semaphore.WaitAsync().ConfigureAwait(false); + try { - await m_semaphore.WaitAsync().ConfigureAwait(false); - securityConfiguration.ApplicationCertificate.Certificate = null; await securityConfiguration.ApplicationCertificate.LoadPrivateKeyEx( @@ -268,10 +268,10 @@ await securityConfiguration.ApplicationCertificate.LoadPrivateKeyEx( /// public void ResetValidatedCertificates() { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - InternalResetValidatedCertificates(); } finally @@ -301,10 +301,10 @@ public bool AutoAcceptUntrustedCertificates get => m_autoAcceptUntrustedCertificates; set { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - m_protectFlags |= ProtectFlags.AutoAcceptUntrustedCertificates; if (m_autoAcceptUntrustedCertificates != value) { @@ -327,10 +327,10 @@ public bool RejectSHA1SignedCertificates get => m_rejectSHA1SignedCertificates; set { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - m_protectFlags |= ProtectFlags.RejectSHA1SignedCertificates; if (m_rejectSHA1SignedCertificates != value) { @@ -353,10 +353,10 @@ public bool RejectUnknownRevocationStatus get => m_rejectUnknownRevocationStatus; set { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - m_protectFlags |= ProtectFlags.RejectUnknownRevocationStatus; if (m_rejectUnknownRevocationStatus != value) { @@ -379,10 +379,10 @@ public ushort MinimumCertificateKeySize get => m_minimumCertificateKeySize; set { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - m_protectFlags |= ProtectFlags.MinimumCertificateKeySize; if (m_minimumCertificateKeySize != value) { @@ -405,10 +405,10 @@ public bool UseValidatedCertificates get => m_useValidatedCertificates; set { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - m_protectFlags |= ProtectFlags.UseValidatedCertificates; if (m_useValidatedCertificates != value) { @@ -547,10 +547,10 @@ public virtual void Validate(X509Certificate2Collection chain, ConfiguredEndpoin try { + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - InternalValidateAsync(chain, endpoint).GetAwaiter().GetResult(); // add to list of validated certificates. @@ -569,10 +569,10 @@ public virtual void Validate(X509Certificate2Collection chain, ConfiguredEndpoin } // add to list of peers. + m_semaphore.Wait(); + try { - m_semaphore.Wait(); - Utils.LogCertificate(LogLevel.Warning, "Validation errors suppressed: ", certificate); m_validatedCertificates[certificate.Thumbprint] = new X509Certificate2(certificate.RawData); } @@ -600,7 +600,7 @@ private void HandleCertificateValidationException(ServiceResultException se, X50 // save the chain in rejected store to allow to add certs to a trusted or issuer store Task.Run(async () => await SaveCertificatesAsync(chain).ConfigureAwait(false)); - LogInnerServiceResults(LogLevel.Error, se.Result.InnerResult); + LogInnerServiceResults(LogLevel.Information, se.Result.InnerResult); throw new ServiceResultException(se, StatusCodes.BadCertificateInvalid); } @@ -727,7 +727,11 @@ private async Task SaveCertificatesAsync(X509Certificate2Collection certificateC try { - await m_semaphore.WaitAsync(kSaveCertificatesTimeout, ct).ConfigureAwait(false); + if (!await m_semaphore.WaitAsync(kSaveCertificatesTimeout, ct).ConfigureAwait(false)) + { + Utils.LogTrace("SaveCertificatesAsync: Timed out waiting, skip job to reduce CPU load."); + return; + } try { @@ -830,7 +834,7 @@ private bool Match( } // check for serial number match. - if (!String.IsNullOrEmpty(serialNumber)) + if (!string.IsNullOrEmpty(serialNumber)) { if (certificate.SerialNumber != serialNumber) { @@ -840,7 +844,7 @@ private bool Match( } // check for authority key id match. - if (!String.IsNullOrEmpty(authorityKeyId)) + if (!string.IsNullOrEmpty(authorityKeyId)) { X509SubjectKeyIdentifierExtension subjectKeyId = X509Extensions.FindExtension(certificate); @@ -1736,7 +1740,7 @@ private bool FindDomain(X509Certificate2 serverCertificate, Uri endpointUrl) bool isLocalHost = false; if (endpointUrl.HostNameType == UriHostNameType.Dns) { - if (String.Equals(dnsHostName, "localhost", StringComparison.OrdinalIgnoreCase)) + if (string.Equals(dnsHostName, "localhost", StringComparison.OrdinalIgnoreCase)) { isLocalHost = true; } @@ -1763,8 +1767,8 @@ private bool FindDomain(X509Certificate2 serverCertificate, Uri endpointUrl) for (int ii = 0; ii < domains.Count; ii++) { - if (String.Equals(hostname, domains[ii], StringComparison.OrdinalIgnoreCase) || - String.Equals(dnsHostName, domains[ii], StringComparison.OrdinalIgnoreCase)) + if (string.Equals(hostname, domains[ii], StringComparison.OrdinalIgnoreCase) || + string.Equals(dnsHostName, domains[ii], StringComparison.OrdinalIgnoreCase)) { domainFound = true; break; diff --git a/Stack/Opc.Ua.Core/Types/Utils/Utils.cs b/Stack/Opc.Ua.Core/Types/Utils/Utils.cs index 6816af4e8..4a0c55e73 100644 --- a/Stack/Opc.Ua.Core/Types/Utils/Utils.cs +++ b/Stack/Opc.Ua.Core/Types/Utils/Utils.cs @@ -2529,7 +2529,7 @@ public static void UpdateExtension(ref XmlElementCollection extensions, XmlQu extensions.Add(document.DocumentElement); } } -#endregion + #endregion #region Reflection Helper Functions /// @@ -2712,24 +2712,23 @@ public static byte[] Append(params byte[][] arrays) /// /// Creates a X509 certificate object from the DER encoded bytes. /// - public static X509Certificate2 ParseCertificateBlob(byte[] certificateData) + public static X509Certificate2 ParseCertificateBlob(ReadOnlyMemory certificateData) { - // macOS X509Certificate2 constructor throws exception if a certchain is encoded // use AsnParser on macOS to parse for byteblobs, #if !NETFRAMEWORK bool useAsnParser = RuntimeInformation.IsOSPlatform(OSPlatform.OSX); -#else - bool useAsnParser = false; #endif try { +#if !NETFRAMEWORK if (useAsnParser) { var certBlob = AsnUtils.ParseX509Blob(certificateData); return CertificateFactory.Create(certBlob, true); } else +#endif { return CertificateFactory.Create(certificateData, true); } @@ -2748,30 +2747,32 @@ public static X509Certificate2 ParseCertificateBlob(byte[] certificateData) /// /// The certificate data. /// - public static X509Certificate2Collection ParseCertificateChainBlob(byte[] certificateData) + public static X509Certificate2Collection ParseCertificateChainBlob(ReadOnlyMemory certificateData) { X509Certificate2Collection certificateChain = new X509Certificate2Collection(); - List certificatesBytes = new List(certificateData); + // macOS X509Certificate2 constructor throws exception if a certchain is encoded // use AsnParser on macOS to parse for byteblobs, #if !NETFRAMEWORK bool useAsnParser = RuntimeInformation.IsOSPlatform(OSPlatform.OSX); -#else - bool useAsnParser = false; #endif - while (certificatesBytes.Count > 0) + int offset = 0; + int length = certificateData.Length; + while (offset < length) { X509Certificate2 certificate; try { +#if !NETFRAMEWORK if (useAsnParser) { - var certBlob = AsnUtils.ParseX509Blob(certificatesBytes.ToArray()); + var certBlob = AsnUtils.ParseX509Blob(certificateData.Slice(offset)); certificate = CertificateFactory.Create(certBlob, true); } else +#endif { - certificate = CertificateFactory.Create(certificatesBytes.ToArray(), true); + certificate = CertificateFactory.Create(certificateData.Slice(offset), true); } } catch (Exception e) @@ -2783,7 +2784,7 @@ public static X509Certificate2Collection ParseCertificateChainBlob(byte[] certif } certificateChain.Add(certificate); - certificatesBytes.RemoveRange(0, certificate.RawData.Length); + offset += certificate.RawData.Length; } return certificateChain; diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateFactoryTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateFactoryTest.cs index b6e28a798..5e7382dac 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateFactoryTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateFactoryTest.cs @@ -289,7 +289,7 @@ public void ParseCertificateBlob() var certBlob = byteServerCertificateChain.ToArray(); - var singleBlob = AsnUtils.ParseX509Blob(certBlob); + byte[] singleBlob = AsnUtils.ParseX509Blob(certBlob).ToArray(); Assert.NotNull(singleBlob); var certX = new X509Certificate2(singleBlob); Assert.NotNull(certX); diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs index f7e376b41..c871d6620 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs +++ b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs @@ -301,7 +301,7 @@ public async Task VerifySelfSignedAppCertsThrow() Assert.AreEqual((StatusCode)StatusCodes.BadCertificateUntrusted, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message); } - Thread.Sleep(1000); + await Task.Delay(1000).ConfigureAwait(false); Assert.AreEqual(m_appSelfSignedCerts.Count, validator.RejectedStore.Enumerate().Result.Count); } } From 48d64b530c8656351d011c51c35fb25a716023de Mon Sep 17 00:00:00 2001 From: Martin Regen Date: Fri, 6 Sep 2024 12:36:11 +0200 Subject: [PATCH 5/6] Remove https package from OPCFoundation.NetStandard.Opc.Ua dependencies to avoid build issues. (#2751) instead manually add the https package for server support, if needed. --- nuget/Opc.Ua.Symbols.nuspec | 5 ++--- nuget/Opc.Ua.nuspec | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/nuget/Opc.Ua.Symbols.nuspec b/nuget/Opc.Ua.Symbols.nuspec index 48ed38e3c..1d7aea606 100644 --- a/nuget/Opc.Ua.Symbols.nuspec +++ b/nuget/Opc.Ua.Symbols.nuspec @@ -2,7 +2,7 @@ OPCFoundation.NetStandard.Opc.Ua.Symbols - 1.04.372.0 + 1.05.374.0 OPC UA .Net Standard Library Symbols This package contains the OPC UA reference implementation debug version and is targeting the .NET Standard Library. OPC Foundation @@ -11,11 +11,10 @@ images/logo.jpg licenses/LICENSE.txt - OPCFoundation OPC-UA netstandard ios linux dotnet net netcore uwp + OPCFoundation OPC-UA netstandard ios linux dotnet net netcore - diff --git a/nuget/Opc.Ua.nuspec b/nuget/Opc.Ua.nuspec index a42c28c33..c1cf6b8e8 100644 --- a/nuget/Opc.Ua.nuspec +++ b/nuget/Opc.Ua.nuspec @@ -2,7 +2,7 @@ OPCFoundation.NetStandard.Opc.Ua - 1.04.372.0 + 1.05.375.0 OPC UA .NET Standard Library This package contains the OPC UA reference implementation and is targeting the .NET Standard Library. OPC Foundation @@ -12,11 +12,10 @@ licenses/LICENSE.txt README.md - OPCFoundation OPC-UA netstandard ios linux dotnet net netcore uwp + OPCFoundation OPC-UA netstandard ios linux dotnet net netcore - From 41363bd1e1d310e7e5a35563a5016a3a188d3e9c Mon Sep 17 00:00:00 2001 From: Martin Regen Date: Fri, 6 Sep 2024 17:07:06 +0200 Subject: [PATCH 6/6] Release testing: Fix channel remove issue and channel exhaustion on reconnect (#2749) - During a reconnect fault the channels remain open and are quickly exhausted. Immediately close channels in Opening state similar to channels in Connecting state instead of waiting for timeout cleanup. - The channel concurrentDictionary sometimes failed to remove a channel. Removing ContainsKey and retry with TryAdd fixes the problem. --- .../Stack/Tcp/TcpListenerChannel.cs | 41 ++++-- .../Stack/Tcp/TcpTransportListener.cs | 134 +++++++++--------- 2 files changed, 94 insertions(+), 81 deletions(-) diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs index f00813f7f..763b71f1a 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpListenerChannel.cs @@ -233,10 +233,10 @@ protected void ForceChannelFault(ServiceResult reason) } bool close = false; - if (State != TcpChannelState.Connecting) + if (State != TcpChannelState.Connecting && State != TcpChannelState.Opening) { - int socketHandle = (Socket != null) ? Socket.Handle : 0; - if (socketHandle != -1) + int? socketHandle = Socket?.Handle; + if (socketHandle != null && socketHandle != -1) { Utils.LogError( "{0} ForceChannelFault Socket={1:X8}, ChannelId={2}, TokenId={3}, Reason={4}", @@ -249,7 +249,7 @@ protected void ForceChannelFault(ServiceResult reason) } else { - // Close immediately if the client never got out of connecting state + // Close immediately if the client never got out of connecting or opening state close = true; } @@ -268,13 +268,11 @@ protected void ForceChannelFault(ServiceResult reason) if (close) { // close channel immediately. - ChannelClosed(); - } - else - { - // notify any monitors. - NotifyMonitors(reason, false); + ChannelFaulted(); } + + // notify any monitors. + NotifyMonitors(reason, close); } } @@ -313,15 +311,13 @@ private void OnCleanup(object state) /// /// Closes the channel and releases resources. + /// Sets state to Closed and notifies monitors. /// protected void ChannelClosed() { try { - if (Socket != null) - { - Socket.Close(); - } + Socket?.Close(); } finally { @@ -333,6 +329,23 @@ protected void ChannelClosed() } } + /// + /// Closes the channel and releases resources. + /// Sets state to Faulted. + /// + protected void ChannelFaulted() + { + try + { + Socket?.Close(); + } + finally + { + State = TcpChannelState.Faulted; + m_listener.ChannelClosed(ChannelId); + } + } + /// /// Sends an error message over the socket. /// diff --git a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs index ea7711512..bb0d93bf7 100644 --- a/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs +++ b/Stack/Opc.Ua.Core/Stack/Tcp/TcpTransportListener.cs @@ -247,7 +247,7 @@ public void ChannelClosed(uint channelId) } else { - Utils.LogInfo("ChannelId {0}: closed channel not found", channelId); + Utils.LogInfo("ChannelId {0}: closed, but channel was not found", channelId); } } @@ -526,63 +526,72 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) return; } - int channelCount = m_channels?.Count ?? 0; - bool serveChannel = !(m_maxChannelCount > 0 && m_maxChannelCount < channelCount); - if (!serveChannel) + var channels = m_channels; + if (channels != null) { - Utils.LogError("OnAccept: Maximum number of channels {0} reached, serving channels is stopped until number is lower or equal than {1} ", - channelCount, m_maxChannelCount); - Utils.SilentDispose(e.AcceptSocket); - } + // TODO: .Count is flagged as hotpath, implement separate counter + int channelCount = channels.Count; + bool serveChannel = !(m_maxChannelCount > 0 && m_maxChannelCount < channelCount); + if (!serveChannel) + { + Utils.LogError("OnAccept: Maximum number of channels {0} reached, serving channels is stopped until number is lower or equal than {1} ", + channelCount, m_maxChannelCount); + Utils.SilentDispose(e.AcceptSocket); + } - // check if the accept socket has been created. - if (serveChannel && e.AcceptSocket != null && e.SocketError == SocketError.Success && m_channels != null) - { - try + // check if the accept socket has been created. + if (serveChannel && e.AcceptSocket != null && e.SocketError == SocketError.Success) { - if (m_reverseConnectListener) + try { - // create the channel to manage incoming reverse connections. - channel = new TcpReverseConnectChannel( - m_listenerId, - this, - m_bufferManager, - m_quotas, - m_descriptions); + if (m_reverseConnectListener) + { + // create the channel to manage incoming reverse connections. + channel = new TcpReverseConnectChannel( + m_listenerId, + this, + m_bufferManager, + m_quotas, + m_descriptions); + } + else + { + // create the channel to manage incoming connections. + channel = new TcpServerChannel( + m_listenerId, + this, + m_bufferManager, + m_quotas, + m_serverCertificate, + m_serverCertificateChain, + m_descriptions); + } + + if (m_callback != null) + { + channel.SetRequestReceivedCallback(new TcpChannelRequestEventHandler(OnRequestReceived)); + channel.SetReportOpenSecureChannelAuditCallback(new ReportAuditOpenSecureChannelEventHandler(OnReportAuditOpenSecureChannelEvent)); + channel.SetReportCloseSecureChannelAuditCallback(new ReportAuditCloseSecureChannelEventHandler(OnReportAuditCloseSecureChannelEvent)); + channel.SetReportCertificateAuditCallback(new ReportAuditCertificateEventHandler(OnReportAuditCertificateEvent)); + } + + uint channelId; + do + { + // get channel id + channelId = GetNextChannelId(); + + // save the channel for shutdown and reconnects. + // retry to get a channel id if it is already in use. + } while (!channels.TryAdd(channelId, channel)); + + // start accepting messages on the channel. + channel.Attach(channelId, e.AcceptSocket); } - else + catch (Exception ex) { - // create the channel to manage incoming connections. - channel = new TcpServerChannel( - m_listenerId, - this, - m_bufferManager, - m_quotas, - m_serverCertificate, - m_serverCertificateChain, - m_descriptions); + Utils.LogError(ex, "Unexpected error accepting a new connection."); } - - if (m_callback != null) - { - channel.SetRequestReceivedCallback(new TcpChannelRequestEventHandler(OnRequestReceived)); - channel.SetReportOpenSecureChannelAuditCallback(new ReportAuditOpenSecureChannelEventHandler(OnReportAuditOpenSecureChannelEvent)); - channel.SetReportCloseSecureChannelAuditCallback(new ReportAuditCloseSecureChannelEventHandler(OnReportAuditCloseSecureChannelEvent)); - channel.SetReportCertificateAuditCallback(new ReportAuditCertificateEventHandler(OnReportAuditCertificateEvent)); - } - - // get channel id - uint channelId = GetNextChannelId(); - - // start accepting messages on the channel. - channel.Attach(channelId, e.AcceptSocket); - - // save the channel for shutdown and reconnects. - m_channels.TryAdd(channelId, channel); - } - catch (Exception ex) - { - Utils.LogError(ex, "Unexpected error accepting a new connection."); } } @@ -616,18 +625,19 @@ private void OnAccept(object sender, SocketAsyncEventArgs e) /// private void DetectInactiveChannels(object state = null) { - List channels; + var channels = new List(); - channels = new List(); + bool cleanup = false; foreach (var chEntry in m_channels) { if (chEntry.Value.ElapsedSinceLastActiveTime > m_quotas.ChannelLifetime) { channels.Add(chEntry.Value); + cleanup = true; } } - if (channels.Count > 0) + if (cleanup) { Utils.LogInfo("TCPLISTENER: {0} channels scheduled for IdleCleanup.", channels.Count); foreach (var channel in channels) @@ -748,20 +758,10 @@ private void OnProcessRequestComplete(IAsyncResult result) /// private uint GetNextChannelId() { - lock (m_lock) - { - do - { - uint nextChannelId = ++m_lastChannelId; - if (nextChannelId != 0 && m_channels?.ContainsKey(nextChannelId) != true) - { - return nextChannelId; - } - } while (true); - } + // wraps at Int32.MaxValue back to 1 + return (uint)Utils.IncrementIdentifier(ref m_lastChannelId); } - /// /// Sets the URI for the listener. /// @@ -806,7 +806,7 @@ private void SetUri(Uri baseAddress, string relativeAddress) private ChannelQuotas m_quotas; private X509Certificate2 m_serverCertificate; private X509Certificate2Collection m_serverCertificateChain; - private uint m_lastChannelId; + private int m_lastChannelId; private Socket m_listeningSocket; private Socket m_listeningSocketIPv6; private ConcurrentDictionary m_channels;