Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async capability when using ConfigurePrimaryHttpMessageHandler to configure certificates #71861

Open
jernejg opened this issue Jul 8, 2022 · 12 comments

Comments

@jernejg
Copy link

jernejg commented Jul 8, 2022

Hey @davidfowl (apologies for the direct tag), but in Unable to use dynamic client certificate with HttpClientFactory #932 you asked if we figured it out in the last post.

We didn't (well we did sort of with a hack).

We are a shop with hundreds of certificates, and which one to use is decided by the context of the current request. In order to get the certificate, we need to talk to the database and we would like to do that in an asynchronous way.

We sort of have a work around that is a bit dirty and it looks something like this:

public class TransportPrimaryMessageHandler : HttpClientHandler
{
    private readonly ICertificateDb _db;
    private readonly Lazy<Task<int>> _triggerOnceOnlyInit;

    public TransportPrimaryMessageHandler(ICertificateDb db)
    {
        _db = db;
        _triggerOnceOnlyInit = new Lazy<Task<int>>(InitializeTransportCertificate);
    }

    private async Task<int> InitializeTransportCertificate()
    {
        var secretInfo = await _db.GetCurrentCertificate();
        var certificate = new X509Certificate2(secretInfo.TransportCertificate, "pwd");
        return ClientCertificates.Add(certificate);
    }

    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
        CancellationToken cancellationToken)
    {
        await _triggerOnceOnlyInit.Value;
        return await base.SendAsync(request, cancellationToken);
    }
}

The lazy initiation ensures that the certificate will only be fetched and added at the first call and it is done asynchronously.

But this kinda doesn't feel right. Are there any details available for using SocketsHttpHandler as you suggested in your post?
Thanks!

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 8, 2022
@ghost
Copy link

ghost commented Jul 8, 2022

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

Issue Details

Hey @davidfowl (apologies for the direct tag), but in Unable to use dynamic client certificate with HttpClientFactory #932 you asked if we figured it out in the last post.

We didn't (well we did sort of with a hack).

We are a shop with hundreds of certificates, and which one to use is decided by the context of the current request. In order to get the certificate, we need to talk to the database and we would like to do that in an asynchronous way.

We sort of have a work around that is a bit dirty and it looks something like this:

public class TransportPrimaryMessageHandler : HttpClientHandler
{
    private readonly ICertificateDb _db;
    private readonly Lazy<Task<int>> _triggerOnceOnlyInit;

    public TransportPrimaryMessageHandler(ICertificateDb db)
    {
        _db = db;
        _triggerOnceOnlyInit = new Lazy<Task<int>>(InitializeTransportCertificate);
    }

    private async Task<int> InitializeTransportCertificate()
    {
        var secretInfo = await _db.GetCurrentCertificate();
        var certificate = new X509Certificate2(Convert.FromBase64String(secretInfo.TransportCertificate), "pwd");
        return ClientCertificates.Add(certificate);
    }

    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
        CancellationToken cancellationToken)
    {
        await _triggerOnceOnlyInit.Value;
        return await base.SendAsync(request, cancellationToken);
    }
}

The lazy initiation ensures that the certificate will only be fetched and added at the first call and it is done asynchronously.

But this kinda doesn't feel right. Are there any details available for using SocketsHttpHandler as you suggested in your post?
Thanks!

Author: jernejg
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@davidfowl
Copy link
Member

How are you managing client instances? Do you have one per tenant? The problem is you can’t safely mutate the handler like this. You need one per customer to avoid crossing the streams by mistake

@jernejg
Copy link
Author

jernejg commented Jul 10, 2022

Thanks for the heads up. I incorrectly assumed that this is solved by registering TransportPrimaryMessageHandler as transient/scoped.

You need one per customer to avoid crossing the streams by mistake

Our use case is a bit more complex. The selection of the certificate depends on a compound key of 3 values :
<customer, destination, id?> id is optional.

This means we would need to traverse all the possibilities at the startup to create all the possible http clients? I think we definitely need to approach this in a different way.

@jernejg
Copy link
Author

jernejg commented Jul 10, 2022

I guess my question is not just about async context for configuring HttpClientHandler, but is the same as this How to create HttpClientHandler later than configuration time #521 - (without pre-creating all the possible named http clients).

I will read the above thread a couple more times since I still don't understand why #521 was closed as completed.

