[Bug 277228] Device permissions security hole with partitioning (/dev/geom.ctl)

From: <bugzilla-noreply_at_freebsd.org>
Date: Fri, 23 Feb 2024 11:49:18 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277228

Poul-Henning Kamp <phk@FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |phk@FreeBSD.org

--- Comment #1 from Poul-Henning Kamp <phk@FreeBSD.org> ---
Oh boy...

The 'operator' group predates FreeBSD, and I speculate that the intent was to
enable non-root users to backup filesystems with dump(8), which reads the raw
disk, rather than go through the filesystem.

See for instance 386BSD's MAKEDEV script, which grants 'operator' read access
to disk devices:

    fd*|wd*)
            umask 2 ; unit=`expr $i : '..\(.*\)'`
            case $i in
            fd*) name=fd; blk=2; chr=9;;
            wd*) name=wd; blk=0; chr=3;;
            esac
            case $unit in
            0|1)
                    mknod ${name}${unit}a   b $blk `expr $unit '*' 8 + 0`
                    […]
                    chgrp operator ${name}${unit}[a-h] r${name}${unit}[a-h]
                    chmod 640 ${name}${unit}[a-h] r${name}${unit}[a-h]

When I implemented DEVFS (3f54a085a60) I retained those permissions, which is
why ``sys/conf.h`` knows the numeric gid of the operator group, and for similar
reasons it ended up knowing about a dozen non-zero uids and gids, in order to
stay compatible with userland and user expectations.

I did consider having DEVFS expose everything as ``600 root:wheel`` but there
were no sane way to have a trusted program in userland fix the permissions to
what tradition and users expected.  (Also: RAM was /expensive/, so adding
another background process was unthinkable.)

GEOM had a much tougher row to hoe and it took a number of gyrations to finally
get access to "on-disk metadata" such as disklabels, MBR's and other stuff
sorted out, while trying to remain compatible with what people had on their
disks at the time.

Along that route I tried to get rid of the magic ``GID_OPERATOR`` in the
kernel, but as (0a2ece0481909f7) attest, people relied on it still.

I do not recall anything depending on ``/dev/geom.ctl`` having ``640
root:operator`` permission, and I suspect it may have gotten those as a side
effect of reusing common geom functions.

Now that we have devd(8), we could zap ``UID_*`` and ``GID_*`` from
``sys/conf.h`` and move the adjustment of permissions to userland where it
belongs, and maybe we should, or maybe it is not worth the effort.

However, for multifunctional "control devices", typically named ``*ctl``, the
file system access control is insufficient, because only the parameters to the
actual syscalls can tell if something innocent like ``gpart show`` or
privileged like ``gpart destroy -F`` is being attempted and jails are another
complication.

The ioctl(2) implementation magic know some things about "read/write/both" for
the ioctl(2) argument, but unfortunately that does distinctions do not readily
map to "harmless" vs "consequential", the way it may have back in the 1980'ies.

And we are not just talking ``geom.ctl`` here, other magic devices like
``mlx5ctl``, ``cam/ctl``, ``sa%d.ctl``, ``apmctl`` probably also grant
``GID_OPERATOR`` near-root behaviours.

My suggestion would be to modify ``sys/conf.h`` to define ``GID_OPERATOR`` as
zero, and if nothing very important breaks, leave it like that.

-- 
You are receiving this mail because:
You are the assignee for the bug.