unix: implement basic SO_PASSRIGHTS functionality

[kevans]

Summary

With exception to sockopt functionality, implement the so_options flag
in unix(4) itself. The general argument for the flag is that SCM_RIGHTS
can be used maliciously for, e.g., a DoS that the receiving side can't
avoid if it is expecting other control messages.

This option gives the receiver a way to disable SCM_RIGHTS on the
sender-side, surfacing an EPERM to them instead. This seems to match
the semantics that Linux offers.

If an SCM_RIGHTS was already sent before we disabled SO_PASSRIGHTS, then
a subsequent recvmsg(2) will silently discard any in-flight files. This
has the downside of punting a file with the potential to hang over to
the deferred-close task, but perhaps usage of the option would
discourage folks from attempting to take advantage of that possibility
anyways.

Various manpages updated to describe the new behavior.

[bms]

The rationale is pretty clear. Whilst it would be preferable to be default-deny (for much the same reasons as security models built on top of this are also default-deny) it cannot be so for compatibility reasons (yet; perhaps in future).

Open vSwitch implements defensive SCM_RIGHTS handling precisely because of the vulnerability this change proposes to close, and because of the asynchronous nature of SCM_RIGHTS, static analysis cannot easily find the vulnerability (as discussed with des).

[kib]

sys/kern/uipc_usrreq.c

2068

This goto increments td_ru.ru_msgsnd++. Same EPERM in uipc_sosend_stream_or_seqpacket() seems to not. Is this intended?

[kevans]

sys/kern/uipc_usrreq.c

2068

Right, I meant to write about this- I don't really understand the ru_msgsnd accounting in general, and I forgot to re-evaluate the other one you mention. For instance, we can hit an ENOBUFS below and we'll still account for it in ru_msgsnd- that seems like it's probably wrong to me, but I don't know if it's still necessarily to increment it for certain classes of errors (but not others).

[markj]

share/man/man4/unix.4

173

"at the sendmsg(2) call" is kind of confusing to me... something like the following might be better, though kind of verbose: "The SO_PASSRIGHTS socket option can be used to configure the receiving socket to reject SCM_RIGHTS messages, in which case any sendmsg(2) call which attempts to send an SCM_RIGHTS to the socket message will fail with .Dv EPERM."

sys/kern/uipc_usrreq.c

531

Suppose SO_PASSRIGHTS is cleared on a listening socket. solisten_clone() will copy some socket options from the parent socket, and then here we unconditionally set PASSRIGHTS. Is that the right behaviour? Should the flag be inherited?

1185

since you don't hold the socket lock here.

2066

3559

[kevans]

sys/kern/uipc_usrreq.c

531

Linux doesn't seem to inherit it, but I think propagating it from a listening socket would make more sense- otherwise you have a window after accept(2) where we coud EPERM a malicious file rather than hang the deferred-free thread.

My initial ptch actually flipped the sense of the socket option in so_options, implementing both SO_NORIGHTS (so_options) and SO_PASSRIGHTS (*sockopt(2) only) so that we captured the exceptional case while still using the Linux-compatible name. Barring a way to tell here that we're an accept(2)ed socket (which I'm not convinced is a great idea, anyways), maybe that would be a better idea.

[markj]

sys/kern/uipc_usrreq.c

531

By "hang the deferred-free thread" you mean the case where the receiving thread has to call fo_close on the file description, and this takes a particularly long time?

I think it's clear to inherit this attribute from the listening socket, but that would make our implementation incompatible with Linux. I'm not sure how important compatibility is here.

[glebius]

A test case would be very much appreciated.

[glebius]

Now I see following commits. Sorry for noise.

[kevans]

sys/kern/uipc_usrreq.c

531

Right- the theoretical threat model is that a process opens an fd backed by NFS or fuse that will hang on close and send it over to another process as a form of DoS -- so discarding it on the recv side means that DoS gets kicked over there instead.

[kevans]

To be clear: are we OK with inverting this to SO_NO_PASSRIGHTS (or SO_NO_RIGHTS) so that we reflect default-on and clean propagation from listening sockets? I'd still like to maintain SO_PASSRIGHTS as the *sockopt(2) interface, at least.

[glebius]

In D57423#1318410, @kevans wrote:

To be clear: are we OK with inverting this to SO_NO_PASSRIGHTS (or SO_NO_RIGHTS) so that we reflect default-on and clean propagation from listening sockets? I'd still like to maintain SO_PASSRIGHTS as the *sockopt(2) interface, at least.

I don't think it is a good idea to make the API different to Linux. In the Linux the new socket option is quite new (about a year), I wonder how settled are they on the API.

[kevans]

In D57423#1318439, @glebius wrote:

In D57423#1318410, @kevans wrote:

To be clear: are we OK with inverting this to SO_NO_PASSRIGHTS (or SO_NO_RIGHTS) so that we reflect default-on and clean propagation from listening sockets? I'd still like to maintain SO_PASSRIGHTS as the *sockopt(2) interface, at least.

I don't think it is a good idea to make the API different to Linux. In the Linux the new socket option is quite new (about a year), I wonder how settled are they on the API.

Right, that's why I'd end up adding both SO_PASSIGHTS (in the 'additional options' section) and SO_NO_RIGHTS (in so_options); internally we only accept SO_PASSRIGHTS for setsockopt()/getsockopt() to set SO_NO_RIGHTS in so_options. Unless you mean the subtlety of it being inherited by an accepted socket? I can poke the Linux folks and see what they think.

[glebius]

I don't see SO_NO_RIGHTS anywhere in the diff. The diff LGTM as is.

[kevans]

In D57423#1318444, @glebius wrote:

I don't see SO_NO_RIGHTS anywhere in the diff. The diff LGTM as is.

Right, that was my proposal in response to the discussion with Mark above. Maybe we just go with this for now because it seems to match Linux semantics for non-inheritance from a listening socket, and I'll raise the question with them in the interim?

[markj]

In D57423#1318448, @kevans wrote:

In D57423#1318444, @glebius wrote:

I don't see SO_NO_RIGHTS anywhere in the diff. The diff LGTM as is.

Right, that was my proposal in response to the discussion with Mark above. Maybe we just go with this for now because it seems to match Linux semantics for non-inheritance from a listening socket, and I'll raise the question with them in the interim?

That's ok with me. If we're going to keep the same name as Linux, it'd be better to have the same semantics.