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>

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;