nix-community / impermanence

Modules to help you handle persistent state on systems with ephemeral root storage [maintainer=@talyz]
MIT License
1.11k stars 81 forks source link

nixos: bind mount with correct permissions #3

Closed lovesegfault closed 4 years ago

lovesegfault commented 4 years ago

This is yet another step towards #1.

With this the full directory structure is replicated with the correct permissions, thus allowing for non-root files and dirs to be bind-mounted. This enforces that the directories and files in the ephemeral storage have exactly matching ownership to their persistent storage correspondents.

I've tested this locally and have managed to bind mount things in my homedir without major issues.

It increases our bash complexity a little bit, but I tried to make the code well documented.

etu commented 4 years ago

I've encountered some really bad issues with this when trying it out.

I'm not sure the model with looking at the persistent stored file works fully if you happen to point at a symlink...

So this would make more sense to do more if we had a users api that took the username into account.

And I think the points that @talyz bring up here is important and good: https://github.com/nix-community/impermanence/issues/1#issuecomment-640484933

So I think this was a fun experiment that brought us an assertion about neededForBoot, which is great and well on the way to be implemented in #5

lovesegfault commented 4 years ago

I think it works if we add a call to realpath to resolve the reference given to chmod/chown. Let me give it a shot.

lovesegfault commented 4 years ago

@etu try it out :)

I'd like to get this merged into next and then we can remove the bind mount for files in a subsequent PR.

lovesegfault commented 4 years ago

@talyz @etu Please take a new look, I've rebased this onto master

lovesegfault commented 4 years ago

Ping @talyz @etu

talyz commented 4 years ago

Since you're not really doing any advanced splicing of nix strings into the bash script, I would suggest making the script take the only two values spliced as arguments instead and either create the script inline or move it out of the file to avoid bloating the activation script too much.

lovesegfault commented 4 years ago

Since you're not really doing any advanced splicing of nix strings into the bash script, I would suggest making the script take the only two values spliced as arguments instead and either create the script inline or move it out of the file to avoid bloating the activation script too much.

I plan to move the script out of the file and add a shellcheck step to CI to make sure it's valid. I just want to do it in a subsequent PR.

lovesegfault commented 4 years ago

I'm still a bit unclear on the use case and purpose of this, though. Have I understood it correctly that this is to account for when the source path has stricter permissions set somewhere prior to the bound directory?

Without this: Suppose you have /state/foo/bar/ where foo/ and bar/ are owned by user alice. You then choose to bind mount /foo/bar/, but the code does a simple mkdir -p as root resulting in /foo/ owned by root, and /bar/ originally owned by root, but then by alice when the bind mount happens. Regardless, alice can't access /foo/bar/ now because /foo/ is owned by not her.

With this: Suppose you have /state/foo/bar/ where foo/ and bar/ are owned by user alice. You then choose to bind mount /foo/bar/, the code correctly transverses that path and creates an identical structure, with identical modes and ownership. alice now owns /foo/ and /foo/bar/ and her bind mount works as expected.

talyz commented 4 years ago

Without this: Suppose you have /state/foo/bar/ where foo/ and bar/ are owned by user alice. You then choose to bind mount /foo/bar/, but the code does a simple mkdir -p as root resulting in /foo/ owned by root, and /bar/ originally owned by root, but then by alice when the bind mount happens. Regardless, alice can't access /foo/bar/ now because /foo/ is owned by not her.

Well, she can access it and edit files in it, since the default permissions allow it, even if /foo happens to be owned by root. I have this setup for my /etc/nixos directory, and it works fine. She wouldn't be able to put files in /foo, though, which this should fix.

talyz commented 4 years ago

I just tried this out and it seems to work; I don't have anything in my setup that actually tests the permission reassignment, but it doesn't break anything for me. I'm getting some error messages on boot, though:

Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/log/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/log/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/bluetooth/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/libvirt/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/docker/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/var/lib/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/systemd/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/var/lib/systemd/coredump/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/etc/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/etc/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/etc/NetworkManager/': File exists
Jun 14 22:11:32 flora stage-2-init: mkdir: cannot create directory '/persistent/etc/NetworkManager/system-connections/': File exists

Could you add a check for the existence of the directory before running mkdir?

lovesegfault commented 4 years ago

@talyz Done!

talyz commented 4 years ago

@lovesegfault Looks good! Great work! Before we merge this, can you please squash this down to one commit? Looking at the changes aggregated, it's pretty clear what is done - it's one connected thing and you provide good comments in the code, so I don't think it needs to be more than one commit in its final form.

lovesegfault commented 4 years ago

@talyz Done!