Re: [PATCH 2/2] openssh: allow empty passwords if PAM allows it as well

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2013-10-14 at 10:51 +0200, Koen Kooi wrote:
> Op 14 okt. 2013, om 10:25 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
> 
> > On Sun, 2013-10-13 at 17:30 +0200, Koen Kooi wrote:
> >> Op 13 okt. 2013, om 15:39 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
> >> 
> >>> On Sun, 2013-10-13 at 12:01 +0200, Koen Kooi wrote:
> >>>> Op 12 okt. 2013, om 10:37 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
> >>>> 
> >>>>> On Fri, 2013-10-11 at 15:37 +0200, Koen Kooi wrote:
> >>>>>> Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
> >>>>>> ---
> >>>>>> meta/recipes-connectivity/openssh/openssh-6.2p2/sshd_config | 2 +-
> >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>> 
> >>>>>> diff --git a/meta/recipes-connectivity/openssh/openssh-6.2p2/sshd_config b/meta/recipes-connectivity/openssh/openssh-6.2p2/sshd_config
> >>>>>> index 4f9b626..175e8f3 100644
> >>>>>> --- a/meta/recipes-connectivity/openssh/openssh-6.2p2/sshd_config
> >>>>>> +++ b/meta/recipes-connectivity/openssh/openssh-6.2p2/sshd_config
> >>>>>> @@ -59,7 +59,7 @@ Protocol 2
> >>>>>> 
> >>>>>> # To disable tunneled clear text passwords, change to no here!
> >>>>>> #PasswordAuthentication yes
> >>>>>> -#PermitEmptyPasswords no
> >>>>>> +PermitEmptyPasswords yes
> >>>>>> 
> >>>>>> # Change to no to disable s/key passwords
> >>>>>> #ChallengeResponseAuthentication yes
> >>>>> 
> >>>>> I'm struggling to connect the "if PAM allows it as well" part of the
> >>>>> shortlog to this change? How is this conditional on PAM?
> >>>> 
> >>>> If PAM disallows empty passwords this option doesn't do anything. The
> >>>> PAM rules run before the openssh config options get applied.
> >>> 
> >>> What if PAM isn't being used?
> >> 
> >> I haven't tested that, but I suspect it will only allow empty passwords if you set it to 'yes'.
> > 
> > Let me put this a different way. I think this commit allows empty
> > passwords for users both using PAM and those who are not.
> 
> Right
> 
> > I think the
> > commit message needs to clearly say that as its a fairly serious
> > security change for both cases.
> 
> Right again.
> 
> > I'm not actually sure this makes sense as a default and it may be better
> > off being configurable, defaulting to off...
> 
> Allowing passwordless (well, null passwords to be exact) logins is the
> current default for both PAM and dropbear, openssh is the odd one out.
> I don't really care what the default should be, just that all 3 should
> use the same :)

Agreed, and I wish you'd said that in the original commit message as it
does make a difference ;-).

> So should I resubmit this patch with an amended commit message or
> rework it and change the defaults in PAM and dropbear as well?

I'd resend with a revised commit message including the above
justification. I'm not guaranteeing I'll take it, I'm hoping we'll get
some further discussion on the subject.

I think ultimately we need to have a config option for this which pam,
dropbear and openssh all honour. Could you add an enhancement request to
the Yocto bugzilla to that end please? That way people can clearly opt
in to specific configurations. I think its important to get security
details like this right.

Cheers,

Richard