[PATCH V3 1/3] base-files: move shells generating process to pkg_postinst

[PATCH V3 1/3] base-files: move shells generating process to pkg_postinst

[Ming Liu]

The "shells" file lists several valid login shells, of which some might be
not present in a particular rootfs, this is unreasonable.

Fix it by moving "shells" file generating process to pkg_postinst, for other
shells must do the same thing to register themselves to this file.

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
 meta/recipes-core/base-files/base-files/shells    |    8 --------
 meta/recipes-core/base-files/base-files_3.0.14.bb |   20 ++++++++++++++++++--
 2 files changed, 18 insertions(+), 10 deletions(-)
 delete mode 100644 meta/recipes-core/base-files/base-files/shells

diff --git a/meta/recipes-core/base-files/base-files/shells b/meta/recipes-core/base-files/base-files/shells
deleted file mode 100644
index ce39b3d..0000000
--- a/meta/recipes-core/base-files/base-files/shells
+++ /dev/null
@@ -1,8 +0,0 @@
-# /etc/shells: valid login shells
-/bin/sh
-/bin/ash
-/bin/bash
-/bin/dash
-/bin/ksh
-/usr/bin/ksh
-/usr/bin/screen
diff --git a/meta/recipes-core/base-files/base-files_3.0.14.bb b/meta/recipes-core/base-files/base-files_3.0.14.bb
index 054fefa..8e8ee9b 100644
--- a/meta/recipes-core/base-files/base-files_3.0.14.bb
+++ b/meta/recipes-core/base-files/base-files_3.0.14.bb
@@ -16,7 +16,6 @@ SRC_URI = "file://rotation \
            file://inputrc \
            file://host.conf \
            file://profile \
-           file://shells \
            file://fstab \
            file://filesystems \
            file://issue.net \
@@ -92,7 +91,6 @@ do_install () {
 	install -m 0644 ${WORKDIR}/usbd ${D}${sysconfdir}/default/usbd
 	sed -i "s#ROOTHOME#${ROOT_HOME}#" ${WORKDIR}/profile
 	install -m 0644 ${WORKDIR}/profile ${D}${sysconfdir}/profile
-	install -m 0644 ${WORKDIR}/shells ${D}${sysconfdir}/shells
 	install -m 0755 ${WORKDIR}/share/dot.profile ${D}${sysconfdir}/skel/.profile
 	install -m 0755 ${WORKDIR}/share/dot.bashrc ${D}${sysconfdir}/skel/.bashrc
 	install -m 0644 ${WORKDIR}/inputrc ${D}${sysconfdir}/inputrc
@@ -137,6 +135,24 @@ do_install_append_linuxstdbase() {
         done
 }
 
+pkg_postinst_${PN} () {
+	if [ ! -f $D${sysconfdir}/shells ]; then
+		touch $D${sysconfdir}/shells
+	fi
+
+	grep -q "^/bin/sh$" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
+}
+
+pkg_postrm_${PN} () {
+	if [ -f $D${sysconfdir}/shells ]; then
+		printf "$(grep -v "^/bin/sh$" $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
+
+		if [ ! -s $D${sysconfdir}/shells ]; then
+			rm $D${sysconfdir}/shells
+		fi
+	fi
+}
+
 PACKAGES = "${PN}-doc ${PN} ${PN}-dev ${PN}-dbg"
 FILES_${PN} = "/"
 FILES_${PN}-doc = "${docdir} ${datadir}/common-licenses"
-- 
1.7.1

[PATCH V3 2/3] screen: add pkg_postinst to register entry to /etc/shells

[Ming Liu]

Also add pkg_postrm to remove the entry.

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
 meta/recipes-extended/screen/screen_4.0.3.bb |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/meta/recipes-extended/screen/screen_4.0.3.bb b/meta/recipes-extended/screen/screen_4.0.3.bb
index 1a7eb20..a3b3715 100644
--- a/meta/recipes-extended/screen/screen_4.0.3.bb
+++ b/meta/recipes-extended/screen/screen_4.0.3.bb
@@ -41,3 +41,21 @@ do_install_append () {
 		fi
 	done
 }
