Set corresponding security signature hash and validate ECDSA security…

… signature hash algorithm length for ECC certificates (OPCFoundation#2912)

* Use correctly and validate certificate ECDSA signature hash algorithm

* Added tests for validating length of EC certificate signature

* Improved invalidation condition of SignatureAlgorithm

* Changed result text

* Test fix

* Added helper method to set hash algorithm depending on key size;
Reverted ApplicationInstance code

* Fix test CreateSelfSignedForECDsaDefaultTest

Libraries/Opc.Ua.Configuration/ApplicationInstance.cs

@@ -2,7 +2,7 @@
  * Copyright (c) 2005-2021 The OPC Foundation, Inc. All rights reserved.
  *
  * 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
  * files (the "Software"), to deal in the Software without
@@ -11,7 +11,7 @@
  * copies of the Software, and to permit persons to whom the
  * Software is furnished to do so, subject to the following
  * conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be
  * included in all copies or substantial portions of the Software.
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
@@ -499,7 +499,7 @@ public async Task<bool> CheckApplicationInstanceCertificates(
 
         #region Private Methods
         /// <summary>
-        /// 
+        ///
         /// </summary>
         /// <param name="id"></param>
         /// <param name="silent"></param>
@@ -715,7 +715,7 @@ await configuration.CertificateValidator.ValidateAsync(
                 configuration.CertificateValidator.CertificateValidation -= certValidator.OnCertificateValidation;
             }
 
-            // check key size 
+            // check key size
             int keySize = X509Utils.GetPublicKeySize(certificate);
             if (minimumKeySize > keySize)
             {

Libraries/Opc.Ua.Security.Certificates/X509Certificate/CertificateBuilderBase.cs

@@ -239,9 +239,16 @@ public virtual ICertificateBuilder AddExtension(X509Extension extension)
         public virtual ICertificateBuilderCreateForECDsaAny SetECCurve(ECCurve curve)
         {
             m_curve = curve;
+
+            // HashAlgorithmName.SHA256 is the default value
+            if (m_hashAlgorithmName == X509Defaults.HashAlgorithmName)
+            {
+                SetHashAlgorithmSize(curve);
+            }
             return this;
         }
 
+
         /// <inheritdoc/>
         public abstract ICertificateBuilderCreateForECDsaAny SetECDsaPublicKey(byte[] publicKey);
 
@@ -275,6 +282,26 @@ public virtual ICertificateBuilderIssuer SetIssuer(X509Certificate2 issuerCertif
         }
         #endregion
 
+        #region Private methods
+        /// <summary>
+        /// Set the hash algorithm depending on the curve size
+        /// </summary>
+        /// <param name="curve"></param>
+        private void SetHashAlgorithmSize(ECCurve curve)
+        {
+            if (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.nistP384.Oid.FriendlyName) == 0 ||
+               (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.brainpoolP384r1.Oid.FriendlyName) == 0))
+            {
+                SetHashAlgorithm(HashAlgorithmName.SHA384);
+            }
+            if (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.nistP521.Oid.FriendlyName) == 0 ||
+               (curve.Oid.FriendlyName.CompareTo(ECCurve.NamedCurves.brainpoolP512r1.Oid.FriendlyName) == 0))
+            {
+                SetHashAlgorithm(HashAlgorithmName.SHA512);
+            }
+        }
+        #endregion
+
         #region Protected Methods
         /// <summary>
         /// The issuer CA certificate.

Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs

@@ -30,7 +30,7 @@ namespace Opc.Ua
     /// </summary>
     public class CertificateValidator : ICertificateValidator
     {
-        // default number of rejected certificates for history 
+        // default number of rejected certificates for history
         const int kDefaultMaxRejectedCertificates = 5;
 
         #region Constructors
@@ -1414,7 +1414,23 @@ protected virtual async Task InternalValidateAsync(X509Certificate2Collection ce
                     null, null, "SHA1 signed certificates are not trusted.", null, sresult);
             }
 
