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

Add TrustedRootCertificates property to HttpClientHandler #39835

Open
kakone opened this issue Jul 23, 2020 · 25 comments
Open

Add TrustedRootCertificates property to HttpClientHandler #39835

kakone opened this issue Jul 23, 2020 · 25 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http
Milestone

Comments

@kakone
Copy link

kakone commented Jul 23, 2020

Background and Motivation

It's actually complicate to add a custom root certificate authority to HttpClientHandler. You need to use the ServerCertificateCustomValidationCallback and it's not very easy to use to compare a certificate in the certification chain to your custom root certificate. So, I see everywhere only a returns true; in the ServerCertificateCustomValidationCallback 😱 (a lot of "good" answers in stackoverflow do this).

Proposed API

Like we have a ClientCertificates property, we could have a TrustedRootCertificates property in HttpClientHandler class (like in the AndroidClientHandler class) :

public System.Security.Cryptography.X509Certificates.X509CertificateCollection TrustedRootCertificates { get; }
@kakone kakone added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jul 23, 2020
@kakone
Copy link
Author

kakone commented Jul 23, 2020

I see the case very regularly (the last time in this Twitter thread of @davidfowl).

@vcsjones
Copy link
Member

it's not very easy to use to compare a certificate in the certification chain to your custom root certificate

This is a lot easier now in .NET 5 with CustomTrustStore. For example, you could do something like:

HttpClientHandler handler = new HttpClientHandler();
handler.ServerCertificateCustomValidationCallback = (message, cert, chain, _) => {
    chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
    chain.ChainPolicy.CustomTrustStore.Add(/* your custom root, add as many roots as you need */);
    chain.ChainPolicy.ExtraStore.Add(/* add any additional intermediate certs */);
    // any additional chain building settings you want
    return chain.Build(cert);
};

we could have a TrustedRootCertificates property in HttpClientHandler class

Perhaps a new API would make this useful, even though X509Chain now makes this only a few lines of code. That said, if we want to add something like TrustedRootCertificates, there is probably merit for ExtraCertificates as well.

The use-case for something like TrustedRootCertificates is not having a root in a trust store. If you don't have the root in the trust store, then you probably don't have any of its intermediates either. So the ExtraCertificates would fill that need. AIA fetching might work, but in the case of private CAs with custom roots, I often see them set up with no AIA URL at all.

@vcsjones
Copy link
Member

cc @bartonjs @wfurt

@kakone
Copy link
Author

kakone commented Jul 23, 2020

This is a lot easier now in .NET 5 with CustomTrustStore

Great, I hadn't noticed these new properties on ChainPolicy. Indeed, it's much easier now.

The use-case for something like TrustedRootCertificates is not having a root in a trust store. If you don't have the root in the trust store, then you probably don't have any of its intermediates either. So the ExtraCertificates would fill that need.

Yes, you are totally right, it's exactly that. To add an ExtraCertificates property would be perfect and would cover most needs.

@wfurt
Copy link
Member

wfurt commented Jul 23, 2020

If anything, I would add it to https://docs.microsoft.com/en-us/dotnet/api/system.net.security.sslclientauthenticationoptions?view=net-5.0

But as @vcsjones it is pretty simple now.

This is dup of #39590.

@ghost
Copy link

ghost commented Jul 23, 2020

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

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

Rather than needing to add more properties and/or mode selectors in the future that can poorly interact, maybe a better answer is to provide some delegate factories.

