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

nixos: sign kernel modules; add security.enforceModuleSignature option #39286

Closed
wants to merge 2 commits into from

Conversation

symphorien
Copy link
Member

This enables the following kernel options:

MODULE_SIG: when a module is loaded, a signature is checked. If it is
not present or invalid, then:

  • if the kernel booted with module.sig_enforce (which is the consequence
    of setting security.enforceModuleSignature=true) then the module is
    rejected
  • otherwise the kernel is just tainted.

MODULE_SIG_ALL: when building the kernel, a key is created and modules
are automatically signed with it. When the build is finished, we cannot
persist the key since this would put it world readable in the store so
it is just dropped. As a consequence, only upstream kernel modules are
signed
. Third party modules like zfs cannot be signed and cannot be
loaded with security.enforceModuleSignature=true.

A nixos test is provided to test that everything works as intended.

Reference:
https://www.kernel.org/doc/html/v4.10/admin-guide/module-signing.html

Motivation for this change

Enabling module signature

Alternative designs

require the users to create themselves a key and point us to it in their configuration so that out of tree
modules can be signed
This would be way more complicated, just for the sake of out-of-tree modules users.

Note that tainting the kernel just for having loaded an unsigned module even for users who don't care about this feature is not a drawback since loading an out-of-tree module already entails tainting the kernel.

Things done

Only tested with 4.14 because building a full kernel is quite long, but the feature exists since 4.3 in this
form, so this should be fine. Anyway, there is a test.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 501+ labels Apr 20, 2018
@matthewbauer
Copy link
Member

So a question on signatures in Linux kernel- what makes the signature invalid? Does the kernel just have 1 valid signature that it accepts for modules? I have concerns that this could break Nix's substitution logic.

@symphorien
Copy link
Member Author

The kernel only accepts modules signed with the key it was built with. So you cannot use modules from another kernel build. This would need to persist the key across builds, and thus putting it world readable in the store.

@Nadrieril
Copy link
Member

@matthewbauer : since the modules are compressed, I believe nix's substitution logic wouldn't touch them, thus preserving the signatures

@xeji
Copy link
Contributor

xeji commented Apr 23, 2018

The kernel only accepts modules signed with the key it was built with. So you cannot use modules from another kernel build. This would need to persist the key across builds, and thus putting it world readable in the store.

Not sure whether kernel builds are binary reproducible now, but this would prevent them from ever being reproducible.

@symphorien
Copy link
Member Author

Fair point. I had not thought of it. Anyway, for now, kernel builds are not reproducible: build ids contain the date

Linux nlaptop 4.9.93 #1-NixOS SMP Sun Apr 8 10:13:01 UTC 2018 x86_64 GNU/Linux

at least :)

