[gnome-keyring] daemon: Stop exposing a GNOME_KEYRING_PID variable



commit 90a3ae6656960b36a1d2277f336222bd49d5eece
Author: Stef Walter <stefw gnome org>
Date:   Thu Mar 6 21:18:38 2014 +0100

    daemon: Stop exposing a GNOME_KEYRING_PID variable
    
    We exit with the DBus session bus. Remove this clutter from the
    environment.  PAM module no longer cares about the lifetime of
    the deamon, except in one case: where it started the daemon in
    order to change a password and the auto_start argument wasn't set.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=725801

 daemon/gkd-main.c    |   16 +++----
 daemon/gkd-util.c    |    1 -
 pam/gkr-pam-client.c |    4 +-
 pam/gkr-pam-module.c |  101 ++++++++++++--------------------------------------
 4 files changed, 34 insertions(+), 88 deletions(-)
---
diff --git a/daemon/gkd-main.c b/daemon/gkd-main.c
index 7cf99a9..5c5381c 100644
--- a/daemon/gkd-main.c
+++ b/daemon/gkd-main.c
@@ -493,13 +493,11 @@ clear_login_password (void)
 }
 
 static void
-print_environment (pid_t pid)
+print_environment (void)
 {
        const gchar **env;
        for (env = gkd_util_get_environment (); *env; ++env)
                printf ("%s\n", *env);
-       if (pid)
-               printf ("GNOME_KEYRING_PID=%d\n", (gint)pid);
        fflush (stdout);
 }
 
@@ -614,7 +612,7 @@ fork_and_print_environment (void)
        int fd, i;
 
        if (run_foreground) {
-               print_environment (getpid ());
+               print_environment ();
                return;
        }
 
@@ -635,8 +633,8 @@ fork_and_print_environment (void)
                                exit (WEXITSTATUS (status));
 
                } else {
-                       /* Not double forking, we know the PID */
-                       print_environment (pid);
+                       /* Not double forking */
+                       print_environment ();
                }
 
                /* The initial process exits successfully */
@@ -666,8 +664,8 @@ fork_and_print_environment (void)
                        if (pid == -1)
                                exit (1);
 
-                       /* We've done two forks. Now we know the PID */
-                       print_environment (pid);
+                       /* We've done two forks. */
+                       print_environment ();
 
                        /* The intermediate child exits */
                        exit (0);
@@ -899,7 +897,7 @@ main (int argc, char *argv[])
                         * Another daemon was initialized, print out environment
                         * for any callers, and quit or go comatose.
                         */
-                       print_environment (0);
+                       print_environment ();
                        if (run_foreground)
                                while (sleep(0x08000000) == 0);
                        cleanup_and_exit (0);
diff --git a/daemon/gkd-util.c b/daemon/gkd-util.c
index 431633b..d277c9f 100644
--- a/daemon/gkd-util.c
+++ b/daemon/gkd-util.c
@@ -43,7 +43,6 @@
 const gchar *GKD_UTIL_OUT_ENVIRONMENT[] = {
        "SSH_AUTH_SOCK",
        "GNOME_KEYRING_CONTROL",
-       "GNOME_KEYRING_PID",
        "SSH_AGENT_PID",
        NULL
 };
diff --git a/pam/gkr-pam-client.c b/pam/gkr-pam-client.c
index d2ce8b5..5c92cec 100644
--- a/pam/gkr-pam-client.c
+++ b/pam/gkr-pam-client.c
@@ -310,7 +310,9 @@ keyring_daemon_op (struct sockaddr_un *addr,
         * and an empty (only result code) return. 
         */
         
-       assert (op == GKD_CONTROL_OP_CHANGE || op == GKD_CONTROL_OP_UNLOCK);
+       assert (op == GKD_CONTROL_OP_CHANGE ||
+               op == GKD_CONTROL_OP_UNLOCK ||
+               op == GKD_CONTROL_OP_QUIT);
 
        ret = connect_daemon (addr, &sock);
        if (ret != GKD_CONTROL_RESULT_OK)
diff --git a/pam/gkr-pam-module.c b/pam/gkr-pam-module.c
index e82e30d..572c516 100644
--- a/pam/gkr-pam-module.c
+++ b/pam/gkr-pam-module.c
@@ -66,7 +66,6 @@ enum {
 };
 
 #define ENV_CONTROL             "GNOME_KEYRING_CONTROL"
-#define ENV_PID                 "GNOME_KEYRING_PID"
 
 /* read & write ends of a pipe */
 #define  READ_END   0
@@ -305,12 +304,6 @@ get_any_env (pam_handle_t *ph, const char *name)
 }
 
 static void
