The security/sudo port has a slight deficiency in handling the PAM conversation. If a PAM module requests that passwords be echoed as they are typed in, the 'echo on' state remains active for the whole of the conversation, even for modules that have explicitly requested that echo be turned off. This can lead to serious security problems, if OPIE or S/Key is used in conjunction with other authentication mechanisms, since both OPIE and S/Key allow the user to enter the password in 'echo on' mode - any other PAM module invoked after that will have the pass phrases echoed back, which may be undesirable in the pam_unix, pam_krb5, or indeed most other cases. Fix: NOTE: this fix will NOT fix the problem completely on at least 4.7-STABLE systems! Please see PR bin/46025, 'OPIE and S/Key PAM prompt echoing fixes', for a description and a fix of another related problem in the OPIE and S/Key PAM modules themselves. Still, I believe that this fix should be applied before FreeBSD 5.0 is released; I will submit it to the sudo developers shortly. How-To-Repeat: Configure S/Key or OPIE authentication for a user. Try to log in. Press 'Enter' on the first S/Key or OPIE prompt, so the 'echo on' prompt is displayed. Press 'Enter' or enter an invalid passphrase at the 'echo on' prompt. Wait for the next auth module's 'Password:' prompt to appear, then watch in horrified fascination as your password is echoed straight back at you as you type it in.
Responsible Changed From-To: freebsd-ports->mharo Over to maintainer; IMHO, this should be submitted to portmgr for committing before 5.0.
On Fri, Dec 06, 2002 at 02:46:03PM -0000, Peter Pentchev wrote: > > >Number: 46026 > >Category: ports > >Synopsis: [PATCH] fix security/sudo PAM echo handling [snip] > NOTE: this fix will NOT fix the problem completely on at least > 4.7-STABLE systems! Please see PR bin/46025, 'OPIE and S/Key PAM prompt > echoing fixes', for a description and a fix of another related problem > in the OPIE and S/Key PAM modules themselves. > > Still, I believe that this fix should be applied before FreeBSD 5.0 is > released; I will submit it to the sudo developers shortly. After an exchange of e-mail with Todd Miller, he committed a fix to the sudo CVS repository. Attached is the patch itself, as extracted from the repository mirrored from rsync.sudo.ws. G'luck, Peter -- Peter Pentchev roam@ringlet.net roam@FreeBSD.org PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 If this sentence didn't exist, somebody would have invented it. Index: Makefile =================================================================== RCS file: /home/ncvs/ports/security/sudo/Makefile,v retrieving revision 1.53 diff -u -r1.53 Makefile --- Makefile 14 Sep 2002 19:27:18 -0000 1.53 +++ Makefile 6 Dec 2002 14:04:17 -0000 @@ -7,7 +7,7 @@ PORTNAME= sudo PORTVERSION= 1.6.6 -PORTREVISION= 0 +PORTREVISION= 1 CATEGORIES= security MASTER_SITES= http://www.sudo.ws/sudo/dist/ \ ftp://ftp.cs.colorado.edu/pub/sysadmin/sudo/ \ Index: files/patch-auth::pam.c =================================================================== RCS file: files/patch-auth::pam.c diff -N files/patch-auth::pam.c --- files/patch-auth::pam.c 1 Jan 1970 00:00:00 -0000 +++ files/patch-auth::pam.c 13 Dec 2002 20:49:42 -0000 @@ -0,0 +1,47 @@ +Index: auth/pam.c +=================================================================== +RCS file: /home/cvs/sudo/sudo/auth/pam.c,v +retrieving revision 1.30 +retrieving revision 1.31 +diff -u -r1.30 -r1.31 +--- auth/pam.c 22 Nov 2002 19:41:13 -0000 1.30 ++++ auth/pam.c 13 Dec 2002 16:33:26 -0000 1.31 +@@ -66,7 +66,7 @@ + #include "sudo_auth.h" + + #ifndef lint +-static const char rcsid[] = "$Sudo: pam.c,v 1.29 2002/01/22 16:43:23 millert Exp $"; ++static const char rcsid[] = "$Sudo: pam.c,v 1.30 2002/11/22 19:41:13 millert Exp $"; + #endif /* lint */ + + static int sudo_conv __P((int, PAM_CONST struct pam_message **, +@@ -205,7 +205,7 @@ + PAM_CONST struct pam_message *pm; + const char *p = def_prompt; + volatile char *pass; +- int n; ++ int n, flags; + extern int nil_pw; + + if ((*response = malloc(num_msg * sizeof(struct pam_response))) == NULL) +@@ -213,17 +213,17 @@ + (void) memset(*response, 0, num_msg * sizeof(struct pam_response)); + + for (pr = *response, pm = *msg, n = num_msg; n--; pr++, pm++) { ++ flags = tgetpass_flags; + switch (pm->msg_style) { + case PAM_PROMPT_ECHO_ON: +- tgetpass_flags |= TGP_ECHO; ++ flags |= TGP_ECHO; + case PAM_PROMPT_ECHO_OFF: + /* Only override PAM prompt if it matches /^Password: ?/ */ + if (strncmp(pm->msg, "Password:", 9) || (pm->msg[9] != '\0' + && (pm->msg[9] != ' ' || pm->msg[10] != '\0'))) + p = pm->msg; + /* Read the password. */ +- pass = tgetpass(p, def_ival(I_PASSWD_TIMEOUT) * 60, +- tgetpass_flags); ++ pass = tgetpass(p, def_ival(I_PASSWD_TIMEOUT) * 60, flags); + pr->resp = estrdup(pass ? pass : ""); + if (*pr->resp == '\0') + nil_pw = 1; /* empty password */
On Fri, Dec 13, 2002 at 11:07:13PM +0200, Peter Pentchev wrote: > On Fri, Dec 06, 2002 at 02:46:03PM -0000, Peter Pentchev wrote: > > > > >Number: 46026 > > >Category: ports > > >Synopsis: [PATCH] fix security/sudo PAM echo handling > [snip] > > NOTE: this fix will NOT fix the problem completely on at least > > 4.7-STABLE systems! Please see PR bin/46025, 'OPIE and S/Key PAM prompt > > echoing fixes', for a description and a fix of another related problem > > in the OPIE and S/Key PAM modules themselves. > > > > Still, I believe that this fix should be applied before FreeBSD 5.0 is > > released; I will submit it to the sudo developers shortly. > > After an exchange of e-mail with Todd Miller, he committed a fix to the > sudo CVS repository. Attached is the patch itself, as extracted from > the repository mirrored from rsync.sudo.ws. Errr... oops. Here it is again, without the MIME / PGP signature. <mutter>sleepy fingers..</mutter> G'luck, Peter -- Peter Pentchev roam@ringlet.net roam@FreeBSD.org PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 I am jealous of the first word in this sentence. Index: Makefile =================================================================== RCS file: /home/ncvs/ports/security/sudo/Makefile,v retrieving revision 1.53 diff -u -r1.53 Makefile --- Makefile 14 Sep 2002 19:27:18 -0000 1.53 +++ Makefile 6 Dec 2002 14:04:17 -0000 @@ -7,7 +7,7 @@ PORTNAME= sudo PORTVERSION= 1.6.6 -PORTREVISION= 0 +PORTREVISION= 1 CATEGORIES= security MASTER_SITES= http://www.sudo.ws/sudo/dist/ \ ftp://ftp.cs.colorado.edu/pub/sysadmin/sudo/ \ Index: files/patch-auth::pam.c =================================================================== RCS file: files/patch-auth::pam.c diff -N files/patch-auth::pam.c --- files/patch-auth::pam.c 1 Jan 1970 00:00:00 -0000 +++ files/patch-auth::pam.c 13 Dec 2002 20:49:42 -0000 @@ -0,0 +1,47 @@ +Index: auth/pam.c +=================================================================== +RCS file: /home/cvs/sudo/sudo/auth/pam.c,v +retrieving revision 1.30 +retrieving revision 1.31 +diff -u -r1.30 -r1.31 +--- auth/pam.c 22 Nov 2002 19:41:13 -0000 1.30 ++++ auth/pam.c 13 Dec 2002 16:33:26 -0000 1.31 +@@ -66,7 +66,7 @@ + #include "sudo_auth.h" + + #ifndef lint +-static const char rcsid[] = "$Sudo: pam.c,v 1.29 2002/01/22 16:43:23 millert Exp $"; ++static const char rcsid[] = "$Sudo: pam.c,v 1.30 2002/11/22 19:41:13 millert Exp $"; + #endif /* lint */ + + static int sudo_conv __P((int, PAM_CONST struct pam_message **, +@@ -205,7 +205,7 @@ + PAM_CONST struct pam_message *pm; + const char *p = def_prompt; + volatile char *pass; +- int n; ++ int n, flags; + extern int nil_pw; + + if ((*response = malloc(num_msg * sizeof(struct pam_response))) == NULL) +@@ -213,17 +213,17 @@ + (void) memset(*response, 0, num_msg * sizeof(struct pam_response)); + + for (pr = *response, pm = *msg, n = num_msg; n--; pr++, pm++) { ++ flags = tgetpass_flags; + switch (pm->msg_style) { + case PAM_PROMPT_ECHO_ON: +- tgetpass_flags |= TGP_ECHO; ++ flags |= TGP_ECHO; + case PAM_PROMPT_ECHO_OFF: + /* Only override PAM prompt if it matches /^Password: ?/ */ + if (strncmp(pm->msg, "Password:", 9) || (pm->msg[9] != '\0' + && (pm->msg[9] != ' ' || pm->msg[10] != '\0'))) + p = pm->msg; + /* Read the password. */ +- pass = tgetpass(p, def_ival(I_PASSWD_TIMEOUT) * 60, +- tgetpass_flags); ++ pass = tgetpass(p, def_ival(I_PASSWD_TIMEOUT) * 60, flags); + pr->resp = estrdup(pass ? pass : ""); + if (*pr->resp == '\0') + nil_pw = 1; /* empty password */
State Changed From-To: open->closed I committed the patch with Michael Haro's permission.