[API Proposal]: Add ECDiffieHellman.DeriveSecretAgreement method

[renestein]

Background and motivation

When the need to use the shared secret (agreement) arises, we use our wrapper of the CNG API on Windows (10). We use reflection on non-Windows platforms because the method DeriveSecretAgreement in EcDiffieHellmanOpenSsl class is declared private.

private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher)

runtime/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs

Line 75 in e55c908

private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher)

As always, the code that uses reflection depends on the private implementation of the class, which we do not control, so the code is fragile and error-prone. The code without obvious optimizations might look like the following code. We would rather prefer to use public and supported API than this "hack".

 private static CoreECDHAsymmetricAlgorithm TryCreateFrom(object asymmetric, bool failOnError, bool ownsAlgorithm)
        {
#if STD21
            var inner = asymmetric as ECDiffieHellman;
            if (inner == null)
#else
            var inner = asymmetric as AsymmetricAlgorithm;
            if (inner == null || !_ecdhType.IsInstanceOfType(inner))
#endif
            {
                if (failOnError) throw new CryptographicException("ECDiffieHellman algorithm expected.");
                return null;
            }

            if (Environment.Version.Major >= 7)
            {
                FieldInfo wrappedField = inner.GetType().GetField("_wrapped", BindingFlags.Instance | BindingFlags.NonPublic);
                if (wrappedField != null)
                {
#if STD21
                    var wrapped = wrappedField.GetValue(inner) as ECDiffieHellman;
                    if (wrapped != null)
#else
                    var wrapped = wrappedField.GetValue(inner) as AsymmetricAlgorithm;
                    if (_ecdhType.IsInstanceOfType(wrapped))
#endif
                    {
                        inner = wrapped;
                    }
                }
            }

#if STD21
            MethodInfo deriveSecretAgreementMethod = inner.GetType().GetMethod("DeriveSecretAgreement", BindingFlags.Instance | BindingFlags.NonPublic, null, new Type[] { typeof(ECDiffieHellmanPublicKey), typeof(IncrementalHash) }, null);
            if (deriveSecretAgreementMethod != null && deriveSecretAgreementMethod.ReturnType != typeof(byte[]))
            {
                deriveSecretAgreementMethod = null;
            }
#else
            MethodInfo deriveSecretAgreementMethod = inner.GetType().GetMethod("DeriveSecretAgreement", BindingFlags.Instance | BindingFlags.NonPublic, null, new Type[] { _ecdhPublicKeyType, typeof(IncrementalHash) }, null);
            if (deriveSecretAgreementMethod == null || deriveSecretAgreementMethod.ReturnType != typeof(byte[]))
            {
                if (failOnError) throw new CryptographicException("Unsupported instance of ECDiffieHellman algorithm.");
                return null;
            }
#endif

//CoreECDHAsymmetricAlgorithm is our class that derives shared secret.
            return new CoreECDHAsymmetricAlgorithm(inner, deriveSecretAgreementMethod, ownsAlgorithm);
        }
        
        
        

API Proposal

namespace System.Security.Cryptography
{
    public partial class ECDiffieHellman : ECAlgorithm
    {
        public virtual byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey);
    }
}

API Usage

//Create ECDH instance for Bob
using var bobEcdh = ECDiffieHellman.Create();
//Create ECDH instance for Alice
using var aliceEcdh = ECDiffieHellman.Create();
//Generate keys
bobEcdh.GenerateKey(eccCurve);
aliceEcdh.GenerateKey(eccCurve);
//Obtain shared secret
var sharedSecret = bobEcdh.DeriveSecretAgreement(aliceEcdh.PublicKey);

Alternative Designs

Add public method DeriveSecretAgreement only to specific ECDH classes (EcDiffieHellmanOpenSsl , ECDiffieHellmanAndroid.cs?, iOS?).

Better method name?

namespace System.Security.Cryptography;

 public abstract partial class ECDiffieHellman : ECAlgorithm
{

   public virtual byte[] DeriveSharedSecret(ECDiffieHellmanPublicKey otherPartyPublicKey)
   {
            throw DerivedClassMustOverride();   
   }
}

Risks

Shared raw secret (Truncate method in CNG BCryptDeriveKey) is available only on Windows 10?

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