-cleanup_free (pam_handle_t *ph, void *data, int pam_end_status)
-{
-       free_safe (data);
-}
-
-static void
 cleanup_free_password (pam_handle_t *ph, void *data, int pam_end_status)
 {
        free_password (data);
@@ -449,7 +442,6 @@ static int
 setup_environment (char *line, void *arg)
 {
        pam_handle_t *ph = (pam_handle_t*)arg;
-       char *x;
        int ret;
        
        /* 
@@ -466,19 +458,14 @@ setup_environment (char *line, void *arg)
                        
        line = strbtrim (line);
        ret = pam_putenv (ph, line);
-       
-       /* If it's the PID line then we're interested in it */
-       if (strncmp (line, ENV_PID, strlen (ENV_PID)) == 0) { 
-               x = line + strlen (ENV_PID);
-               if (x[0] == '=')
-                       pam_set_data (ph, "gkr-pam-pid", strdup (x + 1), cleanup_free);
-       }
-       
+
        return ret;
 }
 
 static int
-start_daemon (pam_handle_t *ph, struct passwd *pwd, const char *password)
+start_daemon (pam_handle_t *ph,
+              struct passwd *pwd,
+              const char *password)
 {
        struct sigaction defsact, oldsact, ignpipe, oldpipe;
        int inp[2] = { -1, -1 };
@@ -573,8 +560,9 @@ start_daemon (pam_handle_t *ph, struct passwd *pwd, const char *password)
                        strerror (errno));
                goto done;
        }
-       
+
        failed = !WIFEXITED (status) || WEXITSTATUS (status) != 0;
+
        if (outerr && outerr[0])
                foreach_line (outerr, log_problem, &failed);
        
@@ -605,43 +593,28 @@ done:
 }
 
 static int
-stop_daemon (pam_handle_t *ph, struct passwd *pwd)
+stop_daemon (pam_handle_t *ph,
+             struct passwd *pwd)
 {
-       const char *spid = NULL;
-       char *apid = NULL;
-       pid_t pid;
-       
+       const char *control;
+       int res;
+
        assert (pwd);
 
-       pam_get_data (ph, "gkr-pam-pid", (const void**)&spid);
-       
-       /* 
-        * No pid, no worries, maybe we didn't start gnome-keyring-daemon
-        * Or this the calling (PAM using) application is hopeless and 
-        * wants to call different PAM callbacks from different processes.
-        * 
-        * In any case we live and let live.
-        */
-       if (!spid)
-               goto done;
-       
-       /* Make sure it parses out nicely */
-       pid = (pid_t)atoi (spid);
-       if (pid <= 0) {
-               syslog (GKR_LOG_ERR, "gkr-pam: invalid gnome-keyring-daemon process id: %s", spid);
-               goto done;
+       control = get_any_env (ph, ENV_CONTROL);
+
+       res = gkr_pam_client_run_operation (pwd, control, GKD_CONTROL_OP_QUIT, 0, NULL);
+
+       /* Daemon had already gone away */
+       if (res == GKD_CONTROL_RESULT_NO_DAEMON) {
+               return PAM_SUCCESS;
+
+       } else if (res != GKD_CONTROL_RESULT_OK) {
+               syslog (GKR_LOG_ERR, "gkr-pam: couldn't stop the daemon");
+               return PAM_SERVICE_ERR;
        }
-       
-       if (kill (pid, SIGTERM) < 0 && errno != ESRCH) {
-               syslog (GKR_LOG_ERR, "gkr-pam: couldn't kill gnome-keyring-daemon process %d: %s", 
-                       (int)pid, strerror (errno));
-               goto done;
-       }               
 
-done:
-       free_safe (apid);
-       
-       /* Don't bother user when daemon can't be stopped */
+       syslog (GKR_LOG_NOTICE, "gkr-pam: stopped the daemon");
        return PAM_SUCCESS;
 }
 
@@ -716,7 +689,7 @@ change_keyring_password (pam_handle_t *ph,
        syslog (GKR_LOG_NOTICE, "gkr-pam: changed password for login keyring");
        return PAM_SUCCESS;
 }
- 
+
 /* -----------------------------------------------------------------------------
  * PAM STUFF
  */
@@ -911,32 +884,6 @@ pam_sm_open_session (pam_handle_t *ph, int flags, int argc, const char **argv)
 }
 
 PAM_EXTERN int
-pam_sm_close_session (pam_handle_t *ph, int flags, int argc, const char **argv)
-{
-       struct passwd *pwd;
-       const char *user;
-       int ret;
-       
-       ret = pam_get_user (ph, &user, NULL);
-       if (ret != PAM_SUCCESS) {
-               syslog (GKR_LOG_ERR, "gkr-pam: couldn't get user from pam: %s", 
-                       pam_strerror (ph, ret));
-               return PAM_SERVICE_ERR;
-       }
-       
-       pwd = getpwnam (user);
-       if (!pwd) {
-               syslog (GKR_LOG_ERR, "gkr-pam: error looking up user information for: %s", user);
-               return PAM_SERVICE_ERR;
-       }
-
-       stop_daemon (ph, pwd);
-       
-       /* Don't bother user when daemon can't be stopped */
-       return PAM_SUCCESS; 
-}
-
-PAM_EXTERN int
 pam_sm_setcred (pam_handle_t * ph, int flags, int argc, const char **argv)
 {
        return PAM_SUCCESS;     


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