Re: _ftello() modification requires additional capsicum rights, breaking tcpdump and dhclient

On Mon, Sep 8, 2014 at 4:42 PM, Andrey Chernov <ache_at_freebsd.org> wrote:

> On 09.09.2014 0:28, Patrick Kelsey wrote:
> > In r268997, _ftello() was modified to use _fcntl(F_GETFL) in the
> > non-append, write-only path.  Consequently, programs that use _ftello()
> > (via ftell, fgetpos, fsetpos, fseek, rewind...) on non-append,
> > write-only files and that use capsicum to restrict capabilities on the
> > associated fds to [CAP_SEEK, CAP_WRITE] broke as all ftell() (and
> > friends) calls on those files fail with ENOTCAPABLE due to lack of
> > CAP_FCNTL rights.  There appear to be only two affected programs in the
> > tree - tcpdump and dhclient.  This affects both CURRENT and 10-STABLE
> > (including 10.1-PRERELEASE)
> >
> > tcpdump, when configured to write to capture files rotated by size,
> > fails to rotate and captures indefinitely to the first file in the
> > series.  This can be reproduced by a command such as: tcpdump -i
> > <ifname> -C 1 -W 2 -w packets -v
> >
> > By inspection, dhclient will fail to trim old data from its client
> > leases file when rewriting that file with a lesser amount of data than
> > it currently contains.  See the ftruncate() call in
> > dhclient.c:rewrite_client_leases().
> >
> > The attached patch adds CAP_FCNTL to the limited rights established for
> > non-append, write-only files used by tcpdump and dhclient.  It also
> > restricts the fcntl rights to CAP_FCNTL_GETFL.
> >
> > The current need to have CAP_FCNTL rights in order to get or set the
> > file position on non-append, write-only files is subtle.  Perhaps part
> > of the answer is to define a CAP_FSEEK right in sys/capability.h that
> > resolves to CAP_SEEK|CAP_FCNTL, or to modify the CAP_SEEK description in
> > rights(4) to note the need for CAP_FCNTL when using ftell() and friends.
> >
> > -Patrick
>
> Stdio code use fcntl(F_GETFL) already in many places, f.e. fdopen(),
> freopen(). libc code in general use it in rpc code. According to your
> note, all that places are currently broken in anyway.
>

You make a godo point about the wider use of fcntl() in libc - aside from
the rpc code, by my count there are 14 other entry points in libc that use
fcntl in their implementation.  To experience breakage, programs that use
those entry points would also have to be supplying them fds with restricted
rights that do not include CAP_FCNTL.  By my count, there are currently
only 12 programs in -current that call cap_rights_limit().  I don't think
these counts inform us very well as to the presence and extent of any
capsicum+libc issues similar to the one that I've raised.  Those 12
programs mentioned above would have to be audited to determine if any of
the 15 libc entry points (including fcntl) that use fcntl are being used on
those restricted fds without being granted CAP_FCNTL rights, and whether
there are overt or potential failures occurring as a result.  Consider that
the failure mode in tcpdump that I found requires that you be using
multiple capture files with size-based rotation, otherwise all works fine.
Also consider that the failure mode in dhclient only occurs when a
rewritten client lease file is smaller than its predecessor.


>
> I don't think that this read-only fcntl(F_GETFL) which doesn not modify
> anything deserves any special rights at all (i.e. can be just enabled by
> default in contrast to F_SETFL), but I am not capsicum expert.
>

I don't think I am in a position to comment on the implications of
permanent F_GETFL rights either.  I do think that the point about wider use
of fcntl(F_GETFL) in libc does argue against making a CAP_FSEEK right in
sys/capability.h, as it would appear users of capsicum and libc are more in
need of a map of capsicum rights required by libc entry points than they
are of convenience #defines.

-Patrick