pacman wrapper in order to safely include a wildcard in sudoers

sharethewisdom · 2018-03-20 11:50:50

I'd like some feedback or help. I' drafting scripts in /usr/local/bin that allow me to specify, for example, which flags to allow for a given user/group. I usually only allow an admin account to run some very specific commands as root without password, as shown somewhere in the wiki.

A lot of guides show wildcards in sudoers, which is generally a very bad idea. Unintentionally, an atacker can aquire rights to edit sensitive files or chain commands this way.

/usr/local/bin/functions.sh

# vim:set ai sw=2 ts=2 sts=2:
_owch() {
  if [[ $quiet != 1 ]]; then
    builtin printf "\033[31m%s \033[0m" "fatal: $@" >&2
    echo
  fi
}
_oops() {
  if [[ $quiet != 1 ]]; then
    builtin printf "\033[35m%s \033[0m" "alert: $@" >&2
    echo
  fi
}
_info() {
  if [[ $quiet != 1 ]]; then
    builtin printf "\033[34m%s \033[0m" "$@" >&1
    echo
  fi
}
# ...

The purpose of /usr/local/bin/pac is to allow any user of the group wheel to make common package/group name queries and to disallow changing the filesystem using pacman.

TODO:

better parsing of target
allow more flag combinations
review the purpose and write comments

#!/bin/sh
# vim:set ai sw=2 ts=2 sts=2:
source `dirname $0`/functions.sh
_showUsage() {
  if [[ $quiet != 1 ]]; then
   # TODO: reflect usage
    _info "Usage: $(basename $0) [-hq] -[QS][sil] [PACKAGE]"
  fi
}
_allowed() {
  for arg in "$@"; do
    _owch "That's not allowed. Try $arg." && _cleanup && exit 1
  done
}
_cleanup() {
  return
}

if [ $EUID != 0 ]; then
  _owch "run as root, dummy!"
  builtin exit 1
fi

trap _cleanup INT QUIT
builtin shopt -s extglob

# default to local query
cmd="-Qs"
pkg=()

for arg in $@; do
  if [[ ${arg} = [-]* ]]; then
    case "$arg" in
      -q) quiet=1 && shift
        ;;
      -h) _showUsage && exit 0
        ;;
      -Q) cmd="$arg"
        ;;
      # todo these could be used twice: ktsgiu
      -Q[cdegiklmnopqstu]) cmd="$arg"
        ;;
      -Q[abfhjrvwxyz]) _allowed "-Q[cdegiklmnopqstu]"
        ;;
      -S[silw]) cmd="$arg"
        ;;
      -S*) _allowed "-S[silw]"
        ;;
      -Fs) cmd="$arg"
        ;;
      -*)   _owch "$arg is not allowed!" && _cleanup && exit 1
        ;;
    esac
  else
    case "$arg" in
      [A-Za-z_-]*) pkg+=("$arg")
        ;;
      [[:alpha:]]+2-[[:alpha:]]+) pkg+=("$arg")
        ;;
      *) _owch "$arg is not allowed!" && _cleanup && exit 1
        ;;
    esac
  fi
done

#[[ $# < 2 ]]  && _showUsage && exit 1
[[ -z ${pkg} ]] && _showUsage && exit 1

/usr/bin/pacman --color always "$cmd" "${pkg[@]}"

The purpose of /usr/local/bin/pacu is to allow the admin user to install packages using specific file patterns using makekg/yaourt or tllocalmgr.

#!/bin/sh
# vim:set ai sw=2 ts=2 sts=2:
source `dirname $0`/functions.sh
_usage() {
  _info "Example usage: $(basename $0) [-h] /tmp/yaourt-tmp-admin/*/*.pkg.tar.xz" 
  _info "(Packages in /home/admin/.texlive/texmf-var/arch/builds/ are also allowed.)"
}

deptest=false
upgrade=false
sync=false
color=false
opts=()
err=0

echo "$@"

[[ $# = 0 ]] && _usage && builtin exit 0
for arg in $@; do
  if [[ ${arg} = -* ]]; then
    [[ ${arg} = "-h" ]] && _usage && builtin exit 0
    if [[ ${arg} = -T ]]; then
      deptest=true
    elif [[ ${arg} = -U ]]; then
      upgrade=true
    elif [[ ${arg} = --color ]]; then
      color=true
    elif [[ ${arg} = --asdeps ]]; then
      opts+=($arg)
    elif [[ ${arg} = --needed ]]; then
      opts+=($arg)
    elif [[ ${arg} = -Dk ]]; then
      # Checks that all files owned by the given
      # package(s) are present on the system
      /usr/sbin/pacman -Dk && builtin exit 0
    elif [[ ${arg} = -Q ]]; then
      # I guess yaourt runs makepkg which I think
      # rarely gives an argument to -Q?
      /usr/sbin/pacman -Q && builtin exit 0
    elif [[ ${arg} = -S ]]; then
      sync=true
    else
      err+=1
      _owch "$arg is not allowed!"
      _owch "in $@"
    fi
  fi
done

if ((err)); then
  _owch "$err" && builtin exit 1
fi

pkg=()
if [[ $deptest = true ]] || [[ $sync = true ]]; then
  for arg in $@; do
    # if [[ ${arg} = +([[:alnum:]]) ]] || [[ ${arg} = +([[:alnum:]])-+([[:alnum:]]) ]]; then
      pkg+=("$arg")
    # fi
  done
  [[ $deptest = true ]] && opt=-T
  [[ $sync    = true ]] && opt=-S
  if [[ $color = true ]]; then
    /usr/sbin/pacman --color auto "${opts[@]}" ${opt} "${pkg[@]}"
  else
    /usr/sbin/pacman "${opts[@]}" ${opt} "${pkg[@]}"
  fi
elif [[ $sync = true ]]; then
  if [[ $color = true ]]; then
    /usr/sbin/pacman --color auto "${opts[@]}" -Syuu "${pkg}"
  else
    /usr/sbin/pacman "${opts[@]}" -U "${file}"
  fi
elif [[ $upgrade = true ]]; then
  if [ $EUID != 0 ]; then
    _owch "run as root, dummy!"
    builtin exit 1
  fi
  for arg in $@; do
    if [[ -f ${arg} ]] && [[ ${arg} = /tmp/yaourt-tmp-+([[:alpha:]])/PKGDEST+([[:graph:]]).pkg.tar.xz ]]; then
      file=("$arg")
    elif [[ -f ${arg} ]] && [[ ${arg} = /home/admin/.texlive/texmf-var/arch/builds/+([[:alpha:]])/texlive-local-+([[:graph:]]).pkg.tar.xz ]]; then 
      file=("$arg")
    fi
  done
  if [[ $color = true ]]; then
    /usr/sbin/pacman --color auto "${opts[@]}" -U "${file}"
  else
    /usr/sbin/pacman "${opts[@]}" -U "${file}"
  fi
else
  err+=1
  _owch "there's no valid option in $@"
fi

if ((err)); then
  _owch "$err errors" && builtin exit 1
fi

Next, here's a quick script /usr/local/bin/pacsyu to sync packages from remote repos the way I think is appropriate. I like to ensure that the keyring is up-to-date and download packages without interruption when I'm in a hurry, or on a flaky wireless connection.

TODO:

better parsing of target
change the behaviour when my laptop is wired
put the stuck-in-memory check in a hook

#!/bin/sh
# vim:set ai sw=2 ts=2 sts=2:
# This script parses arguments for pacman to
# only allow strings containing letters
# This is done to ensure that the sudoers file
# does not contain wildcards, which an attacker
# could use to chain arbitrary commands.
source `dirname $0`/functions.sh
_cleanup() {
  return
}

if [ $EUID != 0 ]; then
  _owch "run as root, dummy!"
  builtin exit 1
fi

trap _cleanup INT QUIT
builtin shopt -s extglob

pkg=()
for arg in $@; do
  case "$arg" in
    -h) _info "Usage: $(basename $0) [-h] [PACKAGE]" && exit 0
      ;;
    -d) _owch "You're not allowed to skip dependency version checks"
      ;;
    --force) _owch "You're not allowed to forcefully overwrite files"
      ;;
    [A-Za-z]*[A-Za-z_-]*) pkg+=("$arg")
      ;;
    python2-[a-z_-]*) pkg+=("$arg")
      ;;
    *) _owch "$arg is not allowed!" && _cleanup && exit 1
      ;;
  esac
done

