Modify

Opened 8 years ago

Last modified 4 years ago

#7667 reopened defect

preserving SUID/SGID permissions when building images

Reported by: ermo <rune.morling+openwrt@… Owned by: jow
Priority: normal Milestone: Barrier Breaker 14.07
Component: toolchain Version:
Keywords: permissions image rootfs tar image.mk suid sgid Cc:

Description

I'm trying to build a customized squashfs4 image for my NSLU2 (from the backfire branch) which includes znc, sudo and a smattering of network troubleshooting tools.

I want to run znc as an unprivileged user and allow said user to also be able to use selected network troubleshooting tools. However, sudo appears to have been stripped of its SUID bit in the final image, which unfortunately makes it non-functional.

After spending some time figuring out how the build system works, I think I have pinpointed the errors leading to the non-functional sudo, as it appears that the sudo Makefile correctly sets the SUID bit.

1) In ipkg-build (creating the sudo .ipk ), tar is invoked as a non-root user, which means it does NOT preserve permissions (which it does when invoked as root). Adding the '-p' parameter to the tar command sequences that create/extract the internal data.tar.gz data makes ipkg-build preserve permissions inside the finished .ipk

2) In ipkg (extracting the sudo .ipk to the directory the image is built from) tar is also invoked as a non-root user. Again, adding the '-p' parameter to the relevant tar command sequences makes ipkg preserve permissions when unpacking the .ipk to the rootfs directory.

3a) In include/image.mk, binaries with their executable bit set are coerced into mode 0755, which strips SUID/SGID/Sticky bits. Adding an extra check to avoid messing with binaries with SUID/SGID bits fixes this.

3b) Also, the tar commands responsible for generating a tar.gz of the contents of the rootfs directory do not supply the -p parameter as described above.

On my system, fixing all of the above appears to make the build system preserve the SUID/SGID bits in the finished images. As an aside, mksquashfs4 preserves permissions by default.

Attachments (6)

ipkg-build-tar-preserve-permissions.patch (528 bytes) - added by ermo <rune.morling+openwrt@… 8 years ago.
ipkg-build -- preserve permissions when creating data.tar.gz
ipkg-tar-preserve-permissions.patch (1.2 KB) - added by ermo <rune.morling+openwrt@… 8 years ago.
ipkg -- preserve permissions when unpacking .ipk
image.mk-preserve-permissions.patch (1.1 KB) - added by ermo <rune.morling+openwrt@… 8 years ago.
include/image.mk -- don't clobber SUID/SGID bits in rootfs binaries and preserve permissions in rootfs.tar.gz
ipkg-build-tar-preserve-permissions_v2.patch (562 bytes) - added by ermo <rune.morling+openwrt@…> 7 years ago.
Updated ipkg-build patch; apply in backfire/build_dir/host/ with 'patch -p0 < ipkg-build-tar-preserve-permissions_v2.patch'
image.mk-preserve-permissions_v2.patch (1.2 KB) - added by ermo <rune.morling+openwrt@…> 7 years ago.
Update image.mk patch; apply in backfire/ with 'patch -p0 < image.mk-preserve-permissions_v2.patch'
image.mk-normalize_permissions.patch (1.8 KB) - added by ermo <rune.morling+openwrt@…> 7 years ago.
Make image.mk be a little smarter about permissions during its normalization pass

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by ermo <rune.morling+openwrt@…

ipkg-build -- preserve permissions when creating data.tar.gz

Changed 8 years ago by ermo <rune.morling+openwrt@…

ipkg -- preserve permissions when unpacking .ipk

Changed 8 years ago by ermo <rune.morling+openwrt@…

include/image.mk -- don't clobber SUID/SGID bits in rootfs binaries and preserve permissions in rootfs.tar.gz

comment:1 Changed 8 years ago by anonymous

Steps to reproduce:

Build an image including sudo in the squashfs and notice how sudo will not have its SUID bit set correctly in the following locations:

  • the build_dir/target-ARCH/root-ARCH directory and
  • the build_dir/target-ARCH/sudo-<version>/ipkg-ARCH
  • the staging_dir/target-ARCH/root-ARCH directory
  • the finished squashfs

With the attached patches applied, the permissions look like this on my system:

