rvaiya / keyd

A key remapping daemon for linux.
MIT License
2.97k stars 175 forks source link

Feature request: allow non-root execution #692

Closed peesock closed 6 months ago

peesock commented 8 months ago

Thanks to linux udev rules, it is possible for a normal user in a specific group (usually "input" or "uinput") to access /dev/input files.

In my opinion, hard-coding root-only actions, namely setgid and nice (with negative values) into the daemon, especially when these actions can be taken care of by the init/service manager, isn't a good idea. It reduces flexibility and adds potentially unwanted behavior, like running commands as root.

I was able to get the basic daemon working by removing root functions and adding command line options to customize the paths for SOCKET_PATH and CONFIG_DIR.

Assuming most people run keyd using the systemd service, the setgid and nice behavior can be replaced with these service options

[Service]
...
User=keyd
Group=keyd
Nice=-20
...

to avoid breaking current behavior. If keyd's built-in version of this isn't a big deal for most people, then perhaps it could be locked behind new CLI options, or removed entirely.

peesock commented 8 months ago

after running tests, it seems that non-root keyd cannot detect new input devices, such as those created by itself and t/runner.py.

t/run.sh test.log with sudo:

CONFIG: parsing /tmp/tmp.oCB10Miv4U/test.conf
    WARNING: line 9: chord_interkey_timeout is not a valid global option
Starting keyd v2.4.3 ()
DEVICE: ignoring 413c:2010  (Dell Dell USB Keyboard)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini Consumer Control)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini)
DEVICE: ignoring 0fac:1ade  (keyd virtual pointer)
DEVICE: match    2fac:2ade  /tmp/tmp.oCB10Miv4U/test.conf   (test keyboard)
DEVICE: removed 2fac:2ade test keyboard

without sudo:

CONFIG: parsing /tmp/tmp.47s7kgris1/test.conf
    WARNING: line 9: chord_interkey_timeout is not a valid global option
Starting keyd v2.4.3 (b7cb828)
failed to open /dev/input/event25
failed to open /dev/input/event26
DEVICE: ignoring 413c:2010  (Dell Dell USB Keyboard)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini Consumer Control)
DEVICE: ignoring 3554:f508  (pulsar X2 V2 Mini)
failed to open /dev/input/event27

which means runner.py detects no keys and fails the test.

this is also seen when unplugging and replugging my keyboard/mouse; same error, different event files.

not sure what's causing this issue. running keyd in gdb and slowly going through functions before getting to evloop() seemed to remove the error, so perhaps time is a factor.

changes: https://github.com/peesock/keyd/commit/8639aa727399e6cf02b9452e4ce28934331da893

peesock commented 8 months ago

by unplugging and replugging my mouse while running a test C program with strace that opens and closes a /dev/input/event* file, i was able to confirm that it's a time issue.

1: openat(AT_FDCWD, "/dev/input/event5", O_RDWR|O_NONBLOCK|O_CLOEXEC) = -1 ENOENT (No such file or directory)
2: clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=0, tv_nsec=100000000}, NULL) = 0
3: openat(AT_FDCWD, "/dev/input/event5", O_RDWR|O_NONBLOCK|O_CLOEXEC) = -1 EACCES (Permission denied)
4: clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=0, tv_nsec=100000000}, NULL) = 0
5: openat(AT_FDCWD, "/dev/input/event5", O_RDWR|O_NONBLOCK|O_CLOEXEC) = 3
6: close(3)

line 1: mouse device file does not exist yet line 3: device file appears, but udev rules have not fully applied line 5: udev rules apply

so it looks like evloop needs to retry on failed event* file opens.

edit: fixed with https://github.com/peesock/keyd/commit/a15c0e181ab5af91dc64f7920e60cc26f2af1606. non-root users will see temporary errors saying "permission denied" before re-connecting, but i figure that should stay in for the sake of simplicity, unless anyone has a better idea.

tests are passing, so i'm going to start actually using this cool project now that i can run it as a user.

rvaiya commented 8 months ago

In my opinion, hard-coding root-only actions, namely setgid and nice (with negative values) into the daemon, especially when these actions can be taken care of by the init/service manager, isn't a good idea.

keyd is deliberately written to run as root. My view is that if a user has compromised an account that can read or write input events, your system is effectively compromised anyway (apart from injection attacks, the attacker can just keylog everything).

It reduces flexibility and adds potentially unwanted behavior, like running commands as root.

This is a valid point, and there was some debate about adding command(), but its intended purpose is for running system level commands. It is not intended to be a substitute for window manager/user level bindings (sxhkd exists for this purpose). If the user can edit /etc/keyd/ they already have root access.

peesock commented 7 months ago

My view is that if a user has compromised an account that can read or write input events, your system is effectively compromised anyway

yeah, that makes sense. projects like swhkd use a privileged daemon that communicates directly to a specific user-owned process, and that process only spawns shell commands, creating a model where all /dev/input access has to be explicitly granted per-process by root.

this is quite secure, but not how i personally run my desktop. i basically give my user infinite permissions (sudo doesn't even have a password on it) and assume all programs i run are trusted, except in the case of sandboxes or running things as specific users. i'm aware this is vulnerable to unknown exploits, like if firefox suddenly had a terrible RCE vulnerability, but it really simplifies things to have one user manage almost everything and not have to worry about complicated systems like polkit.

this is just me. i'm not advocating for per-user usage to be a common thing, only the flexibility to permit it. if it doesn't belong, i'm fine with maintaining my 3-commit fork.

rvaiya commented 7 months ago

Sorry for the prolonged response.

yeah, that makes sense. projects like swhkd https://github.com/waycrate/swhkd/ use a privileged daemon that communicates directly to a specific user-owned process, and that process only spawns shell commands, creating a model where all /dev/input access has to be explicitly granted per-process by root.

I'm not sure how this applies to keyd. As I understand it, swhkd is a root level process that still reads all the input (like keyd), it just delegates command dispatch to a user level daemon ('the server'). The daemon which reads the config file still requires access to /dev/input/*, so it is unclear to me where the additional security comes from.

this is quite secure, but not how i personally run my desktop. i basically give my user infinite permissions [...] sudo doesn't even have a password on it

Don't we all ;).

peesock commented 7 months ago

I'm not sure how this applies to keyd.

It doesn't, just citing an example of something more secure than running things directly, as I initially wanted from this project :) I'm not positive on how swhkd actually does things under the hood (since i don't want to use it), but at least the creator says it's safe.

Anyway, i dug more into the matter of running as a user and am learning about seat management to maybe add this capability in the future. No promises. I don't even know how lock screens would be handled, in case you want to use keyd as a sxhkd replacement...

After posting this issue, i started thinking a lot more about security and how a universal hotkey daemon could work. Generally, i have to learn more about wayland.

rvaiya commented 6 months ago

Adding proper support for this is non trivial and involves significantly increasing the attack surface. It is closely tied to implementing per-user configs, which is a feature I actually have a working prototype for that relies on the same kind of VT monitoring tricks that seatd does.

Adding command() on a per user basis would probably also require a per user daemon to properly account for state, which also adds another moving part. I am not inclined to add such a thing at the moment, but I am toying with the idea for the next major release (which will be backward incompatible).

I am going to close this for now, since I am trying to prune the issue tree, but feel free to reopen it if you want to add something.

Konfekt commented 6 months ago

Thank you for being clear on this and still open for future support.

keyd is deliberately written to run as root. My view is that if a user has compromised an account that can read or write input events, your system is effectively compromised anyway

Indeed, any administered company laptop is compromised, but it would still be useful to, say, map Capslock to Escape and Control as a user.