# First run -Sy archlinux-keyring
/usr/bin/pacman -Sy --needed --noconfirm archlinux-keyring
# Retrieve installed packages from the server
/usr/bin/pacman --color always -Squuw --noconfirm
# Ask for up- or downgrade and download additional packages
/usr/bin/pacman --color always -Squu "${pkg[@]}"

# Check for used files (stuck in memory) as an indication for
# reboot or logout.
lsof +c 0 | grep 'DEL.*lib' | awk '1 { printf "%-20s: %s\n", $1, $NF }' | sort -u

These scripts are probably faulty. They're also probably not very flavour consistent. How do I improve these? Do you know of a good parsing or garbage testing tool?

Last edited by sharethewisdom (2018-03-20 12:42:10)

eschwartz · 2018-03-20 13:35:02

I'd like to first point out that you seem to have a misunderstanding of what sudoers allows. You cannot chain commands, `sudo pacman -Qi bash; pacman -U /tmp/badpkg.tar.xz` doesn't work regardless as there is no way to pass multiple commands in one like that without wrapping it in e.g. `bash -c` which is another story...

The sudoers manpage goes into some detail about the dangers of globbing, but specifically references that the * will match "across word boundaries" -- the example it gives is that `/bin/cat /var/log/messages*` would match `sudo cat /var/log/messages.1` but also `sudo cat /var/log/messages /etc/shadow`.

...