-            if (!isECDsaSignature)
+            // check if certificate signature algorithm length is sufficient
+            if (isECDsaSignature)
+            {
+                int publicKeySize = X509Utils.GetPublicKeySize(certificate);
+                bool isInvalid = (certificate.SignatureAlgorithm.Value == Oids.ECDsaWithSha256 &&
+                                  publicKeySize > 256) ||
+                                 (certificate.SignatureAlgorithm.Value == Oids.ECDsaWithSha384 &&
+                                  (publicKeySize <= 256 || publicKeySize > 384)) ||
+                                 (certificate.SignatureAlgorithm.Value == Oids.ECDsaWithSha512 &&
+                                  publicKeySize <= 384);
+                if (isInvalid)
+                {
+                    sresult = new ServiceResult(StatusCodes.BadCertificatePolicyCheckFailed,
+                        null, null, "Certificate doesn't meet minimum signature algorithm length requirement.", null, sresult);
+                }
+            }
+            else // RSA
             {
                 int keySize = X509Utils.GetRSAPublicKeySize(certificate);
                 if (keySize < m_minimumCertificateKeySize)

Tests/Opc.Ua.Client.Tests/ClientTest.cs

@@ -1705,6 +1705,7 @@ public async Task OpenSessionECCUserCertIdentityToken(
                 }
                 if (eccurveHashPair.Curve.Oid.FriendlyName.Contains(extractedFriendlyNamae))
                 {
+
                     X509Certificate2 cert = CertificateBuilder.Create("CN=Client Test ECC Subject, O=OPC Foundation")
                     .SetECCurve(eccurveHashPair.Curve)
                     .CreateForECDsa();

Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs

@@ -39,6 +39,7 @@
 using System.Threading.Tasks;
 using NUnit.Framework;
 using Opc.Ua.Security.Certificates;
+using Opc.Ua.Security.Certificates.Tests;
 using Assert = NUnit.Framework.Legacy.ClassicAssert;
 
 #if NETCOREAPP2_1 || !ECC_SUPPORT
@@ -55,6 +56,11 @@ namespace Opc.Ua.Core.Tests.Security.Certificates
     [SetCulture("en-us")]
     public class CertificateValidatorTest
     {
+        #region DataPoints
+        [DatapointSource]
+        public static readonly ECCurveHashPair[] ECCurveHashPairs = CertificateTestsForECDsa.GetECCurveHashPairs();
+        #endregion
+
         #region Test Setup
         public const string RootCASubject = "CN=Root CA Test Cert, O=OPC Foundation";
 
@@ -1268,6 +1274,36 @@ public async Task TestMinimumKeyRejected(bool trusted)
             certValidator.CertificateValidation -= approver.OnCertificateValidation;
         }
 
+        /// <summary>
+        /// Test that Hash sizes lower than public key sizes of certificates are not valid
+        /// </summary>
+        /// <param name="ecCurveHashPair"></param>
+        /// <returns></returns>
+        [Theory]
+        public async Task ECDsaHashSizeLowerThanPublicKeySize(
+            ECCurveHashPair ecCurveHashPair
+            )
+        {
+            if (ecCurveHashPair.HashSize > 0)
+            {
+                // default signing cert with custom key
+                X509Certificate2 cert = CertificateBuilder.Create("CN=LowHash")
+                    .SetHashAlgorithm(HashAlgorithmName.SHA512)
+                    .SetECCurve(ecCurveHashPair.Curve)
+                    .CreateForECDsa();
+
+                var validator = TemporaryCertValidator.Create();
+                await validator.TrustedStore.Add(cert).ConfigureAwait(false);
+                var certValidator = validator.Update();
+
+                var serviceResultException = Assert.Throws<ServiceResultException>(() => certValidator.Validate(cert));
+                Assert.AreEqual((StatusCode)StatusCodes.BadCertificatePolicyCheckFailed, (StatusCode)serviceResultException.StatusCode, serviceResultException.Message);
+                Assert.NotNull(serviceResultException.InnerResult);
+                ServiceResult innerResult = serviceResultException.InnerResult.InnerResult;
+                Assert.Null(innerResult);
+            }
+        }
+
         /// <summary>
         /// Test auto accept.
         /// </summary>

Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs

@@ -135,7 +135,7 @@ public void CreateSelfSignedForECDsaDefaultTest(ECCurveHashPair eccurveHashPair)
                 Assert.NotNull(publicKey);
                 publicKey.ExportParameters(false);
             }
-            Assert.AreEqual(X509Defaults.HashAlgorithmName, Oids.GetHashAlgorithmName(cert.SignatureAlgorithm.Value));
+            Assert.AreEqual(eccurveHashPair.HashAlgorithmName, Oids.GetHashAlgorithmName(cert.SignatureAlgorithm.Value));
             Assert.GreaterOrEqual(DateTime.UtcNow, cert.NotBefore);
             Assert.GreaterOrEqual(DateTime.UtcNow.AddMonths(X509Defaults.LifeTime), cert.NotAfter.ToUniversalTime());
             TestUtils.ValidateSelSignedBasicConstraints(cert);
@@ -388,20 +388,7 @@ ECCurveHashPair ecCurveHashPair
         #endregion
 
         #region Private Methods
-        private static ECCurveHashPair[] GetECCurveHashPairs()
-        {
-            var result = new ECCurveHashPairCollection {
-                { ECCurve.NamedCurves.nistP256, HashAlgorithmName.SHA256 },
-                { ECCurve.NamedCurves.nistP384, HashAlgorithmName.SHA384 } };
-            if (!RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
-            {
-                result.AddRange(new ECCurveHashPairCollection {
-                { ECCurve.NamedCurves.brainpoolP256r1, HashAlgorithmName.SHA256 },
-                { ECCurve.NamedCurves.brainpoolP384r1, HashAlgorithmName.SHA384 }});
-            }
-            return result.ToArray();
-        }
-
+
         private void WriteCertificate(X509Certificate2 cert, string message)
         {
             TestContext.Out.WriteLine(message);
@@ -437,6 +424,43 @@ private static byte[] GetPublicKey(ECDsa ecdsa)
         }
         #endregion
 
+        #region Public static
+        public static ECCurveHashPair[] GetECCurveHashPairs()
+        {
+            var result = new ECCurveHashPairCollection {
+                { ECCurve.NamedCurves.nistP256, HashAlgorithmName.SHA256 },
+                { ECCurve.NamedCurves.nistP384, HashAlgorithmName.SHA384 },
+                { ECCurve.NamedCurves.brainpoolP256r1, HashAlgorithmName.SHA256 },
+                { ECCurve.NamedCurves.brainpoolP384r1, HashAlgorithmName.SHA384 }
+            };
+
+            int i = 0;
+            while (i < result.Count)
+            {
+                ECDsa key = null;
+
+                // test if curve is supported
+                try
+                {
+                    key = ECDsa.Create(result[i].Curve);
+                }
+                catch
+                {
+                    result.RemoveAt(i);
+                    continue;
+                }
+                finally
+                {
+                    Utils.SilentDispose(key);
+                }
+                i++;
+            }
+
+            return result.ToArray();
+        }
+
+        #endregion
+
         #region Private Fields
         #endregion
     }