fix can't login when debug-tweaks is not in IMAGE_FEATURES

PATCH In review Robert Yang 8 participants 22 messages Nov 23, 2016 – Nov 30, 2016

Share & tools

Robert Yang

author

2016-11-23 07:49 UTC #0 lore raw

cover fix can't login when debug-tweaks is not in IMAGE_FEATURES

Fixed 2 bugs:
- Can't login as root when debug-tweaks/empty-root-password is not in IMAGE_FEATURES since no passwd.
- When set root passwd and debug-tweaks/empty-root-password is in IMAGE_FEATURES, passwd is *required* to login.

Filed https://bugzilla.yoctoproject.org/show_bug.cgi?id=10710, and marked doc changes required as yes.

// Robert

The following changes since commit a675b2c89e477af088faee9b3be96eae19a85f0b:

sanity.bbclass: fix logging of an error (2016-11-15 15:18:50 +0000)

are available in the git repository at:

git://git.openembedded.org/openembedded-core-contrib rbt/root
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/root

Robert Yang (2):
rootfs-postcommands.bbclass: fix zap_empty_root_password
base-passwd: set root's default password to 'root'

 meta/classes/rootfs-postcommands.bbclass                 |  8 ++++----
 .../base-passwd/base-passwd/passwd_master.patch          | 16 ++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb      |  1 +
 3 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 meta/recipes-core/base-passwd/base-passwd/passwd_master.patch

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-23 07:49 UTC #1 lore raw

PATCH 1/2 rootfs-postcommands.bbclass: fix zap_empty_root_password

[YOCTO #10710]

The previous logic works when root passwd is blank, but it doesn't work when there is a root passws, for example:
1) Set a root passwd in base-passwd
2) IMAGE_FEATURES += "debug-tweaks"
Start the target, it still requires passwd to login.

Empty passwd if debug-tweaks is in IMAGE_FEATURES, else do nothing can fix problem.

Signed-off-by Robert Yang <[email protected]>

---
 meta/classes/rootfs-postcommands.bbclass | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

meta/classes/rootfs-postcommands.bbclass +4 -4

index 0c7ceea..353dc4f 100644
@@ -1,6 +1,6 @@
 
 # Zap the root password if debug-tweaks feature is not enabled
-ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains_any("IMAGE_FEATURES", [ 'debug-tweaks', 'empty-root-password' ], "", "zap_empty_root_password ; ",d)}'
+ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains_any("IMAGE_FEATURES", [ 'debug-tweaks', 'empty-root-password' ], "empty_root_password ; ", "",d)}'
 
 # Allow dropbear/openssh to accept logins from accounts with an empty password string if debug-tweaks is enabled
 ROOTFS_POSTPROCESS_COMMAND += '${@bb.utils.contains_any("IMAGE_FEATURES", [ 'debug-tweaks', 'allow-empty-password' ], "ssh_allow_empty_password; ", "",d)}'