To start with, you may want to deduplicate some logic provided by makepkg. Take a look at the excellent messaging functions in /usr/share/makepkg/util/message.sh or, upcoming in pacman 5.1, /usr/share/makepkg/util/parseopts.sh for "exploding" getopt-like short and long messages (so, you wouldn't need to do clumsy matches against -S[silw]* )

Also consider, instead of doing lots of argument parsing and conditional if/else logic, adding detected arguments like --color to a bash array and then simply including the array in the final `pacman` command line.

Moving on to the actual scripts, I'm not sure what the purpose of `pac` is, since "make common package/group name queries and to disallow changing the filesystem using pacman" does not require root permissions at all. Or, well, for some uses of -Q *only* you might need root permissions to check the database consistency, which requires that pacman have permission to open files like /etc/shadow which have 0600 permissions.

You can easily do that with sudoers on its own, just allow /usr/bin/pacman -Q* as any possible combination of options must ultimately pass through -Q which is... pretty safe. You would not, however, be able to use`/usr/bin/pacman * -Q*` as that would match `sudo pacman -U -- -Qbadpkg.tar.xz` so if you really need to match things like --dbpath, --color, --quiet, etc. *before* the -Q then yeah, you'll need a wrapper script, but I think that is silly.

Your pacu command seems to do nothing other than allow any package located in /tmp/yaourt-tmp-* to be freely installed, which means any user can create /tmp/yaourt-tmp-ipwnedyou/ and then install packages there (if they happen to be in the wheel group to get sudoers rights to `pacu`), which defeats the purpose of not just allowing `pacman -U *`
I'm not sure what your selection criteria for what is a safe package is supposed to be anyway.

pacsyu seems more or less okay at a quick glance, assuming that you truly consider it safe for the wheel users to install any package they want as long as it came from a configured repository. Which is, um, probably safe.
I'm not sure why you offer python2-* packages two chances to be allowed.

sharethewisdom · 2018-03-20 23:53:25

Thanks for the feedback and pointing me to het makepkg utilities. I should have read sudoers(5) again, especially "Preventing shell escapes" before waisting time and posting my silly work here...

I intended to add some permission checks in pacu. Indeed, being able to query as a regular user makes pac rather pointless. I remember that I needed more permissions to make pacman queries on another system some time ago. I was using selinux and other restrictions, maybe that was the cause.

To clarify: I don't allow the wheel group to gain root privileges, except for sudoedit.

Anyway,... I'll observe yaourt and other wrappers and I'll look for a fitting sudo configuration for my admin user.

Maybe something like:

Defaults!/usr/sbin/pacman noexec
Defaults:admin !lecture
admin ALL=(root) /usr/bin/su [!-]*, !/usr/bin/su *root*
admin ALL=(root) NOPASSWD: /usr/sbin/pacman -Syu*
admin ALL=(root) NOPASSWD: /usr/sbin/pacman -Rns*
admin ALL=(root) NOPASSWD: /usr/sbin/pacman -U*
# prevent path traversal
admin ALL=(root) !/usr/sbin/pacman *..*
admin ALL=(root) !/usr/sbin/pacman -U *[[\:blank\:]]*
user ALL=(admin) /usr/bin/zsh, /usr/bin/bash

thanks again for your lengthy reply.

Trilby · 2018-03-21 00:05:42

sharethewisdom wrote:

To clarify: I don't allow the wheel group to gain root privileges, except for sudoedit.

Which in turn gives them the power to grant themselves root priveleges to do whatever they want.

sharethewisdom · 2018-03-21 08:01:15

Trilby wrote:

sharethewisdom wrote:
To clarify: I don't allow the wheel group to gain root privileges, except for sudoedit.
Which in turn gives them the power to grant themselves root priveleges to do whatever they want.

How? Do you mean, by design, or because of an exploit? Could you give an example and a sudoers file? To further clarify, wheel users are still required to enter their passphrase when running sudoedit.

btw, I think I'll hash /usr/bin/pacman and /usr/local/bin/pacsyu:

# sed -i "1cCmnd_Alias PAC=sha256:$(openssl dgst -binary -sha256 /usr/bin/pacman|openssl base64) /usr/bin/pacman" /etc/sudoers.d/Cmnd_Alias.conf
# sed -i "2cCmnd_Alias PACSYU=sha256:$(openssl dgst -binary -sha256 /usr/local/bin/pacsyu|openssl base64) /usr/local/bin/pacsyu" /etc/sudoers.d/Cmnd_Alias.conf

Last edited by sharethewisdom (2018-03-21 09:16:38)

progandy · 2018-03-21 08:14:28

@Trilby: sudoedit is not the same as visudo. [Edit: Don't know how I came to that. Maybe that was my first though...]
@sharethewisdom: It depends on your rules. If any of the sudoedit rules allows you to edit /etc/sudoers, then it is insecure.
Edit: This is a good writeup: http://bencane.com/2012/02/26/sudoedit- … dit-files/

Last edited by progandy (2018-03-21 12:24:23)

sharethewisdom · 2018-03-21 10:07:25

would this be ok?

%wheel specifichost=sudoedit /etc/*,!sudoedit *sudoers*,!sudoedit *..*

I think it would prevent editing /etc/sudoers or anything in /etc/sudoers.d/, like

sudoedit /etc/hosts /etc/sudoers
sudoedit /etc/security/../sudoers
sudoedit /etc/sudoers.d/file.conf

progandy · 2018-03-21 10:57:50

Bad idea, you might have prevented direct editing of sudoers, but you can edit /etc/passwd and /etc/shadown to change the root password. Or create a systemd service that will run on the next boot, edit /etc/profile, ...
You should only allow access to specific files, but it would be even better if you could use setfacl to allow wheel write access only to the necessary files without sudoedit.

Last edited by progandy (2018-03-21 10:58:56)

Trilby · 2018-03-21 11:37:39

progandy wrote:

@Trilby: sudoedit is not the same as visudo.

progandy wrote:

Bad idea, you might have prevented direct editing of sudoers, but you can edit /etc/passwd and /etc/shadown to change the root password. Or create a systemd service that will run on the next boot, edit /etc/profile, ...

Precisely my point. If wheel users can edit any file, they can gain root access in several ways I can think of and probably countless many more I wouldn't think of. So trying to enumerate the badness of this idea and patch each problem would account for 2 of the 6 dumbest ideas in computer security.

On top of the bit prograndy mentioned, they could edit the pacu script itself and replace it's contents with any command at all that they wanted, then run it as root. 1) edit pacu to remove all existing content, 2) replace content with a single `printf root:mypassword | chpasswd`, 3) `sudo pacu`, 4) su root.

Last edited by Trilby (2018-03-21 11:44:00)

progandy · 2018-03-21 12:13:04

@Trilby: I expected he had limited the arguments/files for the sudoedit command, not a general rule for everything in /etc. I can't see a way to edit pacu if it is in /usr/... and sudoedit is only allowed for /etc. What will work is editing /etc/pacman.conf and setting XferCommand.
Edit: Oh right. wildcard after /etc, put it as a second argument...

Last edited by progandy (2018-03-21 12:15:46)

Trilby · 2018-03-21 12:16:38

The parameter specification came after my original concern about allowing sudoedit (which originally had no qualification). But my main point stands. Trying to enumerate all the possible ways this could go wrong and patch against only the problems we can foresee is ridiculous. It is inherently insecure.

EDIT: your edit very nicely highlights my point.

Last edited by Trilby (2018-03-21 12:23:09)

eschwartz · 2018-03-21 12:21:16

progandy wrote:

@Trilby: sudoedit is not the same as visudo.

Who said anything about visudo? visudo doesn't even ask for root privileges. You can trivially use it to create or modify any file that your user has read/write privileges to, and it saves the file with 0440 permissions as the running user.

Whether that file is used by sudo itself, depends on whether the sudoers file is loaded by the sudoers policy, which requires being in root-owned "/etc/sudoers*"...

sharethewisdom · 2018-03-21 16:15:33

progandy wrote:

I expected he had limited the arguments/files for the sudoedit command

No worries, I actually did. I've been using setfacl too: the purpose of an 'admin' user is to alleviate the need to run things as root. I quickly brought up the rule above to illustrate patterns. It was late, I was tired.

But you're right @Trilby, one can't enumerate all the possible ways this could go wrong.

Has any of you come across a sudo configuration that would have allowed for arbitrary code execution because of the use of a wildcard? I'd like to have an overview of commands that commonly end up in sudoers, that are susceptible to unintended use, like command wrapping, either by using a plain option or by some sort of magic with control characters and whatnot. Is there any reason, then, to make a wrapper script for a specific command? (apart from the clumsiness of having to allow a set of long options for pacman -U in different arrangements, for example) Which commands would you give the NOEXEC tag?

Trilby · 2018-03-21 16:35:14

sharethewisdom wrote:

But you're right @Trilby, one can't enumerate all the possible ways this could go wrong.
I'd like to have an overview of commands that commonly end up in sudoers, that are susceptible to unintended use

So you acknowledge that it is impossible to enumerate the problems, and you follow up by asking us to enumerate the problems?!

If you don't want a user to have root access on the machine, do not give them root access on the machine. Period.

What is it you think non-root users should be allowed to do? Don't allow them to do everything then try to make exceptions for all the things you can forsee as being a problem, just grant them rights to do just what you know they need to do (again see the linked article - this is now touching on the #1 dumbest idea).

sharethewisdom · 2018-03-21 20:51:23

Trilby wrote:

just grant them rights to do just what you know they need to do

I'm worried that I made you think that I don't practice the "default deny" approach, because of my little suggestive %wheel rule below. In firejail, ublock origin, nftables, my router's config, ... I always take the whitelisting aproach.

I deny admin the use of paman (as root) with all options, except the ones needed for yaourt and tllocalmgr. The clumsiness of having to allow a set of options in different arrangements in /etc/sudoers drove me to creating a script. This also allows me to sanitize the target. At the same time, I expect running a script like pacu as root to be problematic. Anything that parses and spawns other processes is a hazard. That's the reason for my concerns and why I'm sharing some of these /usr/local/bin drafts. In the mean time I took another look at man sudoers(5) and I found the schaXXX: prefix, which nicely complements the chmod 700 I ran on the root:root owned pacu.

I don't think that enumerating badness nescessarily leads to a clouding of judgment or bad practice as suggested in the article. I acknowledged that it is impossible to enumerate all the problems. But I get the point.

Anyway, thanks for the article. I will be unable to answer messages the folowing days. cheers

eschwartz · 2018-03-21 20:58:41

I've uh tried to point out that allowing pacman -U allows one to install arbitrary packages that run arbitrary bash scripts as post_install commands, or install /usr/bin/suid-bash, or do literally every conceivable form of attack...

R00KIE · 2018-03-21 21:05:41

I've only skimmed through the discussion but what stuck from the OP is this:

sharethewisdom wrote:

The purpose of /usr/local/bin/pac is to allow any user of the group wheel to make common package/group name queries and to disallow changing the filesystem using pacman.

Pacman does not need root rights to perform queries, any user can do it.

The rest of the discussion seems to be deviating from the thread title into how to safely configure the sudoers file and use sudo. That said, since you seem to want to prevent other users from doing stupid things™, do you really want to let them run an AUR helper to install packages?

If you can install things from the AUR then anyone can subvert your perfectly crafted sudoers setup, a user only needs to create a package adding a file to sudoers.d granting them rights to do everything. Who's going to be merging pacnew files? I see many corner cases and details that might be more trouble to fine tune than it's worth.

progandy · 2018-03-21 21:06:20

Even if you managed to limit "pacman -U" to packages that come only from the AUR, nothing can stop your admin from uploading a custom package (e.g. a passwordless su) to the AUR and installing it.

Edit: Too slow.

Last edited by progandy (2018-03-21 21:06:46)

eschwartz · 2018-03-21 21:22:29

R00KIE wrote:

Pacman does not need root rights to perform queries, any user can do it.

I mentioned this above, but this is actually untrue. pacman -Qkk requires root in order to open protected files like /etc/shadow and check that their contents match the packaged version. Granted, in this case it is a backup file...

Trilby · 2018-03-21 21:22:54

Perhaps there is some miscommunication here. I don't think anyone in this thread would want to tell you whether or not you should let any user run any and all commands: that's really up to you.

What I - and I'm pretty sure everyon else here - is trying to convey is that your strategies so far in this thread will not do what you seem to want them to do. If it is important to you that wheel users don't get full root access, your current plans/configurations will not acheive that goal as they have more holes than swiss cheese.

You have an extremely complex configuration for the functional equivalent of the more traditional set up of the wheel group having full root access. If this result is acceptable to you, ditch the complexities and just let wheel members do what they want. If this is unacceptable to you, ditch the complexities even faster before one of those untrusted users takes over the system and locks you out.

Last edited by Trilby (2018-03-21 21:24:20)

R00KIE · 2018-03-21 22:14:09

@Eschwartz
Yes I did forget about 'pacman -Qkk' but in my defence the OP did say 'common package/group name queries' I'd say 'pacman -Ss' would fit the description of common better

That said, Trilby summarized it nicely: complex configuration full of holes that a motivated malicious user can get around quite easily.

Arch Linux

#1 2018-03-20 11:50:50

pacman wrapper in order to safely include a wildcard in sudoers

#2 2018-03-20 13:35:02

Re: pacman wrapper in order to safely include a wildcard in sudoers

#3 2018-03-20 23:53:25

Re: pacman wrapper in order to safely include a wildcard in sudoers

#4 2018-03-21 00:05:42

Re: pacman wrapper in order to safely include a wildcard in sudoers

#5 2018-03-21 08:01:15

Re: pacman wrapper in order to safely include a wildcard in sudoers

#6 2018-03-21 08:14:28

Re: pacman wrapper in order to safely include a wildcard in sudoers

#7 2018-03-21 10:07:25

Re: pacman wrapper in order to safely include a wildcard in sudoers

#8 2018-03-21 10:57:50

Re: pacman wrapper in order to safely include a wildcard in sudoers

#9 2018-03-21 11:37:39

Re: pacman wrapper in order to safely include a wildcard in sudoers

#10 2018-03-21 12:13:04

Re: pacman wrapper in order to safely include a wildcard in sudoers

#11 2018-03-21 12:16:38

Re: pacman wrapper in order to safely include a wildcard in sudoers

#12 2018-03-21 12:21:16

Re: pacman wrapper in order to safely include a wildcard in sudoers

#13 2018-03-21 16:15:33

Re: pacman wrapper in order to safely include a wildcard in sudoers

#14 2018-03-21 16:35:14

Re: pacman wrapper in order to safely include a wildcard in sudoers

#15 2018-03-21 20:51:23

Re: pacman wrapper in order to safely include a wildcard in sudoers

#16 2018-03-21 20:58:41

Re: pacman wrapper in order to safely include a wildcard in sudoers

#17 2018-03-21 21:05:41

Re: pacman wrapper in order to safely include a wildcard in sudoers

#18 2018-03-21 21:06:20

Re: pacman wrapper in order to safely include a wildcard in sudoers

#19 2018-03-21 21:22:29

Re: pacman wrapper in order to safely include a wildcard in sudoers

#20 2018-03-21 21:22:54

Re: pacman wrapper in order to safely include a wildcard in sudoers

#21 2018-03-21 22:14:09

Re: pacman wrapper in order to safely include a wildcard in sudoers

Board footer