[Buildroot] [PATCH 0/3] core/pkg-infra: allow packages to provide permisions in a file

[Buildroot] [PATCH 0/3] core/pkg-infra: allow packages to provide permisions in a file

[Yann E. MORIN]

Hello All!

This series allows packages to provide a permission table in a file
rather than in-line in the .mk fiile.

That permission file can be generated. It is usefull for the SELinux
stuff and busybox, where individual applets should have a suid bit, but
we only know what applets exist at configure time, not when parsing the
.mk file.

The last patch is *not* to be applied; it serves just as an example of
how the new permisions file can be generated.


Regards,
Yann E. MORIN.


The following changes since commit 0c5beb6501707dd5cb80484562bf2b0cbe2b4423

  libcurl: security bump to 7.52.0 (2016-12-21 19:44:44 +0100)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 6fa811da3de50b45790aaeafbf81f2225761abf4

  [RFC] package/busybox: gneerate permissions for enabeld applets (2016-12-21 20:01:39 +0100)


----------------------------------------------------------------
Yann E. MORIN (3):
      core: allow packages to declare a permission file
      docs/manual: document FOO_PERMISSIONS_FILE
      [RFC] package/busybox: gneerate permissions for enabeld applets

 docs/manual/adding-packages-generic.txt |  8 +++++++-
 fs/common.mk                            |  3 +++
 package/busybox/Config.in               |  3 +++
 package/busybox/busybox.mk              | 15 +++++++++++++++
 package/pkg-generic.mk                  |  1 +
 5 files changed, 29 insertions(+), 1 deletion(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] core: allow packages to declare a permission file

[Yann E. MORIN]

Currently, packages can define a variable that holds all the permissions
to set on the files it installs. This can be used to set various
permissions, like ownership, mode, suid/sgid/sticky bits to individual
files.

However, this variable has to contain entries that are known the moment
we scan the .mk file; it is not possible to conditionally add permisions
for files which presence depend on post-parse conditions.

This is the case for example for Busybox, for which we don't know whether
a specific applet will be enabled or not until after the configure
command has run.

Introduce a new variable that packages can set to point to a file that
contains a permission table. That filewill only be used when a filesystem
image is asembled, so the file can be generated, either at configure or
build time, with no problem.

Since this variable can be empty (when no package provides such a
permissions file), we must ensure not to cat anything (or that would
stale, cat-ing stdin). But since this variable is not fully known by the
time we parse the fs infra (e.g. packages from a br2-external tree are
not yet parsed), we can not test it with make syntas (ifneq...endif).
Teting it with shell syntac is not trivial either, becaue the variable
would not be mpty (it would be only spaces). sO, we just iterate over
the the files and cat them one by one with a shell-level for-loop, which
is happy with nothing to iterate over.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>

---
Changes v1 -> v2;
  - protect against empty list  (Peter)
  - fix variable name  (Peter)

---
Note: this will be usefull for Busybox, to properly handle the SELinux
contexts of the individual applets.
---
 fs/common.mk           | 3 +++
 package/pkg-generic.mk | 1 +
 2 files changed, 4 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 396b1c2..0bbcca4 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -91,6 +91,9 @@ ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
 endif
 endif
 	$$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(FULL_DEVICE_TABLE)
+	for f in $$(PACKAGES_PERMISSIONS_FILES); do \
+		cat $$$${f} >> $$(FULL_DEVICE_TABLE) || exit 1; \
+	done
 	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
 	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
 		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3ca71b0..5665005 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -904,6 +904,7 @@ PACKAGES += $(1)
 ifneq ($$($(2)_PERMISSIONS),)
 PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep)
 endif
+PACKAGES_PERMISSIONS_FILES += $$($(2)_PERMISSIONS_FILE)
 ifneq ($$($(2)_DEVICES),)
 PACKAGES_DEVICES_TABLE += $$($(2)_DEVICES)$$(sep)
 endif
-- 
2.7.4

[Buildroot] [PATCH 2/3] docs/manual: document FOO_PERMISSIONS_FILE

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>

---
changes v1 -> v2;
  - fix grammar and semantics
---
 docs/manual/adding-packages-generic.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 0e111e1..b0b4bf2 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -385,7 +385,13 @@ information is (assuming the package name is +libfoo+) :
 * +LIBFOO_PERMISSIONS+ lists the changes of permissions to be done at
   the end of the build process. The syntax is once again the makedevs one.
   You can find some documentation for this syntax in the xref:makedev-syntax[].
-  This variable is optional.
+  This variable is optional; its value must be known when the .mk file
+  is parsed.
+
+* +LIBFOO_PERMISSIONS_FILE+, like +LIBFOO_PERMISSIONS+ but points to a
+  file that contains the list of permissions. Unlike +LIBFOO_PERMISSIONS+,
+  its content need not be known when the .mk file is parsed, so it can be
+  generated. This variable is optional, and you should seldom need it.
 
 * +LIBFOO_USERS+ lists the users to create for this package, if it installs
   a program you want to run as a specific user (e.g. as a daemon, or as a
-- 
2.7.4

[Buildroot] [PATCH 3/3] [RFC] package/busybox: gneerate permissions for enabeld applets

[Yann E. MORIN]

DO NOT COMMIT THIS.

This is just an example how to use FOO_PERMISSIONS_FILE. It's even
probably not 100% correct.

You need to enable BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES in Buildroot,
as well as CONFIG_BUILD_LIBBUSYBOX and CONFIG_FEATURE_INDIVIDUAL in
busybox.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/busybox/Config.in  |  3 +++
 package/busybox/busybox.mk | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/package/busybox/Config.in b/package/busybox/Config.in
index 7ef3f0e..1e4c252 100644
--- a/package/busybox/Config.in
+++ b/package/busybox/Config.in
@@ -54,6 +54,9 @@ config BR2_PACKAGE_BUSYBOX_SELINUX
 	  crond, then individual binaries have to be enabled for the
 	  SELinux type transitions to occur properly.
 
+config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
+	bool "Install individual binaries"
+
 config BR2_PACKAGE_BUSYBOX_WATCHDOG
 	bool "Install the watchdog daemon startup script"
 	help
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 40fb4d9..c2a8ca0 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
 BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
 BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+BUSYBOX_PERMISSIONS_FILE = $(BUSYBOX_DIR)/busybox.permissions
+define BUSYBOX_GEN_PERMISSIONS
+	for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE" $(@D)/include/applets.h \
+		| sed -e 's/,.*//' -e 's/.*(//'`; \
+	do \
+		temp=`grep -w $${app} $(@D)/busybox.links`; \
+		if [ -n "$${temp}" ]; then \
+			echo "$${temp} f 4755 0  0 - - - - -"; \
+		fi; \
+	done >$(BUSYBOX_PERMISSIONS_FILE)
+endef
+BUSYBOX_POST_INSTALL_TARGET_HOOKS += BUSYBOX_GEN_PERMISSIONS
+else
 define BUSYBOX_PERMISSIONS
 	/bin/busybox                     f 4755 0  0 - - - - -
 endef
+endif
 
 # If mdev will be used for device creation enable it and copy S10mdev to /etc/init.d
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),y)
-- 
2.7.4

[Buildroot] [PATCH 3/3] [RFC] package/busybox: gneerate permissions for enabeld applets

[Yann E. MORIN]

Peter, All,

