ssh-tpm-agent: keyutils (fixes #505869)#505874
ssh-tpm-agent: keyutils (fixes #505869)#505874vorburger wants to merge 1 commit intoNixOS:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses nixpkgs issue #505869 by attempting to ensure ssh-tpm-agent can access keyctl (from keyutils) in minimal/non-graphical environments where it may not already be present.
Changes:
- Add
keyutilsas an input to thessh-tpm-agentderivation. - Include
keyutilsinbuildInputsalongsideopenssl.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vendorHash = "sha256-84ZB1B+RczJS08UToCWvvVfWrD62IQxy0XoBwn+wBkc="; | ||
|
|
||
| buildInputs = [ | ||
| openssl | ||
| keyutils | ||
| ]; |
There was a problem hiding this comment.
Adding keyutils to buildInputs alone typically won’t make keyctl available at runtime for ssh-tpm-agent (the Go binary won’t reference the keyutils store path, and systemd/user shells may not have it on PATH). To actually fix the missing keyctl dependency, wrap ssh-tpm-agent (e.g., via makeWrapper + postInstall with --prefix PATH : ${lib.makeBinPath [ keyutils ]}) or otherwise ensure keyutils ends up in the runtime closure and PATH.
|
Hey! Congrats on your first contrib and fix. I reviewed it and I am not going to listen to what Copilot said, and say that yes including a package in the buildInputs arg of a drv adds them as a runtime dependency, however make sure this 100% by building it and running it from /result/bin as specified in the PR template. Good stuff! |
|
I think Copilot is correct here. This won't work. The You can use the following command to check the dependency of a package. The result I got for this PR: You can read this nixpills article for more information. |
|
@wishstudio Thank You, I think I see what you mean. How would I write an (currently expected to be failing) Integration Test in nixpkgs which "runs keyctl on the PATH that package ssh-tpm-agent has at runtime" ? |
|
So after reading the src of go's unix library that ssh-tpm-agent interacts with to to keyctl stuff, I originally thought that it would just link against the keyctl library ( keyctl itself is a syscall ), but if its does really need the binary keyctl, then yes you need to wrap it to set the path. Make sure to import postFixup = ''
wrapProgram $out/bin/ssh-tpm-agent \
--set PATH ${lib.makeBinPath [
keyutils
]}
'';This builds on my system, however I haven't done any tests. If this is anything to go by, then it works: |
NickCao
left a comment
There was a problem hiding this comment.
I believe that line of code is misleading, if you look into https://github.com/Foxboron/ssh-tpm-agent/tree/e8cd6307ad832c37dc613942cc7298e43694348e/internal/keyring, it's doing syscalls directly, not using the keyctl cli.
Not an expert in this field. But if you look at the manpage of keyutils: and in kernel doc: So the kernel may call back to the userspace utility when executing the system call. |
Yes, but the request-key helper does not come from PATH, See |
|
Yet, /run/current-system/sw/bin/request-key doesn't exist on my system and I'm assuming on his too, so the kernel will try to call into userspace and fail and the program interprets that as keyutils don't exist. |
In that case, add |
Nevermind. I misunderstood what you mean. |
Then this is the solution. @vorburger, you can create a module for this package that starts up a service as well as adds the keyutils package to systemPackages. |
Sounds like a plan! We'll close this? |
Sure. |
Things done
Fixes #505869.
This is my very 1st PR to
nixpkgs... let me know if this is cool, or if anything more is needed?passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.