pam_setcred in slock not working

[cruegge]

Looks like I spoke a little too early...

I'm trying to use slock with the pam-auth patch . The patch itself doesnt call pam_setcred, so naturally as with any suckless tool, I'm trying to implement it myself. Judging from the physlock and xsecurelock PRs, it looks like it should be a simple addition. So, I thought this would work:

            switch (ksym) {
            case XK_Return:
                passwd[len] = '\0';
                errno = 0;
                #if PAMAUTH_PATCH
                retval = pam_start(pam_service, hash, &pamc, &pamh);
                color = PAM;
                for (screen = 0; screen < nscreens; screen++) {
                    XSetWindowBackground(dpy, locks[screen]->win, locks[screen]->colors[color]);
                    XClearWindow(dpy, locks[screen]->win);
                    XRaiseWindow(dpy, locks[screen]->win);
                }
                XSync(dpy, False);

                if (retval == PAM_SUCCESS)
                    retval = pam_authenticate(pamh, 0);
                if (retval == PAM_SUCCESS)
                    retval = pam_acct_mgmt(pamh, 0);

                running = 1;
                if (retval == PAM_SUCCESS) {
                    pam_setcred(pamh, PAM_REFRESH_CRED);
                    running = 0;
                }
                else
                    fprintf(stderr, "slock: %s\n", pam_strerror(pamh, retval));
                pam_end(pamh, retval);

But it doesn't seem to be the case. Is there something else that needs to be called before pam_setcred?

Originally posted by @Barbarossa93 in https://github.com/cruegge/pam-gnupg/issues/7#issuecomment-856297852

[cruegge]

That should work, it's the exact same sequence of calls that xsecurelock is using, apart from not calling pam_chauthtok if pam_acct_mgmt fails, which is optional anyway. Since slock does unlock (I assume), it should definitely have called pam_setcred. Can you comment out the call to pam_acct_mgmt, just to exclude that as the cause? That's the way i3lock does it. Also, try adding the debug option to the pam_gnupg.so lines in the pam config and check if anything turns up in journalctl.

I don't have my notebook available currently on which I use pam-gnupg myself, so I can't test it directly, unfortunately.

[Barbaross93]

Commenting out the lines

                           if (retval == PAM_SUCCESS)
                    retval = pam_acct_mgmt(pamh, 0);

Didn't change anything. Enabling the debug option I see

Jun 08 16:42:07 Forester slock[32913]: pam_gnupg(system-auth:auth): stored passphrase
Jun 08 16:42:07 Forester slock[32913]: pam_gnupg(system-auth:setcred): helper terminated with exit code 1
Jun 08 16:42:07 Forester slock[32913]: pam_gnupg(system-auth:setcred): presetting failed, retaining passphrase
Jun 08 16:42:17 Forester kernel: audit: type=1105 audit(1623184937.075:575): pid=33059 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:42:17 Forester audit[33059]: USER_START pid=33059 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:42:17 Forester sudo[33059]: pam_gnupg(sudo:session): unable to obtain stored passphrase
Jun 08 16:43:19 Forester audit[33059]: USER_END pid=33059 uid=1000 auid=1000 ses=1 msg='op=PAM:session_close grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:43:19 Forester kernel: audit: type=1106 audit(1623184999.218:590): pid=33059 uid=1000 auid=1000 ses=1 msg='op=PAM:session_close grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:43:33 Forester audit[33572]: USER_START pid=33572 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:43:33 Forester sudo[33572]: pam_gnupg(sudo:session): unable to obtain stored passphrase
Jun 08 16:43:33 Forester kernel: audit: type=1105 audit(1623185013.308:594): pid=33572 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:44:00 Forester kernel: audit: type=1106 audit(1623185040.481:595): pid=33572 uid=1000 auid=1000 ses=1 msg='op=PAM:session_close grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:44:00 Forester audit[33572]: USER_END pid=33572 uid=1000 auid=1000 ses=1 msg='op=PAM:session_close grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/13 res=success'
Jun 08 16:45:41 Forester audit[34774]: USER_START pid=34774 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/15 res=success'
Jun 08 16:45:41 Forester sudo[34774]: pam_gnupg(sudo:session): unable to obtain stored passphrase
Jun 08 16:45:41 Forester kernel: audit: type=1105 audit(1623185141.761:635): pid=34774 uid=1000 auid=1000 ses=1 msg='op=PAM:session_open grantors=pam_limits,pam_unix,pam_permit,pam_gnupg acct="root" exe="/usr/bin/sudo" hostname=? addr=? terminal=/dev/pts/15 res=succes

It seems like the credentials aren't being stored somewhere by slock?

[cruegge]

Ah, slock installs itself as setuid root to be able to read /etc/shadow (precisely because it doesn't use PAM), and then drops privileges to user ‘nobody’. pam-gnupg, on the other hand, tries to become the authenticated user, in order to read the config file ~/.pam-gnupg, and aborts if that fails, which it will since ‘nobody’ is not allowed to become you.

The easiest workaround is probably to hardcode your real user as the drop user in config.h.

Otherwise, you can rip out all of the setuid functionality, which is not really needed with PAM (with one exception, see below):

• Turn gethash into:

  static const char gethash(void) { struct passwd pw;

    /* Check if the current user has a password entry */
    errno = 0;
    if (!(pw = getpwuid(getuid()))) {
        if (errno)
            die("slock: getpwuid: %s\n", strerror(errno));
        else
            die("slock: cannot retrieve password entry\n");
    }
    return pw->pw_name;

  }

• Remove the /* validate drop-user and -group */ and /* drop privileges */ blocks in main.

• You may need to remove the call to dontkillme (and the then unused function itself). This one's actually useful even with PAM, since it adjusts the OOM score in order to prevent the kernel from killing slock when the machine runs out of memory. But I'm not sure it works without root, so you'll have to test. slock will write a message to stderr if it fails due to that.

• Remove the chmod u+s line in Makefile.

• There will be unused variables after this, so you'll probably get compiler warnings. The HAVE_SHADOW_H will also be obsolete. You can of course remove the leftovers if you want to.

Note that I didn't test this, you'll have to see what happens.

[Barbaross93]

I do have my username as the drop user, although the thing I'm not comfortable is what should be the drop group? I currently have it set to "users"

[cruegge]

It should be your primary group, the one displayed by id -gn.

[Barbaross93]

Welp, that was it! I had the wrong group set. It now works! Thank you so much for all of your help!!

[Ingvix]

• Remove the chmod u+s line in Makefile.

Pardon my ignorance, but why is this necessary? If I leave the SUID be, the dontkillme() function works fine, but not without it, as you speculated. Does it pose some security risk, if the privileges are not dropped as vanilla slock does?

EDIT: Forgot to mention that it seems to work just fine otherwise with your proposed changes and cleaning the unused variables.

[cruegge]

It's been a while, so I don't remember exactly. I think it's simply that running as root is not exactly best practice.

[Ingvix]

Then how about introducing dropping the privileges back, this time automatically to the real user, making it pretty much as secure as it would be with just using your own user and group for the drop targets?

Here's my current diff of the changes in case someone comes looking for it here, or if there are any suggestions to improve it. I might also submit it as a patch to suckless.org a bit later.

diff --git a/config.def.h b/config.def.h
index 9855e21..19e7f62 100644
--- a/config.def.h
+++ b/config.def.h
@@ -6,7 +6,11 @@ static const char *colorname[NUMCOLS] = {
    [INIT] =   "black",     /* after initialization */
    [INPUT] =  "#005577",   /* during input */
    [FAILED] = "#CC3333",   /* wrong password */
+   [PAM] =    "#9400D3",   /* waiting for PAM */
 };

 /* treat a cleared input like a wrong password (color) */
 static const int failonclear = 1;
+
+/* PAM service that's used for authentication */
+static const char* pam_service = "login";
diff --git a/config.mk b/config.mk
index 514c236..f57bb4e 100644
--- a/config.mk
+++ b/config.mk
@@ -12,10 +12,10 @@ X11LIB = /usr/X11R6/lib

 # includes and libs
 INCS = -I. -I/usr/include -I${X11INC}
-LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr
+LIBS = -L/usr/lib -lc -lcrypt -L${X11LIB} -lX11 -lXext -lXrandr -lpam

 # flags
-CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE -DHAVE_SHADOW_H
+CPPFLAGS = -DVERSION=\"${VERSION}\" -D_DEFAULT_SOURCE
 CFLAGS = -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
 LDFLAGS = -s ${LIBS}
 COMPATSRC = explicit_bzero.c
diff --git a/slock.c b/slock.c
index b2f14e3..f97a1fa 100644
--- a/slock.c
+++ b/slock.c
@@ -1,8 +1,5 @@
 /* See LICENSE file for license details. */
 #define _XOPEN_SOURCE 500
-#if HAVE_SHADOW_H
-#include <shadow.h>
-#endif

 #include <ctype.h>
 #include <errno.h>
@@ -18,16 +15,22 @@
 #include <X11/keysym.h>
 #include <X11/Xlib.h>
 #include <X11/Xutil.h>
+#include <security/pam_appl.h>
+#include <security/pam_misc.h>

 #include "arg.h"
 #include "util.h"

 char *argv0;
+static int pam_conv(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr);
+struct pam_conv pamc = {pam_conv, NULL};
+char passwd[256];

 enum {
    INIT,
    INPUT,
    FAILED,
+   PAM,
    NUMCOLS
 };

@@ -57,6 +60,31 @@ die(const char *errstr, ...)
    exit(1);
 }

+static int
+pam_conv(int num_msg, const struct pam_message **msg,
+       struct pam_response **resp, void *appdata_ptr)
+{
+   int retval = PAM_CONV_ERR;
+   for(int i=0; i<num_msg; i++) {
+       if (msg[i]->msg_style == PAM_PROMPT_ECHO_OFF &&
+               strncmp(msg[i]->msg, "Password: ", 10) == 0) {
+           struct pam_response *resp_msg = malloc(sizeof(struct pam_response));
+           if (!resp_msg)
+               die("malloc failed\n");
+           char *password = malloc(strlen(passwd) + 1);
+           if (!password)
+               die("malloc failed\n");
+           memset(password, 0, strlen(passwd) + 1);
+           strcpy(password, passwd);
+           resp_msg->resp_retcode = 0;
+           resp_msg->resp = password;
+           resp[i] = resp_msg;
+           retval = PAM_SUCCESS;
+       }
+   }
+   return retval;
+}
+
 #ifdef __linux__
 #include <fcntl.h>
 #include <linux/oom.h>
@@ -83,10 +111,9 @@ dontkillme(void)
 }
 #endif

