Do we need secure_getenv()?

[whot]

Came up in #108.

Using secure_getenv() basically guarantees that our getenv() calls are useless since they'll always return NULL where the caller has capabilities. This is the case for mutter which I guess is a large portion of our use-case.

As of 13b30f4 with the changes from #108, our usage of getenv is limited to:

git grep -h --only-matching 'getenv(".*)' xkbcommon src | sort | uniq
getenv("HOME")
getenv("LANG") - false positive, inside a comment
getenv("LC_ALL") - false positive, inside a comment
getenv("LC_CTYPE") - false positive, inside a comment
getenv("XCOMPOSEFILE")
getenv("XDG_CONFIG_HOME")
getenv("XKB_CONFIG_ROOT")
getenv("XKB_DEFAULT_LAYOUT")
getenv("XKB_DEFAULT_MODEL")
getenv("XKB_DEFAULT_OPTIONS")
getenv("XKB_DEFAULT_RULES")
getenv("XKB_DEFAULT_VARIANT")
getenv("XKB_LOG_LEVEL")
getenv("XKB_LOG_VERBOSITY")
getenv("XLOCALEDIR")

So the question now is - do we need secure_getenv() or are we happy with getenv()?

The original motivation for not allowing compilation of arbitrary keymaps (109fe70) still stands of course. It's just that we have since added explicit support for arbitrary keymaps (3a91788 and e23f106), so... 🤷‍♂️

[bluetech]

It's just that we have since added explicit support for arbitrary keymaps (3a91788 and e23f106), so...

Well not exactly -- if the process is privileged, secure_gettenv(HOME) returns NULL and ~/.xkb isn't added (same for XDG).

---

Just to frame the discussion, the "threat model" here is based on these assumptions:

1. xkbcommon is a huge chunk of legacy C code which does a ton of parsing - almost guaranteed to have arbitrary code execution vulnerabilities in there somewhere given attacker-controlled input. (To be clear, we do our best to avoid that...).
2. Files in the system path are safe.
3. From the list, HOME, XDG_CONFIG_HOME, XKB_CONFIG_ROOT give user-controlled input for XKB, and HOME, XLOCALEDIR and XCOMPOSEFILE give user-controlled input for Compose.

Obviously, arbitrary code execution even for an unprivileged process is not great, but at least in the UNIX domain it doesn't let the user do anything they can't already do, and is fait accompli anyway.

But in a case like gnome-shell, a user will be able to get arbitrary cap_sys_nice which they can't otherwise.

---

Some options I can think of:

1. Drop secure_getenv.
2. Make the compositors "opt in" to the user paths, either by adding them manually, or setting some xkb_context flag. Basically shift the blame to them :)
3. Convince compositors to drop their file capabilities. If all they need is a sched_setscheduler(pid, SCHED_RR) call (link), I think it should be possible to offload to a specialized setcap binary?

[whot]

I think 2 and 3 are the better choices tbh. More specifically - "if you don't want to do 3, you'll have to do 2".

The second one could be helped by some explicit xkb_add_user_home_paths() so we don't need to sync things across projects but tbh I don't really anticipate this ever changing much anyway - xdg is well established by now.

It does somewhat matter for #123 though but only because that needs to be in sync too.

[whot]

Relevant for mutter: there's a pull request to move the caps to a separate thread, albeit it appears to have stalled.

[bluetech]

xref #308 - I think we can do 2.