<ermo@dante>:~/work/OpenWrt/backfire
(2)$ find -name sudo -type f -exec ls -l {} \;
-rwsr-xr-x 1 ermo ermo 114004 2010-07-25 16:40 ./build_dir/target-armeb_v5te_uClibc-0.9.30.1_eabi/root-ixp4xx/usr/bin/sudo
-rwxr-xr-x 2 ermo ermo 139627 2010-07-25 16:40 ./build_dir/target-armeb_v5te_uClibc-0.9.30.1_eabi/sudo-1.7.2p6/ipkg-install/usr/bin/sudo
-rwxr-xr-x 1 ermo ermo 139627 2010-07-25 16:40 ./build_dir/target-armeb_v5te_uClibc-0.9.30.1_eabi/sudo-1.7.2p6/sudo
-rwsr-xr-x 1 ermo ermo 114004 2010-07-25 16:40 ./build_dir/target-armeb_v5te_uClibc-0.9.30.1_eabi/sudo-1.7.2p6/ipkg-ixp4xx/sudo/usr/bin/sudo
-rwsr-xr-x 1 ermo ermo 139627 2010-07-25 16:40 ./staging_dir/target-armeb_v5te_uClibc-0.9.30.1_eabi/root-ixp4xx/usr/bin/sudo
<ermo@dante>:~/work/OpenWrt/backfire
(3)$ staging_dir/host/bin/unsquashfs4 -ll bin/ixp4xx/openwrt-ixp4xx-generic-squashfs.img |egrep '\/sudo$'
-rwsr-xr-x root/root            114004 2010-07-25 16:40 squashfs-root/usr/bin/sudo
<ermo@dante>:~/work/OpenWrt/backfire
(4)$ 

comment:2 Changed 7 years ago by jow

  • Owner changed from developers to jow
  • Status changed from new to accepted

Changed 7 years ago by ermo <rune.morling+openwrt@…>

Updated ipkg-build patch; apply in backfire/build_dir/host/ with 'patch -p0 < ipkg-build-tar-preserve-permissions_v2.patch'

Changed 7 years ago by ermo <rune.morling+openwrt@…>

Update image.mk patch; apply in backfire/ with 'patch -p0 < image.mk-preserve-permissions_v2.patch'

comment:3 Changed 7 years ago by jow

  • Resolution set to fixed
  • Status changed from accepted to closed

Added in r26258 and r26259 - thank you!

comment:4 Changed 7 years ago by ermo <rune.morling+openwrt@…>

  • Resolution fixed deleted
  • Status changed from closed to reopened

After much discussion, I propose a slightly tweaked normalization approach which only executes find once (thanks to markmentovai for pointers):

# Note that 'a=rX,u+w,go-w' is the same as 0644 or 0755 (preserving executable bits)

- $(FIND) $(TARGET_DIR) \( -type f -o -type d \) \
-a ! \( -perm /6000 -o -perm /0004 -o -regex '(ssh_host_|_host_key|passwd|shadow)' \) -print0 \
| $(XARGS) -0 chmod a=rX,u+w,go-w

# or (dumping SGID)

- $(FIND) $(TARGET_DIR) \( -type f -o -type d \) \
-a ! \( -perm /4000 -o -perm /0004 -o -regex '(ssh_host_|_host_key|passwd|shadow)' \) -print0 \
| $(XARGS) -0 chmod a=rX,u+w,go-w

The idea is to avoid normalizing stuff that is either:

  • setuid/setgid
  • NOT world readable to begin with (/etc/shadow- or /etc/sudoers for instance)
  • NOT known to be sensitive (ssh host keys are considered sensitive)

