SUID Disabler and Permission Hardener

Nice changes by @madaidan:
SUID Disabler and Permission Hardener: Difference between revisions - Whonix

Inspired me to improve a bit more:
SUID Disabler and Permission Hardener: Difference between revisions - Whonix



Great idea!

dpkg-statoverride does not support that yet. feature request: dpkg-statoverride: support for capabilities

Meaning there might be no standard way in which packages implement setting capabilities yet. Therefore I wanted to look at a package that sets capabilities. For example package iputils-ping contains capability enabled binary ping. From the package postinst script:

/var/lib/dpkg/info/iputils-ping.postinst

#!/bin/sh

set -e

if [ "$1" = configure ]; then
    # If we have setcap is installed, try setting cap_net_raw+ep,
    # which allows us to install our binaries without the setuid
    # bit.
    if command -v setcap > /dev/null; then
        if setcap cap_net_raw+ep /bin/ping; then
            chmod u-s /bin/ping
        else
            echo "Setcap failed on /bin/ping, falling back to setuid" >&2
            chmod u+s /bin/ping
        fi
    else
        echo "Setcap is not installed, falling back to setuid" >&2
        chmod u+s /bin/ping
    fi
fi



exit 0

# Local variables:
# mode: shell-script
# tab-width: 4
# indent-tabs-mode: nil
# end:

dpkg -S /sbin/setcap

libcap2-bin: /sbin/setcap

cat debian/control | grep cap

Recommends: libcap2-bin

So if libcap2-bin does not get installed early enough (before iputils-ping) is installed, the Debian maintainer script will set SUID instead of using capabilities. Assuming that other packages might be implemented in a similar way, we should find a way to make sure package libcap2-bin is installed during the build process as early as possible.

Other random examples having a similar use of setcap:

grep -rl setcap /var/lib/dpkg/info

/var/lib/dpkg/info/iproute2.config
/var/lib/dpkg/info/iproute2.postinst
/var/lib/dpkg/info/libcap2-bin.md5sums
/var/lib/dpkg/info/libcap2-bin.list
/var/lib/dpkg/info/libgstreamer1.0-0:amd64.postinst
/var/lib/dpkg/info/gnome-keyring.postinst
/var/lib/dpkg/info/wireshark-common.postinst
/var/lib/dpkg/info/wireshark-common.templates
/var/lib/dpkg/info/iproute2.templates
/var/lib/dpkg/info/kinit.postinst
/var/lib/dpkg/info/iputils-ping.postinst

Meanwhile for packages by Whonix or Kicksecure we can just add Depends: libcap2-bin in debian/control and run setcap during postinst.

Happy to work towards a completely SUID / SGID free by default system. Worthwhile development goal to even get rid of Whonix / Kicksecure requiring sudo / any /etc/sudoers.d exceptions?

Would be specifically interesting in context of:

Now this list of sudoers exceptions needs to be ported to capabilities.

./sdwdate/etc/sudoers.d/sdwdate
./dist-base-files/etc/sudoers.d/30_default-password-lecture
./anon-gw-anonymizer-config/etc/sudoers.d/anonymizer-config-gateway
./tor-control-panel/etc/sudoers.d/tor-control-panel
./tor-control-panel/etc/sudoers.d/restart-tor-gui
./uwt/etc/sudoers.d/uwt
./security-misc/etc/sudoers.d/xfce-security-misc
./security-misc/etc/sudoers.d/security-misc
./security-misc/etc/sudoers.d/pkexec-security-misc
./tb-updater/etc/sudoers.d/tpo-downloader
./whonix-setup-wizard/etc/sudoers.d/whonix-setup-wizard
./usability-misc/etc/sudoers.d/tunnel_unpriv
./usability-misc/etc/sudoers.d/sudo-lecture-disable
./usability-misc/etc/sudoers.d/pwfeedback
./usability-misc/etc/sudoers.d/user-passwordless
./usability-misc/etc/sudoers.d/upgrade-passwordless
./live-config-dist/etc/sudoers.d/live-config-dist
./qubes-whonix/etc/sudoers.d/qubes-whonix
./whonixsetup/etc/sudoers.d/whonixsetup
./sdwdate-gui/etc/sudoers.d/sdwdate-gui
./whonix-xfce-desktop-config/etc/sudoers.d/whonix-xfce-desktop-config
./vm-config-dist/etc/sudoers.d/power-savings-disable-in-vms
./whonixcheck/etc/sudoers.d/whonixcheck
./msgcollector/etc/sudoers.d/msgcollector
./tb-starter/etc/sudoers.d/tb-starter

Help welcome!

Would we need a wrapper around setcap? Because https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502580 mentions several conditions under which setcap can fail.

  • Take into account systems no supporting fcaps, this includes:
    • (for now) non-Linux systems,
    • Linux w/ old kernels or kernels w/o fcap compiled in,
    • file systems not supporting fcaps, and/or w/o mount time enabled
      xattr.

Or don’t care much about this and just ignore errors using setcap [...] || true?

Added to wiki:

  • It does not search folders /root because no SUID binaries should be there by default. That folder is by default readable only by root. If root was to create a custom SUID and move it there, then root should be able to execute it.

Good point. Now mentioned in wiki.

1 Like

Permission Hardener does already support setting capabilities: https://github.com/Whonix/security-misc/blob/master/usr/lib/security-misc/permission-hardening#L458

This is used by anon-apps-config to remove capabilities from ping: https://github.com/Whonix/anon-apps-config/blob/master/etc/permission-hardening.d/30_ping.conf

The lack of dpkg-statoverride support is an issue however. Maybe we can implement something ourselves via an apt / dpkg hook? I.e. on package upgrade, it checks /etc/permission-hardening.d/ for any needed capabilities and if so, sets them.

Capabilities don’t have the same configurability as sudoers exceptions. We cannot restrict which user can execute the program with higher privileges or what arguments they can use.

It may even be better to replace capabilities or setuid with sudoers exceptions as we can restrict execution of those binaries to specific users. For example, if /bin/example is setuid, it can be executed by any user and potentially exploited whereas if a sudoers exception was made, it will only be able to be executed by user user, preventing a compromised sdwdate user from exploiting that program.

Those conditions seem rather niche so yes.

1 Like

We should add support for basic bash/AppArmor-style regex so we can, for example, shorten:

/bin/ nosuid
/usr/local/bin/ nosuid
/usr/bin/ nosuid
/usr/local/usr/bin/ nosuid
/sbin/ nosuid
/usr/local/sbin/ nosuid
/usr/sbin/ nosuid
/usr/local/usr/sbin/ nosuid
/lib/ nosuid
/usr/local/lib/ nosuid
/lib32/ nosuid
/usr/local/lib32/ nosuid
/lib64/ nosuid
/usr/local/lib64/ nosuid
/usr/lib/ nosuid
/usr/local/usr/lib/ nosuid
/usr/lib32/ nosuid
/usr/local/usr/lib32/ nosuid
/usr/lib64/ nosuid
/usr/local/usr/lib64/ nosuid

to:

/{,usr/,usr/local/}{,s}bin/ nosuid
/{,usr/,usr/local/}lib{,32,64}/ nosuid

like in apparmor-profile-everything.

Also, do directories like /usr/local/usr/bin/ even exist?

1 Like

Issue indeed… The essential issue is this…
(You probably know but I am writing this down as reminder to self and everyone else reading.)

sudo getcap /bin/ping

(Capabilities of /bin/ping already removed.)

sudo apt install --reinstall iputils-ping 
sudo getcap /bin/ping

/bin/ping = cap_net_raw+ep

Even if we could, there would a race condition. A time window of vulnerability. Malware with the ability to abuse a capability could use inotifywait (or some other mechanism (perhaps brute force trying)) to wait until the binary is updated and the capability reinstated.

Perhaps once [1] Multiple Boot Modes for Better Security: an Implementation of Untrusted Root existed, malware lurking under user user should be deactivated once rebooted into admin mode. Then malware running under user user couldn’t exploit the vulnerable time window of the capability briefly being re-introduced. If we go for that, we should probably remove upgrade-nonroot command. It would still be an incomplete solution until… Even if rebooted into admin mode, there might be daemons running under for example user www-data. Once [1] is implemented, probbly when booting into admin mode only a limited amount of systemd services should be executed.

Nothing really is as great as a proper implementation of dpkg-statoverride. In case of permissions, dpkg never changes back to the different/original/weaker permissions (won’t re-enable SUID even briefly).

Which kind of configurablity do we need? Can we re-implement these?

Can the output of whoami or some alternative command be trusted? Or can that be fooled with some LD_PRELOAD trick or something?

Let’s pick livecheck.sh [archive] as an example. The script currently uses sudo --non-interactive /bin/lsblk --noheadings --all --raw --output RO. This could probably be translated to some capability.

  1. You’re right. We don’t want all users to have that capability. If output of whoami or similar could be trusted, the script itself could check if it’s running under user user and exit if not so?
  2. The capability might give access to a lot more than sudo --non-interactive /bin/lsblk --noheadings --all --raw --output RO.

Am I re-inventing SUID or sudo here?

My conclusion for now: An SUID free desktop system (including free of sudo) is impractical.

Now on a second thought… Perhaps livecheck.sh could be implemented in a more sophisticated way. A systemd unit running as root [2] could run sblk --noheadings --all --raw --output RO, then write the output to a world readable file. [3] livecheck.sh could then read from that file instead of running a command with SUID (meaning sudo).

Perhaps similar solutions could be invented for other cases currently dependent on a /etc/sudoers.d exception.

Seems difficult. I would not know how to implement this without over complicating the code. I don’t think many people will read that config, let alone edit it or view the logs.

Currently config doesn’t even support white spaces in folder names. (Untested.) Such as basic feature should be implemented before going fancy with bash/AppArmor-style regex?

Config parsing, the script looks complex enough already for my taste.

/usr/local seems to hold anything that also the root / could hold. I found mentions of all newly added entries on Google when putting the serach term into quotes such as.

“/usr/local/usr/bin/”

Also for consistency, thought good to add. At worst in costs a second or so and a log entry.


[2] Or limited user with sudoers exception or capability? Too complex, fancy?
[3] Or only user user? Too complex, fancy?

1 Like

For example, https://github.com/Whonix/whonixcheck/blob/master/etc/sudoers.d/whonixcheck restricts a lot of commands to the whonixcheck user or specific arguments. When switching to capabilities, we can’t really do this.

But we cannot add whoami checks to every program. For example, if instead of the following sudoers exception:

whonixcheck ALL=NOPASSWD: /bin/dmesg

, we added the CAP_SYSLOG capability to the /bin/dmesg binary, any user would now be able to execute /bin/dmesg with that capability. Only by shipping our own /bin/dmesg program would we be able to implement user checks which is infeasible when considering the large amount of programs we’d need to do this for.

Also, LD_PRELOAD wouldn’t work on binaries with capabilities. It’s sanitized to prevent trivial privilege escalation, similar to setuid binaries.

Bash supports this type of regex already.

echo /{,usr/,usr/local/}{,s}bin/

/bin/ /sbin/ /usr/bin/ /usr/sbin/ /usr/local/bin/ /usr/local/sbin/

All we’d need to do on our end is to not handle the strings literally and some basic sanitizing.

This feature wouldn’t only be useful for readability purposes. Permission Hardener currently restricts the permissions of /home/user/:

/home/user/ 0700 user user

But this misses any other user’s home directory whereas if it supported regex, we could change this to:

/home/*/ 0700
1 Like

Why not? Not every program indeed. Just to every script by Whonix currently using sudo. I guess Whonix using /etc/sudoers.d is rather uncommon anyhow, and should be avoided? Not a big deal now. But also not great? Uncommon…

dpkg -S /etc/sudoers.d/*

msgcollector: /etc/sudoers.d/msgcollector
security-misc: /etc/sudoers.d/pkexec-security-misc
usability-misc: /etc/sudoers.d/pwfeedback
qubes-core-agent: /etc/sudoers.d/qt_x11_no_mitshm
qubes-core-agent-passwordless-root: /etc/sudoers.d/qubes
qubes-input-proxy-sender: /etc/sudoers.d/qubes-input-trigger
sudo: /etc/sudoers.d/README
security-misc: /etc/sudoers.d/security-misc
usability-misc: /etc/sudoers.d/sudo-lecture-disable
tb-starter: /etc/sudoers.d/tb-starter
tb-updater: /etc/sudoers.d/tpo-downloader
usability-misc: /etc/sudoers.d/tunnel_unpriv
qubes-core-agent: /etc/sudoers.d/umask
usability-misc: /etc/sudoers.d/upgrade-passwordless
usability-misc: /etc/sudoers.d/user-passwordless
security-misc: /etc/sudoers.d/xfce-security-misc

On my system only Whonix and Qubes dropping files into /etc/sudoers.d.

Could be a configurable wrapper somewhere in /etc, perhaps even .d, to allow extending the list of permitted users per program. A script function to be sourceed so it doesn’t have to be re-invented for every script.

Yes, we certainly shouldn’t add capabilities to /bin/dmesg. What I had in mind…

That’s what I had in mind. Creating wrapper scripts for that. The wrapper script would:

  • check whoami (based on sourceed shell function and config folder)
  • have required capability
  • run command with all parameters (example: /bin/journalctl --boot --no-pager -u whonix-firewall)

Simplified example:

#!/bin/bash

if [ ! “$(whoami)” = “user” ]; then
exit 1
fi

/bin/journalctl --boot --no-pager -u whonix-firewall

In result:

  • Only the whitelisted user(s) would be permitted to run it.
  • Using capabilities.
  • Still only a very specific command.

There would be the risk of malicious input in the script being able to run other commands with the given capability but since these wrapper scripts should be similarly simple as above and not parse any command line parameters I think that risk is worth the gain. In other words, I think it’s very doable to get these scripts right and benefit from a fully SUID free system that doesn’t depend on sudo.

I don’t think we have such a massive amount of /etc/sudoers.d reliance that porting to capability enabled wrapper scripts is infeasible?

Nice, that’s something.

What do you think?

But then my design proposal might fail because capabilities cannot be set on scripts. Only on programs. As per:

Any solution for that? We’d have to invent C(++) based wrappers? Perhaps a thin, perhaps even generic wrapper that sets the capability and then runs the helper script?

I guess the config parser could run the function doing the actual work (which runs find) several times per config entry. But I don’t see much benefit. I don’t foresee a lot users or even packages dropping config snippets there, changing expanding the default configuration. This is not going to be the new /etc/apparmor.d with lots of contributions which really deserves a sophisticated configuration language.

vs

Second one lacks user names.

How to auto detect the user name then? If we want to harden permissions for different user names in /home, wouldn’t it be better to have a special function in the script? In that case would be easier to have a function (feature) parsing /home rather than inventing a complex config parsing just for /home folder permission hardening.

(That feature would then be enabled in config using harden_home_folder_permissions=true or so. Or default buit-in enabled and disabled with harden_home_folder_permissions=false.)

1 Like

This may be a good idea then.

Wrappers in C should be easy to implement.

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <pwd.h>
#include <string.h>

int main() {
  const char *permitted_user = "user";
  struct passwd *current_user = getpwuid(geteuid());

  if (strcmp(current_user->pw_name, permitted_user) == 0) {
    system("/bin/journalctl --boot --no-pager -u whonix-firewall"); 
    exit(0);
  }

  puts("ERROR: You are not the correct user.");
  exit(1);
}

stat -c "%U %G"

1 Like

working fine (tested in qubes) except it all add additional boot delay.

1 Like

related issue:

Performance issues which might cause trouble in Qubes-Whonix:

WIth debugging.

time sudo bash -x /usr/libexec/security-misc/permission-hardening
real	0m7.729s
user	0m0.688s
sys	0m2.020s

Dropping debugging doesn’t make much of a difference.

time sudo /usr/libexec/security-misc/permission-hardening
real 0m8.759s
user 0m0.730s
sys 0m2.100s

Some solution has to be found so it won’t add boot delay.

/usr/bin/time -f %E sudo /usr/libexec/security-misc/permission-hardening

As mentioned in above ticket, I am now considering to not run SUID Disabler at boot time but only at package installation and update time. This would prevent issue Sometimes qubes don't start the first time - If Whonix/Kicksecure Hardening Features for Testers Enabled · Issue #7959 · QubesOS/qubes-issues · GitHub and help to move this ticket forward, i.e. enabling SUID Disabler by default.

SUID Disabler and Permission Hardener is now enabled by default at package installation time.

This is now in the developers repository.

1 Like
1 Like
1 Like

It would also break sudo

1 Like

Done.