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

login
register
mail settings
Submitter Koen Kooi
Date Oct. 11, 2013, 1:37 p.m.
Message ID <1381498665-21514-2-git-send-email-koen@dominion.thruhere.net>
Download mbox | patch
Permalink /patch/59655/
State New
Headers show

Comments

Koen Kooi - Oct. 11, 2013, 1:37 p.m.
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(-)
Richard Purdie - Oct. 12, 2013, 8:37 a.m.
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?

Cheers,

Richard
Koen Kooi - Oct. 13, 2013, 10:01 a.m.
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.

regards,

Koen
Richard Purdie - Oct. 13, 2013, 1:39 p.m.
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?

Cheers,

Richard
Koen Kooi - Oct. 13, 2013, 3:30 p.m.
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'.

regards,

Koen
Richard Purdie - Oct. 14, 2013, 8:25 a.m.
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. I think the
commit message needs to clearly say that as its a fairly serious
security change for both cases.

I'm not actually sure this makes sense as a default and it may be better
off being configurable, defaulting to off...

Cheers,

Richard
Koen Kooi - Oct. 14, 2013, 8:51 a.m.
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 :)

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

regards,

Koen
Richard Purdie - Oct. 14, 2013, 9:32 a.m.
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

Patch

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