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

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

> On 09.09.2014 1:13, Patrick Kelsey wrote:
> > 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.
>
> Just to note by quick glance:
> tcpdump use fdopen(), so in some cases probably already broken without
> F_GETFL rights.
> openssh use fdopen(), so suspicious about F_GETFL too, but I don't
> traverse the order in which fdopen() and cap_rights_* there are applied.
>

I have now looked at all of the programs in -current that call
cap_rights_limit() (dhclient, hastd, ping, tcpdump, rwhod, ctld, iscsid,
kdump, rwho, units, uniq, and sshd) and examined them to see which file
descriptors cap_rights_limit() is invoked on, with what rights, and whether
libc functions that require fcntl rights (fcntl, fdopendir, fdopen,
freopen, fseek, ftell, popen, lockf, etc) are subsequently used on those
descriptors.  In most cases, the programs are simple and/or the application
of cap_rights_limit() is otherwise limited in scope, and it is easy to see
that they have sufficient rights on the restricted fds for the operations
performed on those fds.  This was a mostly manual inspection, and of course
I may have missed something, but I did not find any further issues related
to insufficient capsicum rights when using libc.

In the case of tcpdump, fdopen() is not used on a file descriptor whose
rights have been restricted via cap_rights_limit().

In the case of openssh, cap_rights_limit() is used by sshd to sandbox the
unprivileged child process when using privilege separation by restricting
the child's stdin, stdout, and stderr, the child's end of the socketpair
used to communicate with the privileged parent and the child's end of the
pipe used to log to the privileged parent.  fdopen() is not used on any of
those descriptors.


> >     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.
>
> Theoretically it will be possible to get rid of fcntl(F_GETFL) in
> fseek(), but O_APPEND flag need to be stored somewhere in that case, and
> stdio _flags already have all bit occupied for 16bit short. So the price
> will be changing size of the main stdio structure __sFILE to add new
> space for flags, which is undesirable I think.
>
>
I don't think it is worth the trouble, as given the larger pattern of libc
routines requiring multiple capsicum rights, it seems one will in general
have to have libc implementation knowledge when using it in concert with
capsicum.  For example, consider the limitfd() routine in kdump.c, which
provides rights for the TIOCGETA ioctl to be used on stdout so the eventual
call to isatty() via printf() will work as intended.

I think the above kdump example is a good one for the subtle issues that
can arise when using capsicum with libc.  That call to isatty() is via a
widely-used internal libc routine __smakebuf().  __smakebuf() also calls
__swhatbuf(), which in turn calls _fstat(), all to make sure that output to
a tty is line buffered by default.  It would appear that programs that
restrict rights on stdout without allowing CAP_IOCTL and CAP_FSTAT could be
disabling the normally default line buffering when stdout is a tty.  kdump
goes the distance, but dhclient does not (restricting stdout to CAP_WRITE
only).

In any event, the patch attached to my first message is seeming like the
way to go.

-Patrick