cvs commit: src/lib/libpam/modules/pam_unix pam_unix.8 pam_unix.c

[Yar Tikhiy]

On Thu, Apr 26, 2007 at 01:54:59PM +0300, Alexandr Kovalenko wrote:
> Hello, Yar Tikhiy!
> 
> On Thu, Apr 26, 2007 at 06:39:01AM +0000, you wrote:
> 
> > yar         2007-04-26 06:39:01 UTC
> > 
> >   FreeBSD src repository
> > 
> >   Modified files:        (Branch: RELENG_6)
> >     lib/libpam/modules/pam_unix pam_unix.8 pam_unix.c 
> >   Log:
> >   MFC:
> >           pam_unix.c      1.52
> >           pam_unix.8      1.13
> >   
> >     In account management, verify whether the account has been locked
> >     with `pw lock', so that it's impossible to log into a locked account
> >     using an alternative authentication mechanism, such as an ssh key.
> >     This change affects only accounts locked with pw(8), i.e., having a
> >     `*LOCKED*' prefix in their password hash field, so people still can
> >     use a different pattern to disable password authentication only.
> 
> Using the very same logic you should also add checking for '*', and for
> any other string, which cannot be in password hash of different
> algorithms. By the way, what if some crypto algorithm, which will be
> used for password hashing can produce hash, which contains substring
> '*LOCKED*' ?

Please don't over-generalize.  My change adds a check for a *LOCKED*
prefix only, which cannot appear in a password hash unless its current
format is broken.  Neither an old DES hash nor a new multi-algorithm
hash can start with *LOCKED*.

> But anyway, I think that it is not expected behavour of sshd/pam_unix.
> 
> >From the pw manual page:
> 
> USER LOCKING
> 	 The pw utility supports a simple _password_ locking mechanism for
> 	 users; it works by prepending the string `*LOCKED*' to the
> 	 beginning of the password field in master.passwd to prevent
> 	 successful authentication.
> 
> Please note word _password_. There is nothing about locking _account_
> completely.

I believe account locking was implied in the days pw(8) was written.

> Please consider reviewing this PR and, hopefully, back out this commit.
> At least for a lot of people - it is POLA violation.

Just run adduser(8) and see how it implements account locking and
password auth disabling.  That's the system policy my change is in
keeping with.

> >     Mention all account management criteria in the manpage.
> >   
> >   PR:             bin/71147  http://www.FreeBSD.org/cgi/query-pr.cgi?pr=71147
> >   
> >   Revision  Changes    Path
> >   1.11.2.2  +16 -3     src/lib/libpam/modules/pam_unix/pam_unix.8
> >   1.51.2.1  +6 -0      src/lib/libpam/modules/pam_unix/pam_unix.c
> 
> -- 
> NEVE-RIPE, will build world for food
> Ukrainian FreeBSD User Group
> http://uafug.org.ua/

-- 
Yar