Furthermore, the above expression is POSIX compliant (the currently used expression using '-not' and '+0100' isn't)

In addition, it might be beneficial to list the 'special' files during a verbose build (make V=99) like so:

# Explicitly print out the stuff we didn't touch
- $(FIND) $(TARGET_DIR) \( -type f -o -type d \) \
-a \( -perm /6000 -o -perm /0004 -o  -regex '(ssh_host_|_host_key|passwd|shadow)' \) -ls

To confirm that the squashfs is normalized correctly, the following test can be performed:

staging_dir/host/bin/unsquashfs4 -ll <squashfs4 image file> | egrep '^.((..s......)|(.....S...)|(......-..)) '

As a final note, unfortunately it appears that mkfs.jffs strips suid/sgid bits :(

comment:5 Changed 7 years ago by ermo <rune.morling+openwrt@…>

Hrm. Previous comment had faulty find logic.

Here's an attempt at correcting it (which might be both GNU/FreeBSD/Mac OS X compatible):

define Image/mkfs/prepare/default
	# Make the normalization pass a little smarter by not touching stuff
	# which is either a file or a directory AND either of the following:
	# * has its SUID or SGID bits set
	# * is not world readable
	# * is known to be sensitive
	# Note that a=rX,u+w,go-w is equal to 0644 or 0755
	- $(FIND) $(TARGET_DIR) \( -type f -o -type d \) -a ! \( \
	-perm /4000 -o ! -perm /0004 \
	-o -regex '.*\(ssh_host_\|_host_key\)' -o -regex '.*\/etc/\(passwd\|shadow\)-?' \
	\) -print0 | $(XARGS) -0 chmod a=rX,u+w,go-w
	# Explicitly print out the stuff we didn't touch (notice the missing '!')
	- $(FIND) $(TARGET_DIR) \( -type f -o -type d \) -a \( \
	-perm /4000 -o ! -perm /0004 \
	-o -regex '.*\(ssh_host_\|_host_key\)' -o -regex '.*\/etc/\(passwd\|shadow\)-?' \
	\) -ls
	# *phew*
	$(INSTALL_DIR) $(TARGET_DIR)/tmp
	chmod 0777 $(TARGET_DIR)/tmp
endef

Here's a test expression that can be used in e.g. OpenWrt/<branch/

find . \( -type f -o -type d \) -a \( \
-perm /4000 -o ! -perm /0004 \
-o -regex '.*\(ssh_host_\|_host_key\)' -o -regex '.*\/etc/\(passwd\|shadow\)-?' \
\) -ls

As an aside, it appears that the dropbear package doesn't even attempt to protect its host keys?

comment:6 Changed 7 years ago by ermo <rune.morling+openwrt@…>

Nvm on the dropbear comment. It appears that the keys are secured when they are generated (the files in the dropbear package are empty).

Changed 7 years ago by ermo <rune.morling+openwrt@…>

Make image.mk be a little smarter about permissions during its normalization pass

comment:7 Changed 7 years ago by ermo <rune.morling+openwrt@…>

Cleaned up image.mk normalization pass permission handling.

Example output:

rm -f /home/ermo/work/OpenWrt/backfire/build_dir/linux-brcm47xx/tmpfile.*
# Note that a=rX,u+w is equal to 0644 or 0755
/usr/bin/find /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx '(' -type f -o -type d ')' -a '(' -path '*/etc/config/*'  -o '(' '(' -perm /0004 -a ! -perm /6000 ')' -a ! '(' -path '*ssh_host*' -o -path '*_host_key*' -o -path '*/etc/passwd*' -o -path '*/etc/shadow*' ')' ')' ')' -print0 | xargs -r -0 chmod -vv a=rX,u+w | grep -v retained
mode of `/home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/config/ntpclient' changed to 0644 (rw-r--r--)
mode of `/home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/config/uhttpd' changed to 0644 (rw-r--r--)
mode of `/home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/config/upnpd' changed to 0644 (rw-r--r--)
# Explicitly print out the stuff we didn't touch (notice the '!')
/usr/bin/find /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx '(' -type f -o -type d ')' -a ! '(' -path '*/etc/config/*'  -o '(' '(' -perm /0004 -a ! -perm /6000 ')' -a ! '(' -path '*ssh_host*' -o -path '*_host_key*' -o -path '*/etc/passwd*' -o -path '*/etc/shadow*' ')' ')' ')' -ls
5522939    4 drwxr-x---   2 ermo     ermo         4096 Mar 25 19:29 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/lib/firmware/b43
5392245  148 -rwsr-xr-x   1 ermo     ermo       148799 Mar 22 15:31 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/usr/bin/sudo
5391326    0 -rw-r--r--   1 ermo     ermo            0 Mar 22 15:26 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/dropbear/dropbear_dss_host_key
5391327    0 -rw-r--r--   1 ermo     ermo            0 Mar 22 15:26 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/dropbear/dropbear_rsa_host_key
5391080    4 -rw-r--r--   1 ermo     ermo          198 Mar 14 12:38 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/passwd
5391919    4 -rw-------   1 ermo     ermo         1551 Mar 22 15:38 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/collectd.conf
5522982    4 -rw-------   1 ermo     ermo           41 Mar 22 15:40 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/ppp/chap-secrets
5392249    4 -r--r-----   1 ermo     ermo         2849 Mar 22 15:31 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/etc/sudoers
# *phew*
install -d -m0755 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/tmp
chmod 0777 /home/ermo/work/OpenWrt/backfire/build_dir/target-mipsel_uClibc-0.9.30.1/root-brcm47xx/tmp

comment:8 Changed 7 years ago by ermo <rune.morling+openwrt@…>

I should probably mention that the idea with the hardcoded exception list is to demonstrate that it is feasible to make the build system support the scenario where a custom built image has one or more non-root users and we want to ensure that basic security is not compromised if busybox is built w/shadow passwords enabled.

I completely understand that this may be somewhat out of scope for the default use case, though.

I guess the more important question is whether or not to keep the normalization pass, noting that a sane umask is already set when building packages.

comment:9 Changed 7 years ago by jow

  • Milestone changed from Backfire 10.03.1 to Attitude Adjustment (trunk)
  • Version Backfire 10.03 deleted

This is not going to happen for backfire, moving it to the next major.

comment:10 Changed 4 years ago by jow

  • Milestone changed from Attitude Adjustment 12.09 to Barrier Breaker 14.07

Milestone Attitude Adjustment 12.09 deleted

Add Comment

Modify Ticket

Action
as reopened .
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.