public partial class SslStream
{
    public static RemoteCertificateValidationCallback CreateCustomRootRemoteValidator(X509Certificate2Collection trustedRoots, X509Certificate2Collection intermediates = null)
    {
        if (trustedRoots == null)
            throw new ArgumentNullException(nameof(trustedRoots));
        if (trustedRoots.Count == 0)
             throw new ArgumentException("No trusted roots were provided", nameof(trustedRoots));

        // Let's avoid complex state and/or race conditions by making copies of these collections.
        // Then the delegates should be safe for parallel invocation (provided they are given distinct inputs, which they are).
        X509Certificate2Collection roots = new X509Certificate2Collection(trustedRoots);
        X509Certificate2Collection intermeds = null;

        if (intermediates != null)
        {
               intermeds = new X509Certificate2Collection(intermediates);
        }

        intermediates = null;
        trustedRoots = null;

        return (sender, serverCert, chain, errors) =>
        {
            // Missing cert or the destination hostname wasn't valid for the cert.
            if ((errors & ~SslPolicyErrors.RemoteCertificateChainErrors) != 0)
            {
                return false;
            }

            for (int i = 1; i < chain.ChainElements.Count; i++)
            {
                chain.ChainPolicy.ExtraStore.Add(chain.ChainElements[i].Certificate);
            }

            if (intermeds != null)
            {
                chain.ChainPolicy.ExtraStore.AddRange(intermeds);
            }

            chain.ChainPolicy.CustomTrustStore.Clear();
            chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
            chain.ChainPolicy.CustomTrustStore.AddRange(roots);
            return chain.Build(serverCert);
        };
    }
}

And then at the HttpClient layer:

public partial class HttpClientHandler
{
    public static Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> CreateCustomRootValidator(X509Certificate2Collection trustedRoots, X509Certificate2Collection intermediates = null)
    {
        RemoteCertificateValidationCallback callback = SslStream.CreateCustomRootRemoteValidator(trustedRoots, intermediates);
        return (message, serverCert, chain, errors) => callback(null, serverCert, chain, errors);
    }
}

Yeah, new methods would have to / could be added for "custom anchors" (being able to trust non-root CAs for the call), when that support ever gets added to X509Chain, but making delegate factories reuses the existing infrastructure.

@progmars
Copy link

progmars commented Sep 3, 2021

I'm wondering what should I do if I want to combine both - system trust store and my own root certificate? It seems strange that X509ChainTrustMode both modes are exclusive. I imagined CustomTrustStore would work as an addition to the system trust store, as if I had temporarily installed my custom root cert into the system Trusted Root store.

If the combination of both stores is made possible, it would make custom validators work "automagically" without any code changes, even if the remote server decided to drop its custom CA and move to a public CA instead.

@bartonjs
Copy link
Member

I'm wondering what should I do if I want to combine both - system trust store and my own root certificate?

Essentially, chain.ChainPolicy.CustomTrustStore.AddRange(new X509Store(StoreName.Root, StoreLocation.CurrentMachine, OpenFlags.ReadOnly).Certificates);, though X509Store is IDisposable, and disposing it is goodness. (StoreLocation.CurrentUser may also be interesting).

It seems strange that X509ChainTrustMode both modes are exclusive.

This is how all of the libraries that .NET depends on operates.

The main scenario is for certificate chain pinning, e.g. "I know that this cert will come from Let's Encrypt. If the chain says it comes from elsewhere then there's a man in the middle that I don't want to talk to ... including some network proxy that my domain administrator set up".

@rellis-of-rhindleton
Copy link

Hey folks -- looking over the examples above, trying to make sure I'm handling a certificate pinning scenario correctly. (Specifically public key pinning, as in Certificate and public key pinning.)

The documentation that I've seen isn't thorough and I don't have a good feel for what the situation is at the point the callback is called. Is it safe to assume that the underlying code has already checked for things like subject/name mismatch? Would that show up in the provided errors parameter? Or do I need to check those explicitly?

@wfurt
Copy link
Member

wfurt commented Apr 20, 2022

Yes, all checks will be done and result would be passing in as parameter. Basically the chain is always constructed after the handshake completes, SslStream calls chain.Build and passes status and chain itself to the callback.

@rellis-of-rhindleton
Copy link

So the callback is just to override or add extra checks? Works for me, thank you.

@wfurt
Copy link
Member

wfurt commented Apr 20, 2022

