Bug 442

Summary:

sshd allows login via public-key when account locked

Product:

Portable OpenSSH

Reporter:

Darren Tucker <dtucker>

Component:

sshd

Assignee:

OpenSSH Bugzilla mailing list <openssh-bugs>

Status:

CLOSED FIXED

Severity:

security

Keywords:

patch

Priority:

Version:

-current

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

627

Attachments:

Description	Flags
Test for locked account in allowed_user()	none
Make check for locked pw not depend on HAS_SHADOW_EXPIRE	none
Check for locked accounts, as specified by configure.	none
Fix PAM case and add configure defs for IRIX	none
Update patch to current to fix conflicts	none
Check for locked accounts: update patch to -current.	none
Fix options.use_pam and log -> logit.	djm: ok+

Description Darren Tucker 2002-11-24 14:23:22 AEDT

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.

Comment 1 Darren Tucker 2002-11-24 14:25:11 AEDT

Created attachment 181 [details]
Test for locked account in allowed_user()

Tested on Redhat 8 and Solaris 8.

Comment 2 Darren Tucker 2002-11-24 17:53:23 AEDT

Created attachment 183 [details]
Make check for locked pw not depend on HAS_SHADOW_EXPIRE

Comment 3 Damien Miller 2003-01-07 12:19:41 AEDT

Applied - thanks.

Comment 4 Damien Miller 2003-01-18 16:25:09 AEDT

I have reverted the patch, until we can work out a) if we really want it and b)
how best to implement it.

Comment 5 Darren Tucker 2003-02-23 22:53:18 AEDT

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.

Comment 6 Darren Tucker 2003-02-24 19:23:05 AEDT

Created attachment 239 [details]
Fix PAM case and add configure defs for IRIX

Comment 7 Darren Tucker 2003-03-14 20:47:13 AEDT

Created attachment 249 [details]
Update patch to current to fix conflicts

Comment 8 Darren Tucker 2003-05-11 12:07:11 AEST

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.

Comment 9 Darren Tucker 2003-08-24 11:39:45 AEST

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 10 Damien Miller 2003-08-25 10:13:51 AEST

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.

Comment 11 Darren Tucker 2003-08-25 10:37:40 AEST

Created attachment 370 [details]
Fix options.use_pam and log -> logit.

Whoops, missed that (and a log -> logit).  Both fixed.

Comment 12 Damien Miller 2003-08-25 11:29:27 AEST

Comment on attachment 370 [details]
Fix options.use_pam and log -> logit.

ok djm@

Comment 13 Darren Tucker 2003-08-25 11:52:19 AEST

Applied, thanks.

Comment 14 Damien Miller 2004-04-14 12:24:18 AEST

Mass change of RESOLVED bugs to CLOSED