swaywm / swaylock

Screen locker for Wayland
MIT License
832 stars 197 forks source link

Use explicit_bzero(3) to clear passwords, if it is available #353

Open nmeum opened 6 months ago

nmeum commented 6 months ago

This is a revived version of #148 which is less invasive by only modifying the implementation of clear_buffer().

Relying on the volatile keyword to clear memory is, in many cases, insufficient and may result in the clearing operation being optimized out by the compiler. Recognizing the difficulty of zero'ing memory, many libcs provide an explicit_bzero(3) function (which the libc guarantees not to be optimized out). This is presently implemented by OpenBSD, musl libc and glibc. This patch makes swaylock use this function and, if it is not available, falls back to the existing code and prints a warning during build.

For more details on the complexity of zero'ing memory, refer to the following 35c3 talk: https://media.ccc.de/v/35c3-9788-memsad

michaelortmann commented 6 months ago

you could also opt to use a more generic solution to clear_buffer(). i can recommend to just copy the code from here: https://github.com/jedisct1/libsodium/blob/master/src/libsodium/sodium/utils.c#L125-L157 but ive got the feeling, i recommended this before, and this project opted not to import "that much" code and doesnt fear security issues here.

nmeum commented 6 months ago

The libsodium compat code is also recommended in the referenced talk. However, given the prior discussion in #148 and the comments regarding non-posix functions, I assumed that something like this is even more less likely to get merged. My goal here is to propose a minimally invasive integration, in the hopes that this will be considered for inclusion. Also recall that explicit_bzero should nowadays be supported by all major platforms targeted by swaylock, hence I am not sure if additional fallbacks are even needed these days.

emersion commented 6 months ago

Note, the standard equivalent of this is C23's memset_explicit. However, it doesn't seem implemented in libcs yet.

nmeum commented 6 months ago

Keep in mind though that memset_explicit has only recently been standardized with C23, hence it will so it will be a while before it is widely available.