[Bug 181741] [kernel] [patch] Packet loss when 'control' messages are present with large data (sendmsg(2))

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=181741

Jilles Tjoelker <jilles at FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jilles at FreeBSD.org

--- Comment #15 from Jilles Tjoelker <jilles at FreeBSD.org> ---
I consider the issue brought up in comments 11 and 12 a blocker.

The patch "[PATCH 3/4] uipc_finalizecontrol: read-lock the unp link" introduces
a change not mentioned in the commit message: datagram sockets always behave as
if the LOCAL_CREDS socket option (UNP_WANTCRED flag in the kernel) is set on
them.

That change seems a bad idea to me. Apart from the general problems with
sending unsolicited control messages (full buffers, code that examines only the
first control message if only one is expected), the specifics of SCM_CREDS make
this even more dangerous. A strange thing about SCM_CREDS is that there are two
flavours of it: one containing a struct cmsgcred, generated when the sender
explicitly attaches an SCM_CREDS message, and one containing a struct sockcred,
generated under certain conditions when the receiver has enabled the
LOCAL_CREDS socket option. If a struct sockcred is attached, a struct cmsgcred
(if any) is removed. The two flavours have incompatible fields, but are not
reliably distinguishable in the general case (if the application programmer
even knows about this problem).

Therefore, behaving as if LOCAL_CREDS is set when it is not may cause
applications to receive a struct sockcred and interpret it as a struct
cmsgcred. For example, an application trying to read cmcred_uid for the real
UID will get the effective UID from sc_euid. This is likely to allow an easy
attack since a plain write(2) from a setuid root program (such as an error
message) will have the struct sockcred attached to it.

The patchset does seem to fix the bug that adding a struct sockcred for
LOCAL_CREDS silently does nothing if m_get() or m_getcl() with M_NOWAIT fails,
possibly leaving a struct cmsgcred from the sender.

The patch "[PATCH 3/4] uipc_finalizecontrol: read-lock the unp link" also adds
a comment about a bug that UNP_WANTCRED may be cleared by a failing send, so
that no struct sockcred is ever sent on the connection. Since this only affects
stream and seqpacket sockets, I think it is best fixed on the application side,
by using getpeereid(3) or LOCAL_PEERCRED instead of LOCAL_CREDS; this also
simplifies the application code and makes the credentials more reliable (since
they are from connect(2)/listen(2) time instead of from the write).

Related, if UNP_WANTCRED is cleared then struct cmsgcred from the sender will
get through, which might lead to a struct cmsgcred being interpreted as a
struct sockcred.

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