nix
nix copied to clipboard
libstore/filetransfer: add support for MTLS authentication
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
- [x] ~Is it okay to support only one certificate globally?
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~
It can be now configured per substituter - [x] ~I've added the settings to globals, as netrc and CA verification settings are already there. IDK if that is the right place.~
Settings are now substituter specific
Add :+1: to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.
@vlaci added a regression test. Please check.
Have a look later at the test... Only tested without nix-build.
Have a look later at the test... Only tested without nix-build.
There is no openssl and python in the test derivation. I am not familiar with Nix's test suite. Should there be an additional layer of indirection using i.e nix-shell shebang or nix-run inside the test script?
For the sake of moving this forward, I've just added to required curl, python3 and openssl dependencies to functional tests.
I'm not clear about the implications of:
Note the subsequent if (!absolute) [...]: the certificate and key are not configured for such "absolute" requests, as their URL components would not be applied to the substituter's "base URL."
Otherwise, this looks good.
I'm not clear about the implications of:
Note the subsequent if (!absolute) [...]: the certificate and key are not configured for such "absolute" requests, as their URL components would not be applied to the substituter's "base URL."
Otherwise, this looks good.
My understanding that it is done that way to ensure that the cert is used only if the request will go to cacheUri. Maybe the variable could have a more expressive name, but I don't really understand when the path should be requested from the cache and when not.
Also, the CI test timing out is not a fluke: the same full test run gets stuck locally as well. I am yet to figure out why and where, as the newly added test seemingly runs to completion.
Hi @tomberek, @vlaci,
I'm not clear about the implications of:
Note the subsequent if (!absolute) [...]: the certificate and key are not configured for such "absolute" requests, as their URL components would not be applied to the substituter's "base URL."
Otherwise, this looks good.
My understanding that it is done that way to ensure that the cert is used only if the request will go to
cacheUri.
Right. Note that the conditions under which cacheUri is used have not changed; that check was previously "inlined" in the FileTransferRequest constructor.
It seems prudent not to indiscriminately apply the configured certificates to other hosts.
Maybe the variable could have a more expressive name, but I don't really understand when the path should be requested from the cache and when not.
I don't know under which circumstances HttpBinaryCacheStore::makeRequest is invoked with absolute HTTP(S) or file: URLs, but it doesn't seem to be under the normal "substituter" use-case. Any clues would be welcome!