@@ -105,12 +105,12 @@ read_only_rootfs_hook () {
 #
 # This function is intended to disallow empty root password if 'debug-tweaks' is not in IMAGE_FEATURES.
 #
-zap_empty_root_password () {
+empty_root_password () {
 	if [ -e ${IMAGE_ROOTFS}/etc/shadow ]; then
-		sed -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/shadow
+		sed -i 's%^root:[^:]*:%root::%' ${IMAGE_ROOTFS}/etc/shadow
         fi
 	if [ -e ${IMAGE_ROOTFS}/etc/passwd ]; then
-		sed -i 's%^root::%root:*:%' ${IMAGE_ROOTFS}/etc/passwd
+		sed -i 's%^root:[^:]*:%root::%' ${IMAGE_ROOTFS}/etc/passwd
 	fi
 }

Signature · 1 line

2.10.2

Robert Yang

author

2016-11-23 07:49 UTC #2 lore raw

PATCH 2/2 base-passwd: set root's default password to 'root'

In reply to Robert Yang

[YOCTO #10710]

Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.

Signed-off-by Robert Yang <[email protected]>

---
 .../base-passwd/base-passwd/passwd_master.patch          | 16 ++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb      |  1 +
 2 files changed, 17 insertions(+)
 create mode 100644 meta/recipes-core/base-passwd/base-passwd/passwd_master.patch

meta/recipes-core/base-passwd/base-passwd/passwd_master.patch new file +16 -0

new file mode 100644
index 0000000..909c2a4
@@ -0,0 +1,16 @@
+Set root's default password to 'root'
+
+Upstream-Status: Inappropriate [OE config specific]
+
+Signed-off-by: Robert Yang <[email protected]>
+
+diff --git a/passwd.master b/passwd.master
+index 82df9e5..c29e76d 100644
+--- a/passwd.master
++++ b/passwd.master
+@@ -1,4 +1,4 @@
+-root::0:0:root:/root:/bin/sh
++root:6UZSjeWUui3JQ:0:0:root:/root:/bin/sh
+ daemon:*:1:1:daemon:/usr/sbin:/bin/sh
+ bin:*:2:2:bin:/bin:/bin/sh
+ sys:*:3:3:sys:/dev:/bin/sh

meta/recipes-core/base-passwd/base-passwd_3.5.29.bb +1 -0

index 10457b2..a3477a1 100644
@@ -12,6 +12,7 @@ SRC_URI = "https://launchpad.net/debian/+archive/primary/+files/${BPN}_${PV}.tar
            file://noshadow.patch \
            file://input.patch \
            file://disable-docs.patch \
+           file://passwd_master.patch \
           "
 
 SRC_URI[md5sum] = "6beccac48083fe8ae5048acd062e5421"

Signature · 1 line

2.10.2

Patrick Ohly

2016-11-23 11:16 UTC #3 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 5 lines · Robert Yang

[YOCTO #10710]
Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.

Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?

That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.

So if you think that having a root password set (instead of empty), then at least make it an opt-in behavior that explicitly has to be selected. Make it an image feature so that images with and without default password can be build in the same build configuration. Changing base-passwd doesn't achieve that.

Even then I'm still wondering what the benefit of a well-known password compared to no password is. Both are equally insecure, so someone who wants to allow logins might as well go with "empty password".

Signature · 6 lines

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Burton, Ross

2016-11-23 14:17 UTC #4 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 8 lines · Patrick Ohly

Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?
That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.

Agreed.

Ross

Robert Yang

author

2016-11-24 02:02 UTC #5 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

In reply to Patrick Ohly

Quoted 21 lines · Patrick Ohly

Quoted 5 lines · Robert Yang
[YOCTO #10710]
Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.
Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?
That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.
So if you think that having a root password set (instead of empty), then at least make it an opt-in behavior that explicitly has to be selected. Make it an image feature so that images with and without default password can be build in the same build configuration. Changing base-passwd doesn't achieve that.

Quoted 3 lines

Even then I'm still wondering what the benefit of a well-known password compared to no password is. Both are equally insecure, so someone who wants to allow logins might as well go with "empty password".

The problem is that when debug-tweaks or empty-root-password is not in IMAGE_FEATURE, there is no way to login by default, which will surprise the user. How about:

1) Let user can set root passwd via a variable when building.

Or/And

2) Warn the user at build time when the image is unable to login.

// Robert

Quoted 1 line

Paul Eggleton

2016-11-24 03:18 UTC #6 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 30 lines · On Thu, 24 Nov 2016 10:01:59 Robert Yang wrote:

Quoted 21 lines · Patrick Ohly
Quoted 5 lines · Robert Yang
[YOCTO #10710]
Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.
Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?
That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.
So if you think that having a root password set (instead of empty), then at least make it an opt-in behavior that explicitly has to be selected. Make it an image feature so that images with and without default password can be build in the same build configuration. Changing base-passwd doesn't achieve that.
Quoted 3 lines
Even then I'm still wondering what the benefit of a well-known password compared to no password is. Both are equally insecure, so someone who wants to allow logins might as well go with "empty password".
The problem is that when debug-tweaks or empty-root-password is not in IMAGE_FEATURE, there is no way to login by default, which will surprise the user. How about:

Quoted 5 lines

1) Let user can set root passwd via a variable when building.
Or/And
2) Warn the user at build time when the image is unable to login.

There are problems with both of these:

1) I'm concerned that by making it trivially easy this will encourage users to set a root password and forget they have done so. This may lead to yet more products going out with default root passwords, and that is not a good thing.

2) Having no root password in this scenario is not necessarily a mistake, it may be intentional. If nobody ever needs to log into your device via a terminal, then why would you need a root password set at all? In that scenario you wouldn't want to be implying "this could be wrong, you should set a root password".

