-
Notifications
You must be signed in to change notification settings - Fork 202
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
Use explicit_bzero(3) to clear passwords, if it is available #353
base: master
Are you sure you want to change the base?
Conversation
Relying on volatile 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
@@ -1,3 +1,4 @@ | |||
#define _BSD_SOURCE // for explicit_bzero |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defining feature tests macro in the source file is somewhat unideal, only doing it here because that's what is also done elsewhere in the code. Refer to this post for more information. Also using _BSD_SOURCE
instead of _GNU_SOURCE
here as the BSDs may not define the prototype otherwise. musl defines it on both _GNU_SOURCE
and _BSD_SOURCE
, not sure about glibc.
you could also opt to use a more generic solution to clear_buffer(). |
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. |
Note, the standard equivalent of this is C23's |
Keep in mind though that |
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