Define KeyClient.GetCryptographyClient to return a CryptographyClient with shared pipeline

[heaths]

Based on recent guidelines, KeyClient should define a GetCryptographyClient that returns a CryptographyClient which shares the same pipeline (transport, diagnostics, policies, etc.) with the KeyClient. This is already being done for an IKeyEncryptionKeyResolver, so we can probably use the same constructors.

Be sure to add funnel a client created this way through some of the same tests to ensure no regressions or behavioral differences.

[dxynnez]

Hi @heaths - I am trying to understanding the best practice for using CryptographyClient as it's a bit different than other clientS (it's tied to each key while others are tied to each key vault account).

By adding the GetCryptographyClient api into the KeyClient, it seems to suggest that the CryptographyClient doesn't have to be used as singleton and we can just get it from the keyclient. Would there be any concern if we do that (performance wise, especially for connection pooling)? The new key vault related clients are a bit different than the previous version which make it inconvenient to access different keyvault accounts as now we would need to cache each client for each account (as recommended by other folks working on azure sdk, we can and should use the clients in the new SDK as singleton as the new SDK has addressed most common problems with http client - e.g. pooling, connection recycling etc.). For CryptographyClient it's even worse and we really don't want to cache it for each keyvault account + individual key that we use for crypto related tasks. Could you elaborate more on what would be the most efficient way to use CryptographyClient to work with different keyvaults & keys in the current design? Thanks so much!

[heaths]

KeyClient.GetCryptographyClient is just a convenience method. It takes a key name and optional key version just like new CryptographyClient does. The only difference is that GetCryptographyClient shares the same pipeline as the KeyClient, which decrease memory consumption, shares custom pipeline policies, etc.

Yes, all new clients in Azure SDKs are tied to a single endpoint by design. You can still share an HttpClient, policies, and other settings by sharing client options passed to each class. In that case, the client classes themselves are nothing more than thin wrappers. The CryptographyClient is tied to a single key for convenience in most cases as well, and the public key material (or entire key if you passed a JWK) is cached.

If you have a scenario where you need to access arbitrary endpoints and/or keys, you could create an indexed cache on, say, the endpoint as the key with a client as the value. A basic example would be:

ConcurrentDictionary<Uri, KeyClient> cache = new();
DefaultAzureCredential credential= new();
KeyClientOptions options = new()
{
    Diagnostics =
    {
        // set common options...
        IsDistributedTracingEnabled = true,
    },
};

Uri endpoint = new Uri("https://myvault.vault.azure.net");
KeyClient client = cache.GetOrAdd(endpoint, e => new KeyClient(e, credential, options));

[dxynnez]

the public key material is cached.

Could you elaborate a bit more on how the public key is cached? It seems to only have information about the key URI. And per my understanding retrieving keys require the keys get permission while crypto related operations have their own permissions, it's hard to imagine how a principal with only crypto related permission would be able to cache the public key...

If you have a scenario where you need to access arbitrary endpoints and/or keys, you could create an indexed cache on, say, the endpoint as the key with a client as the value

This is actually what we are doing right now, but to support crypto operation using arbitrary keys we would need to cache the CryptographyClient per endpoint + key - that would be a lot. Is there any concern if we only cache the KeyClient and use the GetCryptographyClient to get a new instance of the CryptographyClient every time? Per my understanding if we don't use custom transport in our ClientOptions then it should just use a shared transport which pools connections across all clients in the new Azure SDK. The only down side of calling the GetCryptographyClient is probably just the overhead to instantiate the CryptographyClient but as you mentioned it's pretty thin also the pipeline would be shared so I wouldn't expect there to be much impact performance wise. But if there is any built-in caching (e.g. public key) at the instance level then it would be a whole different story...

[heaths]

The first crypto operation you do will attempt to fetch the public key material if you have the keys/get permission. If it can be retrieved, public key operations are done locally. If not, every operation is routed through Key Vault or Managed HSM.

You could call GetCryptographyClient() every time, yes. It would be an extra allocation for the class itself, but minimal compared to constructing a new CryptographyClient from public constructors currently, which create a new pipeline each time.

[dxynnez]

Thanks @heaths . I guess that's only true for the CryptographyClient constructed through the public constructors, not for the one returns by GetCryptographyClient (both remoteprovder & provider are set to the RemoteCryptographyClient)?

May I know when would the v4.3.0 be officially released?

[heaths]

You're right. Actually, that's a bug that it uses the remote provider only. I'll get that fixed and publish an update.

4.3.0 will GA early next year.

[dxynnez]

Thanks @heaths for the clarification! I guess in that case we shouldn't call GetCryptographyClient to get a fresh client every time as that would double the cost of all crypto related operations (first request try to get key, if fail then fire another one to call crypto related API directly). Do I understand it right?

[heaths]

Ideally you cache the returned CryptographyClient. We'll only try to fetch one time once the PR is merged and new package released. But if we can fetch, any public key crypto operation happens locally. So encrypting, wrapping, and verifying can be done locally if the key can be retrieved.

[heaths]

@dxynnez if you'd like to try out the new package with the fix to attempt to cache the key locally from KeyClient.GetCryptographyClient, please use https://www.nuget.org/packages/Azure.Security.KeyVault.Keys/4.3.0-beta.4 and open new issues if you find any.

Thanks!