Honestly, I would like to avoid making signatures optional, as it would force users to rebuild their kernel themselves (as in this module: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/misc/crashdump.nix#L63-L73 ) which is quite long on not-so-powerful devices.

@@ -0,0 +1,23 @@
import ./make-test.nix ({ pkgs, ...} : {
Copy link
Member

Choose a reason for hiding this comment

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

I think we want this file in release.nix so that it is run by Hydra.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just pushed a commit adding it to release.nix.

@GrahamcOfBorg GrahamcOfBorg added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels May 2, 2018
@matthewbauer
Copy link
Member

@GrahamcOfBorg test kernel-signed-modules

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.kernel-signed-modules

Partial log (click to expand)

while evaluating the attribute 'drvPath' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/lib/customisation.nix�[0m:179:13:
while evaluating the attribute 'drvPath' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/lib/customisation.nix�[0m:146:13:
while evaluating the attribute 'buildCommand' of the derivation 'vm-test-run-kernel-signed-modules' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/pkgs/stdenv/generic/make-derivation.nix�[0m:148:11:
while evaluating the attribute 'buildCommand' of the derivation 'nixos-test-driver-kernel-signed-modules' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/pkgs/stdenv/generic/make-derivation.nix�[0m:148:11:
while evaluating the attribute 'buildCommand' of the derivation 'nixos-vm' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/pkgs/stdenv/generic/make-derivation.nix�[0m:148:11:
while evaluating the attribute 'perl' of the derivation 'nixos-system-machine-18.09.git.792cf77' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/pkgs/stdenv/generic/make-derivation.nix�[0m:148:11:
while evaluating 'concatMapStringsSep' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/lib/strings.nix�[0m:64:33, called from �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/nixos/modules/system/activation/top-level.nix�[0m:131:42:
while evaluating anonymous function at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/nixos/modules/system/activation/top-level.nix�[0m:131:67, called from undefined position:
undefined variable 'perlPackages' at �[1m/var/lib/gc-of-borg/nix-test-rs-13/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-13/nixos/modules/system/activation/top-level.nix�[0m:131:113

@GrahamcOfBorg
Copy link

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: tests.kernel-signed-modules

Partial log (click to expand)

while evaluating the attribute 'drvPath' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/lib/customisation.nix:179:13:
while evaluating the attribute 'drvPath' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/lib/customisation.nix:146:13:
while evaluating the attribute 'buildCommand' of the derivation 'vm-test-run-kernel-signed-modules' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute 'buildCommand' of the derivation 'nixos-test-driver-kernel-signed-modules' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute 'buildCommand' of the derivation 'nixos-vm' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute 'perl' of the derivation 'nixos-system-machine-18.09.git.46f9bdb' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating 'concatMapStringsSep' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/lib/strings.nix:64:33, called from /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/nixos/modules/system/activation/top-level.nix:131:42:
while evaluating anonymous function at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/nixos/modules/system/activation/top-level.nix:131:67, called from undefined position:
undefined variable 'perlPackages' at /home/ofborg/ofborg1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/builder-7c6f434c1/nixos/modules/system/activation/top-level.nix:131:113

@symphorien
Copy link
Member Author

mmh apparently the merge is at fault. I will investigate tomorrow.

@GrahamcOfBorg GrahamcOfBorg added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 3, 2018
This enables the following kernel options:

MODULE_SIG: when a module is loaded, a signature is checked. If it is
not present or invalid, then:
* if the kernel booted with module.sig_enforce (which is the consequence
of setting security.enforceModuleSignature=true) then the module is
rejected
* otherwise the kernel is just tainted.

MODULE_SIG_ALL: when building the kernel, a key is created and modules
are automatically signed with it. When the build is finished, we cannot
persist the key since this would put it world readable in the store so
it is just dropped. As a consequence, *only upstream kernel modules are
signed*. Third party modules like zfs cannot be signed and cannot be
loaded with security.enforceModuleSignature=true.

A nixos test is provided to test that everything works as intended.

Reference:
https://www.kernel.org/doc/html/v4.10/admin-guide/module-signing.html
@symphorien
Copy link
Member Author

A rebase on the current nixos-unstable should fix the build.

@GrahamcOfBorg GrahamcOfBorg removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 3, 2018
@xeji
Copy link
Contributor

xeji commented May 3, 2018

@GrahamcOfBorg test kernel-signed-modules

@edolstra
Copy link
Member

edolstra commented May 3, 2018

👎 on this since signing with a random key breaks binary reproducibility (building the kernel twice will produce a different result).

@edolstra
Copy link
Member

edolstra commented May 3, 2018

@symphorien Unless something has broken recently, the timestamp embedded in the kernel is supposed to be SOURCE_DATE_EPOCH, so it should be reproducible.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: tests.kernel-signed-modules

Partial log (click to expand)

zsmalloc               28672  1 zram
dm_mod                143360  0
dax                    24576  1 dm_mod
virtio_rng             16384  0
rng_core               24576  1 virtio_rng
zram                   28672  0
zsmalloc               28672  1 zram
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/51ds279md5q14c1cg4hl9q3wdv4lag5d-vm-test-run-kernel-signed-modules

@symphorien
Copy link
Member Author

Well, I just checked with a small kernel config (just enough to run nixos tests):

let smallPlatform = lib.recursiveUpdate hostPlatform {
  platform.kernelAutoModules  =  false;
  platform.kernelBaseConfig  =  hostPlatform.platform.kernelBaseConfig + " kvmconfig";
}; in
let linux-small = linux.override {
  hostPlatform = smallPlatform;
  extraConfig = ''
    VIRTIO_BALLOON y
    OVERLAY_FS y
    SOUND n
    SND n
    REISERFS_FS n
    JFS_FS n
    XFS_FS n
    GFS2_FS n
    BTRFS_FS n
    NILFS2_FS n
    F2FS_FS n
    FS_DAX n
    EXPORTFS n
    OCFS2_FS n
    UDF_FS n
    NFSD_FS n
    NFS_FS n
    WAN n
    CIFS n
    DRM n
    XEN n
    WLAN n
    WIRELESS n
    SECURITY n
    SECURITY_SELINUX n
    SECURITY_APPARMOR n
    SECURITY_YAMA n
    IPV6 n
    HDMI n
    INPUT_MOUSE n
    INPUT_TABLET n
    INPUT_TOUCHSCREEN n
    USB_PRINTER n
    ATA n
    ACPI n
    SCSI n
    MD n
    HID n
    KEYS n
    USB_HID n
    ETHERNET n
    '';
}; in
let linuxPackages_small = linuxPackagesFor linux-small; in ...

and nix-build --check complains:

error: derivation '/nix/store/wr8zsi78940a8khdc558k1dwq23yn9wi-linux-4.14.38.drv' may not be deterministic: output '/nix/store/4vba7pixf1j2kk9jsxf8gqd6ar0frrk5-linux-4.14.38-dev' differs from '/nix/store/4vba7pixf1j2kk9jsxf8gqd6ar0frrk5-linux-4.14.38-dev.check'

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: tests.kernel-signed-modules

Partial log (click to expand)

cannot build derivation '/nix/store/ph7r772lqmvc3rv1bkf1rkfrv3kf9kaz-etc.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/ir5rprmi18iyn3pfs43dq22czhzxkzan-stage-1-init.sh.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/1s57lyqdhx2y0r5ywslyxk1rvslxbsvk-initrd.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/ahkhfl5nnavgmgs4kx11va9b9yxsc5ra-nixos-system-machine-18.09.git.6ee484b.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/70jmm0h4kvq2absw6qbsmj567d16vx32-closure-info.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/92gfsb9m2xbfncnjdbwqs8wf8kyjv34s-run-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/p1vd9q7iyax11fz206wl8mmwksrw0k38-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/x6v35zcfcr18sbcfpzphz9pwc2mh219d-nixos-test-driver-kernel-signed-modules.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/43wmkc09pq8j557m0my7v2nfyq2c3mjc-vm-test-run-kernel-signed-modules.drv': 1 dependencies couldn't be built
error: build of '/nix/store/43wmkc09pq8j557m0my7v2nfyq2c3mjc-vm-test-run-kernel-signed-modules.drv' failed

@Ekleog
Copy link
Member

Ekleog commented May 3, 2018

I think the case of binary reproducibility vs. security is not a necessary obvious choice in favor of reproducibility (well, at least I would rather have module signature than perfect binary reproducibility of the kernel, as a rebuild doesn't take that long when not trusting the builder), and the best way may be to both enable module signature enforcement and “reproducibility modulo a sanitizer” that would remove the signature-specific bits?

@matthewbauer
Copy link
Member

matthewbauer commented May 3, 2018

It doesn’t seem too bad for reproducibility since it is optional. I guess I’m not sure what use cases you have for it, though. The binary cache has signatures already and you should be able to trust what you build from source.

@edolstra
Copy link
Member

edolstra commented May 3, 2018

Well, the security advantage of signed modules seems pretty minor without full TPM. E.g. an attacker can just build a new kernel and reboot into that. You can get most of the advantages by disabling module loading post-boot.

Also, I don't really believe in the notion that protecting against a compromised root user is a useful thing to do on Linux. You're pretty much screwed at that point anyway.

@Ekleog
Copy link
Member

Ekleog commented May 3, 2018

@matthewbauer The idea of signing modules is mostly to protect against a compromised root user

@edolstra Well, with FDE the attacker can't build a new kernel and reboot into that, as he (normally) doesn't have access to the FDE passphrase.

I do agree with you that, currently, this module only brings only a bit of hardening to NixOS. However, I think this is a good step on the path forward to more hardening in NixOS, as protecting against a compromised root user is among quite a number of the steps taken by hardening.

I also agree that disabling module loading post-boot gets most of the advantages of using only signed modules, but not all (some use cases require module loading at rather random points in time, eg. plugging a usb wireless card). And currently NixOS is a bit lagging behind on security (necessary reference to NixOS/rfcs#5), so I don't think adding a “starting point” for architectural-level hardening (and encouraging future work in this direction, not only in package-level hardening) would be a bad idea? :)

@oxij
Copy link
Member

oxij commented May 3, 2018 via email

@Ekleog
Copy link
Member

Ekleog commented May 3, 2018

Well, the point is he can't dump the key from memory if he's root on a hardened kernel. Root only has ring3 access, not ring0 access, and the key resides only in the kernel memory (and IIRC with the TRESOR patch it even resides only in debug registers, not even in the memory). However, if kernel module loading is enabled without enforcing digital signature (which means, on a kernel without this PR), then root can indeed load any module into ring0 and recover the encryption key.

Basically, you're right without this PR, but with this PR segregating root from ring0 (and thus from the disk encryption key) becomes at least a possibility, even if it's likely not all paths are closed yet :)

As for one-time key signing, indeed that's an issue mentioned in the initial PR message. What could be done for nvidia users (as part of later work, imho) is to make a kernelWithSignedModules derivation that'd take a kernel along with a module set and build a kernel that signs the said signed modules.

An alternative (exposed in the initial PR message too) would be to use an out-of-tree key, and have a hydra key for signing all modules built by hydra, and included by default in the kernel -- I'm somehow less convinced with this idea, as in my opinion it loses the benefit of composability NixOS brings, but it'd likely be a level of security equivalent to what eg. RHEL provides, which likely would be enough.

@oxij
Copy link
Member

oxij commented May 3, 2018 via email

@Ekleog
Copy link
Member

Ekleog commented May 3, 2018

Oh. Thanks for the dmsetup command, I had never heard of it! As for TRESOR, it's also designed a bit against DMA attacks (well, there's TRESOR-HUNT, I can't remember if the advices given there have been followed), but if there's a dmsetup command for this it'll likely not disable it indeed.

So indeed FDE is useless here, but secure boot would still be a way to make sure the kernel is legit, without resorting to the artillery of TPM and measured boot.

About root able to have all the files from all mounted volumes regardless, indeed, with the current state of hardening in NixOS ring0 hardening by itself does quite little, but coupled with a RBAC (that NixOS currently doesn't have either) like SELinux, AppArmor or grsecurity RBAC it would allow to effectively restrain root.

Sure these RBAC's policy should also block module loading (if it's not configured to allow root to load modules for eg. usb wifi chipsets), but the main principle in hardening is to layer security defenses, so kernel module loading restriction is among the restrictions that I think should be put in place when possible, because it costs very little with a potentially big benefit (ring3 → ring0 escalation blocking)

As for the DRM vs security feature, both are quite close, like when grsecurity claimed having been ripped off by an unnamed company for locking users out of devices they owned without respecting the GPL terms. But now we're straying very far off-track, so let's maybe try to recenter the discussion?

What I remember of discussion up to now

  1. In the current state of NixOS (neither secure boot nor measured boot), this PR brings little benefit that wouldn't be easily by-passable
  2. In the ideal world where hardened NixOS is a possibility, this PR would likely be included as an additional layer of defense
  3. The downside of this PR is bit-perfect binary reproducibility of the kernel (but reproducibility modulo-the-signature should still be possible)
  4. Actually, the kernel isn't bit-perfect binary reproducible currently (but there appears to be no fundamental reason why this couldn't be fixed, contrarily to what'd happen with this PR merged)

So the question is whether bit-perfect binary reproducibility is actually a design goal of nixpkgs, and if so whether it is more or less important than hardening.

In my mind what is important is reproducibility modulo-useless-stuff, not strict bit-perfect reproducibility. That is, if two built kernels are bitwise identical with the signatures and public keys stripped, then they are “identical enough” for my purposes (that is detecting rogue builders). Having a procedure for handling this kind of almost-bitwise-reproducibility isn't done yet, though.

Also, I'm thinking such an almost-bitwise-reproducibility may help with eg. license-keyed software: it'd become possible (well, easier) to just embed the license key and say “when checking for equality, don't consider the license key”. It could take the form of eg. another meta.checkReproducibilityOn = pkg: derivation that'd default to pkg and could be overridden to build another derivation that'd be checked to be bit-perfect equal. (that said the exact design of such a system is off-topic too I guess)

All that said, I think the benefits in hardening outstrip the loss in bit-perfect reproducibility (because almost-bit-perfect-reproducibility still comes in), but that's just my opinion, and it isn't as strong as I initially thought when I didn't know of the dmsetup command :)

@symphorien symphorien closed this May 4, 2018
@oxij
Copy link
Member

oxij commented May 12, 2018 via email

@symphorien symphorien deleted the mod_sign branch May 18, 2019 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 501+
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants