diff --git a/Applications/ConsoleReferenceClient/ConsoleReferenceClient.csproj b/Applications/ConsoleReferenceClient/ConsoleReferenceClient.csproj index abf04a59ff..3e257890b8 100644 --- a/Applications/ConsoleReferenceClient/ConsoleReferenceClient.csproj +++ b/Applications/ConsoleReferenceClient/ConsoleReferenceClient.csproj @@ -24,7 +24,7 @@ - + diff --git a/Applications/ConsoleReferenceServer/ConsoleReferenceServer.csproj b/Applications/ConsoleReferenceServer/ConsoleReferenceServer.csproj index 4a51d6cc53..597a9f85a5 100644 --- a/Applications/ConsoleReferenceServer/ConsoleReferenceServer.csproj +++ b/Applications/ConsoleReferenceServer/ConsoleReferenceServer.csproj @@ -33,7 +33,7 @@ - + diff --git a/Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj b/Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj index 07f10d1f21..ca7eb77c44 100644 --- a/Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj +++ b/Fuzzing/Encoders/Fuzz.Tests/Opc.Ua.Encoders.Fuzz.Tests.csproj @@ -20,7 +20,7 @@ - + all diff --git a/Fuzzing/Encoders/Fuzz.Tools/Encoders.Fuzz.Tools.csproj b/Fuzzing/Encoders/Fuzz.Tools/Encoders.Fuzz.Tools.csproj index 1c16b233a4..9b112eff3d 100644 --- a/Fuzzing/Encoders/Fuzz.Tools/Encoders.Fuzz.Tools.csproj +++ b/Fuzzing/Encoders/Fuzz.Tools/Encoders.Fuzz.Tools.csproj @@ -23,7 +23,7 @@ - + diff --git a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs b/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs index 860fb7ceca..063dadf2f1 100644 --- a/Libraries/Opc.Ua.Configuration/ApplicationInstance.cs +++ b/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 CheckApplicationInstanceCertificates( #region Private Methods /// - /// + /// /// /// /// @@ -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) { diff --git a/Libraries/Opc.Ua.PubSub/Encoding/UadpNetworkMessage.cs b/Libraries/Opc.Ua.PubSub/Encoding/UadpNetworkMessage.cs index 970601b5e4..be03543c2c 100644 --- a/Libraries/Opc.Ua.PubSub/Encoding/UadpNetworkMessage.cs +++ b/Libraries/Opc.Ua.PubSub/Encoding/UadpNetworkMessage.cs @@ -117,7 +117,7 @@ public UadpNetworkMessage(UADPNetworkMessageDiscoveryType discoveryType) /// Create new instance of as a DiscoveryResponse of PublisherEndpoints type /// /// - /// + /// public UadpNetworkMessage(EndpointDescription[] publisherEndpoints, StatusCode publisherProvidesEndpoints) : base(null, new List()) { diff --git a/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs b/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs index fa25395da6..217d38e620 100644 --- a/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs +++ b/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs @@ -3021,16 +3021,16 @@ protected virtual ServiceResult Call( List argumentErrors = new List(); VariantCollection outputArguments = new VariantCollection(); - ServiceResult error = method.Call( + ServiceResult callResult = method.Call( context, methodToCall.ObjectId, methodToCall.InputArguments, argumentErrors, outputArguments); - if (ServiceResult.IsBad(error)) + if (ServiceResult.IsBad(callResult)) { - return error; + return callResult; } // check for argument errors. @@ -3085,7 +3085,8 @@ protected virtual ServiceResult Call( // return output arguments. result.OutputArguments = outputArguments; - return ServiceResult.Good; + // return the actual result of the original call + return callResult; } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs index 0ad4f52623..3e1399c0f5 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateIdentifier.cs @@ -12,8 +12,6 @@ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; @@ -243,20 +241,6 @@ public async Task Find(bool needPrivateKey) return certificate; } - /// - /// Updates the object from another object (usage is not updated). - /// - /// The certificate. - private void Paste(CertificateIdentifier certificate) - { - this.SubjectName = certificate.SubjectName; - this.Thumbprint = certificate.Thumbprint; - this.RawData = certificate.RawData; - this.ValidationOptions = certificate.ValidationOptions; - this.Certificate = certificate.Certificate; - this.CertificateType = certificate.CertificateType; - } - /// /// Returns a display name for a certificate. /// @@ -528,9 +512,6 @@ public ushort GetMinKeySize(SecurityConfiguration securityConfiguration) // non RSA return 0; } - - throw new ArgumentException("Certificate type is unknown"); - } diff --git a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs index e029abbbc5..22509eba46 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/CertificateValidator.cs @@ -30,7 +30,7 @@ namespace Opc.Ua /// 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) diff --git a/Stack/Opc.Ua.Core/Security/Certificates/EccUtils.cs b/Stack/Opc.Ua.Core/Security/Certificates/EccUtils.cs index 23006c07c5..ede1b1e5c2 100644 --- a/Stack/Opc.Ua.Core/Security/Certificates/EccUtils.cs +++ b/Stack/Opc.Ua.Core/Security/Certificates/EccUtils.cs @@ -222,8 +222,7 @@ public static string GetECDsaQualifier(X509Certificate2 certificate) /// public static ECDsa GetPublicKey(X509Certificate2 certificate) { - string[] securityPolicyUris; - return GetPublicKey(certificate, out securityPolicyUris); + return GetPublicKey(certificate, out string[] _); } /// @@ -236,14 +235,19 @@ public static ECDsa GetPublicKey(X509Certificate2 certificate, out string[] secu { securityPolicyUris = null; + if (certificate == null) + { + return null; + } + var keyAlgorithm = certificate.GetKeyAlgorithm(); - if (certificate == null || keyAlgorithm != Oids.ECPublicKey) + if (keyAlgorithm != Oids.ECPublicKey) { return null; } - const X509KeyUsageFlags SufficientFlags = + const X509KeyUsageFlags kSufficientFlags = X509KeyUsageFlags.KeyAgreement | X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.NonRepudiation | @@ -256,7 +260,7 @@ public static ECDsa GetPublicKey(X509Certificate2 certificate, out string[] secu { X509KeyUsageExtension kuExt = (X509KeyUsageExtension)extension; - if ((kuExt.KeyUsages & SufficientFlags) == 0) + if ((kuExt.KeyUsages & kSufficientFlags) == 0) { return null; } @@ -343,8 +347,6 @@ public static int GetSignatureLength(X509Certificate2 signingCertificate) return publicKey.KeySize / 4; } - - throw new NotImplementedException(); } /// diff --git a/Stack/Opc.Ua.Core/Security/Constants/SecurityPolicies.cs b/Stack/Opc.Ua.Core/Security/Constants/SecurityPolicies.cs index f3a55fa0d0..6e22449dd4 100644 --- a/Stack/Opc.Ua.Core/Security/Constants/SecurityPolicies.cs +++ b/Stack/Opc.Ua.Core/Security/Constants/SecurityPolicies.cs @@ -621,11 +621,6 @@ public static bool Verify(X509Certificate2 certificate, string securityPolicyUri securityPolicyUri); } } - - throw ServiceResultException.Create( - StatusCodes.BadSecurityChecksFailed, - "Unexpected security policy Uri: {0}", - securityPolicyUri); } #endregion } diff --git a/Stack/Opc.Ua.Core/Stack/State/MethodState.cs b/Stack/Opc.Ua.Core/Stack/State/MethodState.cs index f3b79abb03..e327a82b4a 100644 --- a/Stack/Opc.Ua.Core/Stack/State/MethodState.cs +++ b/Stack/Opc.Ua.Core/Stack/State/MethodState.cs @@ -706,7 +706,7 @@ public virtual ServiceResult Call( } // copy out arguments. - if (ServiceResult.IsGood(result)) + if (ServiceResult.IsGoodOrUncertain(result)) { for (int ii = 0; ii < outputs.Count; ii++) { diff --git a/Stack/Opc.Ua.Core/Types/Utils/ServiceResult.cs b/Stack/Opc.Ua.Core/Types/Utils/ServiceResult.cs index 1d04c49081..40d72b66fc 100644 --- a/Stack/Opc.Ua.Core/Types/Utils/ServiceResult.cs +++ b/Stack/Opc.Ua.Core/Types/Utils/ServiceResult.cs @@ -524,6 +524,19 @@ public static bool IsUncertain(ServiceResult status) return false; } + /// + /// Returns true if the status code is good or uncertain. + /// + public static bool IsGoodOrUncertain(ServiceResult status) + { + if (status != null) + { + return StatusCode.IsGood(status.m_code) || StatusCode.IsUncertain(status.m_code); + } + + return false; + } + /// /// Returns true if the status is good or uncertain. /// diff --git a/Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj b/Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj index 1da96a2c36..d9d5309386 100644 --- a/Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj +++ b/Tests/Opc.Ua.Client.ComplexTypes.Tests/Opc.Ua.Client.ComplexTypes.Tests.csproj @@ -9,7 +9,7 @@ - + all diff --git a/Tests/Opc.Ua.Client.Tests/ClientTest.cs b/Tests/Opc.Ua.Client.Tests/ClientTest.cs index e4fb6b6721..5a94753537 100644 --- a/Tests/Opc.Ua.Client.Tests/ClientTest.cs +++ b/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(); diff --git a/Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj b/Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj index c1916c3608..64fdc94854 100644 --- a/Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj +++ b/Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj @@ -23,7 +23,7 @@ - + all diff --git a/Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj b/Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj index fd58f9d418..98893da620 100644 --- a/Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj +++ b/Tests/Opc.Ua.Configuration.Tests/Opc.Ua.Configuration.Tests.csproj @@ -23,7 +23,7 @@ - + all diff --git a/Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj b/Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj index 71a4a42b19..7d5ecaf24c 100644 --- a/Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj +++ b/Tests/Opc.Ua.Core.Tests/Opc.Ua.Core.Tests.csproj @@ -13,7 +13,7 @@ - + all diff --git a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs b/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs index 1068b783f4..b0f757b511 100644 --- a/Tests/Opc.Ua.Core.Tests/Security/Certificates/CertificateValidatorTest.cs +++ b/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; } + /// + /// Test that Hash sizes lower than public key sizes of certificates are not valid + /// + /// + /// + [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(() => 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); + } + } + /// /// Test auto accept. /// diff --git a/Tests/Opc.Ua.Gds.Tests/Opc.Ua.Gds.Tests.csproj b/Tests/Opc.Ua.Gds.Tests/Opc.Ua.Gds.Tests.csproj index e1a99a28c6..82e9d200fb 100644 --- a/Tests/Opc.Ua.Gds.Tests/Opc.Ua.Gds.Tests.csproj +++ b/Tests/Opc.Ua.Gds.Tests/Opc.Ua.Gds.Tests.csproj @@ -26,7 +26,7 @@ - + all diff --git a/Tests/Opc.Ua.PubSub.Tests/Opc.Ua.PubSub.Tests.csproj b/Tests/Opc.Ua.PubSub.Tests/Opc.Ua.PubSub.Tests.csproj index 34dd6373bf..16abd92cab 100644 --- a/Tests/Opc.Ua.PubSub.Tests/Opc.Ua.PubSub.Tests.csproj +++ b/Tests/Opc.Ua.PubSub.Tests/Opc.Ua.PubSub.Tests.csproj @@ -22,7 +22,7 @@ - + all diff --git a/Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs b/Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs index dbb8f1cd5d..63273df3bb 100644 --- a/Tests/Opc.Ua.Security.Certificates.Tests/CertificateTestsForECDsa.cs +++ b/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 } diff --git a/Tests/Opc.Ua.Security.Certificates.Tests/Opc.Ua.Security.Certificates.Tests.csproj b/Tests/Opc.Ua.Security.Certificates.Tests/Opc.Ua.Security.Certificates.Tests.csproj index 3eb757da87..f479c4ec41 100644 --- a/Tests/Opc.Ua.Security.Certificates.Tests/Opc.Ua.Security.Certificates.Tests.csproj +++ b/Tests/Opc.Ua.Security.Certificates.Tests/Opc.Ua.Security.Certificates.Tests.csproj @@ -25,7 +25,7 @@ - + all @@ -54,7 +54,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/Tests/Opc.Ua.Server.Tests/Opc.Ua.Server.Tests.csproj b/Tests/Opc.Ua.Server.Tests/Opc.Ua.Server.Tests.csproj index 660d8dc02f..dcd8568844 100644 --- a/Tests/Opc.Ua.Server.Tests/Opc.Ua.Server.Tests.csproj +++ b/Tests/Opc.Ua.Server.Tests/Opc.Ua.Server.Tests.csproj @@ -23,7 +23,7 @@ - + all diff --git a/version.props b/version.props index e9b9d796cf..82418c4b55 100644 --- a/version.props +++ b/version.props @@ -1,7 +1,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive