Subject: [alpine-aports] main/busybox: split package into core and suid subpackages

From: Christian Kampka <[email-masked]>
Date: Sat, 17 Oct 2015 16:46:57 +0200

Hi,

since this patch probably needs some explanation, I'd like to give an explanation to why I'd like to see this accepted.
The current structure of the busybox packages, which includes the bbsuid binary as well as busybox
forces every alpine installation to include the suid binary to provide functionalities like mount, passwd or su.
If alpine is run as a chroot or docker container or likewise installation , which is no longer uncommon these days,
having suid binaries included in installation should no longer be required and is imo quite undesirable if you think about security.

The proposed patch splits the busybox package into two subpackages, busybox-core and busybox-suid.
The core package contains everything that is currently included in the busybox package except for the bbsuid binary.
This will be shipped via the busybox-suid package. The busybox package will be turned into a metapackage
that pulls in busybox-core and busybox-suid, so for most use cases nothing will change except for those installations
that desire it explicitly.

I am aware that alot of packages currently depend on the busybox package. I think it would be feasable enough to update those
step by step to required only the subpackages they really need to depend on (which probably is not or should not be suid in most cases).

I'm looking forward to your thoughts.

Cheers,
Christian

---
Unsubscribe:  [email-masked]
Help:         [email-masked]
---

---

From: Natanael Copa <[email-masked]>
Date: Thu, 22 Oct 2015 18:11:11 +0200

On Sat, 17 Oct 2015 16:46:57 +0200
Christian Kampka <[email-masked]> wrote:

> Hi,
> 
> since this patch probably needs some explanation, I'd like to give an
> explanation to why I'd like to see this accepted. The current
> structure of the busybox packages, which includes the bbsuid binary
> as well as busybox forces every alpine installation to include the
> suid binary to provide functionalities like mount, passwd or su. If
> alpine is run as a chroot or docker container or likewise
> installation , which is no longer uncommon these days, having suid
> binaries included in installation should no longer be required and is
> imo quite undesirable if you think about security.

Makes perfect sense.

> The proposed patch splits the busybox package into two subpackages,
> busybox-core and busybox-suid. The core package contains everything
> that is currently included in the busybox package except for the
> bbsuid binary. This will be shipped via the busybox-suid package. The
> busybox package will be turned into a metapackage that pulls in
> busybox-core and busybox-suid, so for most use cases nothing will
> change except for those installations that desire it explicitly.

I wonder if we somehow can solve this with totally 2 packages:
  busybox + busybox-suid 
instead of totally 3:
  busybox-core + busybox-suid + busybox.

We could for example add busybox-suid as a dependency to alpine-base,
or assume that busybox-suid is needed if some other package like openrc
is installed and have install_if="busybox=$pkgver openrc". I wonder
what happens then, if you "apk add !busybox-suid" to opt out?

I suppose the most critical thing we want avoid is someone end up
locked out from remote box due to 'su' not working after an upgrade.

> I am aware that alot of packages currently depend on the busybox
> package. I think it would be feasable enough to update those step by
> step to required only the subpackages they really need to depend on
> (which probably is not or should not be suid in most cases).
> 

---

---

From: Christian Kampka <[email-masked]>
Date: Thu, 22 Oct 2015 18:39:30 +0000

>   busybox-core + busybox-suid + busybox.

I did not go for this option because it introduces a breaking change, but
it is certainly desirable in the long run. Since 3.3 is now in freeze, it
may be OK to possibly break dependencies here.

> what happens then, if you "apk add !busybox-suid" to opt out?

Having busybox-suid as a dependency to alpine-base is crucial, but I think
we need to take care of packages that really require it, eg. mkinitfs
without suid could lead to really interesting problems. Having a
metapackage for the transition would soften the blow.
I don't have the overview yet to really offer a qualified opinion here.

> locked out from remote box due to 'su' not working after an upgrade.