If we need more documentation around this so that people understand how this aspect works (and I don't doubt that we do, people do ask about it) then by all means we should improved the documentation.

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre

Robert Yang

author

2016-11-24 03:38 UTC #7 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 43 lines · Paul Eggleton

Quoted 30 lines · On Thu, 24 Nov 2016 10:01:59 Robert Yang wrote:
Quoted 21 lines · Patrick Ohly
Quoted 5 lines · Robert Yang
[YOCTO #10710]
Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.
Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?
That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.
So if you think that having a root password set (instead of empty), then at least make it an opt-in behavior that explicitly has to be selected. Make it an image feature so that images with and without default password can be build in the same build configuration. Changing base-passwd doesn't achieve that.
Quoted 3 lines
Even then I'm still wondering what the benefit of a well-known password compared to no password is. Both are equally insecure, so someone who wants to allow logins might as well go with "empty password".
The problem is that when debug-tweaks or empty-root-password is not in IMAGE_FEATURE, there is no way to login by default, which will surprise the user. How about:
Quoted 5 lines
1) Let user can set root passwd via a variable when building.
Or/And
2) Warn the user at build time when the image is unable to login.
There are problems with both of these:
1) I'm concerned that by making it trivially easy this will encourage users to set a root password and forget they have done so. This may lead to yet more products going out with default root passwords, and that is not a good thing.

Quoted 5 lines

2) Having no root password in this scenario is not necessarily a mistake, it may be intentional. If nobody ever needs to log into your device via a terminal, then why would you need a root password set at all? In that scenario you wouldn't want to be implying "this could be wrong, you should set a root password".

Hi Paul,

Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and there is no passwd, so that user can login easily without a passwd, I think that current status is more unsafe ? And when user realizes this, he wants to add a passwd, but sorry, there is no easy way.
The anaconda installer's (used by Redhat) kickstart file can easily sets a passwd, you can even set an un-encrypted password, are there many complains about that ? When people can get your device (hardware), it's hard to prevent people login you device.

// Robert

Quoted 8 lines

If we need more documentation around this so that people understand how this aspect works (and I don't doubt that we do, people do ask about it) then by all means we should improved the documentation.
Cheers,
Paul

Patrick Ohly

2016-11-24 07:46 UTC #8 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 3 lines · Robert Yang

Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and there is no passwd, so that user can login easily without a passwd, I think that current status is more unsafe ?

Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.

Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).

Signature · 6 lines

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Robert Yang

author

2016-11-24 08:27 UTC #9 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 9 lines · Patrick Ohly

Quoted 3 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and there is no passwd, so that user can login easily without a passwd, I think that current status is more unsafe ?
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.

I meant add an interface to let user can set their password here.

// Robert

Quoted 5 lines

Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).

Philip Balister

2016-11-24 14:09 UTC #10 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

In reply to Patrick Ohly

Quoted 5 lines · Patrick Ohly

Quoted 3 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and there is no passwd, so that user can login easily without a passwd, I think that current status is more unsafe ?

Quoted 9 lines

Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).

That is the first time I've heard Poky is targeting an audience assumed to be running in a secure environment. Should we document what Poky this somewhere? From where I sit, this seems to be an odd limitation.

Philip

Patrick Ohly

2016-11-24 14:54 UTC #11 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 11 lines · Philip Balister

Quoted 10 lines · Patrick Ohly
Quoted 3 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and there is no passwd, so that user can login easily without a passwd, I think that current status is more unsafe ?
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.

Quoted 7 lines

Quoted 4 lines
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).
That is the first time I've heard Poky is targeting an audience assumed to be running in a secure environment.

At least the default local.conf seems to be meant for that (easy-of-use for developers preferred over security in a hostile environment).

Quoted 2 lines

Should we document what Poky this
somewhere? From where I sit, this seems to be an odd limitation.

I'm not aware of a document explicitly documenting this either. I wouldn't call it a limitation, though: a real product could be built with a configuration that doesn't enable debug-tweaks.

As Paul said before, more documentation about first boot, login mechanisms, security considerations, etc. certainly would be useful.

Signature · 6 lines

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Paul Eggleton

2016-11-24 18:59 UTC #12 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

In reply to Patrick Ohly

Quoted 7 lines · On Thu, 24 Nov 2016 08:46:29 Patrick Ohly wrote:

Quoted 5 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and
there is no passwd, so that user can login easily without a passwd, I think
that current status is more unsafe ?

Quoted 8 lines

Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).

I don't think that's part of the design goals on either side, it's simply about making development easier. The feature is clearly labelled "debug- tweaks" because it's for debugging not for production. It could be that we should make it do other things like append a notice to /etc/issue to avoid people leaving it on for production, if that is a concern.

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre

Khem Raj

2016-11-29 01:57 UTC #13 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 20 lines · Paul Eggleton

Quoted 7 lines · On Thu, 24 Nov 2016 08:46:29 Patrick Ohly wrote:
Quoted 5 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and
there is no passwd, so that user can login easily without a passwd, I think
that current status is more unsafe ?
Quoted 8 lines
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).

Quoted 6 lines

I don't think that's part of the design goals on either side, it's simply about making development easier. The feature is clearly labelled "debug- tweaks" because it's for debugging not for production. It could be that we should make it do other things like append a notice to /etc/issue to avoid people leaving it on for production, if that is a concern.

Sometimes such goals can lead to problems. Making development easier by all means if you can ensure a hard error on production e.g. debug-tweaks can then never be part of production images. Otherwise someone will forget it and it will be discovered on millions of devices in field along with the user project will be red-faced.

Robert Yang

author

2016-11-29 02:46 UTC #14 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 27 lines · Khem Raj

Quoted 20 lines · Paul Eggleton
Quoted 7 lines · On Thu, 24 Nov 2016 08:46:29 Patrick Ohly wrote:
Quoted 5 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and
there is no passwd, so that user can login easily without a passwd, I think
that current status is more unsafe ?
Quoted 8 lines
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).
Quoted 6 lines
I don't think that's part of the design goals on either side, it's simply about making development easier. The feature is clearly labelled "debug- tweaks" because it's for debugging not for production. It could be that we should make it do other things like append a notice to /etc/issue to avoid people leaving it on for production, if that is a concern.

Quoted 5 lines

Sometimes such goals can lead to problems. Making development easier by all means if you can ensure a hard error on production e.g. debug-tweaks can then never be part of production images. Otherwise someone will forget it and it will be discovered on millions of devices in field along with the user project will be red-faced.

Will something like IMAGE_FEATURES += "production" help here ? We may also need something like IMAGE_FEATURES += "test" to make it can work with -ctestimage.

// Robert

Quoted 1 line

Paul Eggleton

2016-11-29 03:45 UTC #15 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 28 lines · On Tue, 29 Nov 2016 10:45:51 Robert Yang wrote:

Quoted 27 lines · Khem Raj
Quoted 26 lines
On Nov 24, 2016, at 10:59 AM, Paul Eggleton
<[email protected]> wrote:>>
Quoted 7 lines · On Thu, 24 Nov 2016 08:46:29 Patrick Ohly wrote:
Quoted 5 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and
there is no passwd, so that user can login easily without a passwd, I think
that current status is more unsafe ?
Quoted 8 lines
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).
I don't think that's part of the design goals on either side, it's simply about making development easier. The feature is clearly labelled "debug- tweaks" because it's for debugging not for production. It could be that we
should make it do other things like append a notice to /etc/issue to avoid
people leaving it on for production, if that is a concern.

Quoted 5 lines

Quoted 5 lines
Sometimes such goals can lead to problems. Making development easier by all means if you can ensure a hard error on production e.g. debug-tweaks can then never be part of production images. Otherwise someone will forget it and it will be discovered on millions of devices in field along with the user project will be red-faced.

Right. FWIW in mitigation I did write the raw material for the following section of the YP manuals, though I don't know how many people have ended up reading it:

http://www.yoctoproject.org/docs/current/dev-manual/dev-manual.html#making-images-more-secure

In there there is an explicit mention of disabling debug-tweaks. Looking around the place it could be that we need more warnings about this being on by default though.

Quoted 1 line

Will something like IMAGE_FEATURES += "production" help here ?

