http://bugzilla.mindrot.org/show_bug.cgi?id=559
Summary: PAM fixes
Product: Portable OpenSSH
Version: 3.6.1p2
Platform: All
OS/Version: All
Status: NEW
Severity: normal
Priority: P3
Component: sshd
AssignedTo: openssh-unix-dev at mindrot.org
ReportedBy: fcusack at fcusack.com
- start PAM with correct username
- don't call pam_authenticate() for null password checking
when not necessary
- set pam_flags correctly for kbdint pam_authenticate() calls
- improve logging
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=559 ------- Additional Comments From fcusack at fcusack.com 2003-05-12 13:55 ------- Created an attachment (id=289) --> (http://bugzilla.mindrot.org/attachment.cgi?id=289&action=view) PAM patches ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=559 ------- Additional Comments From djm at mindrot.org 2003-05-12 17:47 ------- Some comments:> - setproctitle("%s%s", authctxt->pw ? user : "unknown", > + setproctitle("%s%s", user, > use_privsep ? " [net]" : "");We deliberately hide the username in logs and on the process list to avoid password disclosure in situations where the client has entered their password as their username (it happens...)> - PRIVSEP(start_pam(authctxt->pw == NULL ? "NOUSER" : user)); > + PRIVSEP(start_pam(user));I am starting to change my mind that this may be correct. See Bug #117> - PRIVSEP(start_pam("NOUSER")); > + PRIVSEP(start_pam(user)); > + authenticated = -1; /* signal illegal user */authctxt->valid = 0 should obviate the need for the authenticated = -1, no?> + /* > + * REDACTED > + * REDACTED > ...What is this?> - retval = (do_pam_authenticate(0) == PAM_SUCCESS); > + retval = (do_pam_authenticate(options.permit_empty_passwd == 0 > + ? PAM_DISALLOW_NULL_AUTHTOK > + : 0) == PAM_SUCCESS);Is this still necessary with the CVS -current PAM code? ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
Darren J Moffat wrote:> On Mon, 12 May 2003, Damien Miller wrote: > >> The PAM stuff is IMO separate - one may disable empty passwords by >> omitting the "nullok" flag to pam_unix.so in the PAM control file. > > That is an argument specific to one vendors implementation of a specific module. > There is (and probably should not be) any standardization of the arguments > modules take. Some vendors my choose to standardize options across modules > they implement but it is certainly not required.Ok, but the point was that control over the behaviour of PAM modules and what they accept should be done in the PAM control file and not in sshd_config. -d
http://bugzilla.mindrot.org/show_bug.cgi?id=559
fcusack at fcusack.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #289 is|0 |1
obsolete| |
------- Additional Comments From fcusack at fcusack.com 2003-05-13 13:27
-------
Created an attachment (id=292)
--> (http://bugzilla.mindrot.org/attachment.cgi?id=292&action=view)
revised PAM patch
revised patches based on djm comments
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
[..] + * REDACTED + */ + if (!options.password_authentication || !options.permit_empty_passwd) + return(0); Check to ensure your not leaking account information via timing attacks by re-adding this. - Ben On Tue, 13 May 2003 bugzilla-daemon at mindrot.org wrote:> http://bugzilla.mindrot.org/show_bug.cgi?id=559 > > fcusack at fcusack.com changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Attachment #289 is|0 |1 > obsolete| | > > > > ------- Additional Comments From fcusack at fcusack.com 2003-05-13 13:27 ------- > Created an attachment (id=292) > --> (http://bugzilla.mindrot.org/attachment.cgi?id=292&action=view) > revised PAM patch > > revised patches based on djm comments > > > > ------- You are receiving this mail because: ------- > You are the assignee for the bug, or are watching the assignee. > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > http://www.mindrot.org/mailman/listinfo/openssh-unix-dev >
On Tue, 13 May 2003, Damien Miller wrote:> Darren J Moffat wrote: > > On Mon, 12 May 2003, Damien Miller wrote: > > > >> The PAM stuff is IMO separate - one may disable empty passwords by > >> omitting the "nullok" flag to pam_unix.so in the PAM control file. > > > > That is an argument specific to one vendors implementation of a specific module. > > There is (and probably should not be) any standardization of the arguments > > modules take. Some vendors my choose to standardize options across modules > > they implement but it is certainly not required. > > Ok, but the point was that control over the behaviour of PAM modules and > what they accept should be done in the PAM control file and not in > sshd_config.In general yes, but for "empty" passwords there is an explicity flag for pam_authenticate(3pam) PAM_DISALLOW_NULL_AUTHTOK. Personally I think this is the wrong level of abstraction and if there is ever a version 2 of PAM I would hope to remove this because it is individual module policy that should be used here. -- Darren J Moffat
http://bugzilla.mindrot.org/show_bug.cgi?id=559 ------- Additional Comments From djm at mindrot.org 2003-05-14 10:23 -------> @@ -186,8 +186,8 @@ input_userauth_request(int type, u_int32 > m = authmethod_lookup(method); > if (m != NULL) { > debug2("input_userauth_request: try method %s", method); > - authenticated = m->userauth(authctxt); > + authenticated = m->userauth(authctxt) && authctxt->valid; > } > userauth_finish(authctxt, authenticated, method);This chunk is not necessary, as userauth_finish does:> if (!authctxt->valid && authenticated) > fatal("INTERNAL ERROR: authenticated invalid user %s", > authctxt->user);and no auth method should set authenticated = 1 for a non existant user :) ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=559 ------- Additional Comments From djm at mindrot.org 2003-05-14 10:29 ------- The chunks like:> - PRIVSEP(start_pam("NOUSER")); > + PRIVSEP(start_pam(user));have been committed under bug #117 ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=559 ------- Additional Comments From fcusack at fcusack.com 2003-05-14 14:28 -------> > @@ -186,8 +186,8 @@ input_userauth_request(int type, u_int32...> This chunk is not necessary, as userauth_finish does:I didn't want to second guess what userauth_finish() would do (for maintainability going forward). Prior to the patch, userauth_finish() would never be called with authenticated=1 && authctxt->valid=0. Hence the fatal(), I guess! I wanted to preserve that assumption.> and no auth method should set authenticated = 1 for a non existant user :)You can't know what PAM will do. I had another patch where getpwnam() wouldn't run until after PAM was called. This gives PAM the chance to change the username, which it's allowed to do. FWIW, I actually have a valid use for that behavior (not just having a feature for feature's sake). A device that logs folks in to a single role account, but using individual usernames and secrets. Via PAM, that's possible to setup so that (eg) the auth goes to radius for secret verification, then the last module in the stack changes the username. The advantage is: no account maintenance on the device. I couldn't use the :style nicety because that is already used to access specific features when logging in. (I could have done something like :user.style but opted for PAM--seems cleaner.) But regardless of that use, again, you cannot know what PAM will do. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=559
djm at mindrot.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|sshd |PAM support
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.