different capability from different namespace required for prctl_set_mm_exe_file

different capability from different namespace required for prctl_set_mm_exe_file

[Tong Zhang]

Kernel Version: 4.18.5

Problem Description:

We discovered inconsistent check when using prctl_set_mm_exe_file(), which is used to setup exe file link.

It is required to have capable(CAP_SYS_RESOURCE) in prctl_set_mm().
while ns_capable(CAP_SYS_ADMIN) in prctl_set_mm_map().

There are two differences:
1)requiring capability from: user namespace, init namespace.
2)capability bit required is different


- Tong

Re: different capability from different namespace required for prctl_set_mm_exe_file

[Greg KH]

On Tue, Sep 25, 2018 at 01:26:55PM -0400, Tong Zhang wrote:
> Kernel Version: 4.18.5
> 
> Problem Description:
> 
> We discovered inconsistent check when using prctl_set_mm_exe_file(), which is used to setup exe file link.
> 
> It is required to have capable(CAP_SYS_RESOURCE) in prctl_set_mm().
> while ns_capable(CAP_SYS_ADMIN) in prctl_set_mm_map().
> 
> There are two differences:
> 1)requiring capability from: user namespace, init namespace.
> 2)capability bit required is different

Can you submit a patch showing what you think is the correct fix here?

thanks,

greg k-h

Re: different capability from different namespace required for prctl_set_mm_exe_file

[Cyrill Gorcunov]

On Tue, Sep 25, 2018 at 07:37:45PM +0200, Greg KH wrote:
> On Tue, Sep 25, 2018 at 01:26:55PM -0400, Tong Zhang wrote:
> > Kernel Version: 4.18.5
> > 
> > Problem Description:
> > 
> > We discovered inconsistent check when using prctl_set_mm_exe_file(), which is used to setup exe file link.
> > 
> > It is required to have capable(CAP_SYS_RESOURCE) in prctl_set_mm().
> > while ns_capable(CAP_SYS_ADMIN) in prctl_set_mm_map().
> > 
> > There are two differences:
> > 1)requiring capability from: user namespace, init namespace.
> > 2)capability bit required is different
> 
> Can you submit a patch showing what you think is the correct fix here?

It is done this way on purpose. The prctl_set_mm_map is a complex call
which carries a bunch of parameters and allowed if you're inside user-ns admin,
in turn prctl_set_mm allows to modify settings one by one. So no, it is not
an error but rather call specifics.

Re: different capability from different namespace required for prctl_set_mm_exe_file

[Greg KH]

On Tue, Sep 25, 2018 at 09:34:27PM +0300, Cyrill Gorcunov wrote:
> On Tue, Sep 25, 2018 at 07:37:45PM +0200, Greg KH wrote:
> > On Tue, Sep 25, 2018 at 01:26:55PM -0400, Tong Zhang wrote:
> > > Kernel Version: 4.18.5
> > > 
> > > Problem Description:
> > > 
> > > We discovered inconsistent check when using prctl_set_mm_exe_file(), which is used to setup exe file link.
> > > 
> > > It is required to have capable(CAP_SYS_RESOURCE) in prctl_set_mm().
> > > while ns_capable(CAP_SYS_ADMIN) in prctl_set_mm_map().
> > > 
> > > There are two differences:
> > > 1)requiring capability from: user namespace, init namespace.
> > > 2)capability bit required is different
> > 
> > Can you submit a patch showing what you think is the correct fix here?
> 
> It is done this way on purpose. The prctl_set_mm_map is a complex call
> which carries a bunch of parameters and allowed if you're inside user-ns admin,
> in turn prctl_set_mm allows to modify settings one by one. So no, it is not
> an error but rather call specifics.

I was hoping that when the submitter went to create such a patch, they
would have realized that.  You learn more when trying to fix a problem
than when someone has to tell you the answers :)

thanks,

greg k-h

Re: different capability from different namespace required for prctl_set_mm_exe_file

[Cyrill Gorcunov]

On Tue, Sep 25, 2018 at 08:40:54PM +0200, Greg KH wrote:
> > 
> > It is done this way on purpose. The prctl_set_mm_map is a complex call
> > which carries a bunch of parameters and allowed if you're inside user-ns admin,
> > in turn prctl_set_mm allows to modify settings one by one. So no, it is not
> > an error but rather call specifics.
> 
> I was hoping that when the submitter went to create such a patch, they
> would have realized that. You learn more when trying to fix a problem
> than when someone has to tell you the answers :)

True :) Next time I'll wait a couple of days.

Re: different capability from different namespace required for prctl_set_mm_exe_file

[TongZhang]

I can see there are two problems,

First: In kernel/sys.c:2117  capable(CAP_SYS_RESOURCE), seems that ns_capable should be used to check capability against user namespace, instead of init_user_ns. Because a  process in a user namespace may call prctl system call and this should be checked against their user namespace capability instead of init_user_ns capability.

Second: They should both require CAP_SYS_RESOURCE or CAP_SYS_ADMIN, is there any particular reasons for requiring different privilege?

> On Sep 25, 2018, at 2:34 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> 
> On Tue, Sep 25, 2018 at 07:37:45PM +0200, Greg KH wrote:
>> On Tue, Sep 25, 2018 at 01:26:55PM -0400, Tong Zhang wrote:
>>> Kernel Version: 4.18.5
>>> 
>>> Problem Description:
>>> 
>>> We discovered inconsistent check when using prctl_set_mm_exe_file(), which is used to setup exe file link.
>>> 
>>> It is required to have capable(CAP_SYS_RESOURCE) in prctl_set_mm().
>>> while ns_capable(CAP_SYS_ADMIN) in prctl_set_mm_map().
>>> 
>>> There are two differences:
>>> 1)requiring capability from: user namespace, init namespace.
>>> 2)capability bit required is different
>> 
>> Can you submit a patch showing what you think is the correct fix here?
> 
> It is done this way on purpose. The prctl_set_mm_map is a complex call
> which carries a bunch of parameters and allowed if you're inside user-ns admin,
> in turn prctl_set_mm allows to modify settings one by one. So no, it is not
> an error but rather call specifics.

Re: different capability from different namespace required for prctl_set_mm_exe_file

[Cyrill Gorcunov]

On Tue, Sep 25, 2018 at 07:37:14PM -0400, TongZhang wrote:
> I can see there are two problems,
> 
> First: In kernel/sys.c:2117  capable(CAP_SYS_RESOURCE), seems that ns_capable should
> be used to check capability against user namespace, instead of init_user_ns. Because a
> process in a user namespace may call prctl system call and this should be checked against
> their user namespace capability instead of init_user_ns capability.
> 
> Second: They should both require CAP_SYS_RESOURCE or CAP_SYS_ADMIN, is there any particular
> reasons for requiring different privilege?

Yes. We consider changing fields one by one in init_ns as an undesirable action,
mostly because some sysadmins/tools continue relay on this info for monitoring.
And requiring sysadmin here is too much: sysamin can do a way more than just
changing these members. In turn because userns is even more weak than init-ns
we require admin capability instead.

Again: non of the monitoring instrument should rely on the members this prctl
changes, they are not consistent and never was. But we still grip the privileges
here simply to not allow anyone change random members, at least in init-ns.

p.s. pleaase don't top post