I'd like to see something like this - at least give the user some way of saying "I really am in production now, so error out on anything that I shouldn't be doing there". I wonder if it potentially goes further than just conflicting with things like debug-tweaks and empty-root-password.

Quoted 2 lines

We may also need something like IMAGE_FEATURES += "test" to make it can work with -ctestimage.

Not sure I follow your reasoning here - can you explain what this feature would do?

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre

Robert Yang

author

2016-11-29 05:37 UTC #16 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 52 lines · Paul Eggleton

Quoted 28 lines · On Tue, 29 Nov 2016 10:45:51 Robert Yang wrote:
Quoted 27 lines · Khem Raj
Quoted 26 lines
On Nov 24, 2016, at 10:59 AM, Paul Eggleton
<[email protected]> wrote:>>
Quoted 7 lines · On Thu, 24 Nov 2016 08:46:29 Patrick Ohly wrote:
Quoted 5 lines · Robert Yang
Currently, debug-tweaks is in EXTRA_IMAGE_FEATURES by default for poky, and
there is no passwd, so that user can login easily without a passwd, I think
that current status is more unsafe ?
Quoted 8 lines
Both well-known password and no password are unsafe. User "root" with password "root" is not even "more" safe already now, because tools that brute-force logins try that. Choosing something else would be a bit safer for a short while until the tools add it to their dictionary.
Poky is also targeting a different audience than OE-core. Poky can assume to be used in a secure environment, OE-core can't (because it might be used for all kinds of devices).
I don't think that's part of the design goals on either side, it's simply about making development easier. The feature is clearly labelled "debug- tweaks" because it's for debugging not for production. It could be that we
should make it do other things like append a notice to /etc/issue to avoid
people leaving it on for production, if that is a concern.
Quoted 5 lines
Quoted 5 lines
Sometimes such goals can lead to problems. Making development easier by all means if you can ensure a hard error on production e.g. debug-tweaks can then never be part of production images. Otherwise someone will forget it and it will be discovered on millions of devices in field along with the user project will be red-faced.
Right. FWIW in mitigation I did write the raw material for the following section of the YP manuals, though I don't know how many people have ended up reading it:
http://www.yoctoproject.org/docs/current/dev-manual/dev-manual.html#making-images-more-secure
In there there is an explicit mention of disabling debug-tweaks. Looking around the place it could be that we need more warnings about this being on by default though.
Quoted 1 line
Will something like IMAGE_FEATURES += "production" help here ?
I'd like to see something like this - at least give the user some way of saying "I really am in production now, so error out on anything that I shouldn't be doing there". I wonder if it potentially goes further than just conflicting with things like debug-tweaks and empty-root-password.

Quoted 5 lines

Quoted 2 lines
We may also need something like IMAGE_FEATURES += "test" to make it can work with -ctestimage.
Not sure I follow your reasoning here - can you explain what this feature would do?

For example, the "bitbake <image> -ctestimage" requires a few pkgs installed, such as psplash-default, see the testcase in meta/lib/oeqa/runtime/smart.py:

def test_smart_install(self):
self.smart('remove -y psplash-default')
self.smart('install -y psplash-default')

The test would fail without psplash-default installed, and also it requires sshd installed on the target. When IMAGE_FETURES += "test", we can install these required packages, I'm not sure this is a good idea, or maybe we can enhance testimge.bbclass to do it. Another way to fix the problem might be not hardcode the package name.

// Robert

Quoted 4 lines

Cheers,
Paul

Paul Eggleton

2016-11-29 06:27 UTC #17 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

Quoted 16 lines · On Tue, 29 Nov 2016 13:36:58 Robert Yang wrote:

Quoted 6 lines · Paul Eggleton
Quoted 2 lines · On Tue, 29 Nov 2016 10:45:51 Robert Yang wrote:
We may also need something like IMAGE_FEATURES += "test" to make it can work with -ctestimage.
Not sure I follow your reasoning here - can you explain what this feature would do?
For example, the "bitbake <image> -ctestimage" requires a few pkgs installed, such as psplash-default, see the testcase in meta/lib/oeqa/runtime/smart.py:
def test_smart_install(self):
self.smart('remove -y psplash-default')
self.smart('install -y psplash-default')