yes. In most cases people want to override failures and add custom trust. You can add extra restrictions - like pinning - if you want. (or do what ever extra based on your application logic)

@rellis-of-rhindleton
Copy link

Okay, great. That's the best of both worlds.

As @kakone observed, it's common to find suggestions on how to disable certification validation, but if you want to add custom validation logic then I'm not sure there is anything in the Microsoft docs to help. I had to come to this issue to find what I needed. I might try to sum some of this up in a docs pull request.

@wfurt
Copy link
Member

wfurt commented Apr 21, 2022

There is "Edit" button on doc pages and anybody can submit changes for review.

@lukeschlather
Copy link

The SocketsHttpHandler docs seem to imply that HttpClientHandler is deprecated but I don't see anything like ServerCertificateCustomValidationCallback there. Is the best solution to fall back to HttpClientHandler or is there a way to do this with SocketsHttpHandler?

https://learn.microsoft.com/en-us/dotnet/api/system.net.http.socketshttphandler?view=net-7.0

@wfurt
Copy link
Member

wfurt commented Sep 27, 2023

I would not see HttpClientHandler handler as obsolete. That is API shape that is supported on all platforms and .NET Framework and .NET Standard. SocketsHttpHandler simply has more and in it powers HttpClientHandler behind the scenes on most platforms anyway.

I'm not sure I fully understand you comment but if you ant custom validation look at SocketsHttpHandler.SslOptions. That allows you do custom validation and certificate selection with callbacks as well as CertificateChainPolicy starting with .NET 7 & 8.

@lukeschlather
Copy link

Ah ok, so I can just do SocketsHttpHandler.SslOptions.CertificateChainPolicy.CustomTrustStore.Add(). The only examples I could find used SocketsHttpHandler.SslOptions.RemoteCertificateValidationCallback which seemed to require me to rewrite a lot of the certificate validation logic.

@wfurt
Copy link
Member

wfurt commented Sep 27, 2023

That really depends on what you are trying to do. The callback always gets populated X509Chain and the callback can override the default decision.

Perhaps look at

public async Task SslStream_ServerUntrustedCaWithCustomTrust_OK(bool usePartialChain)

Unfortunately there is no simple addition ... that is just how X509Chain works and will also need to set TrustMode

@lukeschlather
Copy link

I just want to trust a root cert. I don't want to rethink how certificate chain validation works, that ought to be the handler's job.

@wfurt
Copy link
Member

wfurt commented Sep 27, 2023

Then just set the TrustMode to X509ChainTrustMode.CustomRootTrust and set you root CA(s). This is basically what the test does.

@lukeschlather
Copy link

Is there a cross-platform way to get the system trust store? I don't want to remove the existing certificate bundles, I want to add to it.

@wfurt
Copy link
Member

wfurt commented Sep 27, 2023

X509Store.Certificates should work AFAIK.

@davidfowl
Copy link
Member

@wfurt, is there an end to end sample documented somewhere?

@wfurt
Copy link
Member

wfurt commented Nov 25, 2024

public async Task SslStream_ServerUntrustedCaWithCustomTrust_OK(bool usePartialChain)
may be good examples.
something like

  var clientOptions = new SslClientAuthenticationOptions() { TargetHost = "target" };
  clientOptions.CertificateChainPolicy = new X509ChainPolicy()
  {
     TrustMode = X509ChainTrustMode.CustomRootTrust
  };
  clientOptions.CertificateChainPolicy.CustomTrustStore.Add(trustedRoots);       
  clientOptions.CertificateChainPolicy.ExtraStore.Add(intermediates_if_needed);

The extra store can be useful to avoid downloads, but not really needed. Also note that when CustomRootTrust is used it does not include normal system roots. If somebody wants union than one would need to add them explictly e.g. open the system root store and use AddRange to add everything to CustomTrustStore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http
Projects
None yet
Development

No branches or pull requests

9 participants