Sorry, I forgot CC: to Geoff.
On Fri, Jan 16, 2009 at 12:21:15PM +0100, Karel Zak wrote:
>
> Hi,
>
> the mount command does not work properly if you replace suid with
> POSIX file capabilities. We still need to check for non-root mounts and
> the command has to work in very restricted mode for non-root users.
>
> See the patch below. The author of the patch is Geoff Johnstone.
>
> The patch allows you to remove suid bit from mount and umount. Note
> that you need a system with filesystem capability support, e.g.
> Fedora 10).
>
> # ls -l /bin/mount
> -rwxr-xr-x 1 root root 65192 2008-11-09 22:59 /bin/mount
>
> # getcap /bin/mount
> /bin/mount = cap_dac_override,cap_sys_admin+ep
>
>
> If anyone has any objections, please let me know.
>
> Karel
>
>
> Don't bypass security checks when [u]mount uses POSIX file capabilities
> rather than setuid root to permit non-root mounts.
>
> Signed-off-by: Geoff Johnstone <geoff.johnstone@xxxxxxxxxxxxxx>
> ---
>
> diff --git a/mount/mount.c b/mount/mount.c
> index f92b23c..23f27d6 100644
> --- a/mount/mount.c
> +++ b/mount/mount.c
> @@ -80,8 +80,8 @@ static int list_with_volumelabel = 0;
> */
> static int mounttype = 0;
>
> -/* True if ruid != euid. */
> -static int suid = 0;
> +/* True if (ruid != euid) or (0 != ruid), i.e. only "user" mounts permitted. */
> +static int restricted = 1;
>
> /* Contains the fd to read the passphrase from, if any. */
> static int pfd = -1;
> @@ -774,12 +774,12 @@ guess_fstype_and_mount(const char *spec, const char *node, const char **types,
> }
>
> /*
> - * suid_check()
> + * restricted_check()
> * Die if the user is not allowed to do this.
> */
> static void
> -suid_check(const char *spec, const char *node, int *flags, char **user) {
> - if (suid) {
> +restricted_check(const char *spec, const char *node, int *flags, char **user) {
> + if (restricted) {
> /*
> * MS_OWNER: Allow owners to mount when fstab contains
> * the owner option. Note that this should never be used
> @@ -1109,7 +1109,7 @@ try_mount_one (const char *spec0, const char *node0, const char *types0,
> if (mount_all && (flags & MS_NOAUTO))
> goto out;
>
> - suid_check(spec, node, &flags, &user);
> + restricted_check(spec, node, &flags, &user);
>
> /* The "mount -f" checks for for existing record in /etc/mtab (with
> * regular non-fake mount this is usually done by kernel)
> @@ -1190,7 +1190,7 @@ mount_retry:
> /* Mount failed, complain, but don't die. */
>
> if (types == 0) {
> - if (suid)
> + if (restricted)
> error (_("mount: I could not determine the filesystem type, "
> "and none was specified"));
> else
> @@ -2027,11 +2027,20 @@ main(int argc, char *argv[]) {
> return print_all (types);
> }
>
> - if (getuid () != geteuid ()) {
> - suid = 1;
> - if (types || options || readwrite || nomtab || mount_all ||
> - fake || mounttype || (argc + specseen) != 1)
> - die (EX_USAGE, _("mount: only root can do that"));
> + {
> + const uid_t ruid = getuid();
> + const uid_t euid = geteuid();
> +
> + /* if we're really root and aren't running setuid */
> + if (((uid_t)0 == ruid) && (ruid == euid)) {
> + restricted = 0;
> + }
> + }
> +
> + if (restricted &&
> + (types || options || readwrite || nomtab || mount_all ||
> + fake || mounttype || (argc + specseen) != 1)) {
> + die (EX_USAGE, _("mount: only root can do that"));
> }
>
> atexit(unlock_mtab);
> diff --git a/mount/umount.c b/mount/umount.c
> index 81f8bdf..246bdd7 100644
> --- a/mount/umount.c
> +++ b/mount/umount.c
> @@ -74,8 +74,8 @@ int nomtab = 0;
> /* Call losetup -d for each unmounted loop device. */
> int delloop = 0;
>
> -/* True if ruid != euid. */
> -int suid = 0;
> +/* True if (ruid != euid) or (0 != ruid), i.e. only "user" umounts permitted. */
> +int restricted = 1;
>
> /* Last error message */
> int complained_err = 0;
> @@ -477,7 +477,7 @@ umount_file (char *arg) {
> if (!mc && verbose)
> printf(_("Could not find %s in mtab\n"), file);
>
> - if (suid) {
> + if (restricted) {
> char *mtab_user = NULL;
>
> if (!mc)
> @@ -640,10 +640,18 @@ main (int argc, char *argv[]) {
> usage (stderr, 1);
> }
>
> - if (getuid () != geteuid ()) {
> - suid = 1;
> - if (all || types || nomtab || force || remount)
> - die (2, _("umount: only root can do that"));
> + {
> + const uid_t ruid = getuid();
> + const uid_t euid = geteuid();
> +
> + /* if we're really root and aren't running setuid */
> + if (((uid_t)0 == ruid) && (ruid == euid)) {
> + restricted = 0;
> + }
> + }
> +
> + if (restricted && (all || types || nomtab || force || remount)) {
> + die (2, _("umount: only root can do that"));
> }
>
> argc -= optind;
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Karel Zak <kzak@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
