Re: [PATCH 5/5] c/r: Add AF_UNIX support (v6)

[Oren Laadan <orenl@librato.com>]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl@librato.com):
>>> OL> Does the following bypass security checks for sys_connect() ?
> 
> [ on sock_unix_restore()->sock_unix_restore_connected()->sock_unix_join() ]
> 
>>> I don't think so.  We're basically replicating sys_socketpair() here,
>>> which does not do a security check, presumably because all you're
>>> doing is hooking two sockets together that both belong to you.  That's
>>> not to say that we're as safe as that limited operation, but I don't
>>> think it's totally clear.  Perhaps someone more confident will
>>> comment.
>> Yes, please ... Serge ?
>>
>> To me it sounds plausible. If we adopt it, then a comment in the
>> code is worthwhile.
> 
> I'm not sure what Oren means "sounds plausible" or should be adopted.
> Using a common helper with sys_connect()?

I meant that Dan's argument sounds plausible, and if we go that
way, it deserves a comment in the code explaining why the security
call is omitted.

Of course, that was before reading your concern about LSM-labeling
of sockets...

Oren.

> 
> At the moment you miss out on the security_socket_connect() call.  That
> may be not as important for unix sockets, but it does look like selinux +
> netlabel can label unix sockets as well.  So I'm not convinced we can
> just ignore it, as once we start properly LSM-labeling tasks and
> sockets we may need to do that to ensure proper restart under selinux.
> 
> The other thing is that some new fancy doohicky might require another
> hook in sys_connect, which may or may not be needed for this path.
> If coded this way, we may not find out until someone reports some
> subtle failure long after the fact.
> 
> Still your code is so customized that perhaps an explicit
> security_socket_connect() call in your sock_unix_join() may be the
> way to go...
> 
> -serge