Attachment 74919 Details for Bug 51833 – password-helper: allow running as non-root

[patch] password-helper: allow running as non-root

0001-password-helper-allow-running-as-non-root.patch (text/plain), 5.60 KB, created by Giovanni Campagna on 2013-02-16 01:50:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Giovanni Campagna

Created: 2013-02-16 01:50:22 UTC

Size: 5.60 KB

patch

obsolete

>From eef65c53fc7d71c91042636c5e97fbed6b92e5b1 Mon Sep 17 00:00:00 2001
>From: Giovanni Campagna <gcampagna@src.gnome.org>
>Date: Sat, 16 Feb 2013 01:03:20 +0100
>Subject: [PATCH 1/3] password-helper: allow running as non-root
>
>Make the helper setuid, and read the old password when running non-root.
>pam_unix will do the rest, asking us for the old password and failing
>if they don't match.
>At the same time, convert return values to be PAM errors, so applications
>can detect more of the failure reason, without parsing standard error.
>---
> src/Makefile.am           |  4 ++++
> src/pam-password-helper.c | 22 ++++++++++++++++------
> src/pam-pin/pam-module.c  | 30 ++++++++++++++++++++++++++++--
> 3 files changed, 48 insertions(+), 8 deletions(-)
>
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 023f932..fe432a8 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -66,3 +66,7 @@ CLEANFILES = $(BUILT_SOURCES)
> install-data-hook:
> 	$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/AccountsService/users"
> 	$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/AccountsService/icons"
>+
>+install-exec-hook:
>+	-chown root $(DESTDIR)$(libexecdir)/accounts-daemon-pam-password-helper
>+	-chmod 4755 $(DESTDIR)$(libexecdir)/accounts-daemon-pam-password-helper
>diff --git a/src/pam-password-helper.c b/src/pam-password-helper.c
>index 54d2794..f61fbe8 100644
>--- a/src/pam-password-helper.c
>+++ b/src/pam-password-helper.c
>@@ -30,6 +30,7 @@
> #include <glib.h>
> 
> static char *password;
>+static char *old_password;
> static char *pin;
> 
> static int
>@@ -54,7 +55,11 @@ answer_pam_message (int                        num_msg,
>         */
>         for (i = 0; i < num_msg; i++) {
>                 if ((*msg)[i].msg_style == PAM_PROMPT_ECHO_OFF) {
>-                        if (strcmp((*msg)[i].msg, "PIN") == 0)
>+                        const char *message = (*msg)[i].msg;
>+
>+                        if (old_password && strstr(message, "current") != NULL)
>+                                resps[i].resp = strdup(old_password);
>+                        else if (strcmp(message, "PIN") == 0)
>                                 resps[i].resp = pin ? strdup(pin) : NULL;
>                         else
>                                 resps[i].resp = strdup(password);
>@@ -98,7 +103,7 @@ read_word (GIOChannel *from,
>                         g_printerr ("Generic error reading from standard input\n");
>                 }
> 
>-                exit(1);
>+                exit(PAM_AUTHTOK_ERR);
>         }
> 
>         str[term_pos] = 0;
>@@ -109,7 +114,7 @@ read_word (GIOChannel *from,
> 
>         if (decoded == NULL) {
>                 g_printerr ("Failed to decode password (probably contained a NUL).\n");
>-                exit(1);
>+                exit(PAM_AUTHTOK_ERR);
>         }
> 
>         g_free (str);
>@@ -128,12 +133,17 @@ main (int    argc,
> 
>         if (argc != 2) {
>                 g_printerr ("Wrong number of arguments passed, 1 expected\n");
>-                return 1;
>+                return PAM_AUTHTOK_ERR;
>         }
> 
>         username = argv[1];
> 
>         stdin = g_io_channel_unix_new (STDIN_FILENO);
>+        if (getuid() != 0) {
>+                /* Running from the user session, read the old password first */
>+                old_password = read_word (stdin, FALSE);
>+        }
>+
>         password = read_word (stdin, FALSE);
>         pin = read_word (stdin, TRUE);
>         g_io_channel_unref (stdin);
>@@ -143,7 +153,7 @@ main (int    argc,
>         if (res != PAM_SUCCESS) {
>                 /* pam_strerror can't be used without a pam handle */
>                 g_printerr ("Pam handle creation failed (not enough memory?)\n");
>-                return 2;
>+                return res;
>         }
> 
>         res = pam_chauthtok (pamh, PAM_SILENT);
>@@ -155,5 +165,5 @@ main (int    argc,
>         g_free (password);
>         g_free (pin);
> 
>-        return res == PAM_SUCCESS ? 0 : 2;
>+        return res;
> }
>diff --git a/src/pam-pin/pam-module.c b/src/pam-pin/pam-module.c
>index fd33de5..a6d27f9 100644
>--- a/src/pam-pin/pam-module.c
>+++ b/src/pam-pin/pam-module.c
>@@ -334,7 +334,8 @@ do_change_authtok(pam_handle_t  *handle)
>         char *filename;
>         char *ciphertext;
>         size_t ciphertext_len;
>-        int result;
>+        int result, ok;
>+        uid_t ruid, euid;
> 
>         result = pam_get_user (handle, &username, "Username: ");
>         if (result != PAM_SUCCESS)
>@@ -342,14 +343,39 @@ do_change_authtok(pam_handle_t  *handle)
> 
>         filename = g_build_filename (PASSWDDIR, username, NULL);
> 
>+        ciphertext = NULL;
>         result = PAM_AUTHTOK_ERR;
> 
>+        /* libgcrypt has the interesting habit to drop
>+           all privileges when allocating secure memory,
>+           and it does so with setuid() instead of seteuid()
>+           Workaround that by dropping the priviliges ourselves
>+           and then regaining them before leaving control to
>+           the rest of the PAM stack.
>+
>+           This is not thread-safe and very, very bad in general!
>+        */
>+        ruid = getuid();
>+        euid = geteuid();
>+
>+        if (euid != ruid) {
>+                ok = seteuid(ruid);
>+                if (ok < 0)
>+                        goto out;
>+        }
>+
>         result = request_and_encrypt_pin (handle, username,
>                                           &ciphertext, &ciphertext_len);
>+
>+        ok = seteuid (euid);
>+
>         if (result != PAM_SUCCESS)
>                 goto out;
>+        if (ok < 0) {
>+                result = PAM_AUTHTOK_ERR;
>+                goto out;
>+        }
> 
>-        result = PAM_AUTHTOK_ERR;
>         if (ciphertext) {
>                 if (!g_file_set_contents (filename, ciphertext, ciphertext_len, NULL))
>                         goto out;
>-- 
>1.8.1.2
>

Actions: View | Splinter Review

Attachments on bug 51833: 63941 | 63942 | 63943 | 63944 | 72158 | 72159 | 72741 | 72743 | 72744 | 72745 | 74904 | 74912 | 74913 | 74914 | 74915 | 74916 | 74917 | 74919

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.