[Feature request / Design proposal] Allow EcdhKeyExchangeProvider to be extensible like the other security providers

[GregDomzalski]

Summary

Microsoft.IdentityModel.Tokens should expose extension points for custom EcdhKeyExchangeProvider implementations where the private key may not be available in memory. Providing these extensions would bring EcdhKeyExchangeProvider in line with the other cryptographic providers within the library.

Motivation and goals

The Microsoft.IdentityModel.Tokens library provides the ability for consumers to provide custom SecurityKey implementations and cryptographic providers. This is useful when one wants (or needs) to store private key material in a way that is otherwise inaccessible to the built-in .NET cryptographic libraries. This could be a key exposed through a 3rd party crypto library such as BouncyCastle, a secrets manager such as 1Password (or an SSH agent), or on a hardware backed device such as an HSM.

Yubico (CC: @JamieHankins @aafortner) is collaborating with a sister-team within Entra ID that requires the use of the ECDH functionality within the Microsoft.IdentityModel.Tokens library. One set of private keys used for key agreement is stored on Yubico's YubiHSM product. These are private keys are never available to the host computer that is executing the Microsoft.IdentityModel.Tokens library. This also implies that the actual ECDH key agreement operation must be performed on the YubiHSM.

The current implementation of EcdhKeyExchangeProvider is currently not extensible. There are no override-able methods like there are on other *Provider classes. The current implementation also assumes in-memory access to the private key which may not always be available in the case of custom implementations.

This feature would allow for custom implementations of EcdhKeyExchangeProvider in a similar manner to how other providers (e.g. SignatureProvider) allow for extension.

A possible implementation has been created here: YubicoLabs azure-activedirectory-identitymodel-extensions-for-dotnet:ecdh-keyexchange-provider

In scope

• Allow overriding or extension of the DeriveKeyFromHash step within the GenerateKdf function.
• Add CreateEcdhKeyExchangeProvider or similar to CryptoProviderFactory to allow instantiation of custom provider implementations.
• Remove any assumptions within EcdhKeyExchangeProvider around access to private key material.
• Modify existing internal uses of EcdhKeyExchangeProvider to use CryptoProviderFactory instead of instantiating the provider directly.
• Any additional properties, constants, glue code to bring EcdhKeyExchangeProvider in line with other crypto providers

Out of scope

• Breaking API signature changes
• Any other providers

Risks / unknowns

• EcdhKeyExchangeProvider is an existing class with existing interface expectations. While I have a proposal that does not appear to cause any breaking API surface changes, it's always possible something has been missed.
• Behavior changes are also challenging to avoid, but through careful refactoring and AppContext switches, it should be possible.
• The trickiest part is the constructor. The existing public constructor of EcdhKeyExchangeProvider is doing some verification which presently assumes access to the private key. The same checks could be implemented in a different way.
• Other crypto providers implement IDisposable and also have a Release mechanism through CryptoProviderFactory.Release*Provider(). EcdhKeyExchangeProvider currently implements neither of these. IDisposable would be a breaking API change. Adding a ReleaseEcdhKeyExchangeProvider method to CryptoProviderFactory could provide a "lesser of two evils" approach to still allowing for resource cleanup. Since there is no built-in implementation of this method for this provider, there should be no behavior change. The built-in version can simply no-op.

Examples

Extending EcdhKeyExchangeProvider:

public class MyProvider : EcdhKeyExchangeProvider
{
    private IKeyStore _keyStore;
    private ICustomKey _myKey;

    public MyProvider(
        IKeyStore keyStore, // Representation of the out-of-band crypto
        SecurityKey privateKey,
        SecurityKey publicKey,
        string alg,
        string enc) :
        base(
            keyStore.GetByKeyId(privateKey.KeyId),
            publicKey,
            alg,
            enc)
    {
        _keyStore = keyStore;
        _myKey = keyStore.GetByKeyId(privateKey.keyId);
    }

    public override byte[] DeriveKeyFromHash(
        ECDiffieHellmanPublicKey otherPartyPublicKey,
        HashAlgorithmName hashAlgorithm,
        byte[] prepend,
        byte[] append)
    {
        byte[] keyAgreement = _keyStore.EcdhKeyAgreement(_myKey, otherPartyPublicKey);
        return _keyStore.Hash(hashAlgorithm, [prepend, keyAgreement, append]);
    }
}

[brentschmaltz]

@GregDomzalski @jennyf19 i think the overall this feature makes sense.

[GregDomzalski]

@brentschmaltz @jennyf19 Great! Would you like me to add the next level of detail mentioned in the issue template for feature requests? Or should we open the PR and go through any feedback there?