Patchwork [2/3] image.bbclass: remove zap_root_password

login
register
mail settings
Submitter Qi.Chen@windriver.com
Date Dec. 10, 2013, 9:58 a.m.
Message ID <51a89b7ff6efb35278daf3373040434a9673dba8.1386669285.git.Qi.Chen@windriver.com>
Download mbox | patch
Permalink /patch/63121/
State New
Headers show

Comments

Qi.Chen@windriver.com - Dec. 10, 2013, 9:58 a.m.
From: Chen Qi <Qi.Chen@windriver.com>

This function replaces the root password with '*' if 'debug-tweaks'
is not in IMAGE_FEATURES. As a result, if we don't have 'debug-tweaks',
we would be locked out of the system. That means, if the user uses a
bbappend file for base-passwd to set the root password, he would not be
able to login as root; if the user uses 'EXTRA_USERS_PARAMS' to set
the root password, he would still not be able to login as root.

In a word, this function should be removed to make things work correctly.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/core-image.bbclass |    3 ---
 meta/classes/image.bbclass      |    9 +--------
 2 files changed, 1 insertion(+), 11 deletions(-)
Paul Eggleton - Dec. 10, 2013, 12:15 p.m.
Hi Qi,

On Tuesday 10 December 2013 17:58:51 Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> This function replaces the root password with '*' if 'debug-tweaks'
> is not in IMAGE_FEATURES. As a result, if we don't have 'debug-tweaks',
> we would be locked out of the system. That means, if the user uses a
> bbappend file for base-passwd to set the root password, he would not be
> able to login as root; if the user uses 'EXTRA_USERS_PARAMS' to set
> the root password, he would still not be able to login as root.
> 
> In a word, this function should be removed to make things work correctly.

Er, unless I'm missing something about what you're adding in the other patch, 
you *cannot* simply remove this. The intentional design of the existing code 
is that having "debug-tweaks" in IMAGE_FEATURES means that you can log in as 
root with no password; but most importantly if "debug-tweaks" is not present 
you cannot log in at all as root (in the absence of anything that sets the 
root password, of course). Any changes must preserve this behaviour.

Cheers,
Paul
Mark Hatle - Dec. 10, 2013, 3:36 p.m.
On 12/10/13, 6:15 AM, Paul Eggleton wrote:
> Hi Qi,
>
> On Tuesday 10 December 2013 17:58:51 Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> This function replaces the root password with '*' if 'debug-tweaks'
>> is not in IMAGE_FEATURES. As a result, if we don't have 'debug-tweaks',
>> we would be locked out of the system. That means, if the user uses a
>> bbappend file for base-passwd to set the root password, he would not be
>> able to login as root; if the user uses 'EXTRA_USERS_PARAMS' to set
>> the root password, he would still not be able to login as root.
>>
>> In a word, this function should be removed to make things work correctly.
>
> Er, unless I'm missing something about what you're adding in the other patch,
> you *cannot* simply remove this. The intentional design of the existing code
> is that having "debug-tweaks" in IMAGE_FEATURES means that you can log in as
> root with no password; but most importantly if "debug-tweaks" is not present
> you cannot log in at all as root (in the absence of anything that sets the
> root password, of course). Any changes must preserve this behaviour.

I agree.  The default behavior on most systems should be absolutely no way to 
directly login as root.  Instead logins should occur based on a non-privileged 
user.  (The other patches in that set look good to me.)

--Mark

> Cheers,
> Paul
>
Qi.Chen@windriver.com - Dec. 11, 2013, 3:19 a.m.
On 12/10/2013 11:36 PM, Mark Hatle wrote:
> On 12/10/13, 6:15 AM, Paul Eggleton wrote:
>> Hi Qi,
>>
>> On Tuesday 10 December 2013 17:58:51 Qi.Chen@windriver.com wrote:
>>> From: Chen Qi <Qi.Chen@windriver.com>
>>>
>>> This function replaces the root password with '*' if 'debug-tweaks'
>>> is not in IMAGE_FEATURES. As a result, if we don't have 'debug-tweaks',
>>> we would be locked out of the system. That means, if the user uses a
>>> bbappend file for base-passwd to set the root password, he would not be
>>> able to login as root; if the user uses 'EXTRA_USERS_PARAMS' to set
>>> the root password, he would still not be able to login as root.
>>>
>>> In a word, this function should be removed to make things work 
>>> correctly.
>>
>> Er, unless I'm missing something about what you're adding in the 
>> other patch,
>> you *cannot* simply remove this. The intentional design of the 
>> existing code
>> is that having "debug-tweaks" in IMAGE_FEATURES means that you can 
>> log in as
>> root with no password; but most importantly if "debug-tweaks" is not 
>> present
>> you cannot log in at all as root (in the absence of anything that 
>> sets the
>> root password, of course). Any changes must preserve this behaviour.
>
> I agree.  The default behavior on most systems should be absolutely no 
> way to directly login as root.  Instead logins should occur based on a 
> non-privileged user.  (The other patches in that set look good to me.)
>
> --Mark
>
>> Cheers,
>> Paul
>>
>

Mark & Paul,

Thanks for your explanation.

I think what we really want is to disallow *empty* root password if 
'debug-tweaks' is not in IMAGE_FEATRUES. And if the root password has 
already been set (via bbappend file or via EXTRA_USERS_PARAMS), we 
should not zap that password. Maybe the function should be 
zap_empty_root_password?

What do you think?

Best Regards,
Chen Qi

> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>

Patch

diff --git a/meta/classes/core-image.bbclass b/meta/classes/core-image.bbclass
index e7c34e2..5ee0d6d 100644
--- a/meta/classes/core-image.bbclass
+++ b/meta/classes/core-image.bbclass
@@ -73,8 +73,5 @@  inherit image
 # Create /etc/timestamp during image construction to give a reasonably sane default time setting
 ROOTFS_POSTPROCESS_COMMAND += "rootfs_update_timestamp ; "
 
-# Zap the root password if debug-tweaks feature is not enabled
-ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "", "zap_root_password ; ",d)}'
-
 # Tweak the mount options for rootfs in /etc/fstab if read-only-rootfs is enabled
 ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "read-only-rootfs", "read_only_rootfs_hook; ", "",d)}'
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 168f283..a5ef244 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -562,13 +562,6 @@  rootfs_uninstall_unneeded () {
 	fi
 }
 
-# set '*' as the root password so the images
-# can decide if they want it or not
-zap_root_password () {
-	sed 's%^root:[^:]*:%root:*:%' < ${IMAGE_ROOTFS}/etc/passwd >${IMAGE_ROOTFS}/etc/passwd.new
-	mv ${IMAGE_ROOTFS}/etc/passwd.new ${IMAGE_ROOTFS}/etc/passwd
-} 
-
 # allow dropbear/openssh to accept root logins and logins from accounts with an empty password string
 ssh_allow_empty_password () {
 	if [ -e ${IMAGE_ROOTFS}${sysconfdir}/ssh/sshd_config ]; then
@@ -648,7 +641,7 @@  rootfs_sysroot_relativelinks () {
 	sysroot-relativelinks.py ${SDK_OUTPUT}/${SDKTARGETSYSROOT}
 }
 
-EXPORT_FUNCTIONS zap_root_password remove_init_link do_rootfs make_zimage_symlink_relative set_image_autologin rootfs_update_timestamp rootfs_no_x_startup
+EXPORT_FUNCTIONS remove_init_link do_rootfs make_zimage_symlink_relative set_image_autologin rootfs_update_timestamp rootfs_no_x_startup
 
 do_fetch[noexec] = "1"
 do_unpack[noexec] = "1"