Bug 1083

Created attachment 955 [details]
Fix for denying login to locked account through public key

The attached patch fixes the problem in those HP-UX systems which lack shadow
passwords. This patch denies login to the account irrespective of whether
shadow password feature is installed or not during system updates.

Comment on attachment 955 [details]
Fix for denying login to locked account through public key

Seems reasonable to me, anyone else see any potential side effects?  Especially
with the HAVE_LIBIAF stuff that was recently added?

(In reply to comment #2)
> (From update of attachment 955 [details] [edit])
> Seems reasonable to me, anyone else see any potential side effects?  Especially
> with the HAVE_LIBIAF stuff that was recently added?
> 

I don't see any problem with this in reguards to the HAVE_LIBIAF bits.

But I fail to see the need for the change. On HP-UX systems that lack shadow
passwords, I would expect USE_SHADOW to not be defned so it'll hit the else
where passwd = pw->pw_passwd;

Or are we running into a situation where the system has shadow.h but doesn't
use shadow passowrds and ssh was configured without using the --without-shadow
option?

From comment #3,
>Or are we running into a situation where the system has shadow.h but doesn't
>use shadow passowrds and ssh was configured without using the --without-shadow
>option?

Yes, youre right. 

Shadow passwords are a run-time installable feature on HP-UX (11.00 and 11.11 I
think) but some of the infrastructure such as the getspnam() call is present on
at least some versions without the feature.

So I guess the question is: can it get into this situation with a normal HP-UX
configuration (as opposed to a misconfigured system with mismatched
includes/runtime)?

Also, a slightly simpler patch would be to put the "passwd = pw->pw_passwd;"
without the "if" before all of the #ifdefs.  This should have the same behaviour
as the proposed patch.

(In reply to comment  #5)
> can it get into this situation with a normal HP-UX configuration?
I assume this means a configuration with shadow feature installed. With this, 
spw will not be set to NULL unless getspnam fails for the user(bogus user). In 
11.11, for a misconfigured system spw will be NULL and thus passwd also NULL. 

So I think this wont affect the Normal configuration and also my testing says it 
worked well for systems with normal configuration and problem happens only with 
misconfigured systems.

>Also, a slightly simpler patch would be to put the "passwd = pw->pw_passwd;"
>without the "if" before all of the #ifdefs.

Yes, it will have same effect. One more thing, can we move that like below,

const char *hostname = NULL, *ipaddr = NULL, *passwd = pw->pw_passwd;

In comment  #6 
> const char *hostname = NULL, *ipaddr = NULL, *passwd = pw->pw_passwd;

As there is a check for pw as given below,
/* Shouldn't be called if pw is NULL, but better safe than sorry... */

I think we can do the passwd = pw->pw_passwd assignment after this. This occurs 
after the first #ifdef which is for shadow structure.

Created attachment 982 [details]
Patch to honour locked accounts for key based auth if shadow is not enabled.

The patch is updated based on comments #5 and  #7. I tested this by
installing/uninstalling shadow feature at run time.

Put this on my to-do list for 4.4.

Created attachment 1148 [details]
Patch 982+removed #else part.

Setting passwd first would also let us slightly untangle the ifdef block.

could this use openbsd-compat/xcrypt.c:shadow_pw() to fetch the password? it already does this...

(In reply to comment #11)
> could this use openbsd-compat/xcrypt.c:shadow_pw() to fetch the
> password? it already does this...

Yeah that looks like a good idea.  I'll give it a spin.

(In reply to comment #11)
> could this use openbsd-compat/xcrypt.c:shadow_pw() to fetch the
> password? it already does this...

OK I finally looked and that won't work because shadow_pw() returns just the passwd string and the shadow password checks need other members of the shadow struct.

Created attachment 1542 [details]
revised patch

attachment #1148 [details] had conflicts, revised to -current. IMO this is OK to go in.

Patch applied - thanks!

Mass update RESOLVED->CLOSED after release of openssh-5.1