+
+pkg_postinst_${PN} () {
+	if [ ! -f $D${sysconfdir}/shells ]; then
+		touch $D${sysconfdir}/shells
+	fi
+
+	grep -q "^${bindir}/screen$" $D${sysconfdir}/shells || echo ${bindir}/screen >> $D${sysconfdir}/shells
+}
+
+pkg_postrm_${PN} () {
+	if [ -f $D${sysconfdir}/shells ]; then
+		printf "$(grep -v "^${bindir}/screen$" $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
+
+		if [ ! -s $D${sysconfdir}/shells ]; then
+			rm $D${sysconfdir}/shells
+		fi
+	fi
+}
-- 
1.7.1

[PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Ming Liu]

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
 meta/recipes-extended/bash/bash.inc |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc
index 64b476f..7c18f37 100644
--- a/meta/recipes-extended/bash/bash.inc
+++ b/meta/recipes-extended/bash/bash.inc
@@ -44,7 +44,19 @@ do_install_ptest () {
 }
 
 pkg_postinst_${PN} () {
-	touch $D${sysconfdir}/shells
-	grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> $D${sysconfdir}/shells
-	grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
+	if [ ! -f $D${sysconfdir}/shells ]; then
+		touch $D${sysconfdir}/shells
+	fi
+
+	grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo ${base_bindir}/bash >> $D${sysconfdir}/shells
+}
+
+pkg_postrm_${PN} () {
+	if [ -f $D${sysconfdir}/shells ]; then
+		printf "$(grep -v "^${base_bindir}/bash$" $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
+
+		if [ ! -s $D${sysconfdir}/shells ]; then
+			rm $D${sysconfdir}/shells
+		fi
+	fi
 }
-- 
1.7.1

Re: [PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Phil Blundell]

On Fri, 2013-10-18 at 19:11 +0800, Ming Liu wrote:
>  pkg_postinst_${PN} () {
> -	touch $D${sysconfdir}/shells
> -	grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> $D${sysconfdir}/shells
> -	grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
> +	if [ ! -f $D${sysconfdir}/shells ]; then
> +		touch $D${sysconfdir}/shells
> +	fi
> +
> +	grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo ${base_bindir}/bash >> $D${sysconfdir}/shells
> +}

This patch contains significant changes to the postinst script which
aren't described in the commit message.

p.

> +
> +pkg_postrm_${PN} () {
> +	if [ -f $D${sysconfdir}/shells ]; then
> +		printf "$(grep -v "^${base_bindir}/bash$" $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
> +
> +		if [ ! -s $D${sysconfdir}/shells ]; then
> +			rm $D${sysconfdir}/shells
> +		fi
> +	fi
>  }

Re: [PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Mark Hatle]

On 10/18/13 9:59 AM, Phil Blundell wrote:
> On Fri, 2013-10-18 at 19:11 +0800, Ming Liu wrote:
>>   pkg_postinst_${PN} () {
>> -	touch $D${sysconfdir}/shells
>> -	grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> $D${sysconfdir}/shells
>> -	grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
>> +	if [ ! -f $D${sysconfdir}/shells ]; then

One note with the above check.  Whichever package is responsible for providing 
the 'shells' file needs to be installed -first-.  So anything that manipulates 
the 'shells' file will need an RDEPENDS on that package.

--Mark

>> +		touch $D${sysconfdir}/shells
>> +	fi
>> +
>> +	grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo ${base_bindir}/bash >> $D${sysconfdir}/shells
>> +}
>
> This patch contains significant changes to the postinst script which
> aren't described in the commit message.
>
> p.
>
>> +
>> +pkg_postrm_${PN} () {
>> +	if [ -f $D${sysconfdir}/shells ]; then
>> +		printf "$(grep -v "^${base_bindir}/bash$" $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
>> +
>> +		if [ ! -s $D${sysconfdir}/shells ]; then
>> +			rm $D${sysconfdir}/shells
>> +		fi
>> +	fi
>>   }
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

Re: [PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Phil Blundell]

