Observed on Redhat and Solaris. When openssh is configured without PAM, an account that is locked (via passwd -l) can still be logged into via public-key authentication. Although the password field is modified (to "*LK*" on Solaris or with a leading "!" on Redhat), allowed_user() does not test for those so if password authentication isn't used, the login still succeeds.
Created attachment 181 [details] Test for locked account in allowed_user() Tested on Redhat 8 and Solaris 8.
Created attachment 183 [details] Make check for locked pw not depend on HAS_SHADOW_EXPIRE
Applied - thanks.
I have reverted the patch, until we can work out a) if we really want it and b) how best to implement it.
Created attachment 237 [details] Check for locked accounts, as specified by configure. Patch take 2. Regarding Damien's a) * the man pages (mostly) refer to "passwd -l" as "locking the account" * we respect locked accounts on some platforms (eg AIX, Tru64 w/SIA, PAM?) * ssh-1.2.33 does "*LK*" checking so there is some precedent Regarding b) see patch. It moves most of the platform-dependant stuff to configure.
Created attachment 239 [details] Fix PAM case and add configure defs for IRIX
Created attachment 249 [details] Update patch to current to fix conflicts
Further info: it appears that in later patch sets, Solaris 8 and 9 now check the password string against *LK* in PAM and deny access even for non-password authentications (eg rhosts). http://groups.google.com/groups?as_umsgid=3ebcff1a%240%2449101%24e4fe514c%40news .xs4all.nl http://groups.google.com/groups?as_umsgid=3ebd1d6e%240%2449115%24e4fe514c%40news .xs4all.nl The last patch is probably broken now, I'm trying to find out if it's worth updating. What's the feeling on a) whether we want to do this and b) if the implementation is acceptable.
Created attachment 368 [details] Check for locked accounts: update patch to -current. Should the test be in a separate function, eg "int check_locked(char *passwd)"? If none of the account checks are defined, the if block is dead code. Is it worth adding "#if defined(LOCKED_PASSWD_STRING) || defined(...."?
Comment on attachment 368 [details] Check for locked accounts: update patch to -current. >-#if defined(HAVE_SHADOW_H) && !defined(DISABLE_SHADOW) && \ >- defined(HAS_SHADOW_EXPIRE) >+#if defined(HAVE_SHADOW_H) && !defined(DISABLE_SHADOW) >+ if (!options.use_pam) >+ spw = getspnam(pw->pw_name); >+#if !defined(USE_PAM) && defined(HAS_SHADOW_EXPIRE) The !USE_PAM should be removed and replaced with a !options.use_pam. The behaviour for UsePAM=no should be the same as being configured --without-pam.
Created attachment 370 [details] Fix options.use_pam and log -> logit. Whoops, missed that (and a log -> logit). Both fixed.
Comment on attachment 370 [details] Fix options.use_pam and log -> logit. ok djm@
Applied, thanks.
Mass change of RESOLVED bugs to CLOSED