Patchwork openssh: allow root login when debug-tweaks is enabled

login
register
mail settings
Submitter Saul Wold
Date Sept. 7, 2012, 6:17 p.m.
Message ID <1347041849-1559-1-git-send-email-sgw@linux.intel.com>
Download mbox | patch
Permalink /patch/36139/
State New
Headers show

Comments

Saul Wold - Sept. 7, 2012, 6:17 p.m.
This allows root to login over ssh with an empty password just like
dropbear when the debug-tweaks are enabled, it's important to disable
debug-tweaks for a production system as this will leave open a security
hole!

Thanks to Marc for the settings.
Cc: Marc Ferland <marc.ferland@gmail.com>

[Yocto #3078]

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 meta/recipes-connectivity/openssh/openssh_6.0p1.bb |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Khem Raj - Sept. 7, 2012, 8:52 p.m.
On Fri, Sep 7, 2012 at 11:17 AM, Saul Wold <sgw@linux.intel.com> wrote:
> +       for i in ${IMAGE_FEATURES};
> +       do
> +               if [ ${i} = "debug-tweaks" ]; then
> +                       sed -i -e "s/^#PermitRootLogin/PermitRootLogin/" ${D}${sysconfdir}/ssh/sshd_config
> +                       sed -i -e "s/^#PermitEmptyPasswords no/PermitEmptyPasswords yes/" ${D}${sysconfdir}/ssh/sshd_config
> +               fi
> +       done

instead of looping could you apply same logic as in dropbear to do this thing ?
Saul Wold - Sept. 7, 2012, 9:04 p.m.
On 09/07/2012 01:52 PM, Khem Raj wrote:
> On Fri, Sep 7, 2012 at 11:17 AM, Saul Wold <sgw@linux.intel.com> wrote:
>> +       for i in ${IMAGE_FEATURES};
>> +       do
>> +               if [ ${i} = "debug-tweaks" ]; then
>> +                       sed -i -e "s/^#PermitRootLogin/PermitRootLogin/" ${D}${sysconfdir}/ssh/sshd_config
>> +                       sed -i -e "s/^#PermitEmptyPasswords no/PermitEmptyPasswords yes/" ${D}${sysconfdir}/ssh/sshd_config
>> +               fi
>> +       done
>
> instead of looping could you apply same logic as in dropbear to do this thing ?
>
>
dropbear seems to do it as a patch, not in the do_install_append, also 
looking at dropbear it may be possible to simplify that it seems like 
it's a 2 step process that could be done in one step.  I think 
DISTRO_TYPE is only used in dropbear.

I guess its possible to export a variable that can be read by the 
install_append(), I will give it a test.

Sau!
Phil Blundell - Sept. 7, 2012, 9:09 p.m.
On Fri, 2012-09-07 at 11:17 -0700, Saul Wold wrote:
> +	for i in ${IMAGE_FEATURES};
> +	do
> +		if [ ${i} = "debug-tweaks" ]; then

Using ${IMAGE_FEATURES} in a package postinst is pretty unwholesome.  If
you do:

$ IMAGE_FEATURES=debug-tweaks bitbake openssh
$ sleep $[21*86400]
$ IMAGE_FEATURES=no-debug-tweaks bitbake my-production-image

then there is a significant risk that you will accidentally get the
insecure openssh in your image without realising it.

I know dropbear does this sort of thing already but we should really try
not to add more of that stuff.  If it's an IMAGE_FEATURE then it should
really be getting handled at the rootfs level, not burned into the
packages in the feed.

p.
Khem Raj - Sept. 7, 2012, 10:49 p.m.
On Fri, Sep 7, 2012 at 2:09 PM, Phil Blundell <philb@gnu.org> wrote:
>
> I know dropbear does this sort of thing already but we should really try
> not to add more of that stuff.  If it's an IMAGE_FEATURE then it should
> really be getting handled at the rootfs level, not burned into the
> packages in the feed.

infact yes thats a good approach. I have something similar locally
implemented as ROOTFS_POSTPROCESS_COMMAND
Paul Eggleton - Sept. 7, 2012, 11:56 p.m.
On Friday 07 September 2012 11:17:29 Saul Wold wrote:
> This allows root to login over ssh with an empty password just like
> dropbear when the debug-tweaks are enabled, it's important to disable
> debug-tweaks for a production system as this will leave open a security
> hole!
> 
> Thanks to Marc for the settings.
> Cc: Marc Ferland <marc.ferland@gmail.com>
> 
> [Yocto #3078]
> 
> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
>  meta/recipes-connectivity/openssh/openssh_6.0p1.bb |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
> b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb index 31202d4..fcd082c
> 100644
> --- a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
> +++ b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
> @@ -7,7 +7,7 @@ SECTION = "console/network"
>  LICENSE = "BSD"
>  LIC_FILES_CHKSUM = "file://LICENCE;md5=e326045657e842541d3f35aada442507"
> 
> -PR = "r3"
> +PR = "r4"
> 
>  DEPENDS = "zlib openssl"
>  DEPENDS += "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)}"
> @@ -75,6 +75,13 @@ do_install_append () {
>  			install -m 0755 ${WORKDIR}/sshd ${D}${sysconfdir}/pam.d/sshd
>  		fi
>  	done
> +	for i in ${IMAGE_FEATURES};
> +	do
> +		if [ ${i} = "debug-tweaks" ]; then
> +			sed -i -e "s/^#PermitRootLogin/PermitRootLogin/"
> ${D}${sysconfdir}/ssh/sshd_config +			sed -i -e "s/^#PermitEmptyPasswords
> no/PermitEmptyPasswords yes/" ${D}${sysconfdir}/ssh/sshd_config +		fi
> +	done
>  	install -d ${D}${sysconfdir}/init.d
>  	install -m 0755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/sshd
>  	rm -f ${D}${bindir}/slogin ${D}${datadir}/Ssh.bin

I'm a bit confused by this because I thought this issue had already been 
solved. Unfortunately when I looked back I see the patch was never merged:

http://patches.openembedded.org/patch/29693/

I agree with Phil, we really don't want to replicate dropbear's usage of 
IMAGE_FEATURES outside of image handling code - in fact there is a bug in the 
Yocto Project bugzilla (#2578) against me to remove this for dropbear.

Cheers,
Paul
Saul Wold - Sept. 8, 2012, 12:03 a.m.
On 09/07/2012 04:56 PM, Paul Eggleton wrote:
> On Friday 07 September 2012 11:17:29 Saul Wold wrote:
>> This allows root to login over ssh with an empty password just like
>> dropbear when the debug-tweaks are enabled, it's important to disable
>> debug-tweaks for a production system as this will leave open a security
>> hole!
>>
>> Thanks to Marc for the settings.
>> Cc: Marc Ferland <marc.ferland@gmail.com>
>>
>> [Yocto #3078]
>>
>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>> ---
>>   meta/recipes-connectivity/openssh/openssh_6.0p1.bb |    9 ++++++++-
>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
>> b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb index 31202d4..fcd082c
>> 100644
>> --- a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
>> +++ b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
>> @@ -7,7 +7,7 @@ SECTION = "console/network"
>>   LICENSE = "BSD"
>>   LIC_FILES_CHKSUM = "file://LICENCE;md5=e326045657e842541d3f35aada442507"
>>
>> -PR = "r3"
>> +PR = "r4"
>>
>>   DEPENDS = "zlib openssl"
>>   DEPENDS += "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)}"
>> @@ -75,6 +75,13 @@ do_install_append () {
>>   			install -m 0755 ${WORKDIR}/sshd ${D}${sysconfdir}/pam.d/sshd
>>   		fi
>>   	done
>> +	for i in ${IMAGE_FEATURES};
>> +	do
>> +		if [ ${i} = "debug-tweaks" ]; then
>> +			sed -i -e "s/^#PermitRootLogin/PermitRootLogin/"
>> ${D}${sysconfdir}/ssh/sshd_config +			sed -i -e "s/^#PermitEmptyPasswords
>> no/PermitEmptyPasswords yes/" ${D}${sysconfdir}/ssh/sshd_config +		fi
>> +	done
>>   	install -d ${D}${sysconfdir}/init.d
>>   	install -m 0755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/sshd
>>   	rm -f ${D}${bindir}/slogin ${D}${datadir}/Ssh.bin
>
> I'm a bit confused by this because I thought this issue had already been
> solved. Unfortunately when I looked back I see the patch was never merged:
>
> http://patches.openembedded.org/patch/29693/
>
It was merged, I just augmented that allow_empty_passwd() with another 1 
line sed to PermitRootLogin also.

> I agree with Phil, we really don't want to replicate dropbear's usage of
> IMAGE_FEATURES outside of image handling code - in fact there is a bug in the
> Yocto Project bugzilla (#2578) against me to remove this for dropbear.
>
Paul, I can have a crack at it if you want.

Sau!

> Cheers,
> Paul
>
Paul Eggleton - Sept. 8, 2012, 12:13 a.m.
On Friday 07 September 2012 17:03:44 Saul Wold wrote:
> On 09/07/2012 04:56 PM, Paul Eggleton wrote:
> > I'm a bit confused by this because I thought this issue had already been
> > solved. Unfortunately when I looked back I see the patch was never merged:
> > 
> > http://patches.openembedded.org/patch/29693/
> 
> It was merged, I just augmented that allow_empty_passwd() with another 1
> line sed to PermitRootLogin also.

Ah yes, now that I search for the correct string I see it was merged.

> > I agree with Phil, we really don't want to replicate dropbear's usage of
> > IMAGE_FEATURES outside of image handling code - in fact there is a bug in
> > the Yocto Project bugzilla (#2578) against me to remove this for
> > dropbear.
>
> Paul, I can have a crack at it if you want.

You are welcome to, but FYI it does involve modifications to dropbear itself as 
it does not have a runtime configuration option for this.

Cheers,
Paul

Patch

diff --git a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
index 31202d4..fcd082c 100644
--- a/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_6.0p1.bb
@@ -7,7 +7,7 @@  SECTION = "console/network"
 LICENSE = "BSD"
 LIC_FILES_CHKSUM = "file://LICENCE;md5=e326045657e842541d3f35aada442507"
 
-PR = "r3"
+PR = "r4"
 
 DEPENDS = "zlib openssl"
 DEPENDS += "${@base_contains('DISTRO_FEATURES', 'pam', 'libpam', '', d)}"
@@ -75,6 +75,13 @@  do_install_append () {
 			install -m 0755 ${WORKDIR}/sshd ${D}${sysconfdir}/pam.d/sshd
 		fi
 	done
+	for i in ${IMAGE_FEATURES};
+	do
+		if [ ${i} = "debug-tweaks" ]; then
+			sed -i -e "s/^#PermitRootLogin/PermitRootLogin/" ${D}${sysconfdir}/ssh/sshd_config
+			sed -i -e "s/^#PermitEmptyPasswords no/PermitEmptyPasswords yes/" ${D}${sysconfdir}/ssh/sshd_config
+		fi
+	done
 	install -d ${D}${sysconfdir}/init.d
 	install -m 0755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/sshd
 	rm -f ${D}${bindir}/slogin ${D}${datadir}/Ssh.bin