@wfurt wfurt removed the untriaged New issue has not been triaged by the area owner label Jul 14, 2022
@wfurt wfurt added this to the Future milestone Jul 14, 2022
@wfurt
Copy link
Member

wfurt commented Jul 14, 2022

triage: we should look into how to integrate it with the factory. Not obvious how to do that at the moment.

@jernejg
Copy link
Author

jernejg commented Jul 15, 2022

Wrt to How to create HttpClientHandler later than configuration time #521 am I right thinking that you could actually add HttpClient's later?

When checking the default implementation of IHttpClientFactory I noticed it is injecting and using IOptionsMonitor<HttpClientFactoryOptions>.

Looking at the implementation of IOptionsMonitor<T>.Get(string name) :

public virtual TOptions Get(string name)
{
    name = name ?? Options.DefaultName;
    return _cache.GetOrAdd(name, () => _factory.Create(name));
}

Doesn't this mean I can create new named instances of HttpClientFactoryOptions at runtime, since if the name doesn't exist it will call _factory.Create(name)?

@jernejg
Copy link
Author

jernejg commented Jul 15, 2022

I can also see that IOptionsMonitorCache<TOptions> (the _cache above) is a thread safe concurrent dictionary with Lazy<TOptions> values which would make creating new HttpClientFactoryOptions at runtime thread safe.

@davidfowl
Copy link
Member

Would you create a unique name based on <customer, destination, id?>?

@jernejg
Copy link
Author

jernejg commented Jul 15, 2022

Still brainstorming but this is what I'm thinking:

  1. At configuration time use the standard way of creating HttpClients, that will serve as a template for creating http clients later (Template by destination - because the cardinality of destinations is a lot smaller than that of customers):
services.AddHttpClient("template-destinationA-", 
httpClient => { httpClient.BaseAddress = new Uri("https://www.destinationA.com"); })
.AddHttpMessageHandler<SomeMessageHandlerA>()
.AddHttpMessageHandler<SomeMessageHandlerB>()
.ConfigurePrimaryHttpMessageHandler<PrimaryMessageHandler>();
  1. Have a wrapper around IHttpClientFactory that would know how to check if HttpClientFactoryOptions with the name <customer, destination, id?> already exists. If it does, just delegate creating http client to the default factory.
  2. In case the HttpClientFactoryOptions with the name <customer, destination, id?> does not yet exist, do the following:
    • Based on current key <customer, destination, id?> get the right template HttpClientFactoryOptions by destination.
    • Register a new named HttpClientFactoryOptions that corresponds to <customer, destination, id?> and copy settings from the HttpClientFactoryOptions template.
    • Now that the new HttpClientFactoryOptions is registered and configured we can delegate creating client to the default factory.

I would still use the same trick to initialise the primary with the certificate at the first call to SendAsync.

@CarnaViire
Copy link
Member

Thanks for the ideas @jernejg. I was actually also thinking in the similar direction.

About initializing primary certificates --
There's a much safer alternative to the trick you came up with -- that's LocalCertificateSelectionCallback. It will be triggered upon HTTP connection creation and allows you to select the cert based on the destination, as well as other things. If set up inside HttpClientFactory, it will look something like this:

services.AddHttpClient(...)
    .ConfigurePrimaryHttpMessageHandler(() =>
    {​​​​​
        return new SocketsHttpHandler()
        {
            SslOptions = new SslClientAuthenticationOptions()
            {
                LocalCertificateSelectionCallback = (object sender, string targetHost, X509CertificateCollection localCertificates, X509Certificate? remoteCertificate, string[] acceptableIssuers) => 
                {
                     // extract certificate from database or whatever cache you have and return it here
                }
            }
        };
    });

HttpClient already has separate connection pools per server, so, while the callback doesn't fully solve your problem, you can at least narrow down your key from <customer, destination, id?> to <customer, id?>.

@jernejg
Copy link
Author

jernejg commented Jul 15, 2022

Thank you @CarnaViire I'll take a look.
Looks like in the end I'll need to decide between less safer way or blocking the thread since LocalCertificateSelectionCallback is not async :/

@davidfowl
Copy link
Member

The cert callback does work if you can use the host name (sni) but it’s not enough because of the other cache keys required. The other problem with the callback is the lack of async

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants