Skip to content

[backport 2.3] Separate auth and logic for the daemon#5640

Merged
Ericson2314 merged 1 commit intoNixOS:2.3-maintenancefrom
obsidiansystems:daemon-auth-2.3
Feb 17, 2025
Merged

[backport 2.3] Separate auth and logic for the daemon#5640
Ericson2314 merged 1 commit intoNixOS:2.3-maintenancefrom
obsidiansystems:daemon-auth-2.3

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Nov 24, 2021

Before, processConnection wanted to know a user name and user id, and nix-daemon --stdio, when it isn't proxying to an underlying daemon, would just assume "root" and 0. But nix-daemon --stdio (no proxying) shouldn't make guesses about who holds the other end of its standard streams.

Now processConnection takes an "auth hook", so nix-daemon can provide the appropriate policy and daemon.cc doesn't need to know or care what it is.

(cherry picked from commit 8d4162f)

Depends on #5650
(not actually, but yes in terms of it wouldn't be useful until then.)

@thufschmitt
Copy link
Member

Care to explain why this needs to be backported? The new test makes me believe that it’s just for the fake ssh feature, but I’m not sure what the link between the two is

@Ericson2314
Copy link
Member Author

Ericson2314 commented Dec 1, 2021

@regnat heh a few things:

  1. I backported it trying to fix cross daemon test failures, and yes it ended up being only that test that excised it. However, I found the remote-store and remote-program query params are also easy to backport (and I did that in [backport 2.3] Allow testing with different daemons #5650), and between all backports it will allow my newer more extensive remote building tests to work.

  2. However, my comment i wrote and backported is overly narrow. The case with the "root" that is causing the problems is hit when nix-daemon --stdio is run, and that daemon wouldn't itself connect to another daemon. So it will also occurs when ssh-ing to machines that are "in single user mode".

@stale
Copy link

stale bot commented Jun 12, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 12, 2022
@stale stale bot removed the stale label Feb 23, 2023
@fricklerhandwerk fricklerhandwerk added the contributor-experience Developer experience for Nix contributors label Mar 3, 2023
@Ericson2314 Ericson2314 marked this pull request as draft November 30, 2023 17:58
@Ericson2314 Ericson2314 marked this pull request as ready for review November 30, 2023 17:58
@Ericson2314 Ericson2314 marked this pull request as draft November 30, 2023 18:48
@Ericson2314
Copy link
Member Author

draft until it is needed.

@dpulls
Copy link

dpulls bot commented Feb 16, 2025

🎉 All dependencies have been resolved !

@Ericson2314 Ericson2314 marked this pull request as ready for review February 17, 2025 17:08
Before, processConnection wanted to know a user name and user id, and
`nix-daemon --stdio`, when it isn't proxying to an underlying daemon,
would just assume "root" and 0. But `nix-daemon --stdio` (no proxying)
shouldn't make guesses about who holds the other end of its standard
streams.

Now processConnection takes an "auth hook", so `nix-daemon` can provide
the appropriate policy and daemon.cc doesn't need to know or care what
it is.

(cherry picked from commit 8d4162f)
@Ericson2314
Copy link
Member Author

My comments from 4 years ago were not that clear :). The reasons to backport this are:

  1. It will enable writing more tests, for cross-version testing purposes

  2. For anyone still using 2.3, it fixes some genuinely weird and unexpected behavior for Nix will attempt to create profiles and GC root dirs for root with nix-daemon --stdio.

@Ericson2314 Ericson2314 merged commit 9cdaeba into NixOS:2.3-maintenance Feb 17, 2025
5 checks passed
@Ericson2314 Ericson2314 deleted the daemon-auth-2.3 branch February 17, 2025 17:48
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2025-02-17-meeting-minutes-213-214/60813/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor-experience Developer experience for Nix contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants