General package infrastructure - nobody really should not mean 'about everybody'

brada4 commented 4 years ago

Maintainer: NA Environment: OpenWrt Description:

Some packages including some interpreting code, like PHP-FPM, uses user nobody to drop privileges. With all other sandboxing absent them unknowingly give others unnecessary insider privileges on others. They should be isolated into separate users as to isolate them from each other.

neheb commented 4 years ago

Maintainer should be @openwrt/package-maintainers

I mostly agree.

edit: this is not always possible though. I tried doing this with usbmuxd, but failed. It needs direct USB access, which udev gives it under a normal distribution. There's nothing like udev in OpenWrt.

brada4 commented 4 years ago

Something udev/hotplug/setcap style can certainly help to isolate different hardware-accessiing packages, here I am concerned about those userland packages dropping privileges to same user causing unwanted interactions, it is not full root, but thing like php webshell can cause some disaster.

neheb commented 4 years ago

I'll pin this.

dl12345 commented 4 years ago

needs direct USB access, which udev gives it under a normal distribution. There's nothing like udev in OpenWrt.

Actually, hotplug can process almost all uevents. I just finished porting Intel quickassist drivers to Openwrt and the userspace code similarly needs access to the device files. I was able to get hotplug to do it in much the same way as udev does

neheb commented 4 years ago

Seems interesting. Maybe I'll look at it some time.

dl12345 commented 4 years ago

We used to have a package eudev in the LEDE repository, which is Gentoo Linux's port of udev. It was removed some time back.

The udev libraries and tools are in fact still fully functional on openwrt. In particular, udevadm is useful; it's really just hotplug that replaces udevd, but the underlying uevent mechanism works the same. I grabbed the package Makefile from the LEDE repo and modified it so it just installs the libs and udevadm, which you can then use to compile code that needs udev functionality and work out what files you need to create for hotplug to handle the event,

To give an example of hotplug's functionality, here is udevadm monitoring the uevents when I insert all the Intel quickassist drivers (there are multiple modules that create multiple devices) which then requires various things to be done on module insertion and device creation. The thing in parentheses at the end of each line of the output below corresponds to the required hotplug directory name

root@openwrt:~# udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent

KERNEL[91771.619608] add      /module/intel_qat (module)
KERNEL[91771.619634] add      /class/qat_adf_ctl (class)
KERNEL[91771.619650] add      /devices/virtual/qat_adf_ctl/qat_adf_ctl (qat_adf_ctl)
KERNEL[91771.619897] add      /class/qat_dev_processes (class)
KERNEL[91771.619915] add      /devices/virtual/qat_dev_processes/qat_dev_processes (qat_dev_processes)
KERNEL[91771.625848] add      /module/usdm_drv (module)
KERNEL[91771.625865] add      /class/usdm_drv (class)
KERNEL[91771.625917] add      /devices/virtual/usdm_drv/usdm_drv (usdm_drv)
KERNEL[91771.631820] add      /module/qat_c3xxx (module)
KERNEL[91772.119194] bind     /devices/pci0000:00/0000:00:06.0/0000:01:00.0 (pci)
KERNEL[91772.119220] add      /bus/pci/drivers/c3xxx (drivers)
KERNEL[91772.133391] add      /module/qat_api (module)
KERNEL[91772.645935] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio0 (uio)
KERNEL[91772.645955] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio1 (uio)
KERNEL[91772.646009] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio2 (uio)
KERNEL[91772.646024] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio3 (uio)
KERNEL[91772.646058] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio4 (uio)
KERNEL[91772.646091] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio5 (uio)
KERNEL[91772.646130] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio6 (uio)
KERNEL[91772.646172] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio7 (uio)
KERNEL[91772.646210] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio8 (uio)
KERNEL[91772.646243] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio9 (uio)
KERNEL[91772.646284] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio10 (uio)
KERNEL[91772.646325] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio11 (uio)
KERNEL[91772.646340] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio12 (uio)
KERNEL[91772.646384] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio13 (uio)
KERNEL[91772.646420] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio14 (uio)
KERNEL[91772.646450] add      /devices/pci0000:00/0000:00:06.0/0000:01:00.0/uio/uio15 (uio)

Most of these events are received by hotplug and can be processed given the right directory is created. See below for some of the hotplug.d scripts I created and how they correspond to the uevents above:

root@openwrt:/etc/hotplug.d# ls -l
total 64
drwxr-xr-x 2 root root 4096 May 27 17:43 block
drwxr-xr-x 2 root root 4096 May 27 17:43 dhcp
drwxr-xr-x 2 root root 4096 May 27 17:43 firewall
drwxr-xr-x 2 root root 4096 May 27 17:43 ieee80211
drwxr-xr-x 2 root root 4096 May 27 17:43 iface
drwxr-xr-x 2 root root 4096 May 27 17:43 ipsec
drwxrwxr-x 2 root root 4096 May 27 17:43 module
drwxr-xr-x 2 root root 4096 May 27 17:43 neigh
drwxrwxr-x 2 root root 4096 May 27 17:43 net
drwxr-xr-x 2 root root 4096 May 27 17:43 ntp
drwxrwxr-x 2 root root 4096 May 27 17:43 qat_adf_ctl
drwxrwxr-x 2 root root 4096 May 27 17:43 qat_dev_processes
drwxr-xr-x 2 root root 4096 May 27 17:43 tftp
drwxrwxr-x 2 root root 4096 May 27 17:43 uio
drwxr-xr-x 2 root root 4096 May 27 17:43 usbmisc
drwxrwxr-x 2 root root 4096 May 27 17:43 usdm_drv

root@openwrt:/etc/hotplug.d# ls module
01-usdm

root@openwrt:/etc/hotplug.d# cat /etc/hotplug.d/module/01-usdm 
#!/bin/sh

HUGE_PAGE_DIR="/dev/hugepages"

[ "$DEVICENAME" != "usdm_drv" ] && exit 0

[ "$ACTION" == "add" ] && {

    if [ -d ${HUGE_PAGE_DIR} ]; then

        mkdir ${HUGE_PAGE_DIR}/qat 2> /dev/null

        if [ $? -ne 0]; then
            logger -t "quickassist(usdm_drv): error creating ${HUGE_PAGE_DIR}/qat"
        else
            chgrp qat ${HUGE_PAGE_DIR}/qat
            chmod 0770 ${HUGE_PAGE_DIR}/qat
        fi

    else
        logger -t "quickassist(usdm_drv): ${HUGE_PAGE_DIR} not found"
        exit 1
    fi

}

[ "$ACTION" == "remove" ] && {

    rmdir ${HUGE_PAGE_DIR}/qat

}

root@openwrt:/etc/hotplug.d# cat /etc/hotplug.d/usdm_drv/01-usdm 
#!/bin/sh

if [ "$DEVICENAME" != "usdm_drv" ] && [ "$ACTION" != "add" ]; then exit 0; fi

chgrp qat /dev/usdm_drv
chmod 0660 /dev/usdm_drv

root@openwrt:/etc/hotplug.d# cat /etc/hotplug.d/qat_adf_ctl/01-qat_adf_ctl 
#!/bin/sh

if [ "$DEVICENAME" != "qat_adf_ctl" ] && [ "$ACTION" != "add" ]; then exit 0; fi

chgrp qat /dev/qat_adf_ctl
chmod 0660 /dev/qat_adf_ctl

root@openwrt:/etc/hotplug.d# cat /etc/hotplug.d/uio/01-uio 
#!/bin/sh

if [ "$ACTION" != "add" ] && [ "$MAJOR" != "249" ]; then exit 0; fi

chgrp qat /dev/${DEVICENAME}
chmod 0660 /dev/${DEVICENAME}

As an aside, I'm not sure why we have libudev-fbsd. It's designed to allow compilation of software that needs a libudev, but it is a totally simply shim that does not implement many of the functions, so it does not work for any real world application.

The eudev package can co-exist quite peacefully inside openwrt as long as udevd is not installed, which allows for applications that require udev functionality to be compiled and run properly. Sure, you'd need to create custom hotplug rules to handle the uevents, but we should consider bringing back a variant of this package that installs the libs into the staging_dir and a modified version of udevadm on the router that just handles the monitor command.

brada4 commented 4 years ago

I think hotplug-based hardening (actually empowering procd which adds in some IPC events to what normal hotplug sees) deserves another ticket.

The original concern is regarding popular userland tools running as all same user under pretext they drop privilege.

pprindeville commented 4 years ago

The original concern is regarding popular userland tools running as all same user under pretext they drop privilege.

Which implies that if they are buggy or get subverted, they can take down all of their peer services which are running identically...

dl12345 commented 4 years ago

Ensuring packages add their own specific user/group via the relevant option in the package Makefile and then procd_set_param user $name in the init.d script is a relatively trivial thing to do.

Sure, a couple of chown commands in the init.d script might be necessary in some cases, but isolating packages in this way would be easy to accomplish.

The current use of the user nobody seems to result more from the lack of a defined package security policy than any technical reason that I can see.

brada4 commented 4 years ago

@pprindeville I am more afraid of eval() in random CMS thrown under php-fpm. That is, any chance of low-privilege application actually getting into using all of granted privileges.

brada4 commented 4 years ago

@dl12345 better to enhance framework to the degree that individual chown-s and mktemp-s are abstracted away.

dl12345 commented 4 years ago

Theoretically yes, but much harder to achieve on the current code base given that procd's init scripts are actually shell scripts rather than a systemd-like control file.

brada4 commented 4 years ago

@dl12345 there are many sides of it. One is to allocate user per package needing it, another is to teach hotplug some necessary udev tricks, yet more sgid and setcap tricks, then if all else is lacking tooth, make script longer (or call a sourceable shell file 'library' like sysvinit does)

stintel commented 4 years ago

Ensuring packages add their own specific user/group via the relevant option in the package Makefile and then procd_set_param user $name in the init.d script is a relatively trivial thing to do.

Unfortunately this mechanism of adding a user via a package does not work on sysupgrade. This mechanism will have to be improved before we can reliably use it.

brada4 commented 4 years ago

it should work already where user is used only to drop privileges nowhere and package has no persistent data whatsoever ? @stintel ?

stintel commented 4 years ago

it should work already where user is used only to drop privileges nowhere and package has no persistent data whatsoever ? @stintel ?

The problem is that at the end of sysupgrade, the /etc/passwd and /etc/shadow files are restored, wiping the ones contained in the image, and nuking any newly added users with it. If a service then requires one of these newly added users, it will simply not start.

brada4 commented 4 years ago

at runs as `nobody, thats sad.

EricLuehrsen commented 4 years ago

@stintel what do you mean exactly? Example, the Unbound package user "unbound" from the make file seems to survive various upgrade paths. To test Unbound, I have done both soft install and make it static in the upgrade image.

@dl12345 we should probably push more packages to work more like some larger distro. Do not chown within /etc/ on ROM. Instead copy or build configuration within /var/lib/application or /var/run/application. It would be better to chown within tmpfs there.

brada4 commented 3 years ago

What grep still gives (I took out where string was in a patch to replace it etc)

[ ] ./admin/gkrellmd/patches/100-conf.patch
[ ] ./admin/netdata/files/netdata.init
[ ] ./libs/avahi/Makefile
[ ] ./libs/libcups/Makefile
[ ] ./libs/tcp_wrappers/patches/001-debian_subset.patch
[ ] ./utils/at/Makefile
[ ] ./utils/at/files/atd.init
[ ] ./utils/airos-dfs-reset/files/airos-dfs-reset.init
[ ] ./utils/collectd/files/collectd.uci
[ ] ./utils/collectd/files/collectd.init
[ ] ./lang/php8/files/php8-fpm-www.conf
[ ] ./lang/php7/files/php7-fpm-www.conf
[ ] ./net/nginx/patches/nginx/200-config.patch
[ ] ./net/fakepop/files/fakepop
[ ] ./net/tinyproxy/files/tinyproxy.config
[ ] ./net/https-dns-proxy/files/https-dns-proxy.init
[ ] ./net/https-dns-proxy/files/https-dns-proxy.config
[ ] ./net/dnscrypt-proxy/files/dnscrypt-proxy.init
[ ] ./net/uanytun/files/uanytun-nocrypt.config
[ ] ./net/rsync/files/rsyncd.conf
[ ] ./net/squid/files/squid.init
[ ] ./net/kcptun/files/kcptun.config
[ ] ./net/kcptun/files/kcptun.init
[ ] ./net/openvpn/files/openvpn.config
[ ] ./net/clamav/Makefile
[ ] ./net/clamav/files/clamav-milter.config
[ ] ./net/clamav/files/clamav.config
[ ] ./net/davfs2/files/davfs2.conf
[ ] ./net/stunnel/files/stunnel.uci
[ ] ./net/stunnel/files/stunnel.conf
[ ] ./net/xray-core/files/xray.init
[ ] ./net/tcpproxy/files/tcpproxy.config
[ ] ./net/tor-hs/files/nextcloud-update.sh
[x] ./net/memcached/files/memcached.config
[x] ./net/memcached/files/memcached.init
[ ] ./net/apinger/patches/002-run_as_user.patch
[ ] ./net/ola/files/olad.init
[ ] ./net/darkstat/files/darkstat.init
[ ] ./net/sslh/files/sslh.init

aparcar commented 3 years ago

What about using the same users as described here? https://gitweb.gentoo.org/repo/gentoo.git/tree/acct-user/

In case some are missing, we can try to make them add them.

brada4 commented 3 years ago

Or fedora's way https://pagure.io/setup/blob/master/f/uidgid

aparcar commented 3 years ago

I don't really care which to chose, the latter seems to be much more incomplete.

brada4 commented 3 years ago

It is not the problem with having a list or not, it is certainly useful where files may be on shared storage - mail, web, kvm etc, ie some niche uses from openwrt's point of view, other users can happily randomize e.g. https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/uidrange.html

aparcar commented 3 years ago

Please see https://github.com/openwrt/packages/pull/16454 for an idea how to fixup existing services.

openwrt / packages

General package infrastructure - nobody really should not mean 'about everybody' #12084