-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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 support for trusting dev certs on linux #56582
base: main
Are you sure you want to change the base?
Conversation
FYI @tmds |
return sawTrustSuccess | ||
? sawTrustFailure | ||
? TrustLevel.Partial | ||
: TrustLevel.Full | ||
: TrustLevel.None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tripped me up a bit when I read it, might be worth putting it in terms of the two things we are checking (openssl and nssdb) and maybe breaking it down into a couple of if
s with a return statement.
try | ||
{ | ||
var existingCert = new X509Certificate2(certPath); | ||
if (!existingCert.RawDataMemory.Span.SequenceEqual(certificate.RawDataMemory.Span)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it enough to compare the thumbprints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been reviewing the other implementations and noticed that they do that. I'm happy to adopt that approach if that's the standard approach. I only went further because I didn't think SHA1 was considered sufficient for anything security related.
if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable(OpenSslCertDirectoryOverrideVariableName))) | ||
{ | ||
// Warn but don't bail. | ||
Log.UnixOpenSslCertificateDirectoryOverrideIgnored(OpenSslCertDirectoryOverrideVariableName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this because the X509Chain does not use the alternative location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. The env var in question configures dev-certs
and not openssl (including X509Chain).
private static bool TryRehashOpenSslCertificates(string certificateDirectory) | ||
{ | ||
try | ||
{ | ||
// First, delete all the existing symlinks, so we don't have to worry about fragmentation or leaks. | ||
|
||
var hashRegex = OpenSslHashFilenameRegex(); | ||
var extensionRegex = OpenSslCertificateExtensionRegex(); | ||
|
||
var certs = new List<FileInfo>(); | ||
|
||
var dirInfo = new DirectoryInfo(certificateDirectory); | ||
foreach (var file in dirInfo.EnumerateFiles()) | ||
{ | ||
var isSymlink = (file.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint; | ||
if (isSymlink && hashRegex.IsMatch(file.Name)) | ||
{ | ||
file.Delete(); | ||
} | ||
else if (extensionRegex.IsMatch(file.Name)) | ||
{ | ||
certs.Add(file); | ||
} | ||
} | ||
|
||
// Then, enumerate all certificates - there will usually be zero or one. | ||
|
||
// c_rehash doesn't create additional symlinks for certs with the same fingerprint, | ||
// but we don't expect this to happen, so we favor slightly slower look-ups when it | ||
// does, rather than slightly slower rehashing when it doesn't. | ||
|
||
foreach (var cert in certs) | ||
{ | ||
if (!TryGetOpenSslHash(cert.FullName, out var hash)) | ||
{ | ||
return false; | ||
} | ||
|
||
var linkCreated = false; | ||
for (var i = 0; i < MaxHashCollisions; i++) | ||
{ | ||
var linkPath = Path.Combine(certificateDirectory, $"{hash}.{i}"); | ||
if (!File.Exists(linkPath)) | ||
{ | ||
// As in c_rehash, we link using a relative path. | ||
File.CreateSymbolicLink(linkPath, cert.Name); | ||
linkCreated = true; | ||
break; | ||
} | ||
} | ||
|
||
if (!linkCreated) | ||
{ | ||
Log.UnixOpenSslRehashTooManyHashes(cert.FullName, hash, MaxHashCollisions); | ||
return false; | ||
} | ||
} | ||
} | ||
catch (Exception ex) | ||
{ | ||
Log.UnixOpenSslRehashException(ex.Message); | ||
return false; | ||
} | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure our tool should change trust for other certificates that aren't our own. We should check that the cert we are going to trust is an https dev cert and skip it otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please elaborate? I started with logic that just manipulated certs and links in 1:1 pairs, but then learned that there are also collisions and switched to the purge-and-rehash approach used by c_rehash.
private static bool TryGetOpenSslHash(string certificatePath, [NotNullWhen(true)] out string? hash) | ||
{ | ||
hash = null; | ||
|
||
try | ||
{ | ||
// c_rehash actually does this twice: once with -subject_hash (equivalent to -hash) and again | ||
// with -subject_hash_old. Old hashes are only needed for pre-1.0.0, so we skip that. | ||
var processInfo = new ProcessStartInfo(OpenSslCommand, $"x509 -hash -noout -in {certificatePath}") | ||
{ | ||
RedirectStandardOutput = true, | ||
RedirectStandardError = true | ||
}; | ||
|
||
using var process = Process.Start(processInfo); | ||
var stdout = process!.StandardOutput.ReadToEnd(); | ||
|
||
process.WaitForExit(); | ||
if (process.ExitCode != 0) | ||
{ | ||
Log.UnixOpenSslHashFailed(certificatePath); | ||
return false; | ||
} | ||
|
||
hash = stdout.Trim(); | ||
return true; | ||
} | ||
catch (Exception ex) | ||
{ | ||
Log.UnixOpenSslHashException(certificatePath, ex.Message); | ||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused here, why do we need to compute the hash using OpenSSL, isn't our c_rehash
replacement enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called from our c_rehash
replacement. Like the original tool, we delegate the actual hashing to openssl.
It would be good to develop a POC for this with both Docker multi-stage build and SDK publish.
I don't grasp why we would do this. |
I'm not sure what these things are. I think you might be saying we should validate that the new functionality works in some specific scenarios, but I'm not familiar with those scenarios.
It's entirely possible that my comment doesn't make sense. What I had in mind is that, for OpenSSL to trust a certificate, it needs to be in a folder that is listed in the |
We can just update our Dockerfile to include it, the same way it has an instruction to expose the right ports. No need to set the environment variable in the container image directly. |
We're unlikely to do that. We don't want to change the trust model of the container images we ship. If you can add certs in the directory, you can set the ENV. I'm asking for E2E examples for the scenario that this feature targets. We can help you if you are not familiar with the specifics. As a general rule, high-level Linux features (and certs are pretty high-level) should be fully validated with the container workflow. I'm seeing a lot of assumptions here. Separately, if you are just on a raw Linux box, why not use |
Fine by me.
I can't tell if you mean you, the
Examples in the sense of user-facing samples or in the sense of manual validation?
That would be great.
I've been using both docker and GUI VMs for my validation, but I expect more real-world tests once it's ready for the Aspire folks to play with.
Tell me more? This is definitely uncertain terrain, but we've been through several rounds of consultation and prototyping.
The main reasons are:
|
Ok. I think I understand a bit better now. Here are some workflows we documented many years ago: https://github.com/dotnet/dotnet-docker/blob/main/samples/run-aspnetcore-https-development.md#linux. I'm assuming for an environment where the browser is your local machine and the app is in a container that the new workflow would be very similar. Is that fair? |
Yeah, I think you'd just additionally run |
We can collaborate offline on a workflow / writeup if you'd like. I did the same with that doc I shared with @javiercn. |
Sounds great. Let me get it through threat modelling first so we don't have to redo the docs if our approach changes. |
Force push is a rebase on top of #56701. |
Force push is just picking up fresh changes from #56701, since it wasn't working on Windows. |
// to its final location in the OpenSSL directory. As a result, any failure up until that point | ||
// is fatal (i.e. we can't trust the cert in other locations). | ||
|
||
var certDir = GetOpenSslCertificateDirectory(homeDirectory)!; // May not exist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this directory is not world readable.
There's no consistent way to do this that works for all clients on all Linux distros, but this approach gives us pretty good coverage. In particular, we aim to support .NET (esp HttpClient), Chromium, and Firefox on Ubuntu- and Fedora-based distros. Certificate trust is applied per-user, which is simpler and preferable for security reasons, but comes with the notable downside that the process can't be completed within the tool - the user has to update an environment variable, probably in their user profile. In particular, OpenSSL consumes the `SSL_CERT_DIR` environment variable to determine where it should look for trusted certificates. We break establishing trust into two categories: OpenSSL, which backs .NET, and NSS databases (henceforth, nssdb), which backs browsers. To establish trust in OpenSSL, we put the certificate in `~/.dotnet/corefx/cryptography/trusted`, run a simplified version of OpenSSL's `c_rehash` tool on the directory, and ask the user to update `SSL_CERT_DIR`. To establish trust in nssdb, we search the home directory for Firefox profiles and `~/.pki/nssdb`. For each one found, we add an entry to the nssdb therein. Each of these locations (the trusted certificate folder and the list of nssdbs) can be overridden with an environment variable. This large number of steps introduces a problem that doesn't exist on Windows or macOS - the dev cert can end up trusted by some clients but not by others. This change introduces a `TrustLevel` concept so that we can produce clearer output when this happens. The only non-bundled tools required to update certificate trust are `openssl` (the CLI) and `certutil`. `sudo` is not required, since all changes are within the user's home directory.
A belt-and-suspenders approach for dotnet trust (i.e. in addition to OpenSSL trust) that has the notable advantage of not requiring any environment variables.
Force push just replaces the merge with a rebase. The code is the same. |
Overview
There's no consistent way to do this that works for all clients on all Linux distros, but this approach gives us pretty good coverage. In particular, we aim to support .NET (esp HttpClient), Chromium, and Firefox on Ubuntu- and Fedora-based distros.
Certificate trust is applied per-user, which is simpler and preferable for security reasons, but comes with the notable downside that the process can't be completed within the tool - the user has to update an environment variable, probably in their user profile. In particular, OpenSSL consumes the
SSL_CERT_DIR
environment variable to determine where it should look for trusted certificates.We break establishing trust into two categories: OpenSSL, which backs .NET, and NSS databases (henceforth, nssdb), which backs browsers.
To establish trust in OpenSSL, we put the certificate in
~/.dotnet/corefx/cryptography/trusted
, run a simplified version of OpenSSL'sc_rehash
tool on the directory, and ask the user to updateSSL_CERT_DIR
.To establish trust in nssdb, we search the home directory for Firefox profiles and
~/.pki/nssdb
. For each one found, we add an entry to the nssdb therein.Each of these locations (the trusted certificate folder and the list of nssdbs) can be overridden with an environment variable.
This large number of steps introduces a problem that doesn't exist on Windows or macOS - the dev cert can end up trusted by some clients but not by others. This change introduces a
TrustLevel
concept so that we can produce clearer output when this happens.The only non-bundled tools required to update certificate trust are
openssl
(the CLI) andcertutil
.sudo
is not required, since all changes are within the user's home directory.Comparison with community tools
There are some community projects that have added this functionality, but they differ from this implementation in some notable ways:
SSL_CERT_DIR
Work outside this repo
We could consider pre-settingSSL_CERT_DIR
in our official docker imagesWe could consider settingSSL_CERT_DIR
in install-dotnet.sh (assuming it's already setting things likeDOTNET_ROOT
)