Skip to contentSkip to conversation

base-files: move shells generating process to pkg_postinst

PATCH In review Ming Liu 3 participants 8 messages Oct 18, 2013 – Oct 20, 2013

lore
Share & tools
lore

Ming Liu

author
#0 lore raw

PATCH v3 1/3 base-files: move shells generating process to pkg_postinst

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 <[email protected]>
---
 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
meta/recipes-core/base-files/base-files/shells deleted +0 -8
deleted file mode 100644
index ce39b3d..0000000
@@ -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
meta/recipes-core/base-files/base-files_3.0.14.bb +18 -2
index 054fefa..8e8ee9b 100644
@@ -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"
Signature · 1 line
1.7.1
author
#1 lore raw

PATCH v3 2/3 screen: add pkg_postinst to register entry to /etc/shells

Also add pkg_postrm to remove the entry.

Signed-off-by Ming Liu <[email protected]>
---
 meta/recipes-extended/screen/screen_4.0.3.bb |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
meta/recipes-extended/screen/screen_4.0.3.bb +18 -0
index 1a7eb20..a3b3715 100644
@@ -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
+}
Signature · 1 line
1.7.1
author
#2 lore raw

PATCH v3 3/3 bash: add pkg_postrm to remove the entry in /etc/shells

In reply to Ming Liu
Signed-off-by Ming Liu <[email protected]>
---
 meta/recipes-extended/bash/bash.inc |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)
meta/recipes-extended/bash/bash.inc +15 -3
index 64b476f..7c18f37 100644
@@ -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
 }
Signature · 1 line
1.7.1
#3 lore raw

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

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 52

Quoted 10 lines · Ming Liu

pkg_postinst_${PN} () {

meta/recipes-extended/bash/bash.inc +6 -3
-	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.

Quoted 10 lines
meta/recipes-extended/bash/bash.inc +9 -0
+
+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
 }
#4 lore raw

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

Quoted 6 lines · Phil Blundell

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 47

Quoted 5 lines · Ming Liu
  pkg_postinst_${PN} () {
meta/recipes-extended/bash/bash.inc +1 -3
-	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

Quoted 28 lines

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 52

Quoted 5 lines
meta/recipes-extended/bash/bash.inc +5 -0
+		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.

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 62

Quoted 10 lines
meta/recipes-extended/bash/bash.inc +9 -0
+
+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
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

#5 lore raw

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

Quoted 11 lines · Mark Hatle
Quoted 6 lines · Phil Blundell

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 47

Quoted 5 lines · Ming Liu
  pkg_postinst_${PN} () {
meta/recipes-extended/bash/bash.inc +1 -3
-	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.

#6 lore raw

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

Quoted 13 lines · Phil Blundell
Quoted 11 lines · Mark Hatle
Quoted 6 lines · Phil Blundell

meta/recipes-extended/bash/bash.inc do_install_ptest () { · line 47

Quoted 5 lines · Ming Liu
   pkg_postinst_${PN} () {
meta/recipes-extended/bash/bash.inc +1 -3
-	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.

Quoted 2 lines

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)

Quoted 2 lines

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.

Quoted 5 lines

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

Quoted 3 lines

p.

author
#7 lore raw

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

In reply to Mark Hatle
Quoted 11 lines · Mark Hatle
Quoted 8 lines · Phil Blundell
Quoted 7 lines · Ming Liu
  pkg_postinst_${PN} () {
(quoted diff) +0 -2
-    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

Quoted 40 lines

--Mark

Quoted 21 lines
Quoted 6 lines
meta/recipes-extended/bash/bash.inc +4 -0
+        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.

Quoted 9 lines
meta/recipes-extended/bash/bash.inc +4 -0
+
+pkg_postrm_${PN} () {
+    if [ -f $D${sysconfdir}/shells ]; then
+        printf "$(grep -v "^${base_bindir}/bash$"

$D${sysconfdir}/shells)\n" > $D${sysconfdir}/shells

meta/recipes-extended/bash/bash.inc +4 -0
+
+        if [ ! -s $D${sysconfdir}/shells ]; then
+            rm $D${sysconfdir}/shells
+        fi
Quoted 9 lines
Quoted 2 lines

+ fi
}

_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core