Skip to content

Commit

Permalink
Application config improvements for cert recreation (#1525)
Browse files Browse the repository at this point in the history 
- do not silent recreate a certificate if a matching cert subject is available, enforce manual deletion or replacement
- allow the application cert to be used when expired or not yet valid
- warn in trace if an app cert is loaded without loading the private key 
fixes #1162 , fixes #1102
  • Loading branch information
@mregen
mregen committed Sep 28, 2021
1 parent 75744e2 commit e5baad9
Show file tree
Hide file tree
Showing 11 changed files with 438 additions and 63 deletions.
47 changes: 19 additions & 28 deletions Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

using System;
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
using System.Threading.Tasks;
using System.Xml;
Expand Down Expand Up @@ -100,51 +101,42 @@ public IApplicationConfigurationBuilderClientSelected AsClient()
var appStoreType = CertificateStoreIdentifier.DetermineStoreType(appRoot);
var pkiRootType = CertificateStoreIdentifier.DetermineStoreType(pkiRoot);
var rejectedRootType = CertificateStoreIdentifier.DetermineStoreType(rejectedRoot);
ApplicationConfiguration.SecurityConfiguration = new SecurityConfiguration
{
ApplicationConfiguration.SecurityConfiguration = new SecurityConfiguration {
// app cert store
ApplicationCertificate = new CertificateIdentifier()
{
ApplicationCertificate = new CertificateIdentifier() {
StoreType = appStoreType,
StorePath = DefaultCertificateStorePath(TrustlistType.Application, appRoot),
SubjectName = Utils.ReplaceDCLocalhost(subjectName)
},
// App trusted & issuer
TrustedPeerCertificates = new CertificateTrustList()
{
TrustedPeerCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.Trusted, pkiRoot)
},
TrustedIssuerCertificates = new CertificateTrustList()
{
TrustedIssuerCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.Issuer, pkiRoot)
},
// Https trusted & issuer
TrustedHttpsCertificates = new CertificateTrustList()
{
TrustedHttpsCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.TrustedHttps, pkiRoot)
},
HttpsIssuerCertificates = new CertificateTrustList()
{
HttpsIssuerCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.IssuerHttps, pkiRoot)
},
// User trusted & issuer
TrustedUserCertificates = new CertificateTrustList()
{
TrustedUserCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.TrustedUser, pkiRoot)
},
UserIssuerCertificates = new CertificateTrustList()
{
UserIssuerCertificates = new CertificateTrustList() {
StoreType = pkiRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.IssuerUser, pkiRoot)
},
// rejected store
RejectedCertificateStore = new CertificateTrustList()
{
RejectedCertificateStore = new CertificateTrustList() {
StoreType = rejectedRootType,
StorePath = DefaultCertificateStorePath(TrustlistType.Rejected, rejectedRoot)
},
Expand Down Expand Up @@ -776,21 +768,21 @@ private string DefaultCertificateStorePath(TrustlistType trustListType, string p
switch (trustListType)
{
case TrustlistType.Application:
return pkiRoot + "/own";
return Path.Combine(pkiRoot, "own");
case TrustlistType.Trusted:
return pkiRoot + "/trusted";
return Path.Combine(pkiRoot, "trusted");
case TrustlistType.Issuer:
return pkiRoot + "/issuer";
return Path.Combine(pkiRoot, "issuer");
case TrustlistType.TrustedHttps:
return pkiRoot + "/trustedHttps";
return Path.Combine(pkiRoot, "trustedHttps");
case TrustlistType.IssuerHttps:
return pkiRoot + "/issuerHttps";
return Path.Combine(pkiRoot, "issuerHttps");
case TrustlistType.TrustedUser:
return pkiRoot + "/trustedUser";
return Path.Combine(pkiRoot, "trustedUser");
case TrustlistType.IssuerUser:
return pkiRoot + "/issuerUser";
return Path.Combine(pkiRoot, "issuerUser");
case TrustlistType.Rejected:
return pkiRoot + "/rejected";
return Path.Combine(pkiRoot, "rejected");
}
}
else if (pkiRootType.Equals(CertificateStoreType.X509Store, StringComparison.OrdinalIgnoreCase))
Expand Down Expand Up @@ -876,8 +868,7 @@ private void AddSecurityPolicies(bool includeSign = false, bool deprecated = fal
private bool InternalAddPolicy(ServerSecurityPolicyCollection policies, MessageSecurityMode securityMode, string policyUri)
{
if (securityMode == MessageSecurityMode.Invalid) throw new ArgumentException("Invalid security mode selected", nameof(securityMode));
var newPolicy = new ServerSecurityPolicy()
{
var newPolicy = new ServerSecurityPolicy() {
SecurityMode = securityMode,
SecurityPolicyUri = policyUri
};
Expand Down
66 changes: 58 additions & 8 deletions Libraries/Opc.Ua.Configuration/ApplicationInstance.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Security.Cryptography.X509Certificates;
using System.Text;
Expand Down Expand Up @@ -416,7 +417,6 @@ string productUri
}

ApplicationConfiguration configuration = m_applicationConfiguration;
bool certificateValid = false;

// find the existing certificate.
CertificateIdentifier id = configuration.SecurityConfiguration.ApplicationCertificate;
Expand All @@ -427,12 +427,28 @@ string productUri
"Configuration file does not specify a certificate.");
}

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

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

// check that it is ok.
if (certificate != null)
{
certificateValid = await CheckApplicationInstanceCertificate(configuration, certificate, silent, minimumKeySize).ConfigureAwait(false);
bool certificateValid = await CheckApplicationInstanceCertificate(configuration, certificate, silent, minimumKeySize).ConfigureAwait(false);

if (!certificateValid)
{
var message = new StringBuilder();
message.AppendLine("The certificate with subject {0} in the configuration is invalid.");
message.AppendLine(" Please update or delete the certificate from this location:");
message.AppendLine(" {1}");
throw ServiceResultException.Create(StatusCodes.BadConfigurationError,
message.ToString(), id.SubjectName, id.StorePath
);
}
}
else
{
Expand Down Expand Up @@ -481,7 +497,7 @@ string productUri
}
}

