swaywm / swaylock

Screen locker for Wayland
MIT License
853 stars 202 forks source link

mlock() hardening does not apply to key events in libwayland #289

Open mstoeckl opened 1 year ago

mstoeckl commented 1 year ago

Currently swaylock uses mlock() when available to ensure that the contents of password buffers (see comm.c#L23 and main.c#L1180 ) are never swapped out. This was introduced by https://github.com/swaywm/swaylock/commit/d8f0fb1378852e1acd4aaaccc51868d4349c3b77 .

However, once a password has been typed in, its contents are still available in memory in other forms. Each wl_keyboard::key event received by swaylock is read into a ring buffer (see wl_connection_read()) and then copied into a heap allocated object (see use of wl_connection_demarshal()) . The compositor's connection also retains a copy of these events, until the compositor's outgoing ringbuffer and any wl_closure objects are overwritten. None of these are protected from being put into swap.

This suggests that the current strategy of using mlock() on buffers with password content is insufficient to protect against the (admittedly very rare) scenarios in which an adversary can ensure some of swaylock's memory is swapped out after a password has been typed in but before the screen has been unlocked and swaylock exits. (Note: mlock() does not affect hibernation or core dumps.)

What should be done about this, I don't know -- perhaps it is possible to prevent the relevant parts of libwayland from swapping, or perhaps applying mlockall() is worth the tradeoffs; or perhaps the current use of mlock() is not worth the increased code complexity, and efforts are better spent ensuring that distributions use encrypted swap by default, if they don't already have full disk encryption; or maybe the best thing to do is nothing.

For comparison: I suspect xsecurelock has the same problem of not mlock()'ing the X11 event queue; xscreensaver doesn't use mlock at all.