From: ebiederm@xmission.com (Eric W. Biederman)
To: Prakash Sangappa <prakash.sangappa@oracle.com>
Cc: David Miller <davem@davemloft.net>,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
drepper@redhat.com
Subject: Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN
Date: Wed, 30 Aug 2017 12:41:21 -0500 [thread overview]
Message-ID: <87inh5ymv2.fsf@xmission.com> (raw)
In-Reply-To: <d23ec1ae-e2f0-659c-ce67-9b1b1e9ad8a5@oracle.com> (Prakash Sangappa's message of "Wed, 30 Aug 2017 08:57:08 -0700")
Prakash Sangappa <prakash.sangappa@oracle.com> writes:
> On 8/29/17 5:10 PM, ebiederm@xmission.com wrote:
>
> "prakash.sangappa" <prakash.sangappa@oracle.com> writes:
>
> On 08/29/2017 04:02 PM, David Miller wrote:
>
> From: Prakash Sangappa <prakash.sangappa@oracle.com>
> Date: Mon, 28 Aug 2017 17:12:20 -0700
>
> Currently passing tid(gettid(2)) of a thread in struct ucred in
> SCM_CREDENTIALS message requires CAP_SYS_ADMIN capability otherwise
> it fails with EPERM error. Some applications deal with thread id
> of a thread(tid) and so it would help to allow tid in SCM_CREDENTIALS
> message. Basically, either tgid(pid of the process) or the tid of
> the thread should be allowed without the need for CAP_SYS_ADMIN capability.
>
> SCM_CREDENTIALS will be used to determine the global id of a process or
> a thread running inside a pid namespace.
>
> This patch adds necessary check to accept tid in SCM_CREDENTIALS
> struct ucred.
>
> Signed-off-by: Prakash Sangappa <prakash.sangappa@oracle.com>
>
> I'm pretty sure that by the descriptions in previous changes to this
> function, what you are proposing is basically a minor form of PID
> spoofing which we only want someone with CAP_SYS_ADMIN over the
> PID namespace to be able to do.
>
> The fix is to allow passing tid of the calling thread itself not of any
> other thread or process. Curious why would this be considered
> as pid spoofing?
>
> This change would enable a thread in a multi threaded process, running
> inside a pid namespace to be identified by the recipient of the
> message easily.
>
> I think a more practical problem is that change, changes what is being
> passed in the SCM_CREDENTIALS from a pid of a process to a tid of a
> thread. That could be confusing and that confusion could be exploited.
>
> It will be upto the application to decide what to pass, either pid of the
> process or tid of the thread and the co-operating process receiving the
> message would know what to expect. It does not change or make it
> mandatory to pass tid.
>
>
> It is definitely confusing because in some instances a value can be both
> a tgid and a tid.
>
>
> I definitely think this needs to be talked about in terms of changing
> what is passed in that field and what the consequences could be.
>
> Agreed that If the receiving process expects a pid and the process sending
> the message sends tid, it can cause confusion, but why would that occur?
> Shouldn't the sending process know what is the receiving process expecting?
>
>
> I suspect you are ok. As nothing allows passing a tid today. But I
> don't see any analysis on why passing a tid instead of a tgid will not
> confuse the receiving application, and in such confusion introduce a
> security hole.
>
> It would seem that there has to be an understanding between the two
> processes what is being passed(pid or tid) when communicating with
> each other.
Which is the issue. SCM_CREDENTIALS is fundamentally about dealing with
processes that are in a less than completely trusting relationship.
> With regards to security, the question basically is what is the consequence
> of passing the wrong id. As I understand it, Interpreting the id to be pid
> or tid, the effective uid and gid will be the same. It would be a problem
> only if the incorrect interpretation of the id would refer a different process.
> But that cannot happen as the the global tid(gettid() of a thread is
> unique.
There is also the issue that the receiving process could look, not see
the pid in proc and assume the sending process is dead. That I suspect
is the larger danger.
> As long as the thread is alive, that id cannot reference another process / thread.
> Unless the thread were to exit and the id gets recycled and got used for another
> thread or process. This would be no different from a process exiting and its
> pid getting recycled which is the case now.
Largely I agree.
If all you want are pid translations I suspect the are far easier ways
thant updating the SCM_CREDENTIALS code.
Eric
next prev parent reply other threads:[~2017-08-30 17:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-29 0:12 [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN Prakash Sangappa
2017-08-29 23:02 ` David Miller
2017-08-29 23:59 ` prakash.sangappa
2017-08-30 0:10 ` Eric W. Biederman
[not found] ` <d23ec1ae-e2f0-659c-ce67-9b1b1e9ad8a5@oracle.com>
2017-08-30 17:41 ` Eric W. Biederman [this message]
2017-09-01 17:30 ` Prakash Sangappa
2017-09-01 19:29 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87inh5ymv2.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=davem@davemloft.net \
--cc=drepper@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=prakash.sangappa@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.