When the need to use the shared secret (agreement) arises, we use our wrapper of the CNG API on Windows (10). We use reflection on non-Windows platforms because the method DeriveSecretAgreement in EcDiffieHellmanOpenSsl class is declared private.

private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher)

runtime/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSsl.Derive.cs

Line 75 in e55c908

private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher)

As always, the code that uses reflection depends on the private implementation of the class, which we do not control, so the code is fragile and error-prone. The code without obvious optimizations might look like the following code. We would rather prefer to use public and supported API than this "hack".

 private static CoreECDHAsymmetricAlgorithm TryCreateFrom(object asymmetric, bool failOnError, bool ownsAlgorithm)
        {
#if STD21
            var inner = asymmetric as ECDiffieHellman;
            if (inner == null)
#else
            var inner = asymmetric as AsymmetricAlgorithm;
            if (inner == null || !_ecdhType.IsInstanceOfType(inner))
#endif
            {
                if (failOnError) throw new CryptographicException("ECDiffieHellman algorithm expected.");
                return null;
            }

            if (Environment.Version.Major >= 7)
            {
                FieldInfo wrappedField = inner.GetType().GetField("_wrapped", BindingFlags.Instance | BindingFlags.NonPublic);
                if (wrappedField != null)
                {
#if STD21
                    var wrapped = wrappedField.GetValue(inner) as ECDiffieHellman;
                    if (wrapped != null)
#else
                    var wrapped = wrappedField.GetValue(inner) as AsymmetricAlgorithm;
                    if (_ecdhType.IsInstanceOfType(wrapped))
#endif
                    {
                        inner = wrapped;
                    }
                }
            }

#if STD21
            MethodInfo deriveSecretAgreementMethod = inner.GetType().GetMethod("DeriveSecretAgreement", BindingFlags.Instance | BindingFlags.NonPublic, null, new Type[] { typeof(ECDiffieHellmanPublicKey), typeof(IncrementalHash) }, null);
            if (deriveSecretAgreementMethod != null && deriveSecretAgreementMethod.ReturnType != typeof(byte[]))
            {
                deriveSecretAgreementMethod = null;
            }
#else
            MethodInfo deriveSecretAgreementMethod = inner.GetType().GetMethod("DeriveSecretAgreement", BindingFlags.Instance | BindingFlags.NonPublic, null, new Type[] { _ecdhPublicKeyType, typeof(IncrementalHash) }, null);
            if (deriveSecretAgreementMethod == null || deriveSecretAgreementMethod.ReturnType != typeof(byte[]))
            {
                if (failOnError) throw new CryptographicException("Unsupported instance of ECDiffieHellman algorithm.");
                return null;
            }
#endif

//CoreECDHAsymmetricAlgorithm is our class that derives shared secret.
            return new CoreECDHAsymmetricAlgorithm(inner, deriveSecretAgreementMethod, ownsAlgorithm);
        }
        
        
        

API Proposal

namespace System.Security.Cryptography;

 public abstract partial class ECDiffieHellman : ECAlgorithm
{
   public virtual byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey)
   {
            throw DerivedClassMustOverride();   
   }
}

API Usage

//Create ECDH instance for Bob
using var bobEcdh = ECDiffieHellman.Create();
//Create ECDH instance for Alice
using var aliceEcdh = ECDiffieHellman.Create();
//Generate keys
bobEcdh.GenerateKey(eccCurve);
aliceEcdh.GenerateKey(eccCurve);
//Obtain shared secret
var sharedSecret = bobEcdh.DeriveSecretAgreement(aliceEcdh.PublicKey);

Alternative Designs

Add public method DeriveSecretAgreement only to specific ECDH classes (EcDiffieHellmanOpenSsl , ECDiffieHellmanAndroid.cs?, iOS?).

Better method name?

namespace System.Security.Cryptography;

 public abstract partial class ECDiffieHellman : ECAlgorithm
{

   public virtual byte[]? DeriveSharedSecret(ECDiffieHellmanPublicKey otherPartyPublicKey)
   {
            throw DerivedClassMustOverride();   
   }
}

Risks

Shared raw secret (Truncate method in CNG BCryptDeriveKey) is available only on Windows 10?

Author:	renestein
Assignees:	-
Labels:	api-suggestion, area-System.Security, untriaged
Milestone:	-

[bartonjs]

