-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
X509Stores, HttpClient: Support custom CA bundle via environment variables #23666
Comments
ManagedHandler just uses SslStream, which in turn uses all of the X509Certificate intrastructure, X509Stores, etc. Why is that insufficient? |
Maybe it is in some way supported already. I've updated the title to request this as a feature of the X509Stores. |
@bartonjs, the only thing that would be needed here is helper code to parse a bundle and add the certs to the appropriate store, right? Or am I missing a larger piece of the puzzle? |
From the libcurl description, I have the impression the envvar actually becomes the entire store. So it isn't 'adding' but 'replacing'. |
And that's important? We should ensure we have the right functionality; for something like this, I don't think it needs to be a drop-in replacement with all possible libcurl features. |
It makes it possible to replace everything. But for the use-cases I listed it is not a must. It may be nice if the same file is usable by curl, dotnet, ... |
We respect the For best results, make a new directory, put the one bundle file you are interested in in it, and point both variables appropriately |
@bartonjs thank you! I will try this out next week. |
@bartonjs @stephentoub I gave this a try and On Fedora 25 using .NET Core 2.0. Using curl:
Using .NET Core, performing
.NET Core is still using the system bundle:
|
@tmds The problem is you didn't also set SSL_CERT_DIR. (From curl that would end up as "read the first certificate from each file in the directory", I guess; whereas we seem to be "read all certs from all files in the directory")
|
Setting SSL_CERT_FILE=/tmp/ca-bundle.crt works for curl.
Perhaps Ubuntu is behaving different from Fedora? |
Maybe. I don't have a Fedora machine available, but I checked it with RHEL: using System;
using System.Security.Cryptography.X509Certificates;
namespace PrintStore
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Opening LocalMachine\\Root store");
using (X509Store store = new X509Store(StoreName.Root, StoreLocation.LocalMachine, OpenFlags.ReadOnly))
{
var certs = store.Certificates;
Console.WriteLine($"Store contains {certs.Count} certificate(s)...");
}
}
}
} $ dotnet run
Opening LocalMachine\Root store
Store contains 193 certificate(s)...
$ SSL_CERT_DIR="" SSL_CERT_FILE="" dotnet run
Opening LocalMachine\Root store
Store contains 0 certificate(s)...
$ SSL_CERT_DIR="" SSL_CERT_FILE="/home/jbarton/trust/test.cer" dotnet run
Opening LocalMachine\Root store
Store contains 1 certificate(s)...
$ SSL_CERT_FILE="/home/jbarton/trust/test.cer" dotnet run
Opening LocalMachine\Root store
Store contains 194 certificate(s)... It's maybe possible that the environment variable names are different on Fedora? You could do something like this to check: $ locate libcrypto
/usr/lib64/.libcrypto.so.1.0.2k.hmac
/usr/lib64/.libcrypto.so.10.hmac
/usr/lib64/libcrypto.so.1.0.2k
/usr/lib64/libcrypto.so.10
$ cd /usr/lib64/
$ strings libcrypto.so.10 | grep SSL_CERT_
SSL_CERT_DIR
SSL_CERT_FILE My system data $ dotnet --info
.NET Command Line Tools (2.0.0)
Product Information:
Version: 2.0.0
Commit SHA-1 hash: cdcd1928c9
Runtime Environment:
OS Name: rhel
OS Version: 7
OS Platform: Linux
RID: rhel.7-x64
Base Path: /opt/rh/rh-dotnet20/root/usr/lib64/dotnet/sdk/2.0.0/
Microsoft .NET Core Shared Framework Host
Version : 2.0.0
Build : N/A Our functions are fairly straightforward if you want to try debugging to see what's happening: https://github.com/dotnet/corefx/blob/release/2.0.0/src/Native/Unix/System.Security.Cryptography.Native/pal_x509_root.cpp |
@bartonjs I will try it next week. We are testing in a different way. Can you check if HttpClient uses those envvars (that is how I am testing)? |
Fedora behaves the same for the X509Store:
So the managed http handler will pick those up. It's a bit weird you need to set both variables to affect the X509Store and only one to affect the The curl handler in .NET Core 2.0 (HttpClient) ignores those variables for each attempt I have done (on Fedora 24, 25 and RHEL 7.3). |
@stephentoub @bartonjs the user of libcurl is responsible for picking up these envvars and passing them to curl via |
For reference: |
@stephentoub If there will still be a curlhandler in 2.1, I can implement the passing |
There very likely will be. We'd only be able to remove it if we're able to implement both HTTP/2 and Negotiate/NTLM auth in ManagedHandler in time, and while I think there's a reasonable chance we'll be able to do some of that, I don't expect we'll be able to do all of it (unless others want to jump in and help).
I'm not entirely following this thread. Can you summarize for me what the issue is, what this would provide, etc.? Thanks! |
Sometimes you want to override the CAs trusted by an application. The openssl way of enabling this is using the The curl command picks up those variables and passes them to libcurl. I can add the equivalent code to corefx. |
Ah, this is the part I was missing. So you're saying that these env vars are respected by curl, and they're respected by ManagedHandler because they're respected by X509Store because they're respected by OpenSSL, but they're not respected by libcurl itself and thus not by CurlHandler? |
This is how I understood it from @bartonjs
Yes. Libcurl isn't picking these up by default. It has options for them. |
Implemented in dotnet/corefx#25219 |
Something similar to curl's CURL_CA_BUNDLE.
from man curl:
This provides a global way to provide custom CA for all dotnet application. It avoids applications breaking and having to implement their own.
This is in particular useful in corporate environments where the https proxy is signed by a custom CA.
@stephentoub @davidsh @geoffkizer @wfurt @karelz
The text was updated successfully, but these errors were encountered: