libstore/filetransfer: add support for MTLS authentication #13030
Conversation
Certificate/private-key pair can be configured globally and it will be handled by libcurl.
| @@ -371,6 +371,11 @@ struct curlFileTransfer : public FileTransfer | |||
| curl_easy_setopt(req, CURLOPT_SSL_VERIFYHOST, 0); | |||
| } | |||
|
|
|||
| if (settings.clientCertFile != "" && settings.clientKeyFile != "") { | |||
| curl_easy_setopt(req, CURLOPT_SSLCERT, settings.clientCertFile.get().c_str()); | |||
There was a problem hiding this comment.
Is there a privacy implication if we start sending this to all binary caches unconditionally?
There was a problem hiding this comment.
My understanding is that the server asks the client to present its certificate by sending the list of accepted issuers in a CertificateRequest message. It won't be presented, unless the configured certificate matches the requested issuer.
There was a problem hiding this comment.
Would this allow to have multiple ssl certificates in a single file and curl than uses whatever is requested? Than there would be a reasonable way to support multiple caches with different certs each.
There was a problem hiding this comment.
There was a problem hiding this comment.
- We could add a config mapping similar to
trusted-public-keysmapping domains to certificate paths. A cert and private key can be put in the same file. - or we could also support a directory structure similar to what docker uses for mtls. This clashes with the existing server CA configuration option a bit, though.
There was a problem hiding this comment.
Oh. That certs.d directory looks really nice. Because if you someone creates tooling for adding caches, all they need to do is to drop the certificate in the right directory and ask the user to extend the substitute list. Will try to get some feedback on this today from the Nix team.
There was a problem hiding this comment.
Another question, what does the error looks like if a certificate is expired? Just want to make sure the error is easy to understand.
There was a problem hiding this comment.
If the error happens during handshake, nix will print something like this
warning: error: unable to download 'https://foobar/hw81q3s8a7bz7jz7gp0h81838y5251p1.narinfo': SSL peer certificate or SSH remote key was not OK (60) SSL certificate problem: certificate has expired; retrying in 2283 ms
EDIT: nope, see below
The error also depends on the server setup. In my case, the server accepts any certificate during the handshake and checks it later, allowing it to return an HTTP 400 response which will be printed to standard output.
There was a problem hiding this comment.
I've actually tested it, and it looks like I was mistaken. If the handshake fails due to client cert error, it returns something like this, currently:
OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 0;
It looks totally implementation defined~, as I see no way for the server to signal an issue with the certificate back to the client in the handshake itself.~
So a nodejs http server will just abort the connection (see above), but nginx for example will send an http error response.
Reading the RFC, it looks like, the server could send, for example a bad_certificate alert message, but it won't contain any detail about the error. I think this is one of the reasons why servers usually signal the error at the http layer.
There was a problem hiding this comment.
So we are still discussing what we do with #11139
(probably we get more updates on Monday), but we potentially can have this option being scoped to a substituter list, which would be more on point.
Certificate/private-key pair can be configured globally and it will be handled by libcurl.
Motivation
In our setup, we use client certificate authentication extensively to access company resources. It would be very easy for us to deploy a binary cache and setup authentication the same way.
Context
Fixes #13002
Questions
I've gone with that as I don't think, that many people use different certificates for different HTTPS services, and implementing per-domain certificate handling would greatly complicate the implementation
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.