On Fri, 2013-10-18 at 10:12 -0500, Mark Hatle wrote:
> On 10/18/13 9:59 AM, Phil Blundell wrote:
> > On Fri, 2013-10-18 at 19:11 +0800, Ming Liu wrote:
> >>   pkg_postinst_${PN} () {
> >> -	touch $D${sysconfdir}/shells
> >> -	grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> $D${sysconfdir}/shells
> >> -	grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
> >> +	if [ ! -f $D${sysconfdir}/shells ]; then
> 
> One note with the above check.  Whichever package is responsible for providing 
> the 'shells' file needs to be installed -first-.  So anything that manipulates 
> the 'shells' file will need an RDEPENDS on that package.

Isn't the whole point of the check above that it now creates /etc/shells
if it didn't exist already?

That said, though, I'm still not entirely convinced that having
semi-random packages create a file that isn't mentioned in either FILES
or CONFFILES is a very good thing.  I'm also not totally clear on what
exactly the problem is that this set of patches is trying to solve: the
original commit message says that having nonexistent files named
in /etc/shells is "unreasonable" but doesn't provide any supporting
evidence for that assertion.

p.

Re: [PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Mark Hatle]

On 10/18/13 10:18 AM, Phil Blundell wrote:
> On Fri, 2013-10-18 at 10:12 -0500, Mark Hatle wrote:
>> On 10/18/13 9:59 AM, Phil Blundell wrote:
>>> On Fri, 2013-10-18 at 19:11 +0800, Ming Liu wrote:
>>>>    pkg_postinst_${PN} () {
>>>> -	touch $D${sysconfdir}/shells
>>>> -	grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> $D${sysconfdir}/shells
>>>> -	grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> $D${sysconfdir}/shells
>>>> +	if [ ! -f $D${sysconfdir}/shells ]; then
>>
>> One note with the above check.  Whichever package is responsible for providing
>> the 'shells' file needs to be installed -first-.  So anything that manipulates
>> the 'shells' file will need an RDEPENDS on that package.
>
> Isn't the whole point of the check above that it now creates /etc/shells
> if it didn't exist already?

Situation  bash has dep on base-files:

base-files package gets install (creates basic /etc/shells)
bash gets installed (checks for /etc/shells, adds /bin/bash)

Alternative situation:

bash has no dep on base-files:

bash gets installed (checks for /etc/shells, doesn't exist)
base-files gets installed (creates basic /etc/shells)

> That said, though, I'm still not entirely convinced that having
> semi-random packages create a file that isn't mentioned in either FILES

I don't want it to create the file, that is the wrong behavior.  The -package- 
needs to depend on the package that provides the base configuration for the 
system.  -something- has to create the file, or be installed first.

> or CONFFILES is a very good thing.  I'm also not totally clear on what
> exactly the problem is that this set of patches is trying to solve: the
> original commit message says that having nonexistent files named
> in /etc/shells is "unreasonable" but doesn't provide any supporting
> evidence for that assertion.

The original problem is that /etc/shells contains too much "crap", and we've got 
customers saying "hey you are opening up potential security holes by having 
things in there that are not valid."  (Beyond the file being sloppy)

So we would prefer that a minimal file exist, and then entries for valid shells 
be added dynamically to the system, only if the packages that provide them are 
supported.

--Mark

> p.
>
>

Re: [PATCH V3 3/3] bash: add pkg_postrm to remove the entry in /etc/shells

[Ming Liu]

On 10/18/2013 11:12 PM, Mark Hatle wrote:
> On 10/18/13 9:59 AM, Phil Blundell wrote:
>> On Fri, 2013-10-18 at 19:11 +0800, Ming Liu wrote:
>>>   pkg_postinst_${PN} () {
>>> -    touch $D${sysconfdir}/shells
>>> -    grep -q "bin/bash" $D${sysconfdir}/shells || echo /bin/bash >> 
>>> $D${sysconfdir}/shells
>>> -    grep -q "bin/sh" $D${sysconfdir}/shells || echo /bin/sh >> 
>>> $D${sysconfdir}/shells
>>> +    if [ ! -f $D${sysconfdir}/shells ]; then
>
> One note with the above check.  Whichever package is responsible for 
> providing the 'shells' file needs to be installed -first-.  So 
> anything that manipulates the 'shells' file will need an RDEPENDS on 
> that package.
Yes, that is a good idea, I will change it as you suggest.

the best,
thank you

>
> --Mark
>
>>> +        touch $D${sysconfdir}/shells
>>> +    fi
>>> +
>>> +    grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo 
>>> ${base_bindir}/bash >> $D${sysconfdir}/shells
>>> +}
>>
>> This patch contains significant changes to the postinst script which
>> aren't described in the commit message.
>>
>> p.
>>
>>> +
>>> +pkg_postrm_${PN} () {
>>> +    if [ -f $D${sysconfdir}/shells ]; then
>>> +        printf "$(grep -v "^${base_bindir}/bash$" 
>>> $D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells
>>> +
>>> +        if [ ! -s $D${sysconfdir}/shells ]; then
>>> +            rm $D${sysconfdir}/shells
>>> +        fi
>>> +    fi
>>>   }
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>