Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 21 Apr 2017 23:45:29 -0700
From: Andy Lutomirski <>
To: Casey Schaufler <>
Cc: Andy Lutomirski <>, Kees Cook <>, 
	Djalal Harouni <>, Linux Kernel Mailing List <>, 
	Andrew Morton <>, "Serge E. Hallyn" <>, 
	"" <>, 
	LSM List <>, 
	Linux API <>, Dongsu Park <>, 
	James Morris <>, Paul Moore <>, 
	Tetsuo Handa <>, 
	Greg Kroah-Hartman <>, Jonathan Corbet <>, 
	Jessica Yu <>, Rusty Russell <>, 
	Arnaldo Carvalho de Melo <>, Mauro Carvalho Chehab <>, Ingo Molnar <>, 
	belakhdar abdeldjalil <>, Peter Zijlstra <>
Subject: Re: [PATCH v3 2/2] modules:capabilities: add a per-task modules
 autoload restriction

On Fri, Apr 21, 2017 at 5:13 PM, Casey Schaufler <> wrote:
> On 4/21/2017 5:00 PM, Andy Lutomirski wrote:
>> On Fri, Apr 21, 2017 at 4:52 PM, Casey Schaufler <> wrote:
>>> On 4/21/2017 4:28 PM, Andy Lutomirski wrote:
>>>> On Fri, Apr 21, 2017 at 4:19 PM, Kees Cook <> wrote:
>>>>> On Wed, Apr 19, 2017 at 7:41 PM, Andy Lutomirski <> wrote:
>>>>>> On Wed, Apr 19, 2017 at 4:43 PM, Kees Cook <> wrote:
>>>>>>> On Wed, Apr 19, 2017 at 4:15 PM, Andy Lutomirski <> wrote:
>>>>>>>> On Wed, Apr 19, 2017 at 3:20 PM, Djalal Harouni <> wrote:
>>>>>>>>> +/* Sets task's modules_autoload */
>>>>>>>>> +static inline int task_set_modules_autoload(struct task_struct *task,
>>>>>>>>> +                                           unsigned long value)
>>>>>>>>> +{
>>>>>>>>> +       if (value > MODULES_AUTOLOAD_DISABLED)
>>>>>>>>> +               return -EINVAL;
>>>>>>>>> +       else if (task->modules_autoload > value)
>>>>>>>>> +               return -EPERM;
>>>>>>>>> +       else if (task->modules_autoload < value)
>>>>>>>>> +               task->modules_autoload = value;
>>>>>>>>> +
>>>>>>>>> +       return 0;
>>>>>>>>> +}
>>>>>>>> This needs to be more locked down.  Otherwise someone could set this
>>>>>>>> and then run a setuid program.  Admittedly, it would be quite odd if
>>>>>>>> this particular thing causes a problem, but the issue exists
>>>>>>>> nonetheless.
>>>>>>> Eeeh, I don't agree this needs to be changed. APIs provided by modules
>>>>>>> are different than the existing privilege-manipulation syscalls this
>>>>>>> concern stems from. Applications are already forced to deal with
>>>>>>> things being missing like this in the face of it simply not being
>>>>>>> built into the kernel.
>>>>>>> Having to hide this behind nnp seems like it'd reduce its utility...
>>>>>> I think that adding an inherited boolean to task_struct that can be
>>>>>> set by unprivileged tasks and passed to privileged tasks is a terrible
>>>>>> precedent.  Ideally someone would try to find all the existing things
>>>>>> like this and kill them off.
>>>>> (Tristate, not boolean, but yeah.)
>>>>> I see two others besides seccomp and nnp:
>>>> Well, that's interesting.  That should presumably be reset on setuid
>>>> exec or something.
>>>> Um.  At least that's just a performance issue.
>>>>> I really don't think this needs nnp protection.
>>>>>> I agree that I don't see how one would exploit this particular
>>>>>> feature, but I still think I dislike the approach.  This is a slippery
>>>>>> slope to adding a boolean for perf_event_open(), unshare(), etc, and
>>>>>> we should solve these for real rather than half-arsing them IMO.
>>>>> I disagree (obviously); this would be protecting the entire module
>>>>> autoload attack surface. That's hardly a specific control, and it's a
>>>>> demonstrably needed flag.
>>>> The list is just going to get longer.  We should probably have controls for:
>>>>  - Use of perf.  Unclear how fine grained they should be.
>>>>  - Creation of new user namespaces.  Possibly also use of things like
>>>> iptables without global privilege.
>>>>  - Ability to look up tasks owned by different uids (or maybe other
>>>> tasks *at all*) by pid/tid.  Conceptually, this is easy.  The API is
>>>> the only hard part, I think.
>>>>  - Ability to bind ports, maybe?
>>> One of my longer term (i.e. after stacking) projects
>>> is to create sensible access control on ports. Why shouldn't
>>> they have owners and mode bits (or ACLs, if you prefer)
>>> or real names. I kind of think we should be able to eliminate
>>> the need for dbus without resorting to kdbus.
>> My implicit_rights concept gives any type of access control you can
>> use on inodes because they *are* inodes.  So you get ACLs, etc.
>> Brief summary for those who didn't read my old email: We add a new
>> kind of filesystem object called a "right".  It's a special kind of
>> socket inode that can't be bound or connected but is instead created
>> by a new syscall.  It has a name, so "port:1234" might be a name of a
>> right.
>> To use an implicit right, you do whatever syscall you would do
>> normally.  The kernel looks for a right object at
>> /dev/implicit_rights/<name>.  If that object exists, is a right of the
>> correct type (i.e. the right's name matches <name>) and you have
>> execute access, you win.  Otherwise you lose.
>> To avoid breaking existing distros, for things like modules_autoload,
>> you would set a sysctl
>> /proc/sys/kernel/required_implicit_rights/modules_autoload=1.  With
>> that set, to autoload a module without CAP_SYS_MODULE, you need the
>> /dev/implicit_rights/modules_autoload.
> Sounds good.
>>> So I don't like the idea of treating that as a special case.
>>> I'd rather see ports controlled properly. (Of course, the
>>> SELinux crowd will point out they have this handled, but I
>>> remain unconvinced of the overall solution)
>> Agreed.  But I think we should address all of these things together.
> What I don't want is to have to buy into a hundred things I
> don't want in order to get the one thing I do. A General mechanism
> is dandy, but I don't want to have to write a gazillion policy
> lines for features I don't want in order to get a simple control.
> The problem with SELinux is not the effort required to protect
> what you care about, it's the effort required to do everything else.

The point is to make it super simple.  chown, chmod and, if you want
to get fancy, setfacl.  You'll need a mkright tool, but that's

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.