[apparmor] [PATCH 3/3] AppArmor: export known rlimit names/value mappings in securityfs

[John Johansen]

On 12/31/2011 01:01 PM, Kees Cook wrote:
> On Sat, Dec 31, 2011 at 12:22:09PM -0800, John Johansen wrote:
>> On 12/31/2011 01:07 AM, Kees Cook wrote:
>>> Since the parser needs to know which rlimits are known to the kernel,
>>> export the name/value mappings via the "rlimit" subdirectory in the
>>> securityfs "features" directory.
>>>
>> So I have a different concern than Seth's, which may make things easier.
>>
>> Basically we don't need the rlimit entry #, it doesn't hurt to have it here
>> it just isn't needed.  The interface defines a fixed ordering, and the kernel
>> remaps that to its internal order if it needs it.
>>
>> Sorry I didn't have the specifics earlier, but I had to dig back and refresh
>> my memory of what was needed.
>>
>> What we need for rlimits is
>> - which ones are supported
>> - whether setting more than the current tasks limit is supported
>>
>> Currently the rlimit rules control setting only the current tasks rlimits.
>> But rlimits where extended a while ago to allow setting other tasks limits
>> as well.
>>
>> We are going to extend our support with the policy db, providing matching
>> against the target tasks profile.
>>
>> I was thinking a mask of the rlimits by name might be sufficient.  It seems
>> silly to use multiple files for that.
>>
>> And then we can have another boolean file indicating the extended match support
>
> Do you mean like this?
>
> features/rlimit/
>      mask ("cpu fsize data stack ...")
>      extended_match ("no")
>
yeah I think that is sufficient, of course if you think we need an individual file
for each, I can live with that too

I think the dividing line for me on the more sysfs vs. procfs style interface
is whether the kernel needs to be able to accept a write/update of the value.
None of these do so how we group them, put them in a directory (individual
files), or are grouped in a single file.  Is a little more based on what feels
right.

I don't really expect the set of rlimits to change much, and it just feels
like a set of related things that could be specified together as a mask like
capabilities even (of course the names are more useful) so I don't mind
sticking them together in a single file.

The extended_match (or what ever we call it) feels like a different piece of info
so it should go in its own file.  As for the extended_match name, that feels like
a really poor choice.  Maybe
   foreign_task?
   task_match?

suggestions welcome


>> So now to messing with the spec,
>>
>> I was thinking perhaps something similar for file and network as well
>>    a mask of the supported permissions
>>    and other files for other parts of what is supported
>>
>>    eg.
>>    file/
>>      mask  (exec,read,write,append,link,..)
>>      owner (true)  #simple ownership test we currently do
>>      link_pair (true)
>>      extended_owner (false)  #arbitrary matching against owner info
>
> Okay, sounds easy to do.
>
>> ...
>> Hrmm thinking about it some of the conditional matching stuff should probably
>> be pulled out of file and stuck in a separate directory.  Call it matching or
>> dfa.
>>    matching/
>>      dfa16 - support the newer dfa16 format
>>      dfa32 - support the newer larger dfa32 format
>>      diff_encode - support differential encoding
>>      kernel_var - support kernel vars
>>      back_refs - support back references
>>      ..
>>      owner
>>      extended owner
>
> So remove "owner" and 'extended_owner" from "file/" and move it here?
>
So, yes.  But the current upstream dfa only supports owner currently.  I don't
think we need to actually mention the other things that are not supported currently.

To be clear
   matching/
     owner

is it right now, unless you really want to put in entries that say "false".


Or we could rework this a little bit so it wasn't an individual entry for each
if you want.  It would be something like
   matching/
     format - notflex dfa16 dfa32
     options - diff_encode, kernel_var, back_refs
     conditionals - owner extended_owner, ...

I don't really care either works for me


>> And while we are at messing with the spec it might be worthwhile grouping all
>> the domain transition ones together in a dir
>>    domain/
>>      change_hat
>>      change_hatv
>>      change_profile
>>      change_onexec
>>
>> and in the future we can add stacking etc.
>
> Okay, I'll split it out.
>

Thanks kees, feel free to propose changes to the spec.