pam: Fix issue with changed password not unlocking keyring

If a user (needs to) change their password while authenticating (via
GDM for example), and pam_gnome_keyring is configured to start the
daemon from the session PAM stage, then we were failing to pass the
changed password to our session handler.

Fix this issue so that this workflow works.

https://bugzilla.gnome.org/show_bug.cgi?id=726196

2 files changed, 76 insertions, 15 deletions

diff --git a/pam/gkr-pam-module.c b/pam/gkr-pam-module.c
index 35927318..a124c2b2 100644
--- a/pam/gkr-pam-module.c
+++ b/pam/gkr-pam-module.c
@@ -780,6 +780,19 @@ parse_args (pam_handle_t *ph, int argc, const char **argv)
 	return args;
 }
 
+static int
+stash_password_for_session (pam_handle_t *ph,
+                            const char *password)
+{
+	if (pam_set_data (ph, "gkr_system_authtok", strdup (password),
+	                  cleanup_free_password) != PAM_SUCCESS) {
+		syslog (GKR_LOG_ERR, "gkr-pam: error stashing password for session");
+		return PAM_AUTHTOK_RECOVER_ERR;
+	}
+
+	return PAM_SUCCESS;
+}
+
 PAM_EXTERN int
 pam_sm_authenticate (pam_handle_t *ph, int unused, int argc, const char **argv)
 {
@@ -821,19 +834,13 @@ pam_sm_authenticate (pam_handle_t *ph, int unused, int argc, const char **argv)
 
 	ret = unlock_keyring (ph, pwd, password, &need_daemon);
 	if (ret != PAM_SUCCESS && need_daemon) {
-		if (args & ARG_AUTO_START) {
-			/* If we started the daemon, its already unlocked, since we passed the password */
+		/* If we started the daemon, its already unlocked, since we passed the password */
+		if (args & ARG_AUTO_START)
 			ret = start_daemon (ph, pwd, password);
 
 		/* Otherwise start later in open session, store password */
-		} else if (pam_set_data (ph, "gkr_system_authtok", strdup (password),
-		                         cleanup_free_password) != PAM_SUCCESS) {
-			syslog (GKR_LOG_ERR, "gkr-pam: error storing authtok");
-			return PAM_AUTHTOK_RECOVER_ERR;
-
-		} else {
-			ret = PAM_SUCCESS;
-		}
+		else
+			ret = stash_password_for_session (ph, password);
 	}
 
 	return ret;
@@ -924,18 +931,20 @@ pam_chauthtok_update (pam_handle_t *ph, struct passwd *pwd, uint args)
 	const char *password, *original;
 	int need_daemon = 0;
 	int ret;
-	
+
+	ret = pam_get_item (ph, PAM_AUTHTOK, (const void**)&password);
+	if (ret != PAM_SUCCESS)
+		password = NULL;
+
 	ret = pam_get_item (ph, PAM_OLDAUTHTOK, (const void**)&original);
 	if (ret != PAM_SUCCESS || original == NULL) {
 		syslog (GKR_LOG_WARN, "gkr-pam: couldn't update the login keyring password: %s",
 		        "no old password was entered");
+		if (password)
+			stash_password_for_session (ph, password);
 		return PAM_IGNORE;
 	}
 		
-	ret = pam_get_item (ph, PAM_AUTHTOK, (const void**)&password);
-	if (ret != PAM_SUCCESS)
-		password = NULL;
-		
 	if (password == NULL) {
 		/* No password was set, and we can't prompt for it */
 		if (args & ARG_USE_AUTHTOK) {
@@ -979,6 +988,13 @@ pam_chauthtok_update (pam_handle_t *ph, struct passwd *pwd, uint args)
 		}
 	}
 
+	/*
+	 * Likely the daemon is being started later in the session if we weren't
+	 * allowed to autostart it here. Store the password for our session handler
+	 */
+	if (!(args & ARG_AUTO_START))
+		stash_password_for_session (ph, password);
+
 	return ret;
 }
 
diff --git a/pam/test-pam.c b/pam/test-pam.c
index 55f49ac6..87d8fe18 100644
--- a/pam/test-pam.c
+++ b/pam/test-pam.c
@@ -509,6 +509,47 @@ test_password_changes_starts (Test *test,
 	g_free (control);
 }
 
+static void
+test_password_change_start_in_session (Test *test,
+                                       gconstpointer user_data)
+{
+	const char *pam_conf = user_data;
+	gchar *control;
+
+	if (test->skipping)
+		return;
+
+	/* This is the PAM config that starts the daemon from session handler */
+	g_assert (strstr (pam_conf, "session-start") != NULL);
+
+	egg_tests_copy_scratch_file (test->directory, SRCDIR "/pam/fixtures/login.keyring");
+	control = g_strdup_printf ("%s/keyring", test->directory);
+
+	/* First we authenticate, but don't start the keyring here */
+	test->password = "booo";
+	g_assert_cmpint (pam_authenticate (test->ph, 0), ==, PAM_SUCCESS);
+
+	test->password = "booo";
+	test->new_password = "changed";
+	g_assert_cmpint (pam_chauthtok (test->ph, 0), ==, PAM_SUCCESS);
+
+	/* No daemon should be running, chauthtok started/stopped it */
+	g_assert (gkd_control_quit (control, GKD_CONTROL_QUIET_IF_NO_PEER) == FALSE);
+
+	/* Now session should be able to start and unlock the keyring */
+	g_assert_cmpint (pam_open_session (test->ph, 0), ==, PAM_SUCCESS);
+
+	/* Initialize the daemon */
+	g_assert (gkd_control_initialize (control, "secrets", PASS_ENVIRON));
+
+	/* Lookup the item */
+	g_assert (check_if_login_keyring_locked (test) == FALSE);
+	g_assert (check_if_login_item_1_exists (test) == TRUE);
+
+	g_assert (gkd_control_quit (control, 0));
+	g_free (control);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -547,5 +588,9 @@ main (int argc, char **argv)
 	            "gnome-keyring-test-no-start",
 	            setup, test_password_changes_starts, teardown);
 
+	g_test_add ("/pam/password-change-start-in-session", Test,
+	            "gnome-keyring-test-session-start",
+	            setup, test_password_change_start_in_session, teardown);
+
 	return g_test_run ();
 }