[gnome-keyring: 1/2] pam: lookup XDG_RUNTIME_DIR using get_any_env



commit b22d058a055ec3e0f31ae16417f16b42baadb42f
Author: Jacob Keller <jacob keller gmail com>
Date:   Fri Oct 5 00:47:27 2018 -0700

    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 <jacob keller gmail com>

 pam/gkr-pam-client.c | 17 +++------------
 pam/gkr-pam-module.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 55 insertions(+), 20 deletions(-)
---
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 <sys/types.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <sys/un.h>
 
 #include <assert.h>
 #include <ctype.h>
@@ -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;
        


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]