-static const char *
-gethash(void)
+static struct passwd *
+getpw(void)
 {
-   const char *hash;
    struct passwd *pw;

    /* Check if the current user has a password entry */
@@ -97,43 +124,20 @@ gethash(void)
        else
            die("slock: cannot retrieve password entry\n");
    }
-   hash = pw->pw_passwd;
-
-#if HAVE_SHADOW_H
-   if (!strcmp(hash, "x")) {
-       struct spwd *sp;
-       if (!(sp = getspnam(pw->pw_name)))
-           die("slock: getspnam: cannot retrieve shadow entry. "
-               "Make sure to suid or sgid slock.\n");
-       hash = sp->sp_pwdp;
-   }
-#else
-   if (!strcmp(hash, "*")) {
-#ifdef __OpenBSD__
-       if (!(pw = getpwuid_shadow(getuid())))
-           die("slock: getpwnam_shadow: cannot retrieve shadow entry. "
-               "Make sure to suid or sgid slock.\n");
-       hash = 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 */
-
-   return hash;
+   return pw;
 }

 static void
 readpw(Display *dpy, struct xrandr *rr, struct lock **locks, int nscreens,
-       const char *hash)
+       const char *pw)
 {
    XRRScreenChangeNotifyEvent *rre;
-   char buf[32], passwd[256], *inputhash;
-   int num, screen, running, failure, oldc;
+   char buf[32];
+   int num, screen, running, failure, oldc, retval;
    unsigned int len, color;
    KeySym ksym;
    XEvent ev;
+   pam_handle_t *pamh;

    len = 0;
    running = 1;
@@ -160,10 +164,27 @@ readpw(Display *dpy, struct xrandr *rr, struct lock **locks, int nscreens,
            case XK_Return:
                passwd[len] = '\0';
                errno = 0;
-               if (!(inputhash = crypt(passwd, hash)))
-                   fprintf(stderr, "slock: crypt: %s\n", strerror(errno));
-               else
-                   running = !!strcmp(inputhash, hash);
+               retval = pam_start(pam_service, pw, &pamc, &pamh);
+               color = PAM;
+               for (screen = 0; screen < nscreens; screen++) {
+                   XSetWindowBackground(dpy, locks[screen]->win, locks[screen]->colors[color]);
+                   XClearWindow(dpy, locks[screen]->win);
+                   XRaiseWindow(dpy, locks[screen]->win);
+               }
+               XSync(dpy, False);
+
+               if (retval == PAM_SUCCESS)
+                   retval = pam_authenticate(pamh, 0);
+               if (retval == PAM_SUCCESS)
+                   retval = pam_acct_mgmt(pamh, 0);
+
+               running = 1;
+               if (retval == PAM_SUCCESS) {
+                   pam_setcred(pamh, PAM_REFRESH_CRED);
+                   running = 0;
+               } else
+                   fprintf(stderr, "slock: %s\n", pam_strerror(pamh, retval));
+               pam_end(pamh, retval);
                if (running) {
                    XBell(dpy, 100);
                    failure = 1;
@@ -307,11 +328,7 @@ int
 main(int argc, char **argv) {
    struct xrandr rr;
    struct lock **locks;
-   struct passwd *pwd;
-   struct group *grp;
-   uid_t duid;
-   gid_t dgid;
-   const char *hash;
+   struct passwd *pw;
    Display *dpy;
    int s, nlocks, nscreens;

@@ -323,26 +340,12 @@ main(int argc, char **argv) {
        usage();
    } ARGEND

-   /* validate drop-user and -group */
-   errno = 0;
-   if (!(pwd = getpwnam(user)))
-       die("slock: getpwnam %s: %s\n", user,
-           errno ? strerror(errno) : "user entry not found");
-   duid = pwd->pw_uid;
-   errno = 0;
-   if (!(grp = getgrnam(group)))
-       die("slock: getgrnam %s: %s\n", group,
-           errno ? strerror(errno) : "group entry not found");
-   dgid = grp->gr_gid;
-
 #ifdef __linux__
    dontkillme();
 #endif

-   hash = gethash();
+   pw = getpw();
    errno = 0;
-   if (!crypt("", hash))
-       die("slock: crypt: %s\n", strerror(errno));

    if (!(dpy = XOpenDisplay(NULL)))
        die("slock: cannot open display\n");
@@ -350,9 +353,9 @@ main(int argc, char **argv) {
    /* drop privileges */
    if (setgroups(0, NULL) < 0)
        die("slock: setgroups: %s\n", strerror(errno));
-   if (setgid(dgid) < 0)
+   if (setgid(pw->pw_gid) < 0)
        die("slock: setgid: %s\n", strerror(errno));
-   if (setuid(duid) < 0)
+   if (setuid(pw->pw_uid) < 0)
        die("slock: setuid: %s\n", strerror(errno));

    /* check for Xrandr support */
@@ -389,7 +392,7 @@ main(int argc, char **argv) {
    }

    /* everything is now blank. Wait for the correct password */
-   readpw(dpy, &rr, locks, nscreens, hash);
+   readpw(dpy, &rr, locks, nscreens, pw->pw_name);

    return 0;
 }