Skip to content

libstore/filetransfer: add support for MTLS authentication #13030

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.

Sign up for GitHub

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

Jump to bottom
Merged
Mic92 merged 2 commits into from
Jan 25, 2026
Merged

libstore/filetransfer: add support for MTLS authentication #13030

Mic92 merged 2 commits into from
Jan 25, 2026

Conversation

@vlaci
Copy link

@vlaci vlaci commented Apr 15, 2025 •
edited
Loading

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

  • 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
  • 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 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@vlaci vlaci requested a review from Ericson2314 as a code owner April 15, 2025 14:18
@Mic92 Mic92 added this to Nix team Apr 15, 2025
@github-project-automation github-project-automation bot moved this to To triage in Nix team Apr 15, 2025
Mic92
Mic92 reviewed Apr 15, 2025
View reviewed changes
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label May 16, 2025
Mic92
Mic92 reviewed May 16, 2025
View reviewed changes
@vlaci vlaci requested a review from edolstra as a code owner May 16, 2025 12:33
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label May 26, 2025
@Mic92
Copy link
Member

Mic92 commented May 26, 2025

@vlaci added a regression test. Please check.

@Mic92
Copy link
Member

Mic92 commented May 26, 2025

Have a look later at the test... Only tested without nix-build.

@vlaci
Copy link
Author

vlaci commented May 29, 2025

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?

@tomberek tomberek added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Jun 4, 2025
@tomberek tomberek self-requested a review June 4, 2025 20:18
@tomberek tomberek moved this from To triage to 🏁 Review in Nix team Jun 4, 2025
@vlaci
Copy link
Author

vlaci commented Jul 18, 2025

For the sake of moving this forward, I've just added to required curl, python3 and openssl dependencies to functional tests.

@tomberek
Copy link
Contributor

tomberek commented Jul 21, 2025

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.

@vlaci
Copy link
Author

vlaci commented Jul 22, 2025

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.

@ztzg
Copy link
Contributor

ztzg commented Jul 28, 2025

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!

lf- pushed a commit to lix-project/lix that referenced this pull request Jul 29, 2025
@Mic92 @vlaci
contrib/plugins: init with mTLS store example
569dcf9 
This is a collection of Lix plugins that showcase how to write one for
various usecases.

The first is a mTLS store plugin that enable mTLS cache URIs
(`https+mtls://`).

We enable meson build system support for this plugin but we are not
going to distribute it in the official packaging of Lix, we will
repackage each relevant plugin downstream in Nixpkgs.

These plugins have *NO* guarantee support, they are provided as useful
references and are possibly production-ready if your usecase is simple
enough.

Reference: NixOS/nix#13030 (this change has
resemblances but our APIs are different, the tests harness is mostly
from CppNix).

Change-Id: Ib354271981b35dff6c134b12c4748c3eaf743fcb
Co-authored-by: Jörg Thalheim <joerg@thalheim.io>
Co-authored-by: László Vaskó <1771332+vlaci@users.noreply.github.com>
Signed-off-by: Raito Bezarius <raito@lix.systems>
@xokdvium xokdvium force-pushed the mtls-auth branch 2 times, most recently from b629b75 to 2322142 Compare January 18, 2026 17:36
@xokdvium
Copy link
Contributor

xokdvium commented Jan 18, 2026

Gave this a slight rebase + renamed ssl -> tls, since that was a misnomer. Also added more lightweight unit tests for HTTPS and HTTPS with mTLS to test redirects (now we only send the certs if the authority matches the cache hostname and uses the same scheme).

ztzg and others added 2 commits January 24, 2026 21:59
@ztzg @xokdvium
http-binary-cache-store: Add 'tls-certificate' and 'tls-private-key' …
36b0beb 
…settings

Those are set via the store's URI, e.g.:

    https://substituter.invalid?tls-certificate=/path/to/cert.pem&tls-private-key=/path/to/key.pem
@xokdvium
libutil-tests: Add unit tests for https binary cache stores with mTLS
0f17a1f 
This addresses the concerns with network isolation that have been raised
previously [1] by only running the tests by default in a network namespace.
This way all networks tests are independent of each other and do not bind
to ports in the host namespace.

This is much neater than doing these sorts of tests in functional suite.

[1]: NixOS#14266 (comment)
@xokdvium xokdvium requested a review from Mic92 January 24, 2026 19:03
Mic92 added a commit to Mic92/dotfiles that referenced this pull request Jan 25, 2026
@Mic92
eve: add cache2.thalheim.io with mTLS authentication
2681791 
Set up an experimental mTLS-authenticated binary cache endpoint to test
the nix client certificate authentication feature from NixOS/nix#13030.

The setup uses clan vars to generate a self-signed CA and client
certificate pair. Nginx is configured to require client certificate
verification against this CA before proxying to the harmonia backend.

This allows testing the new nix substituter options:
  tls-certificate=/path/to/client.crt
  tls-private-key=/path/to/client.key

Tested with:
  nix copy --from 'https://cache2.thalheim.io?tls-certificate=~/.nix-mtls/client.crt&tls-private-key=~/.nix-mtls/client.key' /nix/store/i3zw7h6pg3n9r5i63iyqxrapa70i4v5w-hello-2.12.2
@Mic92 Mic92 mentioned this pull request Jan 25, 2026
Merged
@Mic92
Copy link
Member

Mic92 commented Jan 25, 2026

For future reference/documentation, I tested the code in this pull request against nginx+harmonia: Mic92/dotfiles#4577

Mic92 added a commit to Mic92/dotfiles that referenced this pull request Jan 25, 2026
@Mic92
eve: add cache2.thalheim.io with mTLS authentication
bd478cd 
Set up an experimental mTLS-authenticated binary cache endpoint to test
the nix client certificate authentication feature from NixOS/nix#13030.

The setup uses clan vars to generate a self-signed CA and client
certificate pair. Nginx is configured to require client certificate
verification against this CA before proxying to the harmonia backend.

This allows testing the new nix substituter options:
  tls-certificate=/path/to/client.crt
  tls-private-key=/path/to/client.key

Tested with:
  nix copy --from 'https://cache2.thalheim.io?tls-certificate=~/.nix-mtls/client.crt&tls-private-key=~/.nix-mtls/client.key' /nix/store/i3zw7h6pg3n9r5i63iyqxrapa70i4v5w-hello-2.12.2
@Mic92 Mic92 added this pull request to the merge queue Jan 25, 2026
Merged via the queue into NixOS:master with commit 2eb19a6 Jan 25, 2026
14 checks passed
@github-project-automation github-project-automation bot moved this from 🏁 Review to Done in Nix team Jan 25, 2026
@vlaci vlaci deleted the mtls-auth branch January 28, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mic92 Mic92 Mic92 approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

@tomberek tomberek Awaiting requested review from tomberek

+1 more reviewer

@ztzg ztzg ztzg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

documentation idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

Nix team
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Client certificate authentication support for binary caches

5 participants

@vlaci @Mic92 @tomberek @ztzg @xokdvium