From 49166465db61fd51cd8d1d20dcd4aa51b2ac3057 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 18 Jan 2023 16:42:52 -0500 Subject: [PATCH 1/8] Implement raw ECDH agreement with SecurityTransforms --- .../ECDiffieHellmanSecurityTransforms.cs | 10 +++ .../ECDiffieHellmanTests.NistValidation.cs | 7 +- .../ECDiffieHellmanTests.Raw.cs | 81 +++++++++++++++++++ .../ref/System.Security.Cryptography.cs | 1 + .../Security/Cryptography/ECDiffieHellman.cs | 22 +++++ .../System.Security.Cryptography.Tests.csproj | 2 + 6 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs index a9d6e3df58ba81..5ff8f96f16fc4a 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs @@ -165,6 +165,16 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } + public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + { + ArgumentNullException.ThrowIfNull(otherPartyPublicKey); + ThrowIfDisposed(); + + byte[]? secretAgreement = DeriveSecretAgreement(otherPartyPublicKey, hasher: null); + Debug.Assert(secretAgreement is not null); + return secretAgreement; + } + private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher) { if (!(otherPartyPublicKey is ECDiffieHellmanSecurityTransformsPublicKey secTransPubKey)) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs index 7e5d408c4c9aea..fe90b5fbadc4dc 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs @@ -223,9 +223,12 @@ private static void Verify( HashAlgorithmName zHashAlgorithm, byte[] iutZ) { - byte[] result = iut.DeriveKeyFromHash(cavsPublic, zHashAlgorithm); + byte[] deriveHash = iut.DeriveKeyFromHash(cavsPublic, zHashAlgorithm); byte[] hashedZ = zHasher.ComputeHash(iutZ); - Assert.Equal(hashedZ.ByteArrayToHex(), result.ByteArrayToHex()); + Assert.Equal(hashedZ.ByteArrayToHex(), deriveHash.ByteArrayToHex()); + + byte[] rawDerived = iut.DeriveSecretAgreement(cavsPublic); + Assert.Equal(iutZ.ByteArrayToHex(), rawDerived.ByteArrayToHex()); } } #endif diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs new file mode 100644 index 00000000000000..a80e5ed19ed05b --- /dev/null +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs @@ -0,0 +1,81 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Security.Cryptography; +using Xunit; + +namespace System.Security.Cryptography.EcDiffieHellman.Tests +{ + public partial class ECDiffieHellmanTests + { + [Fact] + public static void RawDerivation_OtherKeyRequired() + { + using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) + { + AssertExtensions.Throws( + "otherPartyPublicKey", + () => ecdh.DeriveSecretAgreement(null)); + } + } + + [Theory] + [MemberData(nameof(MismatchedKeysizes))] + public static void RawDerivation_SameSizeOtherKeyRequired(int aliceSize, int bobSize) + { + using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(aliceSize)) + using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(bobSize)) + using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) + { + AssertExtensions.Throws( + "otherPartyPublicKey", + () => alice.DeriveSecretAgreement(bobPublic)); + } + } + + [Theory] + [MemberData(nameof(EveryKeysize))] + public static void RawDerivation_DeriveSharedSecret_Agree(int keySize) + { + using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(keySize)) + using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(keySize)) + using (ECDiffieHellmanPublicKey alicePublic = alice.PublicKey) + using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) + { + byte[] aliceDerived = alice.DeriveSecretAgreement(bobPublic); + byte[] bobDerived = bob.DeriveSecretAgreement(alicePublic); + Assert.Equal(aliceDerived, bobDerived); + } + } + + [Fact] + public static void RawDerivation_DeriveSharedSecret_Disagree() + { + using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellman eve = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) + using (ECDiffieHellmanPublicKey evePublic = eve.PublicKey) + { + byte[] aliceDerived = alice.DeriveSecretAgreement(bobPublic); + byte[] eveDerived = alice.DeriveSecretAgreement(evePublic); + + Assert.NotEqual(aliceDerived, eveDerived); + } + } + + [Fact] + public static void RawDerivation_DeriveIsStable() + { + using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) + { + byte[] aliceDerived1 = alice.DeriveSecretAgreement(bobPublic); + byte[] aliceDerived2 = alice.DeriveSecretAgreement(bobPublic); + Assert.Equal(aliceDerived1, aliceDerived2); + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs index 41682a4330ee94..d374d2ae5ba7aa 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -969,6 +969,7 @@ protected ECDiffieHellman() { } public virtual byte[] DeriveKeyFromHmac(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, byte[]? hmacKey, byte[]? secretPrepend, byte[]? secretAppend) { throw null; } public virtual byte[] DeriveKeyMaterial(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey) { throw null; } public virtual byte[] DeriveKeyTls(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey, byte[] prfLabel, byte[] prfSeed) { throw null; } + public virtual byte[] DeriveSecretAgreement(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey) { throw null; } public override void FromXmlString(string xmlString) { } public override string ToXmlString(bool includePrivateParameters) { throw null; } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs index a94fe8395d829f..79bb57d16b955a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs @@ -133,6 +133,28 @@ public virtual byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey, throw DerivedClassMustOverride(); } + /// + /// Derive key raw material. + /// + /// The public key of the party with which to derive a mutual secret. + /// The raw key agreement. + /// + /// is . + /// + /// + /// is over a different curve than this key. + /// + /// + /// A derived implementation has not provided an implementation of the method. + /// + /// + /// The current platform does not support raw key agreement. + /// + public virtual byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + { + throw DerivedClassMustOverride(); + } + private static NotImplementedException DerivedClassMustOverride() { return new NotImplementedException(SR.NotSupported_SubclassOverride); diff --git a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj index 3165257e702672..51c65ded7522a6 100644 --- a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj +++ b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj @@ -119,6 +119,8 @@ Link="CommonTest\System\Security\Cryptography\AlgorithmImplementations\ECDiffieHellman\ECDiffieHellmanTests.ImportExport.cs" /> + Date: Wed, 18 Jan 2023 17:22:38 -0500 Subject: [PATCH 2/8] Implement raw ECDH agreement with CNG --- .../Common/src/Interop/Windows/BCrypt/Cng.cs | 1 + .../NCrypt/Interop.NCryptDeriveKeyMaterial.cs | 15 +++++++++++++++ .../Security/Cryptography/ECDiffieHellmanCng.cs | 12 ++++++++++++ .../System.Security.Cryptography.Cng.Tests.csproj | 2 ++ .../Cryptography/ECDiffieHellmanWrapper.cs | 3 +++ 5 files changed, 33 insertions(+) diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs index 4c0fe5e82ad850..31e827630fbcf1 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/Cng.cs @@ -45,6 +45,7 @@ internal static class KeyDerivationFunction public const string Hash = "HASH"; // BCRYPT_KDF_HASH public const string Hmac = "HMAC"; // BCRYPT_KDF_HMAC public const string Tls = "TLS_PRF"; // BCRYPT_KDF_TLS_PRF + public const string Raw = "TRUNCATE"; // BCRYPT_KDF_RAW_SECRET } } diff --git a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs index cb290836f3eb42..23c025e762a846 100644 --- a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs +++ b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs @@ -241,5 +241,20 @@ internal static unsafe byte[] DeriveKeyMaterialTls( flags); } } + + internal static unsafe byte[] DeriveKeyMaterialTruncate( + SafeNCryptSecretHandle secretAgreement, + SecretAgreementFlags flags) + { + byte[] result = DeriveKeyMaterial( + secretAgreement, + BCryptNative.KeyDerivationFunction.Raw, + ReadOnlySpan.Empty, + flags); + + // Win32 returns the result as little endian. So we need to flip it to big endian. + Array.Reverse(result); + return result; + } } } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs index a25cfcc3a4f6c7..92ee7ece5b5478 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs @@ -130,5 +130,17 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey Interop.NCrypt.SecretAgreementFlags.None); } } + + public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + { + ArgumentNullException.ThrowIfNull(otherPartyPublicKey); + + using (SafeNCryptSecretHandle secretAgreement = DeriveSecretAgreementHandle(otherPartyPublicKey)) + { + return Interop.NCrypt.DeriveKeyMaterialTruncate( + secretAgreement, + Interop.NCrypt.SecretAgreementFlags.None); + } + } } } diff --git a/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj b/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj index 8bd0699feeb6d6..e40ea90436f4ca 100644 --- a/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.Cng/tests/System.Security.Cryptography.Cng.Tests.csproj @@ -176,6 +176,8 @@ Link="CommonTest\System\Security\Cryptography\AlgorithmImplementations\ECDiffieHellman\ECDiffieHellmanTests.ImportExport.cs" /> + _wrapped.DeriveKeyTls(Unwrap(otherPartyPublicKey), prfLabel, prfSeed); + public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) => + _wrapped.DeriveSecretAgreement(Unwrap(otherPartyPublicKey)); + public override void FromXmlString(string xmlString) => _wrapped.FromXmlString(xmlString); public override string ToXmlString(bool includePrivateParameters) => From e731f36053db8c96c57a5b7e2ba73f465fc9e642 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 18 Jan 2023 17:40:28 -0500 Subject: [PATCH 3/8] Implement raw ECDH agreement with OpenSSL --- .../Cryptography/ECDiffieHellmanOpenSsl.Derive.cs | 10 ++++++++++ .../System.Security.Cryptography.OpenSsl.Tests.csproj | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs index 6c4a212ec0eac6..a61223277225f7 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs @@ -69,6 +69,16 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } + public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + { + ArgumentNullException.ThrowIfNull(otherPartyPublicKey); + ThrowIfDisposed(); + + byte[]? secretAgreement = DeriveSecretAgreement(otherPartyPublicKey, hasher: null); + Debug.Assert(secretAgreement is not null); + return secretAgreement; + } + /// /// Get the secret agreement generated between two parties /// diff --git a/src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj b/src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj index 3ed8b1f1e208e4..dedc6bdb4bd7fd 100644 --- a/src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj +++ b/src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj @@ -37,6 +37,8 @@ Link="CommonTest\System\Security\Cryptography\AlgorithmImplementations\ECDiffieHellman\ECDiffieHellmanTests.ImportExport.cs" /> + Date: Wed, 18 Jan 2023 17:44:16 -0500 Subject: [PATCH 4/8] Implement raw ECDH agreement with Android/Conscrypt --- .../Cryptography/ECDiffieHellmanAndroid.Derive.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs index 7002da719f4874..1498eccda4a330 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs @@ -72,6 +72,16 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } + public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + { + ArgumentNullException.ThrowIfNull(otherPartyPublicKey); + ThrowIfDisposed(); + + byte[]? secretAgreement = DeriveSecretAgreement(otherPartyPublicKey, hasher: null); + Debug.Assert(secretAgreement is not null); + return secretAgreement; + } + /// /// Get the secret agreement generated between two parties /// From 282d5059a2d3a33757825717dd25f637bd75abb5 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Wed, 18 Jan 2023 17:49:49 -0500 Subject: [PATCH 5/8] Guard Windows 8 --- .../Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs index 23c025e762a846..0bb0ba115eacd2 100644 --- a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs +++ b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveKeyMaterial.cs @@ -246,6 +246,11 @@ internal static unsafe byte[] DeriveKeyMaterialTruncate( SafeNCryptSecretHandle secretAgreement, SecretAgreementFlags flags) { + if (!OperatingSystem.IsWindowsVersionAtLeast(10)) + { + throw new PlatformNotSupportedException(); + } + byte[] result = DeriveKeyMaterial( secretAgreement, BCryptNative.KeyDerivationFunction.Raw, From e74d3e0ead01ede8a656a83f299c3f26cd0dcf04 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 19 Jan 2023 10:39:36 -0500 Subject: [PATCH 6/8] Fix tests on Windows 8 --- .../ECDiffieHellman/ECDiffieHellmanFactory.cs | 3 +++ .../ECDiffieHellmanTests.NistValidation.cs | 11 +++++++-- .../ECDiffieHellmanTests.Raw.cs | 23 +++++++++++++++---- .../tests/ECDiffieHellmanCngProvider.cs | 1 + .../tests/EcDiffieHellmanOpenSslProvider.cs | 1 + .../DefaultECDiffieHellmanProvider.Android.cs | 2 ++ .../DefaultECDiffieHellmanProvider.Unix.cs | 1 + .../DefaultECDiffieHellmanProvider.Windows.cs | 1 + 8 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanFactory.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanFactory.cs index aec1b09fc8e827..6497640809f3ab 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanFactory.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanFactory.cs @@ -13,6 +13,7 @@ public interface IECDiffieHellmanProvider bool IsCurveValid(Oid oid); bool ExplicitCurvesSupported { get; } bool CanDeriveNewPublicKey { get; } + bool SupportsRawDerivation { get; } } public static partial class ECDiffieHellmanFactory @@ -42,5 +43,7 @@ public static bool IsCurveValid(Oid oid) public static bool ExplicitCurvesSupported => s_provider.ExplicitCurvesSupported; public static bool CanDeriveNewPublicKey => s_provider.CanDeriveNewPublicKey; + + public static bool SupportsRawDerivation => s_provider.SupportsRawDerivation; } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs index fe90b5fbadc4dc..fc1286ffc21a62 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs @@ -227,8 +227,15 @@ private static void Verify( byte[] hashedZ = zHasher.ComputeHash(iutZ); Assert.Equal(hashedZ.ByteArrayToHex(), deriveHash.ByteArrayToHex()); - byte[] rawDerived = iut.DeriveSecretAgreement(cavsPublic); - Assert.Equal(iutZ.ByteArrayToHex(), rawDerived.ByteArrayToHex()); + if (ECDiffieHellmanFactory.SupportsRawDerivation) + { + byte[] rawDerived = iut.DeriveSecretAgreement(cavsPublic); + Assert.Equal(iutZ.ByteArrayToHex(), rawDerived.ByteArrayToHex()); + } + else + { + Assert.Throws(() => iut.DeriveSecretAgreement(cavsPublic)); + } } } #endif diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs index a80e5ed19ed05b..393b2825ba5ac8 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs @@ -9,7 +9,9 @@ namespace System.Security.Cryptography.EcDiffieHellman.Tests { public partial class ECDiffieHellmanTests { - [Fact] + public static bool DoesNotSupportRawDerivation => !ECDiffieHellmanFactory.SupportsRawDerivation; + + [ConditionalFact(typeof(ECDiffieHellmanFactory), nameof(ECDiffieHellmanFactory.SupportsRawDerivation))] public static void RawDerivation_OtherKeyRequired() { using (ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create()) @@ -20,7 +22,7 @@ public static void RawDerivation_OtherKeyRequired() } } - [Theory] + [ConditionalTheory(typeof(ECDiffieHellmanFactory), nameof(ECDiffieHellmanFactory.SupportsRawDerivation))] [MemberData(nameof(MismatchedKeysizes))] public static void RawDerivation_SameSizeOtherKeyRequired(int aliceSize, int bobSize) { @@ -34,7 +36,7 @@ public static void RawDerivation_SameSizeOtherKeyRequired(int aliceSize, int bob } } - [Theory] + [ConditionalTheory(typeof(ECDiffieHellmanFactory), nameof(ECDiffieHellmanFactory.SupportsRawDerivation))] [MemberData(nameof(EveryKeysize))] public static void RawDerivation_DeriveSharedSecret_Agree(int keySize) { @@ -49,7 +51,7 @@ public static void RawDerivation_DeriveSharedSecret_Agree(int keySize) } } - [Fact] + [ConditionalFact(typeof(ECDiffieHellmanFactory), nameof(ECDiffieHellmanFactory.SupportsRawDerivation))] public static void RawDerivation_DeriveSharedSecret_Disagree() { using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) @@ -65,7 +67,7 @@ public static void RawDerivation_DeriveSharedSecret_Disagree() } } - [Fact] + [ConditionalFact(typeof(ECDiffieHellmanFactory), nameof(ECDiffieHellmanFactory.SupportsRawDerivation))] public static void RawDerivation_DeriveIsStable() { using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) @@ -77,5 +79,16 @@ public static void RawDerivation_DeriveIsStable() Assert.Equal(aliceDerived1, aliceDerived2); } } + + [ConditionalFact(nameof(DoesNotSupportRawDerivation))] + public static void RawDerivation_NotSupported() + { + using (ECDiffieHellman alice = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) + using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) + { + Assert.Throws(() => alice.DeriveSecretAgreement(bobPublic)); + } + } } } diff --git a/src/libraries/System.Security.Cryptography.Cng/tests/ECDiffieHellmanCngProvider.cs b/src/libraries/System.Security.Cryptography.Cng/tests/ECDiffieHellmanCngProvider.cs index 0470ab527ebfc6..d805dbc6d2b813 100644 --- a/src/libraries/System.Security.Cryptography.Cng/tests/ECDiffieHellmanCngProvider.cs +++ b/src/libraries/System.Security.Cryptography.Cng/tests/ECDiffieHellmanCngProvider.cs @@ -35,6 +35,7 @@ public bool ExplicitCurvesSupported } public bool CanDeriveNewPublicKey => true; + public bool SupportsRawDerivation => PlatformDetection.IsWindows10OrLater; private static bool NativeOidFriendlyNameExists(string oidFriendlyName) { diff --git a/src/libraries/System.Security.Cryptography.OpenSsl/tests/EcDiffieHellmanOpenSslProvider.cs b/src/libraries/System.Security.Cryptography.OpenSsl/tests/EcDiffieHellmanOpenSslProvider.cs index 2890fc477ee047..ef4aa6f83881f5 100644 --- a/src/libraries/System.Security.Cryptography.OpenSsl/tests/EcDiffieHellmanOpenSslProvider.cs +++ b/src/libraries/System.Security.Cryptography.OpenSsl/tests/EcDiffieHellmanOpenSslProvider.cs @@ -27,6 +27,7 @@ public ECDiffieHellman Create(ECCurve curve) public bool ExplicitCurvesSupported => _ecdsaProvider.ExplicitCurvesSupported; public bool CanDeriveNewPublicKey => true; + public bool SupportsRawDerivation => true; } public partial class ECDiffieHellmanFactory diff --git a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Android.cs b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Android.cs index 63ec79d8baeecd..93c0237a8f6cb6 100644 --- a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Android.cs +++ b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Android.cs @@ -21,6 +21,8 @@ public bool IsCurveValid(Oid oid) public bool CanDeriveNewPublicKey => false; + public bool SupportsRawDerivation => true; + private static bool IsValueOrFriendlyNameValid(string friendlyNameOrValue) { if (string.IsNullOrEmpty(friendlyNameOrValue)) diff --git a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Unix.cs b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Unix.cs index 58b7bcfa00f5fd..d00c95b27eaffa 100644 --- a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Unix.cs +++ b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Unix.cs @@ -35,6 +35,7 @@ public bool ExplicitCurvesSupported } public bool CanDeriveNewPublicKey { get; } = !PlatformDetection.IsiOS && !PlatformDetection.IstvOS && !PlatformDetection.IsMacCatalyst; + public bool SupportsRawDerivation => true; private static bool IsValueOrFriendlyNameValid(string friendlyNameOrValue) { diff --git a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Windows.cs b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Windows.cs index c458f74cd8931b..49f8ff15761e17 100644 --- a/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Windows.cs +++ b/src/libraries/System.Security.Cryptography/tests/DefaultECDiffieHellmanProvider.Windows.cs @@ -23,6 +23,7 @@ public bool ExplicitCurvesSupported } public bool CanDeriveNewPublicKey => true; + public bool SupportsRawDerivation => PlatformDetection.IsWindows10OrLater; private static bool NativeOidFriendlyNameExists(string oidFriendlyName) { From 09919b3c3db7ac58c98901aa033c8c8a19c2487f Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 15:00:52 -0500 Subject: [PATCH 7/8] Adjust for API review feedback --- .../Cryptography/ECDiffieHellmanAndroid.Derive.cs | 2 +- .../Security/Cryptography/ECDiffieHellmanCng.cs | 3 ++- .../Cryptography/ECDiffieHellmanOpenSsl.Derive.cs | 3 ++- .../Cryptography/ECDiffieHellmanSecurityTransforms.cs | 2 +- .../ref/System.Security.Cryptography.cs | 2 +- .../System/Security/Cryptography/ECDiffieHellman.cs | 11 +++++++++-- .../Security/Cryptography/ECDiffieHellmanWrapper.cs | 4 ++-- 7 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs index 1498eccda4a330..c3f9fbed2ae4b2 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanAndroid.Derive.cs @@ -72,7 +72,7 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } - public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + public override byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) { ArgumentNullException.ThrowIfNull(otherPartyPublicKey); ThrowIfDisposed(); diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs index 92ee7ece5b5478..df4d0c62420621 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs @@ -131,7 +131,8 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey } } - public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + /// + public override byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) { ArgumentNullException.ThrowIfNull(otherPartyPublicKey); diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs index a61223277225f7..f6fc78762e043c 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs @@ -69,7 +69,8 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } - public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + /// + public override byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) { ArgumentNullException.ThrowIfNull(otherPartyPublicKey); ThrowIfDisposed(); diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs index 2f1f26a5577586..cad96d1017f50b 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanSecurityTransforms.cs @@ -165,7 +165,7 @@ public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey DeriveSecretAgreement); } - public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + public override byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) { ArgumentNullException.ThrowIfNull(otherPartyPublicKey); ThrowIfDisposed(); diff --git a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs index d4d5e3a451965a..341b3c68d18ecc 100644 --- a/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs +++ b/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs @@ -969,7 +969,7 @@ protected ECDiffieHellman() { } public virtual byte[] DeriveKeyFromHmac(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, byte[]? hmacKey, byte[]? secretPrepend, byte[]? secretAppend) { throw null; } public virtual byte[] DeriveKeyMaterial(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey) { throw null; } public virtual byte[] DeriveKeyTls(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey, byte[] prfLabel, byte[] prfSeed) { throw null; } - public virtual byte[] DeriveSecretAgreement(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey) { throw null; } + public virtual byte[] DeriveRawSecretAgreement(System.Security.Cryptography.ECDiffieHellmanPublicKey otherPartyPublicKey) { throw null; } public override void FromXmlString(string xmlString) { } public override string ToXmlString(bool includePrivateParameters) { throw null; } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs index 79bb57d16b955a..d0e0047be410cc 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellman.cs @@ -134,10 +134,14 @@ public virtual byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey, } /// - /// Derive key raw material. + /// Derive raw key material. /// /// The public key of the party with which to derive a mutual secret. /// The raw key agreement. + /// + /// Care must be taking when using the raw derived secret agreement value. The raw value is expected to be used + /// as input in to a Key Derivation Function, and not used directly as key material. + /// /// /// is . /// @@ -150,7 +154,10 @@ public virtual byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey, /// /// The current platform does not support raw key agreement. /// - public virtual byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) + /// + /// The object has already been disposed. + /// + public virtual byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) { throw DerivedClassMustOverride(); } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanWrapper.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanWrapper.cs index a98903a1f00b47..b355ccab0a305a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanWrapper.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanWrapper.cs @@ -43,8 +43,8 @@ public override byte[] DeriveKeyFromHmac( public override byte[] DeriveKeyTls(ECDiffieHellmanPublicKey otherPartyPublicKey, byte[] prfLabel, byte[] prfSeed) => _wrapped.DeriveKeyTls(Unwrap(otherPartyPublicKey), prfLabel, prfSeed); - public override byte[] DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) => - _wrapped.DeriveSecretAgreement(Unwrap(otherPartyPublicKey)); + public override byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey) => + _wrapped.DeriveRawSecretAgreement(Unwrap(otherPartyPublicKey)); public override void FromXmlString(string xmlString) => _wrapped.FromXmlString(xmlString); From b5d3399698af7ea3fb7dc60785759b66acbb3706 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Thu, 23 Feb 2023 15:14:26 -0500 Subject: [PATCH 8/8] Fix tests to use the new name --- .../ECDiffieHellmanTests.NistValidation.cs | 4 ++-- .../ECDiffieHellmanTests.Raw.cs | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs index fc1286ffc21a62..d79d8de3edba17 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.NistValidation.cs @@ -229,12 +229,12 @@ private static void Verify( if (ECDiffieHellmanFactory.SupportsRawDerivation) { - byte[] rawDerived = iut.DeriveSecretAgreement(cavsPublic); + byte[] rawDerived = iut.DeriveRawSecretAgreement(cavsPublic); Assert.Equal(iutZ.ByteArrayToHex(), rawDerived.ByteArrayToHex()); } else { - Assert.Throws(() => iut.DeriveSecretAgreement(cavsPublic)); + Assert.Throws(() => iut.DeriveRawSecretAgreement(cavsPublic)); } } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs index 393b2825ba5ac8..d8d434248574ee 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.Raw.cs @@ -18,7 +18,7 @@ public static void RawDerivation_OtherKeyRequired() { AssertExtensions.Throws( "otherPartyPublicKey", - () => ecdh.DeriveSecretAgreement(null)); + () => ecdh.DeriveRawSecretAgreement(null)); } } @@ -32,7 +32,7 @@ public static void RawDerivation_SameSizeOtherKeyRequired(int aliceSize, int bob { AssertExtensions.Throws( "otherPartyPublicKey", - () => alice.DeriveSecretAgreement(bobPublic)); + () => alice.DeriveRawSecretAgreement(bobPublic)); } } @@ -45,8 +45,8 @@ public static void RawDerivation_DeriveSharedSecret_Agree(int keySize) using (ECDiffieHellmanPublicKey alicePublic = alice.PublicKey) using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) { - byte[] aliceDerived = alice.DeriveSecretAgreement(bobPublic); - byte[] bobDerived = bob.DeriveSecretAgreement(alicePublic); + byte[] aliceDerived = alice.DeriveRawSecretAgreement(bobPublic); + byte[] bobDerived = bob.DeriveRawSecretAgreement(alicePublic); Assert.Equal(aliceDerived, bobDerived); } } @@ -60,8 +60,8 @@ public static void RawDerivation_DeriveSharedSecret_Disagree() using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) using (ECDiffieHellmanPublicKey evePublic = eve.PublicKey) { - byte[] aliceDerived = alice.DeriveSecretAgreement(bobPublic); - byte[] eveDerived = alice.DeriveSecretAgreement(evePublic); + byte[] aliceDerived = alice.DeriveRawSecretAgreement(bobPublic); + byte[] eveDerived = alice.DeriveRawSecretAgreement(evePublic); Assert.NotEqual(aliceDerived, eveDerived); } @@ -74,8 +74,8 @@ public static void RawDerivation_DeriveIsStable() using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) { - byte[] aliceDerived1 = alice.DeriveSecretAgreement(bobPublic); - byte[] aliceDerived2 = alice.DeriveSecretAgreement(bobPublic); + byte[] aliceDerived1 = alice.DeriveRawSecretAgreement(bobPublic); + byte[] aliceDerived2 = alice.DeriveRawSecretAgreement(bobPublic); Assert.Equal(aliceDerived1, aliceDerived2); } } @@ -87,7 +87,7 @@ public static void RawDerivation_NotSupported() using (ECDiffieHellman bob = ECDiffieHellmanFactory.Create(ECCurve.NamedCurves.nistP256)) using (ECDiffieHellmanPublicKey bobPublic = bob.PublicKey) { - Assert.Throws(() => alice.DeriveSecretAgreement(bobPublic)); + Assert.Throws(() => alice.DeriveRawSecretAgreement(bobPublic)); } } }