* [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN @ 2017-08-29 0:12 Prakash Sangappa 2017-08-29 23:02 ` David Miller 0 siblings, 1 reply; 7+ messages in thread From: Prakash Sangappa @ 2017-08-29 0:12 UTC (permalink / raw) To: linux-kernel, netdev; +Cc: davem, ebiederm, drepper, prakash.sangappa 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> --- net/core/scm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/core/scm.c b/net/core/scm.c index b1ff8a4..9274197 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -55,6 +55,7 @@ static __inline__ int scm_check_creds(struct ucred *creds) return -EINVAL; if ((creds->pid == task_tgid_vnr(current) || + creds->pid == task_pid_vnr(current) || ns_capable(task_active_pid_ns(current)->user_ns, CAP_SYS_ADMIN)) && ((uid_eq(uid, cred->uid) || uid_eq(uid, cred->euid) || uid_eq(uid, cred->suid)) || ns_capable(cred->user_ns, CAP_SETUID)) && -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN 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 0 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2017-08-29 23:02 UTC (permalink / raw) To: prakash.sangappa; +Cc: linux-kernel, netdev, ebiederm, drepper 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. Sorry, I'm not applying this. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN 2017-08-29 23:02 ` David Miller @ 2017-08-29 23:59 ` prakash.sangappa 2017-08-30 0:10 ` Eric W. Biederman 0 siblings, 1 reply; 7+ messages in thread From: prakash.sangappa @ 2017-08-29 23:59 UTC (permalink / raw) To: David Miller; +Cc: linux-kernel, netdev, ebiederm, drepper 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. > > Sorry, I'm not applying this. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN 2017-08-29 23:59 ` prakash.sangappa @ 2017-08-30 0:10 ` Eric W. Biederman [not found] ` <d23ec1ae-e2f0-659c-ce67-9b1b1e9ad8a5@oracle.com> 0 siblings, 1 reply; 7+ messages in thread From: Eric W. Biederman @ 2017-08-30 0:10 UTC (permalink / raw) To: prakash.sangappa; +Cc: David Miller, linux-kernel, netdev, drepper "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 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. 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. Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <d23ec1ae-e2f0-659c-ce67-9b1b1e9ad8a5@oracle.com>]
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN [not found] ` <d23ec1ae-e2f0-659c-ce67-9b1b1e9ad8a5@oracle.com> @ 2017-08-30 17:41 ` Eric W. Biederman 2017-09-01 17:30 ` Prakash Sangappa 0 siblings, 1 reply; 7+ messages in thread From: Eric W. Biederman @ 2017-08-30 17:41 UTC (permalink / raw) To: Prakash Sangappa; +Cc: David Miller, linux-kernel, netdev, drepper 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN 2017-08-30 17:41 ` Eric W. Biederman @ 2017-09-01 17:30 ` Prakash Sangappa 2017-09-01 19:29 ` Eric W. Biederman 0 siblings, 1 reply; 7+ messages in thread From: Prakash Sangappa @ 2017-09-01 17:30 UTC (permalink / raw) To: Eric W. Biederman; +Cc: David Miller, linux-kernel, netdev, drepper On 8/30/17 10:41 AM, ebiederm@xmission.com wrote: > Prakash Sangappa <prakash.sangappa@oracle.com> writes: > > >> 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. > Will this not be a bug in the application, if it is sending the wrong id? >> 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. What would be an another easier & efficient way of doing pid translation? Should a new API/mechanism be considered mainly for pid translation purpose for use with pid namespaces, say based on 'pipe' something similar to I_SENDFD? Thanks, -Prakash. > Eric > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN 2017-09-01 17:30 ` Prakash Sangappa @ 2017-09-01 19:29 ` Eric W. Biederman 0 siblings, 0 replies; 7+ messages in thread From: Eric W. Biederman @ 2017-09-01 19:29 UTC (permalink / raw) To: Prakash Sangappa; +Cc: David Miller, linux-kernel, netdev, drepper Prakash Sangappa <prakash.sangappa@oracle.com> writes: > On 8/30/17 10:41 AM, ebiederm@xmission.com wrote: >> Prakash Sangappa <prakash.sangappa@oracle.com> writes: >> >> >>> 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. >> > > Will this not be a bug in the application, if it is sending the wrong > id? No. It could be deliberate and malicious. >>> 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. > > What would be an another easier & efficient way of doing pid translation? > > Should a new API/mechanism be considered mainly for pid translation purpose > for use with pid namespaces, say based on 'pipe' something similar to > I_SENDFD? There are proc files that provide all of the pids of a process you can read those. Other possibilities exist if you want to go that fast. Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-01 19:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2017-09-01 17:30 ` Prakash Sangappa 2017-09-01 19:29 ` Eric W. Biederman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).