Windows certainly went a long time without exposing the raw derived secret directly, suggesting that the need for it should be low. Of course, they did eventually add it, so there must be some scenarios that they recognized. I'd like to understand what your scenario is that can't be accomplished by the existing derivation routines.

For .NET 7 this is a problem, because we still support Windows 7 and 8.1. For .NET 8 it might be doable, though I think we're still supporting Windows Server 2012 and Windows Server 2012 R2, which means we'd still have a support hole in this new method.

[renestein]

@bartonjs Thanks for the response.

I don't know why the CNG "BCRYPT_KDF_RAW_SECRET (L"TRUNCATE")" derivation function was added, but from our point of view, one of the main problems is with the padding of (some) secrets in our SSH and TLS implementation.
(e. g. TLS, SSH )

The comment form the code.

 // MS CNG ECDH provider does not reveal the shared key, which means we have to use its key material deriver.
Unfortunately, this key deriver was not intended for SSH key derivation. Even though we are able to get
it to work in most cases, it's not possible to achieve 100% chance of success due to unsuitable padding
of rare secrets that are shorter than the key.
Unfortunately, until Microsoft updates its CNG providers to be suitable for SSH key derivation,
                // we can't do anything about it. The recommended solution is to use a plugin.

And it is not possible to implement ECDH ciphers for SSH protocol using the .NET ECDiffieHellman class.

I understand that the proposed new method in the ECDiffieHellman class might on older platforms/OS cause the issues you have described. For now, we would be very grateful if the private method
private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher) in the EcDiffieHellmanOpenSsl class would become public. Sounds feasible?

[lukaaash]

Basically, without an API for this, there is no way to implement ECDH ciphers (RFC 5656) for SSH protocol in .NET without resorting to P/Invoke (using different native API on each platform), custom cryptographic primitives, or the clunky "hack" outlined above.

[bartonjs]

Ah, the problem is that the SSH version wants K as an mpint, which may involve either adding or removing some leading 0x00s... which you can't know without seeing the K value. Well, that's a scenario 😄.

For now, we would be very grateful if the private method
private byte[]? DeriveSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey, IncrementalHash? hasher) in the EcDiffieHellmanOpenSsl class would become public. Sounds feasible?

No. That's certainly not the shape we'd use for a public method; all of our public members are designed explicitly to be public and go through the API Review process.

Unless we add the member on the base type there's no way to use it on macOS/iOS/Android (no one has suggested an interop case where they needed to hydrate an instance from a native pointer, or get the native pointer to call a native function, on those platforms), and adding the member with the right shape on types like ECDiffieHellmanOpenSsl creates a slight complication for us when we move it to the base member.

We're already exceeding our capacity for the .NET 7 release, so there's no room to insert this.

[renestein]

@bartonjs
We are looking forward to .NET 8 release.
Until then, we pray for avoiding the refactoring or significant overhaul of the incriminated .NET classes. 😄
Thanks again for your fast replies.

[vcsjones]

@bartonjs the proposal for this seems pretty straight forward. Is there anything holding this up for [api-ready-for-review]?

[bartonjs]

I'm wondering if we should make the method name a bit more... discouraging. DeriveRawSecretAgreement (inserting the infix "Raw"). Just so it's not accidentally suggesting it's the best use. (IIRC, using the raw agreement is discouraged by NIST, which is why CNG didn't offer it at all, initially)

[vcsjones]

@bartonjs

I'm wondering if we should make the method name a bit more... discouraging.

I don't have any particularly strong feels on the name. We can put Raw in the name and put some stern wording the <remarks>

using the raw agreement is discouraged by NIST

Yeah. The use case is more for people that want to KDF the output themselves (e.g. SSH).

[ghost]

I also have a need to get access to the ECDH raw key agreement. The PIN/UV Auth Protocol 2 in CTAP 2.1 (for FIDO) uses ECDH with HMAC-SHA-256 as the KDF, where the key used in the KDF is the raw key agreement. .NET doesn't offer this. (I understand that we want discourage developers from using the raw key agreement except as input into a KDF.)

[terrajobst]

Video

• Looks good as proposed

namespace System.Security.Cryptography;

public partial class ECDiffieHellman : ECAlgorithm
{
    public virtual byte[] DeriveRawSecretAgreement(ECDiffieHellmanPublicKey otherPartyPublicKey);
}