On 2016-12-13 23:32 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > DO NOT COMMIT THIS.
>  > This is jsut an example how to use FOO_PERMISSIONS_FILE.
> 
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > ---
>  >  package/busybox/busybox.mk | 15 +++++++++++++++
>  >  1 file changed, 15 insertions(+)
> 
>  > diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
>  > index f4a241d..9c1f2e8 100644
>  > --- a/package/busybox/busybox.mk
>  > +++ b/package/busybox/busybox.mk
>  > @@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
>  >  BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
>  >  BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
>  
>  > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
>  > +BUSYBOX_PERMISSIONS_FILE = $(@D)/busybox.permissions
> 
> Careful, what does $(@D) refer to here? When it is used here in the file
> it is output/build/busybox-*, but when it is expanded in fs/common.mk it
> refers to output/images:
> 
> cat    /home/peko/source/buildroot/output-test/images/busybox.permissions   >> /home/peko/source/buildroot/output-test/build/_device_table.txt
> cat: /home/peko/source/buildroot/output-test/images/busybox.permissions: No such file or directory
> 
> 
> > +define BUSYBOX_GEN_PERMISSIONS
>  > +	for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE" $(@D)/include/applets.h \
>  > +		| sed -e 's/,.*//' -e 's/.*(//'`; \
>  > +	do \
>  > +		temp=`grep -w $${app} $(@D)/busybox.links`; \
>  > +		if [ -n "$${temp}" ]; then \
>  > +			echo "$${temp} f 4755 0  0 - - - - -"
> 
> You forgot to append ; \ so it doesn't actually work.

No, it does not. Again, as I said in the cover-letter and in this commit
log, it is just a very early smnapshot of the directions we'd have to
follow to allow packages to provide non-constant (i.e. generated)
permissions, and how relatively easy it would be.

If we ever want to do it, of course.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 3/3] [RFC] package/busybox: gneerate permissions for enabeld applets

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > DO NOT COMMIT THIS.
 > This is jsut an example how to use FOO_PERMISSIONS_FILE.

 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > ---
 >  package/busybox/busybox.mk | 15 +++++++++++++++
 >  1 file changed, 15 insertions(+)

 > diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
 > index f4a241d..9c1f2e8 100644
 > --- a/package/busybox/busybox.mk
 > +++ b/package/busybox/busybox.mk
 > @@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
 >  BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
 >  BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
 
 > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
 > +BUSYBOX_PERMISSIONS_FILE = $(@D)/busybox.permissions

Careful, what does $(@D) refer to here? When it is used here in the file
it is output/build/busybox-*, but when it is expanded in fs/common.mk it
refers to output/images:

cat    /home/peko/source/buildroot/output-test/images/busybox.permissions   >> /home/peko/source/buildroot/output-test/build/_device_table.txt
cat: /home/peko/source/buildroot/output-test/images/busybox.permissions: No such file or directory


> +define BUSYBOX_GEN_PERMISSIONS
 > +	for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE" $(@D)/include/applets.h \
 > +		| sed -e 's/,.*//' -e 's/.*(//'`; \
 > +	do \
 > +		temp=`grep -w $${app} $(@D)/busybox.links`; \
 > +		if [ -n "$${temp}" ]; then \
 > +			echo "$${temp} f 4755 0  0 - - - - -"

You forgot to append ; \ so it doesn't actually work.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 3/3] [RFC] package/busybox: gneerate permissions for enabeld applets

[Yann E. MORIN]

DO NOT COMMIT THIS.

This is jsut an example how to use FOO_PERMISSIONS_FILE.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/busybox/busybox.mk | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index f4a241d..9c1f2e8 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
 BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
 BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+BUSYBOX_PERMISSIONS_FILE = $(@D)/busybox.permissions
+define BUSYBOX_GEN_PERMISSIONS
+	for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE" $(@D)/include/applets.h \
+		| sed -e 's/,.*//' -e 's/.*(//'`; \
+	do \
+		temp=`grep -w $${app} $(@D)/busybox.links`; \
+		if [ -n "$${temp}" ]; then \
+			echo "$${temp} f 4755 0  0 - - - - -"
+		fi; \
+	done >$(BUSYBOX_PERMISSIONS_FILE)
+endef
+BUSYBOX_POST_INSTALL_TARGET_HOOKS += BUSYBOX_GEN_PERMISSIONS
+else
 define BUSYBOX_PERMISSIONS
 	/bin/busybox                     f 4755 0  0 - - - - -
 endef
+endif
 
 # If mdev will be used for device creation enable it and copy S10mdev to /etc/init.d
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),y)
-- 
2.7.4