Quoted 5 lines

The test would fail without psplash-default installed, and also it requires sshd installed on the target. When IMAGE_FETURES += "test", we can install these required packages, I'm not sure this is a good idea, or maybe we can enhance testimge.bbclass to do it. Another way to fix the problem might be not hardcode the package name.

We have other means of dealing with that specifically - i.e. have the test skip itself if a package required for the test is not in the image. I think we already have examples of that.

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre

Mike Looijmans

2016-11-24 12:15 UTC #18 lore raw

Re: [PATCH 2/2] base-passwd: set root's default password to 'root'

In reply to Robert Yang

Quoted 24 lines · Robert Yang

Quoted 21 lines · Patrick Ohly
Quoted 5 lines · Robert Yang
[YOCTO #10710]
Otherwise, we can't login as root when debug-tweaks is not in IMAGE_FEATURES, and there is no other users to login by default, so there is no way to login.
Wait a second, are you really suggesting that OE-core should have a default root password in its default configuration?
That's very bad practice and I'm against doing it this way. Having a default password is one of the common vulnerabilities in actual devices on the market today. OE-core should make it hard to make that mistake, not actively introduce it.
So if you think that having a root password set (instead of empty), then at least make it an opt-in behavior that explicitly has to be selected. Make it an image feature so that images with and without default password can be build in the same build configuration. Changing base-passwd doesn't achieve that.

Quoted 7 lines

Quoted 3 lines
Even then I'm still wondering what the benefit of a well-known password compared to no password is. Both are equally insecure, so someone who wants to allow logins might as well go with "empty password".
The problem is that when debug-tweaks or empty-root-password is not in IMAGE_FEATURE, there is no way to login by default, which will surprise the user. How about:

We've used the following workaround for that in settop box images. Basically, what you want is that login as root without password is possible, since that can only be done from a local network connection or a serial port, which implies that you have physical access to the device anyway. But you do NOT want to be able to login using SSH with a blank password, because you'd typically forward that port from a router. So remove "debug-tweaks" but don't kill the logon:

# Some features in image.bbclass we do NOT want, so override them

# to be empty. We want to log in as root, but NOT via SSH. So we want
# to live without debug-tweaks...
zap_root_password () {
	true
}

Quoted 2 lines

1) Let user can set root passwd via a variable when building.
2) Warn the user at build time when the image is unable to login.

Setting a root password at build time is a very very very bad idea. It's only okay if there's ever going to be only one instance of your product in the world.

It's much better to have a blank or missing password. At least that makes it possible to check whether the user has configured it already, like for SSH. By default, SSH won't let you in until you have a password or a keyfile, which allows your device to be hooked up to the internet without a "gap" where you could access it with a trivial password.

Having written that, a bit more thought on the initial access is good. I for one would be glad to get rid of the aforementioned workaround.

Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: [email protected]
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Robert Yang

author

2016-11-30 03:15 UTC #19 lore raw

In reply to Robert Yang

Hello,

According to the discussions, here are things that we may do, please feel free to give your comments.
1) Add an image feature like "production", which will conflict with debug-tweaks, and check for some common security issues ?

2) Add a way like ROOT_PASSWD to let user can set root passwd easily ?

3) Do nothing, leave it as the current status.

// Robert

Quoted 30 lines · Robert Yang

Fixed 2 bugs:
- Can't login as root when debug-tweaks/empty-root-password is not in IMAGE_FEATURES since no passwd.
- When set root passwd and debug-tweaks/empty-root-password is in IMAGE_FEATURES, passwd is *required* to login.
Filed https://bugzilla.yoctoproject.org/show_bug.cgi?id=10710, and marked doc changes required as yes.
// Robert
The following changes since commit a675b2c89e477af088faee9b3be96eae19a85f0b:
sanity.bbclass: fix logging of an error (2016-11-15 15:18:50 +0000)
are available in the git repository at:
git://git.openembedded.org/openembedded-core-contrib rbt/root
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/root
Robert Yang (2):
rootfs-postcommands.bbclass: fix zap_empty_root_password
base-passwd: set root's default password to 'root'
 meta/classes/rootfs-postcommands.bbclass                 |  8 ++++----
 .../base-passwd/base-passwd/passwd_master.patch          | 16 ++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb      |  1 +
 3 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 meta/recipes-core/base-passwd/base-passwd/passwd_master.patch

