From ec46680fe10ffc69007b0a8b29d9e69c72704053 Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Mon, 15 Feb 2016 14:00:56 +0100 Subject: [PATCH 01/16] revert using argv0 and minor fixup - use hardcoded "slock" instead of argv[0] - add "slock: " to fprintf calls, where it was missing - revert `argc--, argv++` shifting --- slock.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/slock.c b/slock.c index a0ffed0..2aa395e 100644 --- a/slock.c +++ b/slock.c @@ -46,15 +46,14 @@ static Bool failure = False; static Bool rr; static int rrevbase; static int rrerrbase; -static char *argv0; static void die(const char *errstr, ...) { va_list ap; + fputs("slock: ", stderr); va_start(ap, errstr); - fprintf(stderr, "%s: ", argv0); vfprintf(stderr, errstr, ap); va_end(ap); exit(1); @@ -256,7 +255,7 @@ lockscreen(Display *dpy, int screen) usleep(1000); } if (!len) { - fprintf(stderr, "unable to grab mouse pointer for screen %d\n", screen); + fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); } else { for (len = 1000; len; len--) { if (XGrabKeyboard(dpy, lock->root, True, GrabModeAsync, GrabModeAsync, CurrentTime) == GrabSuccess) { @@ -266,7 +265,7 @@ lockscreen(Display *dpy, int screen) } usleep(1000); } - fprintf(stderr, "unable to grab keyboard for screen %d\n", screen); + fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); } /* grabbing one of the inputs failed */ running = 0; @@ -283,8 +282,6 @@ main(int argc, char **argv) Display *dpy; int screen; - argv0 = argv[0], argc--, argv++; - #ifdef __linux__ dontkillme(); #endif @@ -317,11 +314,11 @@ main(int argc, char **argv) return 1; } - if (argc >= 1 && fork() == 0) { + if (argc >= 2 && fork() == 0) { if (dpy) close(ConnectionNumber(dpy)); - execvp(argv[0], argv); - die("execvp %s failed: %s\n", argv[0], strerror(errno)); + execvp(argv[1], argv+1); + die("execvp %s failed: %s\n", argv[1], strerror(errno)); } /* Everything is now blank. Now wait for the correct password. */ -- 2.20.1 From 65b8d5278882310eed758e6fbfd6ab9676db883c Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Mon, 15 Feb 2016 14:15:45 +0100 Subject: [PATCH 02/16] Revert "No need for usage()" This reverts most of commit a6dc051e3744ce5b14c54d2d246d3e8258207e76 and fixes some related stuff: - keep spelling fixes from original commit - make -h and -v also work when followed by more arguments - any unknown flag prints usage - fix output of -v to display "slock: version 1.3" instead of "slock: slock-1.3" --- slock.1 | 16 +++++++++++++--- slock.c | 17 +++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/slock.1 b/slock.1 index 467194d..0ef3e15 100644 --- a/slock.1 +++ b/slock.1 @@ -3,17 +3,27 @@ slock \- simple X screen locker .SH SYNOPSIS .B slock -.RB [ -.IR cmd -] +.RB [ \-v +| +.IR cmd ] .SH DESCRIPTION .B slock is an X screen locker. If provided, .IR cmd is executed after the screen has been locked. +.SH OPTIONS +.TP +.B \-v +prints version information to stdout, then exits. .SH EXAMPLES $ slock /usr/sbin/s2ram .SH CUSTOMIZATION .B slock can be customized by creating a custom config.h and (re)compiling the source code. This keeps it fast, secure and simple. +.SH AUTHORS +See the LICENSE file for the authors. +.SH LICENSE +See the LICENSE file for the terms of redistribution. +.SH BUGS +Please report them. diff --git a/slock.c b/slock.c index 2aa395e..c9cdee2 100644 --- a/slock.c +++ b/slock.c @@ -273,15 +273,28 @@ lockscreen(Display *dpy, int screen) return NULL; } -int -main(int argc, char **argv) +static void +usage(void) { + fprintf(stderr, "usage: slock [-v|POST_LOCK_CMD]\n"); + exit(1); +} + +int +main(int argc, char **argv) { #ifndef HAVE_BSD_AUTH const char *pws; #endif Display *dpy; int screen; + if ((argc >= 2) && !strcmp("-v", argv[1])) + die("version %s, © 2006-2016 slock engineers\n", VERSION); + + /* treat first argument starting with a '-' as option */ + if ((argc >= 2) && argv[1][0] == '-') + usage(); + #ifdef __linux__ dontkillme(); #endif -- 2.20.1 From a7afade1701a809f6a33b53525d59dd29b38d381 Mon Sep 17 00:00:00 2001 From: Hiltjo Posthuma Date: Sun, 31 Jul 2016 13:43:00 +0200 Subject: [PATCH 03/16] clear passwords with explicit_bzero Make sure to explicitly clear memory that is used for password input. memset is often optimized out by the compiler. Brought to attention by the OpenBSD community, see: https://marc.info/?t=146989502600003&r=1&w=2 Thread subject: x11/slock: clear passwords with explicit_bzero Changes: - explicit_bzero.c import from libressl-portable. - Makefile: add COMPATSRC for compatibility src. - config.mk: add separate *BSD section in config.mk to simply uncomment it on these platforms. --- Makefile | 6 +++--- config.mk | 4 ++++ explicit_bzero.c | 19 +++++++++++++++++++ slock.c | 8 ++++++-- util.h | 2 ++ 5 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 explicit_bzero.c create mode 100644 util.h diff --git a/Makefile b/Makefile index 86b3437..8b3e248 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ include config.mk -SRC = slock.c +SRC = slock.c ${COMPATSRC} OBJ = ${SRC:.c=.o} all: options slock @@ -35,8 +35,8 @@ clean: dist: clean @echo creating dist tarball @mkdir -p slock-${VERSION} - @cp -R LICENSE Makefile README config.def.h config.mk ${SRC} slock.1 \ - slock-${VERSION} + @cp -R LICENSE Makefile README config.def.h config.mk ${SRC} \ + explicit_bzero.c slock.1 slock-${VERSION} @tar -cf slock-${VERSION}.tar slock-${VERSION} @gzip slock-${VERSION}.tar @rm -rf slock-${VERSION} diff --git a/config.mk b/config.mk index f93879e..3afc061 100644 --- a/config.mk +++ b/config.mk @@ -18,9 +18,13 @@ LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_SHADOW_H CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS} LDFLAGS = -s ${LIBS} +COMPATSRC = explicit_bzero.c # On *BSD remove -DHAVE_SHADOW_H from CPPFLAGS and add -DHAVE_BSD_AUTH # On OpenBSD and Darwin remove -lcrypt from LIBS +#LIBS = -L/usr/lib -lc -L${X11LIB} -lX11 -lXext -lXrandr +#CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_BSD_AUTH -D_BSD_SOURCE +#COMPATSRC = # compiler and linker CC = cc diff --git a/explicit_bzero.c b/explicit_bzero.c new file mode 100644 index 0000000..3e33ca8 --- /dev/null +++ b/explicit_bzero.c @@ -0,0 +1,19 @@ +/* $OpenBSD: explicit_bzero.c,v 1.3 2014/06/21 02:34:26 matthew Exp $ */ +/* + * Public domain. + * Written by Matthew Dempsky. + */ + +#include + +__attribute__((weak)) void +__explicit_bzero_hook(void *buf, size_t len) +{ +} + +void +explicit_bzero(void *buf, size_t len) +{ + memset(buf, 0, len); + __explicit_bzero_hook(buf, len); +} diff --git a/slock.c b/slock.c index c9cdee2..a00fbb9 100644 --- a/slock.c +++ b/slock.c @@ -23,6 +23,8 @@ #include #endif +#include "util.h" + enum { INIT, INPUT, @@ -135,7 +137,7 @@ readpw(Display *dpy, const char *pws) * timeout. */ while (running && !XNextEvent(dpy, &ev)) { if (ev.type == KeyPress) { - buf[0] = 0; + explicit_bzero(&buf, sizeof(buf)); num = XLookupString(&ev.xkey, buf, sizeof(buf), &ksym, 0); if (IsKeypadKey(ksym)) { if (ksym == XK_KP_Enter) @@ -161,14 +163,16 @@ readpw(Display *dpy, const char *pws) XBell(dpy, 100); failure = True; } + explicit_bzero(&passwd, sizeof(passwd)); len = 0; break; case XK_Escape: + explicit_bzero(&passwd, sizeof(passwd)); len = 0; break; case XK_BackSpace: if (len) - --len; + passwd[len--] = 0; break; default: if (num && !iscntrl((int)buf[0]) && (len + num < sizeof(passwd))) { diff --git a/util.h b/util.h new file mode 100644 index 0000000..6f748b8 --- /dev/null +++ b/util.h @@ -0,0 +1,2 @@ +#undef explicit_bzero +void explicit_bzero(void *, size_t); -- 2.20.1 From 3bb868e40873c568acdec74f7783c77f063aa396 Mon Sep 17 00:00:00 2001 From: FRIGN Date: Mon, 22 Aug 2016 00:25:21 +0200 Subject: [PATCH 04/16] Refactor main() - Add arg.h and fix usage Given slock is suid we don't want to have half-measures in place to parse the arguments in case the code is changed in the future with somebody not paying enough attention. Also, fix the usage string output to be more consistent across the suckless toolbase and make it reflect the manpage entry. - Comments Use proper block comments and add/change them where necessary to help in studying the code. - Error messages Consistently prepend them with "slock:" and fix wording and do a proper cleanup before quitting (XCloseDisplay and free the locks), making the die() semantics consistent with st's. - getpwuid() error reporting Properly present an error message if getpwuid() fails. - fork() error reporting Properly present an error message if fork() fails. If we cannot close the connection within the fork context we abort the operation and report an error. - execvp() error handling If execvp fails, we cannot call die() afterwards as this implies calling exit(). We must use _exit() to prevent the libc from doing now "illegal" cleanup-work. --- arg.h | 65 +++++++++++++++++++++++++++++++++++++++++++ slock.c | 85 ++++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 119 insertions(+), 31 deletions(-) create mode 100644 arg.h diff --git a/arg.h b/arg.h new file mode 100644 index 0000000..0b23c53 --- /dev/null +++ b/arg.h @@ -0,0 +1,65 @@ +/* + * Copy me if you can. + * by 20h + */ + +#ifndef ARG_H__ +#define ARG_H__ + +extern char *argv0; + +/* use main(int argc, char *argv[]) */ +#define ARGBEGIN for (argv0 = *argv, argv++, argc--;\ + argv[0] && argv[0][0] == '-'\ + && argv[0][1];\ + argc--, argv++) {\ + char argc_;\ + char **argv_;\ + int brk_;\ + if (argv[0][1] == '-' && argv[0][2] == '\0') {\ + argv++;\ + argc--;\ + break;\ + }\ + for (brk_ = 0, argv[0]++, argv_ = argv;\ + argv[0][0] && !brk_;\ + argv[0]++) {\ + if (argv_ != argv)\ + break;\ + argc_ = argv[0][0];\ + switch (argc_) + +/* Handles obsolete -NUM syntax */ +#define ARGNUM case '0':\ + case '1':\ + case '2':\ + case '3':\ + case '4':\ + case '5':\ + case '6':\ + case '7':\ + case '8':\ + case '9' + +#define ARGEND }\ + } + +#define ARGC() argc_ + +#define ARGNUMF() (brk_ = 1, estrtonum(argv[0], 0, INT_MAX)) + +#define EARGF(x) ((argv[0][1] == '\0' && argv[1] == NULL)?\ + ((x), abort(), (char *)0) :\ + (brk_ = 1, (argv[0][1] != '\0')?\ + (&argv[0][1]) :\ + (argc--, argv++, argv[0]))) + +#define ARGF() ((argv[0][1] == '\0' && argv[1] == NULL)?\ + (char *)0 :\ + (brk_ = 1, (argv[0][1] != '\0')?\ + (&argv[0][1]) :\ + (argc--, argv++, argv[0]))) + +#define LNGARG() &argv[0][0] + +#endif diff --git a/slock.c b/slock.c index a00fbb9..210d5c8 100644 --- a/slock.c +++ b/slock.c @@ -23,8 +23,11 @@ #include #endif +#include "arg.h" #include "util.h" +char *argv0; + enum { INIT, INPUT, @@ -54,7 +57,6 @@ die(const char *errstr, ...) { va_list ap; - fputs("slock: ", stderr); va_start(ap, errstr); vfprintf(stderr, errstr, ap); va_end(ap); @@ -280,8 +282,7 @@ lockscreen(Display *dpy, int screen) static void usage(void) { - fprintf(stderr, "usage: slock [-v|POST_LOCK_CMD]\n"); - exit(1); + die("usage: slock [-v | cmd [arg ...]]\n"); } int @@ -290,64 +291,86 @@ main(int argc, char **argv) { const char *pws; #endif Display *dpy; - int screen; + int s, nlocks; - if ((argc >= 2) && !strcmp("-v", argv[1])) - die("version %s, © 2006-2016 slock engineers\n", VERSION); - - /* treat first argument starting with a '-' as option */ - if ((argc >= 2) && argv[1][0] == '-') + ARGBEGIN { + case 'v': + fprintf(stderr, "slock-"VERSION"\n"); + return 0; + default: usage(); + } ARGEND #ifdef __linux__ dontkillme(); #endif - if (!getpwuid(getuid())) - die("no passwd entry for you\n"); + /* Check if the current user has a password entry */ + errno = 0; + if (!getpwuid(getuid())) { + if (errno == 0) + die("slock: no password entry for current user\n"); + else + die("slock: getpwuid: %s\n", strerror(errno)); + } #ifndef HAVE_BSD_AUTH pws = getpw(); #endif - if (!(dpy = XOpenDisplay(0))) - die("cannot open display\n"); + if (!(dpy = XOpenDisplay(NULL))) + die("slock: cannot open display\n"); + + /* check for Xrandr support */ rr = XRRQueryExtension(dpy, &rrevbase, &rrerrbase); - /* Get the number of screens in display "dpy" and blank them all. */ + + /* get number of screens in display "dpy" and blank them */ nscreens = ScreenCount(dpy); - if (!(locks = malloc(sizeof(Lock*) * nscreens))) - die("Out of memory.\n"); - int nlocks = 0; - for (screen = 0; screen < nscreens; screen++) { - if ((locks[screen] = lockscreen(dpy, screen)) != NULL) + if (!(locks = malloc(sizeof(Lock *) * nscreens))) { + XCloseDisplay(dpy); + die("slock: out of memory\n"); + } + for (nlocks = 0, s = 0; s < nscreens; s++) { + if ((locks[s] = lockscreen(dpy, s)) != NULL) nlocks++; } - XSync(dpy, False); + XSync(dpy, 0); - /* Did we actually manage to lock something? */ - if (nlocks == 0) { /* nothing to protect */ + /* did we actually manage to lock anything? */ + if (nlocks == 0) { + /* nothing to protect */ free(locks); XCloseDisplay(dpy); return 1; } - if (argc >= 2 && fork() == 0) { - if (dpy) - close(ConnectionNumber(dpy)); - execvp(argv[1], argv+1); - die("execvp %s failed: %s\n", argv[1], strerror(errno)); + /* run post-lock command */ + if (argc > 0) { + switch (fork()) { + case -1: + free(locks); + XCloseDisplay(dpy); + die("slock: fork failed: %s\n", strerror(errno)); + case 0: + if (close(ConnectionNumber(dpy)) < 0) + die("slock: close: %s\n", strerror(errno)); + execvp(argv[0], argv); + fprintf(stderr, "slock: execvp %s: %s\n", argv[0], + strerror(errno)); + _exit(1); + } } - /* Everything is now blank. Now wait for the correct password. */ + /* everything is now blank. Wait for the correct password */ #ifdef HAVE_BSD_AUTH readpw(dpy); #else readpw(dpy, pws); #endif - /* Password ok, unlock everything and quit. */ - for (screen = 0; screen < nscreens; screen++) - unlockscreen(dpy, locks[screen]); + /* password ok, unlock everything and quit */ + for (s = 0; s < nscreens; s++) + unlockscreen(dpy, locks[s]); free(locks); XCloseDisplay(dpy); -- 2.20.1 From c2f975773d720e734dbdab9a1e9ae51b0972ae0c Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Tue, 30 Aug 2016 17:33:09 +0200 Subject: [PATCH 05/16] Exit as soon as possible on input grabbing error We want to know at once if slock failed or not to lock the screen, not seing a black screen for a whole second (or two) and then die. Thanks to ^7heo for reporting this. --- slock.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/slock.c b/slock.c index 210d5c8..dd32374 100644 --- a/slock.c +++ b/slock.c @@ -222,7 +222,6 @@ static Lock * lockscreen(Display *dpy, int screen) { char curs[] = {0, 0, 0, 0, 0, 0, 0, 0}; - unsigned int len; int i; Lock *lock; XColor color, dummy; @@ -249,34 +248,31 @@ lockscreen(Display *dpy, int screen) lock->pmap = XCreateBitmapFromData(dpy, lock->win, curs, 8, 8); invisible = XCreatePixmapCursor(dpy, lock->pmap, lock->pmap, &color, &color, 0, 0); XDefineCursor(dpy, lock->win, invisible); - XMapRaised(dpy, lock->win); - if (rr) - XRRSelectInput(dpy, lock->win, RRScreenChangeNotifyMask); /* Try to grab mouse pointer *and* keyboard, else fail the lock */ - for (len = 1000; len; len--) { - if (XGrabPointer(dpy, lock->root, False, ButtonPressMask | ButtonReleaseMask | PointerMotionMask, - GrabModeAsync, GrabModeAsync, None, invisible, CurrentTime) == GrabSuccess) - break; - usleep(1000); - } - if (!len) { + if (XGrabPointer(dpy, lock->root, False, ButtonPressMask | + ButtonReleaseMask | PointerMotionMask, GrabModeAsync, GrabModeAsync, + None, invisible, CurrentTime) != GrabSuccess) { fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); - } else { - for (len = 1000; len; len--) { - if (XGrabKeyboard(dpy, lock->root, True, GrabModeAsync, GrabModeAsync, CurrentTime) == GrabSuccess) { - /* everything fine, we grabbed both inputs */ - XSelectInput(dpy, lock->root, SubstructureNotifyMask); - return lock; - } - usleep(1000); - } + running = 0; + unlockscreen(dpy, lock); + return NULL; + } + + if (XGrabKeyboard(dpy, lock->root, True, GrabModeAsync, GrabModeAsync, + CurrentTime) != GrabSuccess) { fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); + running = 0; + unlockscreen(dpy, lock); + return NULL; } - /* grabbing one of the inputs failed */ - running = 0; - unlockscreen(dpy, lock); - return NULL; + + XMapRaised(dpy, lock->win); + if (rr) + XRRSelectInput(dpy, lock->win, RRScreenChangeNotifyMask); + + XSelectInput(dpy, lock->root, SubstructureNotifyMask); + return lock; } static void -- 2.20.1 From b87bfa234378bcfc1b13273c5089f07902de1725 Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Wed, 31 Aug 2016 00:56:13 +0200 Subject: [PATCH 06/16] Update bsd-auth string. Thanks to Hiltjo for discovering this. --- slock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slock.c b/slock.c index dd32374..847b328 100644 --- a/slock.c +++ b/slock.c @@ -157,7 +157,7 @@ readpw(Display *dpy, const char *pws) case XK_Return: passwd[len] = 0; #ifdef HAVE_BSD_AUTH - running = !auth_userokay(getlogin(), NULL, "auth-xlock", passwd); + running = !auth_userokay(getlogin(), NULL, "auth-slock", passwd); #else running = !!strcmp(crypt(passwd, pws), pws); #endif -- 2.20.1 From d8bec0f6fdc8a246d78cb488a0068954b46fcb29 Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Wed, 31 Aug 2016 00:59:06 +0200 Subject: [PATCH 07/16] fix CVE-2016-6866 --- slock.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/slock.c b/slock.c index 847b328..8ed59ca 100644 --- a/slock.c +++ b/slock.c @@ -123,7 +123,7 @@ readpw(Display *dpy) readpw(Display *dpy, const char *pws) #endif { - char buf[32], passwd[256]; + char buf[32], passwd[256], *encrypted; int num, screen; unsigned int len, color; KeySym ksym; @@ -159,7 +159,11 @@ readpw(Display *dpy, const char *pws) #ifdef HAVE_BSD_AUTH running = !auth_userokay(getlogin(), NULL, "auth-slock", passwd); #else - running = !!strcmp(crypt(passwd, pws), pws); + errno = 0; + if (!(encrypted = crypt(passwd, pws))) + fprintf(stderr, "slock: crypt: %s\n", strerror(errno)); + else + running = !!strcmp(encrypted, pws); #endif if (running) { XBell(dpy, 100); @@ -312,6 +316,8 @@ main(int argc, char **argv) { #ifndef HAVE_BSD_AUTH pws = getpw(); + if (strlen(pws) < 2) + die("slock: failed to get user password hash.\n"); #endif if (!(dpy = XOpenDisplay(NULL))) -- 2.20.1 From a9eddbd94fb03a36186ef2b1e784468dfcddbc19 Mon Sep 17 00:00:00 2001 From: FRIGN Date: Tue, 23 Aug 2016 10:55:34 +0200 Subject: [PATCH 08/16] Convert manpage to mandoc and fix usage In all honor, the previous usage was formally more correct, but for the sake of consistency across all the tools having the v-flag, I separated it from the command-string. Also, make use of the mandoc macros for the manpage. This makes it easier to maintain, extend and change in the future. --- slock.1 | 54 ++++++++++++++++++++++++++---------------------------- slock.c | 2 +- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/slock.1 b/slock.1 index 0ef3e15..2b2b7c8 100644 --- a/slock.1 +++ b/slock.1 @@ -1,29 +1,27 @@ -.TH SLOCK 1 slock\-VERSION -.SH NAME -slock \- simple X screen locker -.SH SYNOPSIS -.B slock -.RB [ \-v -| -.IR cmd ] -.SH DESCRIPTION -.B slock -is an X screen locker. If provided, -.IR cmd +.Dd 2016-08-23 +.Dt SLOCK 1 +.Sh NAME +.Nm slock +.Nd simple X screen locker +.Sh SYNOPSIS +.Nm +.Op Fl v +.Op Ar cmd Op Ar arg ... +.Sh DESCRIPTION +.Nm +is a simple X screen locker. If provided, +.Ar cmd Op Ar arg ... is executed after the screen has been locked. -.SH OPTIONS -.TP -.B \-v -prints version information to stdout, then exits. -.SH EXAMPLES -$ slock /usr/sbin/s2ram -.SH CUSTOMIZATION -.B slock -can be customized by creating a custom config.h and (re)compiling the source -code. This keeps it fast, secure and simple. -.SH AUTHORS -See the LICENSE file for the authors. -.SH LICENSE -See the LICENSE file for the terms of redistribution. -.SH BUGS -Please report them. +.Sh OPTIONS +.Bl -tag -width Ds +.It Fl v +Print version information to stdout and exit. +.El +.Sh EXAMPLES +$ +.Nm +/usr/sbin/s2ram +.Sh CUSTOMIZATION +.Nm +can be customized by creating a custom config.h from config.def.h and +(re)compiling the source code. This keeps it fast, secure and simple. diff --git a/slock.c b/slock.c index 8ed59ca..3e2ab8f 100644 --- a/slock.c +++ b/slock.c @@ -282,7 +282,7 @@ lockscreen(Display *dpy, int screen) static void usage(void) { - die("usage: slock [-v | cmd [arg ...]]\n"); + die("usage: slock [-v] [cmd [arg ...]]\n"); } int -- 2.20.1 From 137f0076c2986306109d637599b885bdaf92cd58 Mon Sep 17 00:00:00 2001 From: FRIGN Date: Tue, 23 Aug 2016 01:45:46 +0200 Subject: [PATCH 09/16] Refactor dontkillme() - Use file pointers instead of raw I/O, inspired by Kernel code. - Use OOM_SCORE_ADJ_MIN from linux/oom.h instead of working with magic values. - Stricter error checking and descriptive error messages. The reasoning for using the constant rather than magic values lies in the fact that this ensures people get the message. With "-1000", a code reviewer would question if that is really the lowest possible number or just an arbitrary value. The kernel ABI probably won't change, but even in the case, we wouldn't have to modify the code. The OOM killer only is guaranteed to not kill you if you have OOM_SCORE_ADJ_MIN. --- slock.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/slock.c b/slock.c index 3e2ab8f..9ec2f71 100644 --- a/slock.c +++ b/slock.c @@ -65,19 +65,27 @@ die(const char *errstr, ...) #ifdef __linux__ #include +#include static void dontkillme(void) { - int fd; + FILE *f; + const char oomfile[] = "/proc/self/oom_score_adj"; - fd = open("/proc/self/oom_score_adj", O_WRONLY); - if (fd < 0 && errno == ENOENT) { - return; + if (!(f = fopen(oomfile, "w"))) { + if (errno == ENOENT) + return; + die("slock: fopen %s: %s\n", oomfile, strerror(errno)); } - if (fd < 0 || write(fd, "-1000\n", (sizeof("-1000\n") - 1)) != - (sizeof("-1000\n") - 1) || close(fd) != 0) { - die("can't tame the oom-killer. is suid or sgid set?\n"); + fprintf(f, "%d", OOM_SCORE_ADJ_MIN); + if (fclose(f)) { + if (errno == EACCES) + die("slock: unable to disable OOM killer. " + "suid or sgid set?\n"); + else + die("slock: fclose %s: %s\n", oomfile, + strerror(errno)); } } #endif -- 2.20.1 From 1f66885fbf36c726b7615060d3c98cbf74218d13 Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Thu, 1 Sep 2016 13:46:19 +0200 Subject: [PATCH 10/16] Add cleanup() to do free(locks) + XCloseDisplay() --- slock.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/slock.c b/slock.c index 9ec2f71..4980325 100644 --- a/slock.c +++ b/slock.c @@ -230,6 +230,13 @@ unlockscreen(Display *dpy, Lock *lock) free(lock); } +static void +cleanup(Display *dpy) +{ + free(locks); + XCloseDisplay(dpy); +} + static Lock * lockscreen(Display *dpy, int screen) { @@ -349,8 +356,7 @@ main(int argc, char **argv) { /* did we actually manage to lock anything? */ if (nlocks == 0) { /* nothing to protect */ - free(locks); - XCloseDisplay(dpy); + cleanup(dpy); return 1; } @@ -358,8 +364,7 @@ main(int argc, char **argv) { if (argc > 0) { switch (fork()) { case -1: - free(locks); - XCloseDisplay(dpy); + cleanup(dpy); die("slock: fork failed: %s\n", strerror(errno)); case 0: if (close(ConnectionNumber(dpy)) < 0) @@ -382,8 +387,7 @@ main(int argc, char **argv) { for (s = 0; s < nscreens; s++) unlockscreen(dpy, locks[s]); - free(locks); - XCloseDisplay(dpy); + cleanup(dpy); return 0; } -- 2.20.1 From e378f735d857f7da124177e3540912d920be5022 Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Thu, 1 Sep 2016 13:46:51 +0200 Subject: [PATCH 11/16] Re-introduce the waiting loop for input grabbing MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We actually “need” to wait a little for input to be released before locking for cases where slock is spawned from other graphical applications using keybindings. This undoes the misbehaviour I introduced in c2f9757, sorry for the mess. --- slock.c | 60 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/slock.c b/slock.c index 4980325..97c7489 100644 --- a/slock.c +++ b/slock.c @@ -223,6 +223,7 @@ unlockscreen(Display *dpy, Lock *lock) return; XUngrabPointer(dpy, CurrentTime); + XUngrabKeyboard(dpy, CurrentTime); XFreeColors(dpy, DefaultColormap(dpy, lock->screen), lock->colors, NUMCOLS, 0); XFreePixmap(dpy, lock->pmap); XDestroyWindow(dpy, lock->win); @@ -241,7 +242,7 @@ static Lock * lockscreen(Display *dpy, int screen) { char curs[] = {0, 0, 0, 0, 0, 0, 0, 0}; - int i; + int i, ptgrab, kbgrab; Lock *lock; XColor color, dummy; XSetWindowAttributes wa; @@ -268,30 +269,45 @@ lockscreen(Display *dpy, int screen) invisible = XCreatePixmapCursor(dpy, lock->pmap, lock->pmap, &color, &color, 0, 0); XDefineCursor(dpy, lock->win, invisible); - /* Try to grab mouse pointer *and* keyboard, else fail the lock */ - if (XGrabPointer(dpy, lock->root, False, ButtonPressMask | - ButtonReleaseMask | PointerMotionMask, GrabModeAsync, GrabModeAsync, - None, invisible, CurrentTime) != GrabSuccess) { - fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); - running = 0; - unlockscreen(dpy, lock); - return NULL; - } + /* Try to grab mouse pointer *and* keyboard for 600ms, else fail the lock */ + for (i = 6, ptgrab = kbgrab = -1; i; --i) { + if (ptgrab != GrabSuccess) { + ptgrab = XGrabPointer(dpy, lock->root, False, + ButtonPressMask | ButtonReleaseMask | + PointerMotionMask, GrabModeAsync, + GrabModeAsync, None, invisible, CurrentTime); + } + if (kbgrab != GrabSuccess) { + kbgrab = XGrabKeyboard(dpy, lock->root, True, + GrabModeAsync, GrabModeAsync, CurrentTime); + } - if (XGrabKeyboard(dpy, lock->root, True, GrabModeAsync, GrabModeAsync, - CurrentTime) != GrabSuccess) { - fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); - running = 0; - unlockscreen(dpy, lock); - return NULL; - } + /* input is grabbed: we can lock the screen */ + if (ptgrab == GrabSuccess && kbgrab == GrabSuccess) { + XMapRaised(dpy, lock->win); + if (rr) + XRRSelectInput(dpy, lock->win, RRScreenChangeNotifyMask); + + XSelectInput(dpy, lock->root, SubstructureNotifyMask); + return lock; + } - XMapRaised(dpy, lock->win); - if (rr) - XRRSelectInput(dpy, lock->win, RRScreenChangeNotifyMask); + /* retry on AlreadyGrabbed but fail on other errors */ + if ((ptgrab != AlreadyGrabbed && ptgrab != GrabSuccess) || + (kbgrab != AlreadyGrabbed && kbgrab != GrabSuccess)) + break; - XSelectInput(dpy, lock->root, SubstructureNotifyMask); - return lock; + usleep(100000); + } + + /* we couldn't grab all input: fail out */ + if (ptgrab != GrabSuccess) + fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); + if (kbgrab != GrabSuccess) + fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); + running = 0; + unlockscreen(dpy, lock); + return NULL; } static void -- 2.20.1 From 39fb855aa100c5a5a8b7f3b6cc1fbb2135fe7dde Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Thu, 1 Sep 2016 13:47:05 +0200 Subject: [PATCH 12/16] Move screen unlocking inside cleanup() --- slock.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/slock.c b/slock.c index 97c7489..0682279 100644 --- a/slock.c +++ b/slock.c @@ -234,6 +234,11 @@ unlockscreen(Display *dpy, Lock *lock) static void cleanup(Display *dpy) { + int s; + + for (s = 0; s < nscreens; ++s) + unlockscreen(dpy, locks[s]); + free(locks); XCloseDisplay(dpy); } @@ -305,8 +310,6 @@ lockscreen(Display *dpy, int screen) fprintf(stderr, "slock: unable to grab mouse pointer for screen %d\n", screen); if (kbgrab != GrabSuccess) fprintf(stderr, "slock: unable to grab keyboard for screen %d\n", screen); - running = 0; - unlockscreen(dpy, lock); return NULL; } @@ -359,19 +362,21 @@ main(int argc, char **argv) { /* get number of screens in display "dpy" and blank them */ nscreens = ScreenCount(dpy); - if (!(locks = malloc(sizeof(Lock *) * nscreens))) { + if (!(locks = calloc(nscreens, sizeof(Lock *)))) { XCloseDisplay(dpy); die("slock: out of memory\n"); } for (nlocks = 0, s = 0; s < nscreens; s++) { if ((locks[s] = lockscreen(dpy, s)) != NULL) nlocks++; + else + break; } XSync(dpy, 0); - /* did we actually manage to lock anything? */ - if (nlocks == 0) { - /* nothing to protect */ + /* did we manage to lock everything? */ + if (nlocks != nscreens) { + running = 0; cleanup(dpy); return 1; } @@ -400,9 +405,6 @@ main(int argc, char **argv) { #endif /* password ok, unlock everything and quit */ - for (s = 0; s < nscreens; s++) - unlockscreen(dpy, locks[s]); - cleanup(dpy); return 0; -- 2.20.1 From 03a87179919eebab7d38c548e3ff8e2911512468 Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Thu, 1 Sep 2016 13:47:19 +0200 Subject: [PATCH 13/16] Localize running and failure inside readpw() They are only needed there, so don't make them global. --- slock.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/slock.c b/slock.c index 0682279..83d426a 100644 --- a/slock.c +++ b/slock.c @@ -46,8 +46,6 @@ typedef struct { static Lock **locks; static int nscreens; -static Bool running = True; -static Bool failure = False; static Bool rr; static int rrevbase; static int rrerrbase; @@ -132,14 +130,15 @@ readpw(Display *dpy, const char *pws) #endif { char buf[32], passwd[256], *encrypted; - int num, screen; + int num, screen, running, failure; unsigned int len, color; KeySym ksym; XEvent ev; static int oldc = INIT; len = 0; - running = True; + running = 1; + failure = 0; /* As "slock" stands for "Simple X display locker", the DPMS settings * had been removed and you can set it with "xset" or some other @@ -253,7 +252,7 @@ lockscreen(Display *dpy, int screen) XSetWindowAttributes wa; Cursor invisible; - if (!running || dpy == NULL || screen < 0 || !(lock = malloc(sizeof(Lock)))) + if (dpy == NULL || screen < 0 || !(lock = malloc(sizeof(Lock)))) return NULL; lock->screen = screen; @@ -376,7 +375,6 @@ main(int argc, char **argv) { /* did we manage to lock everything? */ if (nlocks != nscreens) { - running = 0; cleanup(dpy); return 1; } -- 2.20.1 From a55594fdd69fcfcc10b8c9624d5aba298969d713 Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Fri, 2 Sep 2016 11:49:02 +0200 Subject: [PATCH 14/16] increasing for loops are idiomatic --- slock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slock.c b/slock.c index 83d426a..08ada6f 100644 --- a/slock.c +++ b/slock.c @@ -274,7 +274,7 @@ lockscreen(Display *dpy, int screen) XDefineCursor(dpy, lock->win, invisible); /* Try to grab mouse pointer *and* keyboard for 600ms, else fail the lock */ - for (i = 6, ptgrab = kbgrab = -1; i; --i) { + for (i = 0, ptgrab = kbgrab = -1; i < 6; i++) { if (ptgrab != GrabSuccess) { ptgrab = XGrabPointer(dpy, lock->root, False, ButtonPressMask | ButtonReleaseMask | -- 2.20.1 From 9698224090ff2989659717815bfa076d5d436a70 Mon Sep 17 00:00:00 2001 From: Markus Teich Date: Wed, 7 Sep 2016 10:04:06 +0200 Subject: [PATCH 15/16] make error message prefix consistent --- slock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/slock.c b/slock.c index 08ada6f..62a9841 100644 --- a/slock.c +++ b/slock.c @@ -99,9 +99,9 @@ getpw(void) errno = 0; if (!(pw = getpwuid(getuid()))) { if (errno) - die("getpwuid: %s\n", strerror(errno)); + die("slock: getpwuid: %s\n", strerror(errno)); else - die("cannot retrieve password entry\n"); + die("slock: cannot retrieve password entry\n"); } rval = pw->pw_passwd; @@ -109,7 +109,7 @@ getpw(void) if (rval[0] == 'x' && rval[1] == '\0') { struct spwd *sp; if (!(sp = getspnam(getenv("USER")))) - die("cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); + die("slock: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); rval = sp->sp_pwdp; } #endif @@ -117,7 +117,7 @@ getpw(void) /* drop privileges */ if (geteuid() == 0 && ((getegid() != pw->pw_gid && setgid(pw->pw_gid) < 0) || setuid(pw->pw_uid) < 0)) - die("cannot drop privileges\n"); + die("slock: cannot drop privileges\n"); return rval; } #endif -- 2.20.1 From 04143fd68dbc656905714eff5c208fadb3464e25 Mon Sep 17 00:00:00 2001 From: Quentin Rameau Date: Wed, 7 Sep 2016 13:02:42 +0200 Subject: [PATCH 16/16] Unify how we check passwords between different OSes --- config.mk | 9 ++------- slock.c | 47 +++++++++++++---------------------------------- 2 files changed, 15 insertions(+), 41 deletions(-) diff --git a/config.mk b/config.mk index 3afc061..049305c 100644 --- a/config.mk +++ b/config.mk @@ -20,16 +20,11 @@ CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS} LDFLAGS = -s ${LIBS} COMPATSRC = explicit_bzero.c -# On *BSD remove -DHAVE_SHADOW_H from CPPFLAGS and add -DHAVE_BSD_AUTH +# On *BSD remove -DHAVE_SHADOW_H from CPPFLAGS # On OpenBSD and Darwin remove -lcrypt from LIBS #LIBS = -L/usr/lib -lc -L${X11LIB} -lX11 -lXext -lXrandr -#CPPFLAGS = -DVERSION=\"${VERSION}\" -DHAVE_BSD_AUTH -D_BSD_SOURCE +#CPPFLAGS = -DVERSION=\"${VERSION}\" -D_BSD_SOURCE #COMPATSRC = # compiler and linker CC = cc - -# Install mode. On BSD systems MODE=2755 and GROUP=auth -# On others MODE=4755 and GROUP=root -#MODE=2755 -#GROUP=auth diff --git a/slock.c b/slock.c index 62a9841..da4b099 100644 --- a/slock.c +++ b/slock.c @@ -18,11 +18,6 @@ #include #include -#if HAVE_BSD_AUTH -#include -#include -#endif - #include "arg.h" #include "util.h" @@ -88,7 +83,6 @@ dontkillme(void) } #endif -#ifndef HAVE_BSD_AUTH /* only run as root */ static const char * getpw(void) @@ -96,6 +90,7 @@ getpw(void) const char *rval; struct passwd *pw; + /* Check if the current user has a password entry */ errno = 0; if (!(pw = getpwuid(getuid()))) { if (errno) @@ -109,10 +104,20 @@ getpw(void) if (rval[0] == 'x' && rval[1] == '\0') { struct spwd *sp; if (!(sp = getspnam(getenv("USER")))) - die("slock: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); + die("slock: getspnam: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); rval = sp->sp_pwdp; } -#endif +#else + if (rval[0] == '*' && rval[1] == '\0') { +#ifdef __OpenBSD__ + if (!(pw = getpwnam_shadow(getenv("USER")))) + die("slock: getpwnam_shadow: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); + rval = pw->pw_passwd; +#else + die("slock: getpwuid: cannot retrieve shadow entry (make sure to suid or sgid slock)\n"); +#endif /* __OpenBSD__ */ + } +#endif /* HAVE_SHADOW_H */ /* drop privileges */ if (geteuid() == 0 && @@ -120,14 +125,9 @@ getpw(void) die("slock: cannot drop privileges\n"); return rval; } -#endif static void -#ifdef HAVE_BSD_AUTH -readpw(Display *dpy) -#else readpw(Display *dpy, const char *pws) -#endif { char buf[32], passwd[256], *encrypted; int num, screen, running, failure; @@ -163,15 +163,11 @@ readpw(Display *dpy, const char *pws) switch (ksym) { case XK_Return: passwd[len] = 0; -#ifdef HAVE_BSD_AUTH - running = !auth_userokay(getlogin(), NULL, "auth-slock", passwd); -#else errno = 0; if (!(encrypted = crypt(passwd, pws))) fprintf(stderr, "slock: crypt: %s\n", strerror(errno)); else running = !!strcmp(encrypted, pws); -#endif if (running) { XBell(dpy, 100); failure = True; @@ -320,9 +316,7 @@ usage(void) int main(int argc, char **argv) { -#ifndef HAVE_BSD_AUTH const char *pws; -#endif Display *dpy; int s, nlocks; @@ -338,20 +332,9 @@ main(int argc, char **argv) { dontkillme(); #endif - /* Check if the current user has a password entry */ - errno = 0; - if (!getpwuid(getuid())) { - if (errno == 0) - die("slock: no password entry for current user\n"); - else - die("slock: getpwuid: %s\n", strerror(errno)); - } - -#ifndef HAVE_BSD_AUTH pws = getpw(); if (strlen(pws) < 2) die("slock: failed to get user password hash.\n"); -#endif if (!(dpy = XOpenDisplay(NULL))) die("slock: cannot open display\n"); @@ -396,11 +379,7 @@ main(int argc, char **argv) { } /* everything is now blank. Wait for the correct password */ -#ifdef HAVE_BSD_AUTH - readpw(dpy); -#else readpw(dpy, pws); -#endif /* password ok, unlock everything and quit */ cleanup(dpy); -- 2.20.1