linux-kernel - Re: [PATCH v2] unix: properly account for FDs passed over unix sockets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Feb 2016 21:32:56 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Herrmann <dh.herrmann@...il.com>, Willy Tarreau <w@....eu>,
	"David S. Miller" <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	Марк Коренберг 
	<socketpair@...il.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Simon McVittie <simon.mcvittie@...labora.co.uk>
Subject: Re: [PATCH v2] unix: properly account for FDs passed over unix
 sockets

On 02.02.2016 20:29, Linus Torvalds wrote:
> On Tue, Feb 2, 2016 at 10:29 AM, Hannes Frederic Sowa
> <hannes@...essinduktion.org> wrote:
>>>
>>> Anyway, can someone provide a high-level description of what exactly
>>> this patch is supposed to do? Which operation should be limited, who
>>> should inflight FDs be accounted on, and which rlimit should be used
>>> on each operation? I'm having a hard time auditing existing
>>> user-space, given just the scarce description of this commit.
>>
>> Yes, all your observations are true. I think we need to explicitly
>> need to refer to the sending socket while attaching the fds.
>
> I don't think that really helps. Maybe somebody passed a unix domain
> socket around, and now we're crediting the wrong socket again.

I was struggling a bit what you meant but I think you are referring to 
the following scenario:

process-1 opens up a unix socket and passes it to process-2 (this 
process has different credentials) via af-unix. Process-2 then sends 
multiple fds to another destination over this transferred unix-fd.

True, in this case we again account the fds to the wrong process, which 
is bad.

> So how about we actually add a "struct cred *" to the scm_cookie
> itself, and we initialize it to "get_current_cred()". And then always
> use that.

Unfortunately we never transfer a scm_cookie via the skbs but merely use 
it to initialize unix_skb_parms structure in skb->cb and destroy it 
afterwards.

But "struct pid *" in unix_skb_parms should be enough to get us to 
corresponding "struct cred *" so we can decrement the correct counter 
during skb destruction.

So:

We increment current task's unix_inflight and also check the current 
task's limit during attaching fds to skbs and decrement the inflight 
counter via "struct pid *". This looks like it should work.

> That way it's always the person who actually does the send (rather
> than the opener of the socket _or_ the opener of the file that gets
> passed around) that gets credited, and thanks to the cred pointer we
> can then de-credit them properly.

Exactly, I try to implement that. Thanks a lot!

Hannes