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

Re: If you care about debian's security read this



On Sun, Mar 03, 2002 at 08:56:45PM -0500, Jeff Licquia wrote:
> I agree that there is a security impact to this, though; gnome-sudo
> would be better if it supported this.  With all the flamage, I'm tempted
> to submit a patch to do this.

This seems difficult with support in sudo for it: there doesn't seem
to be any way for the launcher (gnome-sudo) to tell whether sudo's
asked for a password without the program being run telling the launcher
that it's running now. Which means the gnome-sudo-helper needs to be
in sudoers, which means the only option is running sudo twice somehow,
which I think still has problems (if your local config is to always ask
for passwords, eg).

So, here's one way of changing sudo. It essentially adds support for the
SSH_ASKPASS environment variable for querying passwords and such. Since
XAUTHORITY is passed through in the environment, this should do away
with the need for gnome-sudo-helper, and just let gnome-sudo call "sudo
<cmd>" directly.

It appears to work, and doesn't have at least one obvious security hole.
It doesn't do all its dialogue via ssh-askpass, though, which might be
considered a bug.

diff -urb sudo-1.6.5p1/debian/changelog sudo-1.6.5p1-aj/debian/changelog
--- sudo-1.6.5p1/debian/changelog	Mon Mar  4 14:21:24 2002
+++ sudo-1.6.5p1-aj/debian/changelog	Mon Mar  4 14:39:30 2002
@@ -1,3 +1,10 @@
+sudo (1.6.5p1-3.1) unstable; urgency=low
+
+  * Add support for getting passphrase from ssh-askpass.
+  * Fixes free-of-unallocated-memory bug in auth/pam.c (Closes: Bug#131235)
+
+ -- Anthony Towns <aj@azure.humbug.org.au>  Mon,  4 Mar 2002 13:11:54 +1000
+
 sudo (1.6.5p1-3) unstable; urgency=high
 
   * ugly hack to add --disable-saved-ids when building on sparc in response 
diff -urb sudo-1.6.5p1/auth/pam.c sudo-1.6.5p1-aj/auth/pam.c
--- sudo-1.6.5p1/auth/pam.c	Mon Mar  4 14:21:24 2002
+++ sudo-1.6.5p1-aj/auth/pam.c	Mon Mar  4 14:20:37 2002
@@ -220,7 +220,7 @@
 		pr->resp = estrdup((char *) tgetpass(p,
 		    def_ival(I_PASSWD_TIMEOUT) * 60, tgetpass_flags));
 		if (pr->resp == NULL)
-		    pr->resp = "";
+		    pr->resp = strdup(""); /* freed later */
 		if (*pr->resp == '\0')
 		    nil_pw = 1;		/* empty password */
 		break;
diff -urb sudo-1.6.5p1/env.c sudo-1.6.5p1-aj/env.c
--- sudo-1.6.5p1/env.c	Wed Jan 16 09:43:58 2002
+++ sudo-1.6.5p1-aj/env.c	Mon Mar  4 14:24:14 2002
@@ -154,6 +154,10 @@
 
     for (ep = envp; *ep; ep++) {
 	switch (**ep) {
+	    case 'D':
+		if (strncmp("DISPLAY=", *ep, 8) == 0) 
+		    break; /* needed for ssh-askpass to work */
+		continue;
 	    case 'H':
 		if (strncmp("HOME=", *ep, 5) == 0)
 		    break;
@@ -174,6 +178,8 @@
 		    user_shell = *ep + 6;
 		else if (!user_prompt && !strncmp("SUDO_PROMPT=", *ep, 12))
 		    user_prompt = *ep + 12;
+		else if (!user_sshaskpass && !strncmp("SSH_ASKPASS=", *ep, 12))
+		    user_sshaskpass = *ep + 12;
 		continue;
 	    case 'T':
 		if (strncmp("TZ=", *ep, 3) == 0)
Only in sudo-1.6.5p1-aj/: foo.sh
diff -urb sudo-1.6.5p1/sudo.c sudo-1.6.5p1-aj/sudo.c
--- sudo-1.6.5p1/sudo.c	Wed Jan 16 09:43:59 2002
+++ sudo-1.6.5p1-aj/sudo.c	Mon Mar  4 14:44:30 2002
@@ -736,6 +736,14 @@
 	    case 'S':
 		tgetpass_flags |= TGP_STDIN;
 		break;
+	    case 'A':
+		if (!user_sshaskpass) {
+		    (void) fprintf(stderr, 
+			"%s: -A requires SSH_ASKPASS\n", Argv[0]);
+		    usage(1);
+		}
+		tgetpass_flags |= TGP_SSHASKPASS;
+		break;
 	    case '-':
 		NewArgc--;
 		NewArgv++;
@@ -1022,7 +1030,7 @@
 {
 
     (void) fprintf(stderr, "usage: sudo -V | -h | -L | -l | -v | -k | -K | %s",
-	"[-H] [-P] [-S] [-b] [-p prompt]\n            [-u username/#uid] ");
+	"[-A] [-H] [-P] [-S] [-b]\n            [-p prompt] [-u username/#uid] ");
 #ifdef HAVE_LOGIN_CAP_H
     (void) fprintf(stderr, "[-c class] ");
 #endif
diff -urb sudo-1.6.5p1/sudo.h sudo-1.6.5p1-aj/sudo.h
--- sudo-1.6.5p1/sudo.h	Thu Jan 17 07:27:09 2002
+++ sudo-1.6.5p1-aj/sudo.h	Mon Mar  4 13:27:58 2002
@@ -60,6 +60,7 @@
     char *cmnd;
     char *cmnd_args;
     char *class_name;
+    char *sshaskpass;
 };
 
 /*
@@ -135,6 +136,7 @@
 #define user_prompt		(sudo_user.prompt)
 #define user_host		(sudo_user.host)
 #define user_shost		(sudo_user.shost)
+#define user_sshaskpass		(sudo_user.sshaskpass)
 #define safe_cmnd		(sudo_user.cmnd_safe)
 #define login_class		(sudo_user.class_name)
 #define runas_pw		(sudo_user._runas_pw)
@@ -171,6 +173,7 @@
  */
 #define TGP_ECHO	0x01		/* leave echo on when reading passwd */
 #define TGP_STDIN	0x02		/* read from stdin, not /dev/tty */
+#define TGP_SSHASKPASS  0x04		/* if set, use SSH_ASKPASS on DISPLAY */
 
 /*
  * Function prototypes
diff -urb sudo-1.6.5p1/sudo.man.in sudo-1.6.5p1-aj/sudo.man.in
--- sudo-1.6.5p1/sudo.man.in	Thu Jan 17 09:36:09 2002
+++ sudo-1.6.5p1-aj/sudo.man.in	Mon Mar  4 14:26:28 2002
@@ -145,7 +145,7 @@
 .SH "SYNOPSIS"
 .IX Header "SYNOPSIS"
 \&\fBsudo\fR \fB\-V\fR | \fB\-h\fR | \fB\-l\fR | \fB\-L\fR | \fB\-v\fR | \fB\-k\fR | \fB\-K\fR | \fB\-s\fR |
-[ \fB\-H\fR ] [\fB\-P\fR ] [\fB\-S\fR ] [ \fB\-b\fR ] | [ \fB\-p\fR \fIprompt\fR ]
+[ \fB-A\fR ] [ \fB\-H\fR ] [\fB\-P\fR ] [\fB\-S\fR ] [ \fB\-b\fR ] | [ \fB\-p\fR \fIprompt\fR ]
 [ \fB\-c\fR \fIclass\fR|\fI-\fR ] [ \fB\-a\fR \fIauth_type\fR ]
 [ \fB\-u\fR \fIusername\fR|\fI#uid\fR ] \fIcommand\fR
 .SH "DESCRIPTION"
@@ -262,6 +262,13 @@
 The \fB\-s\fR (\fIshell\fR) option runs the shell specified by the \fI\s-1SHELL\s0\fR
 environment variable if it is set or the shell as specified
 in \fIpasswd\fR\|(@mansectform@).
+.Ip "\-A" 4
+.IX Item "-A"
+The \fB\-A\fR option runs the program specified in the \fI\s-1SSH_ASKPASS\s0\fR
+envrionment variable to obtain the password. The program should accept a single
+argument for the prompt, and return a single line on stdout for the password.
+Note that most environment variables have been cleared before this program is
+run.
 .Ip "\-H" 4
 .IX Item "-H"
 The \fB\-H\fR (\fI\s-1HOME\s0\fR) option sets the \f(CW\*(C`HOME\*(C'\fR environment variable
diff -urb sudo-1.6.5p1/tgetpass.c sudo-1.6.5p1-aj/tgetpass.c
--- sudo-1.6.5p1/tgetpass.c	Tue Dec 18 09:56:47 2001
+++ sudo-1.6.5p1-aj/tgetpass.c	Mon Mar  4 14:21:09 2002
@@ -36,6 +36,7 @@
 
 #include <sys/types.h>
 #include <sys/param.h>
+#include <sys/wait.h>
 #ifdef HAVE_SYS_BSDTYPES_H
 # include <sys/bsdtypes.h>
 #endif /* HAVE_SYS_BSDTYPES_H */
@@ -131,6 +132,59 @@
 static void handler __P((int));
 
 /*
+ * Call external command (ssh-askpass) to get password. Useful for GUIs.
+ */
+char *
+tsshaskpass(prompt, buf, bufsize)
+    const char *prompt;
+    char *buf;
+    int bufsize;
+{
+    int fromchild[2];
+    pid_t child;
+    int len, status;
+
+    if (0 != pipe(fromchild)) return NULL;
+
+    child = fork();
+    if (child < 0) {
+	close(fromchild[0]); close(fromchild[1]);
+	return NULL;
+    }
+
+    if (child == 0) { /* I'm the child */
+        set_perms(PERM_FULL_USER, 0);
+
+	close(fromchild[0]);
+
+	if (fromchild[1] != 1) {
+	    dup2(fromchild[1], 1);
+	    close(fromchild[1]);
+	}
+
+	execlp(user_sshaskpass, user_sshaskpass, prompt, NULL);
+	exit(1);
+    }
+
+    /* I'm the parent */
+    close(fromchild[1]);
+
+    /* Read the first line of output from the child */
+    len = read(fromchild[0], buf, bufsize - 1);
+    close(fromchild[0]);
+
+    while (waitpid(child, &status, 0) < 0)
+        if (errno != EINTR)
+	    break;
+    if (len <= 1)
+	return NULL;
+    buf[len] = '\0';
+    buf[strcspn(buf, "\r\n")] = '\0';
+
+    return (buf[0] != '\0') ? buf : NULL;
+}
+
+/*
  * Like getpass(3) but with timeout and echo flags.
  */
 char *
@@ -145,6 +199,10 @@
     int input, output, save_errno;
     struct TERM term, oterm;
     char *pass;
+
+    if (flags & TGP_SSHASKPASS) {
+	return tsshaskpass(prompt, buf, sizeof(buf));
+    }
 
 restart:
     /* Open /dev/tty for reading/writing if possible else use stdin/stderr. */


Cheers,
aj

-- 
Anthony Towns <aj@humbug.org.au> <http://azure.humbug.org.au/~aj/>
We came. We Saw. We Conferenced. http://linux.conf.au/

  ``Debian: giving you the power to shoot yourself in each 
       toe individually.'' -- with kudos to Greg Lehey

Attachment: pgpuJlDtVOOz7.pgp
Description: PGP signature


Reply to: