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

kernel: enable module signing #87426

Draft
wants to merge 5 commits into
base: master
from
Draft

Conversation

@eadwu
Copy link
Member

eadwu commented May 9, 2020

This suffers from the same pitfalls as #86835, if the kernel needs to be rebuilt twice, the generated certificate will be wrong and thus the module is unable to be loaded due to the invalid certificate.

Though with the flawed approach to force the kernel to be the same, I included an example to show it works (zfs).

Motivation for this change

If lockdown is enabled, or module signatures are forced, unsigned modules are unable to be loaded.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
autoModuleSignHook = kernel: makeSetupHook
{ substitutions =
{ kernel = kernel.dev + "/lib/modules/${kernel.modDirVersion}/build";
hash = if (kernel.configfile.structuredConfig ? MODULE_SIG_HASH)

This comment has been minimized.

Copy link
@eadwu

eadwu May 9, 2020

Author Member

Another shortcoming, due to differing hash algos, we need to specify a default or read it in from the output config.

cp $buildRoot/$f $dev/lib/modules/${modDirVersion}/build/certs
fi
done

This comment has been minimized.

Copy link
@ajs124

ajs124 May 15, 2020

Member

With the private signing key in the store, wouldn't this allow anyone on the system to sign arbitrary modules?

This comment has been minimized.

Copy link
@eadwu

eadwu May 15, 2020

Author Member

I thought it wouldn't be as much of a problem as the certs are generated on every kernel build since we don't supply one by default.

I also assumed that derivations in /nix/store wouldn't be writable by other things (if that's compromised I don't think anything I could do would be much better).

This comment has been minimized.

Copy link
@ajs124

ajs124 May 15, 2020

Member

I'm not sure I understand? As long as the key is in the store, you can just take it and sign an arbitrary module with it, without ever needing to touch anything else in the nix store, right?

This comment has been minimized.

Copy link
@eadwu

eadwu May 15, 2020

Author Member

For my specific use case for this PR, it's to modprobe out of tree modules doing the boot phase. The only modules in the initrd are the result of a nix derivation so I'd expect that nothing would be compromised there.

I'm not sure if you can modprobe modules that aren't in the nix store, but if you can, then yeah, that would be a problem.

If it's just the act of signing arbitrary modules, then if the above isn't possible, then it shouldn't really matter, since if you can't use it, it doesn't make a difference?

@eadwu
Copy link
Member Author

eadwu commented May 15, 2020

Depends on #87856 for certain scenarios.

@eadwu eadwu force-pushed the eadwu:kernel/sign-modules branch from 2bf1dc9 to 67b1cdf May 23, 2020
@eadwu eadwu force-pushed the eadwu:kernel/sign-modules branch from 67b1cdf to 8575b86 Jun 30, 2020
@ofborg ofborg bot requested review from teto, layus and thoughtpolice Jun 30, 2020
@eadwu eadwu force-pushed the eadwu:kernel/sign-modules branch 4 times, most recently from 47dfbfd to dafb287 Jun 30, 2020
@ofborg ofborg bot requested a review from abbradar Jul 1, 2020
eadwu added 5 commits May 9, 2020
@eadwu eadwu force-pushed the eadwu:kernel/sign-modules branch from dafb287 to 7ca0592 Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.