if ((certificate == null) || !certificateValid)
if (certificate == null)
{
certificate = await CreateApplicationInstanceCertificate(configuration,
minimumKeySize, lifeTimeInMonths).ConfigureAwait(false);
Expand Down Expand Up @@ -512,6 +528,27 @@ string productUri
#endregion

#region Private Methods
/// <summary>
/// Helper to suppress errors which are allowed for the application certificate validation.
/// </summary>
private class CertValidationSuppressibleStatusCodes
{
public StatusCode[] ApprovedCodes { get; }

public CertValidationSuppressibleStatusCodes(StatusCode[] approvedCodes)
{
ApprovedCodes = approvedCodes;
}

public void OnCertificateValidation(object sender, CertificateValidationEventArgs e)
{
if (ApprovedCodes.Contains(e.Error.StatusCode))
{
e.Accept = true;
}
}
}

/// <summary>
/// Creates an application instance certificate if one does not already exist.
/// </summary>
Expand All @@ -526,12 +563,23 @@ string productUri
return false;
}

// set suppressible errors
var certValidator = new CertValidationSuppressibleStatusCodes(
new StatusCode[] {
StatusCodes.BadCertificateUntrusted,
StatusCodes.BadCertificateTimeInvalid,
StatusCodes.BadCertificateHostNameInvalid,
StatusCodes.BadCertificateRevocationUnknown,
StatusCodes.BadCertificateIssuerRevocationUnknown,
});

Utils.Trace(Utils.TraceMasks.Information, "Checking application instance certificate. {0}", certificate.Subject);

try
{
// validate certificate.
configuration.CertificateValidator.Validate(certificate);
configuration.CertificateValidator.CertificateValidation += certValidator.OnCertificateValidation;
configuration.CertificateValidator.Validate(certificate.HasPrivateKey ? new X509Certificate2(certificate.RawData) : certificate);
}
catch (Exception ex)
{
Expand All @@ -542,6 +590,10 @@ string productUri
return false;
}
}
finally
{
configuration.CertificateValidator.CertificateValidation -= certValidator.OnCertificateValidation;
}

// check key size.
int keySize = X509Utils.GetRSAPublicKeySize(certificate);
Expand Down Expand Up @@ -728,9 +780,7 @@ ushort lifeTimeInMonths
Utils.Trace(Utils.TraceMasks.Information, "Certificate created. Thumbprint={0}", certificate.Thumbprint);

// reload the certificate from disk.
await configuration.SecurityConfiguration.ApplicationCertificate.LoadPrivateKeyEx(passwordProvider).ConfigureAwait(false);

return certificate;
return await configuration.SecurityConfiguration.ApplicationCertificate.LoadPrivateKeyEx(passwordProvider).ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -871,7 +921,7 @@ private async Task<bool> ApproveMessage(string message, bool silent)
}
else
{
Utils.Trace(message);
Utils.Trace(Utils.TraceMasks.Error, message);
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Opc.Ua.Security.Certificates/X509Certificate/CertificateBuilderBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public ICertificateBuilder SetCAConstraint(int pathLengthConstraint = -1)
}

/// <inheritdoc/>
public virtual ICertificateBuilderCreateForRSAAny SetRSAKeySize(int keySize)
public virtual ICertificateBuilderCreateForRSAAny SetRSAKeySize(ushort keySize)
{
if (keySize == 0)
{
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Opc.Ua.Security.Certificates/X509Certificate/ICertificateBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public interface ICertificateBuilderRSAParameter
/// Set the RSA key size in bits.
/// </summary>
/// <param name="keySize">The size of the RSA key.</param>
ICertificateBuilderCreateForRSAAny SetRSAKeySize(int keySize);
ICertificateBuilderCreateForRSAAny SetRSAKeySize(ushort keySize);
}

#if ECC_SUPPORT
Expand Down
11 changes: 10 additions & 1 deletion Stack/Opc.Ua.Core/Security/Certificates/CertificateFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,21 @@ public static X509Certificate2 Load(X509Certificate2 certificate, bool ensurePri
return certificate;
}

if (ensurePrivateKeyAccessible)
{
if (!X509Utils.VerifyRSAKeyPair(certificate, certificate))
{
Utils.Trace(Utils.TraceMasks.Error, "WARNING - Trying to add certificate to cache with invalid private key.");
return null;
}
}

// update the cache.
m_certificates[certificate.Thumbprint] = certificate;

if (m_certificates.Count > 100)
{
Utils.Trace("WARNING - Process certificate cache has {0} certificates in it.", m_certificates.Count);
Utils.Trace(Utils.TraceMasks.Error, "WARNING - Process certificate cache has {0} certificates in it.", m_certificates.Count);
}

}
Expand Down
23 changes: 16 additions & 7 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
Expand Down Expand Up @@ -154,9 +155,9 @@ public X509Certificate2 Certificate
/// <summary>
/// Finds a certificate in a store.
/// </summary>
public async Task<X509Certificate2> Find()
public Task<X509Certificate2> Find()
{
return await Find(false).ConfigureAwait(false);
return Find(false);
}

/// <summary>
Expand All @@ -168,7 +169,7 @@ public Task<X509Certificate2> LoadPrivateKey(string password)
/// <summary>
/// Loads the private key for the certificate with an optional password.
/// </summary>
public async Task<X509Certificate2> LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider)
public Task<X509Certificate2> LoadPrivateKeyEx(ICertificatePasswordProvider passwordProvider)
{
if (this.StoreType == CertificateStoreType.Directory)
{
Expand All @@ -177,19 +178,19 @@ public async Task<X509Certificate2> LoadPrivateKeyEx(ICertificatePasswordProvide
store.Open(this.StorePath);
string password = passwordProvider?.GetPassword(this);
m_certificate = store.LoadPrivateKey(this.Thumbprint, this.SubjectName, password);
return m_certificate;
return Task.FromResult(m_certificate);
}
}

return await Find(true).ConfigureAwait(false);
return Find(true);
}

/// <summary>
/// Finds a certificate in a store.
/// </summary>
/// <param name="needPrivateKey">if set to <c>true</c> the returned certificate must contain the private key.</param>
/// <returns>An instance of the <see cref="X509Certificate2"/> that is emebeded by this instance or find it in
/// the selected strore pointed out by the <see cref="StorePath"/> using selected <see cref="SubjectName"/>.</returns>
/// <returns>An instance of the <see cref="X509Certificate2"/> that is embedded by this instance or find it in
/// the selected store pointed out by the <see cref="StorePath"/> using selected <see cref="SubjectName"/>.</returns>
public async Task<X509Certificate2> Find(bool needPrivateKey)
{
X509Certificate2 certificate = null;
Expand All @@ -213,6 +214,14 @@ public async Task<X509Certificate2> Find(bool needPrivateKey)
if (certificate != null)
{
m_certificate = certificate;

if (needPrivateKey && this.StoreType == CertificateStoreType.Directory)
{
var message = new StringBuilder();
message.AppendLine("Loaded a certificate with private key from the directory store.");
message.AppendLine("Ensure to call LoadPrivateKeyEx with password provider before calling Find(true).");
Utils.Trace(Utils.TraceMasks.Error, message.ToString());
}
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions Stack/Opc.Ua.Core/Security/Certificates/CertificateStoreIdentifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/

using System;
using System.IO;

namespace Opc.Ua
{
Expand Down Expand Up @@ -74,10 +75,11 @@ public override string ToString()
/// The path to the default PKI Root.
/// </summary>
#if NETFRAMEWORK
public static readonly string DefaultPKIRoot = "%CommonApplicationData%/OPC Foundation/pki";
public static readonly string DefaultPKIRoot = Path.Combine("%CommonApplicationData%", "OPC Foundation", "pki");
#else
public static readonly string DefaultPKIRoot = "%LocalApplicationData%/OPC Foundation/pki";
public static readonly string DefaultPKIRoot = Path.Combine("%LocalApplicationData%","OPC Foundation","pki");
#endif

/// <summary>
/// The path to the current user X509Store.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ public virtual async Task UpdateCertificate(SecurityConfiguration securityConfig
securityConfiguration.ApplicationCertificate.Certificate = null;
}

await Update(securityConfiguration).ConfigureAwait(false);
await securityConfiguration.ApplicationCertificate.LoadPrivateKeyEx(
securityConfiguration.CertificatePasswordProvider).ConfigureAwait(false);
await Update(securityConfiguration).ConfigureAwait(false);

lock (m_callbackLock)
{
Expand Down

0 comments on commit e5baad9

Please sign in to comment.