From b22d058a055ec3e0f31ae16417f16b42baadb42f Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Fri, 5 Oct 2018 00:47:27 -0700 Subject: pam: lookup XDG_RUNTIME_DIR using get_any_env The pam_gnome_keyring.so PAM module needs to find the daemon control file, which is stored in $XDG_RUNTIME_DIR/keyring/control. Unfortunately when commit 2ca51a0aef5b ("daemon: Stop exporting the $GNOME_KEYRING_CONTROL env variable", 2014-03-06) switched to using XDG_RUNTIME_DIR preferentially over GNOME_KEYRING_CONTROL, it was looked up using getenv(). Unfortunately XDG_RUNTIME_DIR isn't always set in the environment, but may need to be looked up from pam_getenv. Indeed, the function get_any_env already exists for this purpose. Because of the incorrect environment lookup, lookup_daemon will incorrectly report that the gnome-keyring-daemon is not running, even though it is. This results in starting the daemon multiple times, and potentially failing to shut it down, or start it correctly when changing the password. To fix this, move the code for determining the control file path from gkr-pam-client.c into gkr-pam-module.c This will using get_any_env(), and avoids the need for passing the pam_handle_t variable into gkr-pam-client.c It does mean that the control variable must be allocated with space, since we need to combine the environment value with a different suffix depending on if we use GNOME_KEYRING_CONTROL or XDG_RUNTIME_DIR. Add a function get_control_file, so that the logic for determining the control file path remains in one location. Signed-off-by: Jacob Keller --- pam/gkr-pam-client.c | 17 +++------------ pam/gkr-pam-module.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 20 deletions(-) (limited to 'pam') diff --git a/pam/gkr-pam-client.c b/pam/gkr-pam-client.c index 71766c1d..ebb7f798 100644 --- a/pam/gkr-pam-client.c +++ b/pam/gkr-pam-client.c @@ -148,27 +148,16 @@ lookup_daemon (struct passwd *pwd, struct sockaddr_un *addr) { struct stat st; - const char *suffix; - if (control == NULL) { - control = getenv ("XDG_RUNTIME_DIR"); - if (control == NULL) - return GKD_CONTROL_RESULT_NO_DAEMON; - suffix = "/keyring/control"; - } else { - suffix = "/control"; - } - - if (strlen (control) + strlen (suffix) + 1 > sizeof (addr->sun_path)) { - syslog (GKR_LOG_ERR, "gkr-pam: address is too long for unix socket path: %s/%s", - control, suffix); + if (strlen (control) + 1 > sizeof (addr->sun_path)) { + syslog (GKR_LOG_ERR, "gkr-pam: address is too long for unix socket path: %s", + control); return GKD_CONTROL_RESULT_FAILED; } memset (addr, 0, sizeof (*addr)); addr->sun_family = AF_UNIX; strcpy (addr->sun_path, control); - strcat (addr->sun_path, suffix); /* A bunch of checks to make sure nothing funny is going on */ if (lstat (addr->sun_path, &st) < 0) { diff --git a/pam/gkr-pam-module.c b/pam/gkr-pam-module.c index fa814b23..f6b2188f 100644 --- a/pam/gkr-pam-module.c +++ b/pam/gkr-pam-module.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -68,6 +69,8 @@ enum { #define ENV_CONTROL "GNOME_KEYRING_CONTROL" +#define MAX_CONTROL_SIZE (sizeof(((struct sockaddr_un *)0)->sun_path)) + /* read & write ends of a pipe */ #define READ_END 0 #define WRITE_END 1 @@ -599,16 +602,49 @@ done: return ret; } +/* control must be at least MAX_CONTROL_SIZE */ +static int +get_control_file (pam_handle_t *ph, char *control) +{ + const char *control_root; + const char *suffix; + + control_root = get_any_env (ph, ENV_CONTROL); + if (control_root == NULL) { + control_root = get_any_env (ph, "XDG_RUNTIME_DIR"); + if (control_root == NULL) + return GKD_CONTROL_RESULT_NO_DAEMON; + suffix = "/keyring/control"; + } else { + suffix = "/control"; + } + + if (strlen (control_root) + strlen (suffix) + 1 > MAX_CONTROL_SIZE) { + syslog (GKR_LOG_ERR, "gkr-pam: address is too long for unix socket path: %s/%s", + control, suffix); + return GKD_CONTROL_RESULT_FAILED; + } + + strcpy (control, control_root); + strcat (control, suffix); + + return GKD_CONTROL_RESULT_OK; +} + static int stop_daemon (pam_handle_t *ph, struct passwd *pwd) { - const char *control; + char control[MAX_CONTROL_SIZE]; int res; assert (pwd); - control = get_any_env (ph, ENV_CONTROL); + res = get_control_file(ph, control); + if (res != GKD_CONTROL_RESULT_OK) { + syslog (GKR_LOG_ERR, "gkr-pam: unable to locate daemon control file"); + return PAM_SERVICE_ERR; + } res = gkr_pam_client_run_operation (pwd, control, GKD_CONTROL_OP_QUIT, 0, NULL); @@ -631,13 +667,18 @@ unlock_keyring (pam_handle_t *ph, const char *password, int *need_daemon) { - const char *control; + char control[MAX_CONTROL_SIZE]; int res; const char *argv[2]; assert (pwd); - control = get_any_env (ph, ENV_CONTROL); + res = get_control_file(ph, control); + if (res != GKD_CONTROL_RESULT_OK) { + syslog (GKR_LOG_ERR, "gkr-pam: unable to locate daemon control file"); + return PAM_SERVICE_ERR; + } + argv[0] = password; res = gkr_pam_client_run_operation (pwd, control, GKD_CONTROL_OP_UNLOCK, @@ -666,7 +707,7 @@ change_keyring_password (pam_handle_t *ph, const char *original, int *need_daemon) { - const char *control; + char control[MAX_CONTROL_SIZE]; const char *argv[3]; int res; @@ -674,7 +715,12 @@ change_keyring_password (pam_handle_t *ph, assert (password); assert (original); - control = get_any_env (ph, ENV_CONTROL); + res = get_control_file(ph, control); + if (res != GKD_CONTROL_RESULT_OK) { + syslog (GKR_LOG_ERR, "gkr-pam: unable to locate daemon control file"); + return PAM_SERVICE_ERR; + } + argv[0] = original; argv[1] = password; -- cgit v1.2.1