-
-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
keyd: add keyd service and test #221321
keyd: add keyd service and test #221321
Conversation
I added basic hardening to the systemd service. Now
However, I'm not a a systemd expert, so there are options for further improvements. Some observation:
|
cc @pennae could you review the PR, please? You have already watched half on the matrix server) |
I replaced
I didn't quite understand what was meant. P.S. Thanks for the detailed review. |
the new config format is good. systemd system services can be run as arbitrary users/groups by setting |
I am not sure if this is possible due to the following reasons which I have researched. I wrote this as in kanata (only this, without extra hardening): hardware.uinput.enable = true;
...
{
DynamicUser = true;
DeviceAllow = [
"/dev/uinput rw"
"char-input r"
];
SupplementaryGroups = with config.users.groups; [
input.name
uinput.name
];
} But it didn't work because of the error:
I looked at If necessary, I can try to ask the |
you don't have to use |
I tested with minimum hardening parameters. First I tried this: serviceConfig = {
ExecStart = "${pkgs.keyd}/bin/keyd";
Restart = "always";
DynamicUser = true;
SupplementaryGroups = with config.users.groups; [
input.name
];
};
|
I give up. I don't have enough time/patience anymore to complete this task. I asked similar question here rvaiya/keyd#459. From what I read (this doesn't really apply to our problem):
Although If you accept the current behavior I can squash commits before merging, if not - feel free to close the PR I won't be angry. Anyway, I gained vast knowledge doing this stuff 😌 |
we've had a look out of curiosity on the hunch that your |
A few last questions 🧐.
Should I squash all the commits into one and keep the commit messages, or should I just squash my commits and keep yours separately?
|
that's more of a guideline for committers than for authors. you can merge your commits all you like (as long as the resulting commit train makes sense). do take the test fix from ours too to make sure the test always passes, but please keep the rest of it intact.
yeah, |
The keyd package already exists, but without a systemd service. Keyd requires write access to /var/run to create its socket. Currently the directory it uses can be changed with an environment variable, but the keyd repo state suggests that this may turn into a compile-time option. with that set, and some supplementary groups added, we can run the service under DynamicUser. Co-authored-by: pennae <[email protected]>
I think this PR is now ready for landing. If I misunderstood what you said, I can roll back the commit history using a local copy or the
Ah, cool. |
The keyd package already exists but without a systemd service.
Description of changes
Add systemd service for the keyd package, and write a test.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Also, I tested manually.