Yes, definitely.
Let me know what you decide is the best way to proceed, I'll make the
necessary modifications to the patch.
Thanks for reviewing this.

Cheers,
Christian

---

From: Natanael Copa <[email-masked]>
Date: Mon, 26 Oct 2015 09:36:25 +0100

On Thu, 22 Oct 2015 18:39:30 +0000
Christian Kampka <[email-masked]> wrote:

> Having busybox-suid as a dependency to alpine-base is crucial,

If we can let alpine-base depend on busybox-suid then I think we will
be fine and can probably just drop the busybox-core package.

> but I think
> without suid could lead to really interesting problems.

How? the suid binaries are only:
        "/bin/mount",
        "/bin/ping",
        "/bin/ping6",
        "/bin/umount",
        "/usr/bin/crontab",
        "/usr/bin/passwd",
        "/usr/bin/su",
        "/usr/bin/traceroute",

mkinitfs needs to run as root already so it should not need elevate any
privileges and thus should not need anything suid root.

I think the only things that may need suid are things like ping/ping6
(any scripts using ping?) and interactive use like su and passwd.

I don't think it is simple to find out what packages that actually
needs suid. Even postgresql init.d script uses su but I expect su work
without suid in that case because it will reduce permissions from root
to user 'postgres'.

> Having a metapackage for the transition would soften the blow.

What I'd like to do is avoid the busbyox-core vs busybox package. If we
introduce the busybox-core package and other packages start depend on
it, then we will have problems getting rid of it in future.

> Thanks for reviewing this.

I think, split bbsuid to separate package and adding it as a dependency
to alpine-base should work.

Or am I missing something?

---

---

From: Christian Kampka <[email-masked]>
Date: Mon, 26 Oct 2015 14:48:23 +0100

Hi,

> Or am I missing something?

Actually, I was under the assumption that no bbsuid meant no su,ping,passwd,etc. symlinks either, 
but I somehow missed the fact that busybox will of course create those without suid bit.

That said, I agree that having just busybox and busybox-suid would be the best way to proceed.
I have reworked the patch accordingly.

Cheers,
Christian

---

---

From: Christian Kampka <[email-masked]>
Date: Mon, 26 Oct 2015 14:48:24 +0100

---
 main/alpine-base/APKBUILD    |  2 +-
 main/busybox/APKBUILD        | 18 +++++++++++++-----
 main/busybox/busybox.trigger |  3 +--
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/main/alpine-base/APKBUILD b/main/alpine-base/APKBUILD
index 834d763..cb55dd3 100644
--- a/main/alpine-base/APKBUILD
+++ b/main/alpine-base/APKBUILD
@@ -7,7 +7,7 @@ pkgdesc="Meta package for minimal alpine base"
 url="http://alpinelinux.org"
 arch="noarch"
 license="GPL"
-depends="alpine-baselayout alpine-conf apk-tools busybox busybox-initscripts
+depends="alpine-baselayout alpine-conf apk-tools busybox busybox-suid busybox-initscripts
 	openrc libc-utils alpine-keys"
 makedepends=""
 install=""
diff --git a/main/busybox/APKBUILD b/main/busybox/APKBUILD
index c548540..bd0780d 100644
--- a/main/busybox/APKBUILD
+++ b/main/busybox/APKBUILD
@@ -2,15 +2,13 @@
 # Maintainer: Natanael Copa <[email-masked]>
 pkgname=busybox
 pkgver=1.23.2
-pkgrel=9
+pkgrel=10
 pkgdesc="Size optimized toolbox of many common UNIX utilities"
 url=http://busybox.net
 arch="all"
 license="GPL2"
-depends=
 makedepends="linux-headers"
-install="$pkgname.post-install $pkgname.post-upgrade"
-subpackages="$pkgname-static"
+subpackages="$pkgname-static $pkgname-suid"
 options="suid"
 triggers="busybox.trigger=/bin:/usr/bin:/sbin:/usr/sbin:/lib/modules/*"
 source="http://busybox.net/downloads/$pkgname-$pkgver.tar.bz2
