Skip to content
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.

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

systemd: build with cryptsetup support, add cryptsetup generators #66856

Merged
merged 4 commits into from Aug 6, 2020
Merged

systemd: build with cryptsetup support, add cryptsetup generators #66856

merged 4 commits into from Aug 6, 2020

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Aug 19, 2019 •
edited

Motivation for this change

This adds systemd cryptsetup generators, allowing systemd to translate /etc/crypttab into native systemd units early at boot and when configuration of the system manager is reloaded.

It depends on #93024 for the cleanups, which were needed in first place to allow bootstrapping systemd.

Afterwards, we can also get rid of systemd-cryptsetup-generator (which is now integrated in systemd directly).

I didn't do any manual testing yet, and we definitely should add some automated tests for it too :-)

Closes #75540.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @arianvp @7c6f434c (lvm2)

flokli
flokli commented Aug 19, 2019
View reviewed changes
nixos/modules/system/boot/systemd.nix Outdated
@@ -831,6 +831,7 @@ in
[Sleep]
'';

"tmpfiles.d/lvm2.conf".source = "${pkgs.lvm2}/lib/tmpfiles.d/lvm2.conf";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if systemd.packages could do this automatically.

@flokli flokli mentioned this pull request Aug 19, 2019
Merged
10 tasks
@ofborg ofborg bot requested a review from 7c6f434c August 19, 2019 11:17
@flokli flokli mentioned this pull request Sep 24, 2019
Merged
@flokli flokli mentioned this pull request Oct 4, 2019
Closed
@arianvp arianvp mentioned this pull request Nov 4, 2019
Closed
@ofborg ofborg bot removed the 6.topic: pantheon The Pantheon desktop environment label Dec 2, 2019
@ofborg ofborg bot requested a review from ajs124 July 12, 2020 11:47
@flokli
Copy link
Contributor Author

flokli commented Jul 12, 2020 •
edited

We did some further debugging, and got things to work 🎉 Also, the new lvmSystemdGenerator test needed some tweaking after it succeeded.

It's probably best to break this down into two PRs - one incorporating all the lvm fixes (including the possibilities to build a smaller variant of it), and then a second one using such a smaller lvm package to bootstrap systemd's libdevmapper for its cryptsetup support.

@flokli flokli mentioned this pull request Jul 12, 2020
Merged
10 tasks
@dasJ dasJ mentioned this pull request Jul 13, 2020
Merged
10 tasks
@flokli
Copy link
Contributor Author

flokli commented Jul 14, 2020

With #93024 merged, rebased this on latest staging.

@flokli
Copy link
Contributor Author

flokli commented Jul 14, 2020

For the scope of this PR, I don't intend to yet use this anywhere inside stage1/2 (which is more work), but it's a prerequisite for #72401.

Even without systemd taking care of unlocking root volumes, we should add a test, relying on systemd to unlock a disk only described in a /etc/crypttab - We could provide the keyfile in the activation script for now.

More advanced usecases could be also making use of the _netdev option, to cover cases like having something pushing secrets, or having a mysqld with data inside that mountpoint - but that's probably out of scope for this PR, and for a later one.

@FRidh
Copy link
Member

FRidh commented Jul 31, 2020

status?

@flokli
Copy link
Contributor Author

flokli commented Jul 31, 2020

IIRC, the NixOS tests installer tests at least succeeded, but I'd like to see a small test being added, as commented in #66856 (comment).

I'll see if I can get to that shortly, otherwise I'd appreciate if someone could come up with a small vm test.

flokli and others added 4 commits August 5, 2020 00:46
@flokli
systemd: build with cryptsetup and cryptsetup-generators
72c8ed0 
There's a circular dependency to systemd via cryptsetup and lvm2
(systemd -> cryptsetup -> lvm2 -> udev=systemd).

However, cryptsetup only really needs the devmapper component shipped
with lvm2. So build `pkgs.cryptsetup` with a lvm2 that doesn't come with
udev.
@flokli
systemd: substituteInPlace paths to mkswap and mke2fs (used in new cr…
0a41d69 
…yptsetup-generator.c)
@arianvp @flokli
systemd-cryptsetup-generator: remove
29941db 
This package previously did override the systemd package, and instructed
ninja, systemd's previous build system, to only build the
cryptsetup-specific systemd generators (plus some manual rpath
massaging, as ninja install wasn't used).

Afterwards, users were expected to add this package to their
`systemd.generator-packages` (or since
https://github.com/NixOS/nixpkgs/pull/65376/files `systemd.packages`)
NixOS module options, so systemd will use these generators.

As the previous commit added cryptsetup support directly to the systemd
package (and pkgs.systemd now already ships the cryptsetup generators),
we don't need another package shipping the same generators.
@flokli
nixosTests.systemd: test cryptsetup support
eb58711 
This creates and opens a luks volume, puts its passphrase into a keyfile
and writes a /etc/crypttab. It then reboots the machine, and verifies
systemd parsed /etc/crypttab properly, and was able to unlock the volume
with the keyfile provided (as we try to mount it).

The memorySize of the VM had to be bumped, as luksFormat would otherwise
run out of memory.
@flokli
Copy link
Contributor Author

flokli commented Aug 4, 2020

I rebased this on top of latest staging, and added a test verifying the functionality we enable - PTAL.

@flokli flokli merged commit 8e0b2b9 into NixOS:staging Aug 6, 2020
Staging automation moved this from Ready to Done Aug 6, 2020
@flokli flokli deleted the systemd-cryptsetup-lvm branch August 6, 2020 10:07
@flokli flokli mentioned this pull request Aug 6, 2020
Open
@andersk andersk mentioned this pull request Sep 3, 2020
Merged
10 tasks
andersk added a commit to andersk/nixpkgs that referenced this pull request Sep 3, 2020
@andersk
cryptsetup, lvm2, systemd: Break cyclic dependency at a different point
f4b2c9d 
The cyclic dependency of systemd → cryptsetup → lvm2 → udev=systemd
needs to be broken somewhere.  The previous strategy of building
cryptsetup with an lvm2 built without udev (NixOS#66856) caused the
installer.luksroot test to fail.  Instead, build lvm2 with a udev built
without cryptsetup.

Fixes NixOS#96479.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@flokli flokli mentioned this pull request Sep 28, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lheckemann lheckemann lheckemann approved these changes

@grahamc grahamc Awaiting requested review from grahamc

@Mic92 Mic92 Awaiting requested review from Mic92

@abbradar abbradar Awaiting requested review from abbradar

@lovek323 lovek323 Awaiting requested review from lovek323

@edolstra edolstra Awaiting requested review from edolstra

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

@andir andir Awaiting requested review from andir

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@ajs124 ajs124 Awaiting requested review from ajs124

Assignees
No one assigned
Labels
6.topic: nixos 8.has: clean-up 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@flokli @lovesegfault @andir @lheckemann @ajs124 @FRidh @7c6f434c @arianvp