ChenQi

2016-11-30 09:27 UTC #20 lore raw

We could make use of EXTRA_USERS_PARAMS here. More details are in meta/classes/extrausers.bbclass.

I haven't tested it myself, but maybe lines below could work:
INHERIT += "extrausers"
EXTRA_USERS_PARAMS = "\
usermod -p 'root' root; \
"

This info could also be found in local.conf.sample.extended.

Best Regards,
Chen Qi

Quoted 46 lines · Robert Yang

Hello,
According to the discussions, here are things that we may do, please feel free to give your comments.
1) Add an image feature like "production", which will conflict with debug-tweaks, and check for some common security issues ?
2) Add a way like ROOT_PASSWD to let user can set root passwd easily ?
3) Do nothing, leave it as the current status.
// Robert
Quoted 32 lines · Robert Yang
Fixed 2 bugs:
- Can't login as root when debug-tweaks/empty-root-password is not in IMAGE_FEATURES since no passwd.
- When set root passwd and debug-tweaks/empty-root-password is in IMAGE_FEATURES, passwd is *required* to login.
Filed https://bugzilla.yoctoproject.org/show_bug.cgi?id=10710, and marked doc changes required as yes.
// Robert
The following changes since commit a675b2c89e477af088faee9b3be96eae19a85f0b:
sanity.bbclass: fix logging of an error (2016-11-15 15:18:50 +0000)
are available in the git repository at:
git://git.openembedded.org/openembedded-core-contrib rbt/root
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/root
Robert Yang (2):
rootfs-postcommands.bbclass: fix zap_empty_root_password
base-passwd: set root's default password to 'root'
 meta/classes/rootfs-postcommands.bbclass                 |  8 ++++----
 .../base-passwd/base-passwd/passwd_master.patch          | 16 
++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb      |  1 +
 3 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 
meta/recipes-core/base-passwd/base-passwd/passwd_master.patch

ChenQi

2016-11-30 09:31 UTC #21 lore raw

Quoted 9 lines · ChenQi

We could make use of EXTRA_USERS_PARAMS here. More details are in meta/classes/extrausers.bbclass.
I haven't tested it myself, but maybe lines below could work:
INHERIT += "extrausers"
EXTRA_USERS_PARAMS = "\
usermod -p 'root' root; \
"

It should be '-P' instead of '-p'.
We implement setting clear text password ourselves via allow-for-setting-password-in-clear-text.patch in shadow package.

Regards,
Chen Qi

Quoted 54 lines

This info could also be found in local.conf.sample.extended.
Best Regards,
Chen Qi
Quoted 47 lines · Robert Yang
Hello,
According to the discussions, here are things that we may do, please feel free to give your comments.
1) Add an image feature like "production", which will conflict with debug-tweaks, and check for some common security issues ?
2) Add a way like ROOT_PASSWD to let user can set root passwd easily ?
3) Do nothing, leave it as the current status.
// Robert
Quoted 33 lines · Robert Yang
Fixed 2 bugs:
- Can't login as root when debug-tweaks/empty-root-password is not in IMAGE_FEATURES since no passwd.
- When set root passwd and debug-tweaks/empty-root-password is in IMAGE_FEATURES, passwd is *required* to login.
Filed https://bugzilla.yoctoproject.org/show_bug.cgi?id=10710, and marked doc changes required as yes.
// Robert
The following changes since commit a675b2c89e477af088faee9b3be96eae19a85f0b:
sanity.bbclass: fix logging of an error (2016-11-15 15:18:50 +0000)
are available in the git repository at:
git://git.openembedded.org/openembedded-core-contrib rbt/root
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/root
Robert Yang (2):
rootfs-postcommands.bbclass: fix zap_empty_root_password
base-passwd: set root's default password to 'root'
 meta/classes/rootfs-postcommands.bbclass                 |  8 ++++----
 .../base-passwd/base-passwd/passwd_master.patch          | 16 
++++++++++++++++
 meta/recipes-core/base-passwd/base-passwd_3.5.29.bb      |  1 +
 3 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 
meta/recipes-core/base-passwd/base-passwd/passwd_master.patch