@@ -41,6 +39,7 @@ _sdir="$srcdir"/$pkgname-$pkgver
 _staticdir="$srcdir"/build-static
 _dyndir="$srcdir"/build-dynamic
 _config="$srcdir"/busyboxconfig
+
 prepare() {
 	mkdir -p "$_staticdir" "$_dyndir"
 	#patches
@@ -92,7 +91,6 @@ package() {
 		"$pkgdir"/var/cache/misc "$pkgdir"/bin "$pkgdir"/sbin
 	chmod 1777 "$pkgdir"/tmp
 	install -m755 busybox "$pkgdir"/bin/busybox || return 1
-	install -m4111 bbsuid "$pkgdir"/bin/bbsuid || return 1
 	# we need /bin/sh to be able to execute post-install
 	ln -s /bin/busybox "$pkgdir"/bin/sh
 
@@ -114,6 +112,16 @@ package() {
 		"$pkgdir"/etc/udhcpd.conf || return 1
 }
 
+suid() {
+	pkgdesc="suid binaries of Busybox"
+	depends="${pkgname}"
+	triggers="busybox-suid.trigger=/bin:/usr/bin:/sbin:/usr/sbin"
+
+	cd "$_dyndir"
+	mkdir -p "$subpkgdir"/bin
+	install -m4111 bbsuid "$subpkgdir"/bin/bbsuid || return 1
+}
+
 static() {
 	pkgdesc="Statically linked Busybox"
 	mkdir -p "$subpkgdir"/bin
diff --git a/main/busybox/busybox.trigger b/main/busybox/busybox.trigger
index 39cc6b4..c9f9059 100644
--- a/main/busybox/busybox.trigger
+++ b/main/busybox/busybox.trigger
@@ -14,7 +14,6 @@ for i in "$@"; do
 done
 
 if [ -n "$do_bb_install" ]; then
-	/bin/bbsuid --install
+	[ -e /bin/bbsuid ] && /bin/bbsuid --install
 	/bin/busybox --install -s
 fi
-
-- 
2.6.1

---

---

From: Natanael Copa <[email-masked]>
Date: Mon, 26 Oct 2015 20:33:11 +0100

On Mon, 26 Oct 2015 14:48:24 +0100

> +++ b/main/alpine-base/APKBUILD

...

> +	triggers="busybox-suid.trigger=/bin:/usr/bin:/sbin:/usr/sbin"

There are no trigger script for busybox-suid.

I fixed that and pushed.

Thanks!

-nc

---

---

From: Christian Kampka <[email-masked]>
Date: Sat, 17 Oct 2015 16:46:58 +0200

---
 main/busybox/APKBUILD                  | 57 ++++++++++++++++++++++------------
 main/busybox/busybox-core.post-install |  4 +++
 main/busybox/busybox-core.post-upgrade | 11 +++++++
 main/busybox/busybox-core.trigger      | 18 +++++++++++
 main/busybox/busybox-suid.trigger      |  3 ++
 main/busybox/busybox.post-install      |  4 ---
 main/busybox/busybox.post-upgrade      | 11 -------
 main/busybox/busybox.trigger           | 20 ------------
 8 files changed, 73 insertions(+), 55 deletions(-)
 create mode 100644 main/busybox/busybox-core.post-install
 create mode 100644 main/busybox/busybox-core.post-upgrade
 create mode 100644 main/busybox/busybox-core.trigger
 create mode 100644 main/busybox/busybox-suid.trigger
 delete mode 100644 main/busybox/busybox.post-install
 delete mode 100644 main/busybox/busybox.post-upgrade
 delete mode 100644 main/busybox/busybox.trigger

diff --git a/main/busybox/APKBUILD b/main/busybox/APKBUILD
index c548540..23d24d8 100644
--- a/main/busybox/APKBUILD
+++ b/main/busybox/APKBUILD
@@ -2,17 +2,15 @@
 # Maintainer: Natanael Copa <[email-masked]>
 pkgname=busybox
 pkgver=1.23.2
-pkgrel=9
+pkgrel=10
 pkgdesc="Size optimized toolbox of many common UNIX utilities"
 url=http://busybox.net
 arch="all"
 license="GPL2"
-depends=
+depends="$pkgname-core $pkgname-suid"
 makedepends="linux-headers"
-install="$pkgname.post-install $pkgname.post-upgrade"
-subpackages="$pkgname-static"
+subpackages="$pkgname-static $pkgname-core $pkgname-suid"
 options="suid"
-triggers="busybox.trigger=/bin:/usr/bin:/sbin:/usr/sbin:/lib/modules/*"
 source="http://busybox.net/downloads/$pkgname-$pkgver.tar.bz2
 	bbsuid.c
 
@@ -87,31 +86,49 @@ build() {
 }
 
 package() {
+	mkdir -p "$pkgdir" && return 0
+}
+
+core() {
+	pkgdesc="Core utilities of Busybox"
+	install="$pkgname-core.post-install $pkgname-core.post-upgrade"
+	triggers="busybox-core.trigger=/bin:/usr/bin:/sbin:/usr/sbin:/lib/modules/*"
+	depends=
+
 	cd "$_dyndir"
-	mkdir -p "$pkgdir"/usr/sbin "$pkgdir"/usr/bin "$pkgdir"/tmp \
-		"$pkgdir"/var/cache/misc "$pkgdir"/bin "$pkgdir"/sbin
-	chmod 1777 "$pkgdir"/tmp
-	install -m755 busybox "$pkgdir"/bin/busybox || return 1
-	install -m4111 bbsuid "$pkgdir"/bin/bbsuid || return 1
+	mkdir -p "$subpkgdir"/usr/sbin "$subpkgdir"/usr/bin "$subpkgdir"/tmp \
+		"$subpkgdir"/var/cache/misc "$subpkgdir"/bin "$subpkgdir"/sbin
+	chmod 1777 "$subpkgdir"/tmp
+	install -m755 busybox "$subpkgdir"/bin/busybox || return 1
 	# we need /bin/sh to be able to execute post-install
-	ln -s /bin/busybox "$pkgdir"/bin/sh
+	ln -s /bin/busybox "$subpkgdir"/bin/sh
 
 	#ifupdown needs those dirs to be present
 	mkdir -p \
-		"$pkgdir"/etc/network/if-down.d \
-		"$pkgdir"/etc/network/if-post-down.d \
-		"$pkgdir"/etc/network/if-post-up.d \
-		"$pkgdir"/etc/network/if-pre-down.d \
-		"$pkgdir"/etc/network/if-pre-up.d \
-		"$pkgdir"/etc/network/if-up.d \
+		"$subpkgdir"/etc/network/if-down.d \
+		"$subpkgdir"/etc/network/if-post-down.d \
+		"$subpkgdir"/etc/network/if-post-up.d \
+		"$subpkgdir"/etc/network/if-pre-down.d \
+		"$subpkgdir"/etc/network/if-pre-up.d \
+		"$subpkgdir"/etc/network/if-up.d \
 		|| return 1
 
 	install -Dm644 "$srcdir"/acpid.logrotate \
-		"$pkgdir/etc/logrotate.d/acpid" || return 1
+		"$subpkgdir/etc/logrotate.d/acpid" || return 1
 
-	mkdir -p "$pkgdir"/var/lib/udhcpd || return 1
+	mkdir -p "$subpkgdir"/var/lib/udhcpd || return 1
 	install -Dm644 "$_sdir"/examples/udhcp/udhcpd.conf \
-		"$pkgdir"/etc/udhcpd.conf || return 1
+		"$subpkgdir"/etc/udhcpd.conf || return 1
+}
+
+suid() {
+	pkgdesc="suid binaries of Busybox"
+	depends="${pkgname}-core"
+	triggers="busybox-suid.trigger=/bin:/usr/bin:/sbin:/usr/sbin"
+
+	cd "$_dyndir"
+	mkdir -p "$subpkgdir"/bin
+	install -m4111 bbsuid "$subpkgdir"/bin/bbsuid || return 1
 }
 
 static() {
diff --git a/main/busybox/busybox-core.post-install b/main/busybox/busybox-core.post-install
new file mode 100644
index 0000000..a986b2f
--- /dev/null
+++ b/main/busybox/busybox-core.post-install
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+# We need the symlinks early
+exec /bin/busybox --install -s
diff --git a/main/busybox/busybox-core.post-upgrade b/main/busybox/busybox-core.post-upgrade
new file mode 100644
index 0000000..268f22d
--- /dev/null
+++ b/main/busybox/busybox-core.post-upgrade
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+# remove links that has been relocated
+for link in /bin/install /bin/ip /bin/vi /usr/bin/lspci; do
+	if [ -L "$link" ] && [ "$(readlink $link)" = "/bin/busybox" ]; then
+		rm "$link"
+	fi
+done
+
+# We need the symlinks early
+exec /bin/busybox --install -s
diff --git a/main/busybox/busybox-core.trigger b/main/busybox/busybox-core.trigger
new file mode 100644
index 0000000..0572ade
--- /dev/null
+++ b/main/busybox/busybox-core.trigger
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+do_bb_install=
+
+for i in "$@"; do
+	case "$i" in
+		/lib/modules/*)
+			if [ -d "$i" ]; then
+				/bin/busybox depmod ${i#/lib/modules/}
+			fi
+			;;
+		*) do_bb_install=yes;;
+	esac
+done
+
+if [ -n "$do_bb_install" ]; then
+	/bin/busybox --install -s
+fi
diff --git a/main/busybox/busybox-suid.trigger b/main/busybox/busybox-suid.trigger
new file mode 100644
index 0000000..7520da3
--- /dev/null
+++ b/main/busybox/busybox-suid.trigger
@@ -0,0 +1,3 @@
+#!/bin/sh
+
+/bin/bbsuid --install
diff --git a/main/busybox/busybox.post-install b/main/busybox/busybox.post-install
deleted file mode 100644
index a986b2f..0000000
--- a/main/busybox/busybox.post-install
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-
-# We need the symlinks early
-exec /bin/busybox --install -s
diff --git a/main/busybox/busybox.post-upgrade b/main/busybox/busybox.post-upgrade
deleted file mode 100644
index 268f22d..0000000
--- a/main/busybox/busybox.post-upgrade
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/bin/sh
-
-# remove links that has been relocated
-for link in /bin/install /bin/ip /bin/vi /usr/bin/lspci; do
-	if [ -L "$link" ] && [ "$(readlink $link)" = "/bin/busybox" ]; then
-		rm "$link"
-	fi
-done
-
-# We need the symlinks early
-exec /bin/busybox --install -s
diff --git a/main/busybox/busybox.trigger b/main/busybox/busybox.trigger
deleted file mode 100644
index 39cc6b4..0000000
--- a/main/busybox/busybox.trigger
+++ /dev/null
@@ -1,20 +0,0 @@
-#!/bin/sh
-
-do_bb_install=
-
-for i in "$@"; do
-	case "$i" in
-		/lib/modules/*)
-			if [ -d "$i" ]; then
-				/bin/busybox depmod ${i#/lib/modules/}
-			fi
-			;;
-		*) do_bb_install=yes;;
-	esac
-done
-
-if [ -n "$do_bb_install" ]; then
-	/bin/bbsuid --install
-	/bin/busybox --install -s
-fi
-
-- 
2.6.1

---