Conversation
ObiWahn
commented
Mar 13, 2026
- I agree that my contribution may be distributed under GPL-2.0-only (in addition to the project's primary license)
|
Calling the main script with the correct password looks like this: |
kakra
left a comment
There was a problem hiding this comment.
Since you published it, I felt like I should give a quick first review. The general direction looks good and promising. I think we should find a way to optionally call the body of the for loop from the installer, so we could directly pass the full well known module path as a parameter. So maybe that core logic should be moved to lib/?
What do you think?
| ## Cert Import | ||
|
|
||
| If you alread have a certificate with a key under your control imported into | ||
| the bios you can scipt this section. |
| If you alread have a certificate with a key under your control imported into | ||
| the bios you can scipt this section. | ||
|
|
||
| bla bla |
| fi | ||
|
|
||
| case "$module_path" in | ||
| *xz) |
There was a problem hiding this comment.
You're matching *xz here but strip .xz below...
| key="$MOK_KEY_DIR/MOK.priv" | ||
| x509="MOK_KEY_DIR/MOK.der" | ||
|
|
||
| # TODO - maybe this could be done better without exporting the key to the env |
There was a problem hiding this comment.
You could mktemp a file with restrictive permissions, grab a file descriptor, and remove the file from disk immediately. Then that FD would keep the file opened (and thus persisted on disk just without a filename, it will be lost as soon as the FD closes) and could be piped into other commands. If this is better, tho, I'm not sure because you could still read the files from /proc/PID/fd/ then. It's not very different from looking at /proc/PID/environ, so maybe not worth the effort. I'm not sure if bash scripts could easily access the keyring store.
|
|
||
| # The exact location of `sign-file` might vary depending on your platform. | ||
| # TODO - test with different distributions | ||
| for module_dir in /usr/lib/linux-kbuild*; do |
There was a problem hiding this comment.
At least Gentoo doesn't have this dir. But it has /usr/lib/modules/*/build/scripts/sign-file.c where .../build is a symlink to the installed kernel sources. This has some limitations:
- the build of the kernel may not have run at all
sign-fileis usually not compiled on Gentoo- the kernel sources may no longer exist and only the built kernel modules still exist
|
|
||
| version=${module_dir#/usr/lib/linux-kbuild-} | ||
| # TODO - other platforms?! | ||
| module_dir="/lib/modules/$version-amd64/updates/dkms" |
There was a problem hiding this comment.
There's nothing like /lib/modules/VERSION-amd64 on Gentoo. It looks more like this:
# ls -al /lib/modules/
insgesamt 0
drwxr-xr-x 1 root root 432 7. Mär 11:30 ./
drwxr-xr-x 1 root root 99808 12. Mär 23:30 ../
drwxr-xr-x 1 root root 492 14. Dez 01:12 6.18.0-gentoo/
drwxr-xr-x 1 root root 492 15. Feb 19:55 6.18.10-gentoo/
drwxr-xr-x 1 root root 492 19. Feb 21:37 6.18.12-gentoo/
drwxr-xr-x 1 root root 492 27. Feb 13:36 6.18.13-gentoo/
drwxr-xr-x 1 root root 492 4. Mär 00:18 6.18.14-gentoo/
drwxr-xr-x 1 root root 492 12. Mär 23:31 6.18.16-gentoo/
drwxr-xr-x 1 root root 492 18. Dez 00:34 6.18.1-gentoo/
drwxr-xr-x 1 root root 492 31. Dez 10:03 6.18.2-gentoo/
drwxr-xr-x 1 root root 492 2. Jan 22:08 6.18.2-gentoo-r1/
drwxr-xr-x 1 root root 492 4. Jan 15:18 6.18.3-gentoo/
drwxr-xr-x 1 root root 492 14. Jan 03:03 6.18.4-gentoo/
drwxr-xr-x 1 root root 492 18. Jan 16:38 6.18.5-gentoo/
drwxr-xr-x 1 root root 492 21. Jan 21:34 6.18.6-gentoo/
drwxr-xr-x 1 root root 492 30. Jan 01:45 6.18.7-gentoo/
drwxr-xr-x 1 root root 492 10. Feb 12:23 6.18.8-gentoo/
drwxr-xr-x 1 root root 492 10. Feb 23:45 6.18.9-gentoo/
So I'd assume that amd64 is actually the kernel version suffix provided by the distribution or kernel package, usually derived from the name of multiple kernel variants to choose from? TBH, I've never experienced an explicit architecture suffix, e.g. on Ubuntu the directory names look like 4.15.0-213-generic because that machine, I checked, has the generic kernel variant installed.
The suffix is part of uname -r. It belongs to the version, it's not some independent value.
| rm "$module_path" | ||
| xz --check=crc32 --lzma2=dict=512KiB "${module_path%%.xz}" | ||
| ;; | ||
| *ko) |
There was a problem hiding this comment.
What about other compression methods? xz is common, but modules may be compressed as gz as well (which was common some years ago).
What about modules that have been previously signed? Is this idempotent?
Also I don't like that it seemingly goes through all modules and touches files that may be part of distribution packaging. This can result in broken checksums. It uses "$module_dir"/hid-xpadneo* but can we be sure this touches only the currently managed module?
Also, reviewing this, I found it confusing that the loop var is called module_path but is semantically different from module_dir above. I suggest renaming it to xpadneo_module_path or xpadneo_module.
| echo | ||
| for module_path in "$module_dir"/hid-xpadneo* ; do | ||
| echo -n "module: $module_path ... " | ||
| if ! [[ -e "$module_path" ]] ; then |
There was a problem hiding this comment.
Why would this happen? The only incident would be that the for-loop expansion literally resolved to .../hid-xpadneo* because no such dir or file existed. Shouldn't this rather be -f or -r instead?
|
From my experience, you can use It would also be nice if we could integrate the scripts with the Github testing workflow, to simulate module signing is working. Currently, this only has Ubuntu 22.04 and 24.04 because Azure pipelines to not have much choice. Maybe I'll migrate that to native Github workflows later, and we could even deploy tests using distrobox then (native Github workflows do not offer a lot of distribution choice either). |
|
Hey @kakra, I have to deal with some family business at the moment, I will probably be able to resume the PR later during the next week. Thanks for your input! |