[apparmor] stack and stack_onexec semantics again

[John Johansen]

On 03/08/2016 10:03 PM, Tyler Hicks wrote:

<< snip >>


>>
>> The stack_onexec api however is a different story dependent on when/how
>> the stacking is applied. There are 3 places/ways it can be applied.
>>
>> 1. straight override applied to current confinement when stack_onexec is
>>    called
>>   ie.  A + stack_onexc(B) results in A//&B
> 
> Is this the same behavior as change_onexec()? In other words, does
> "A + change_onexec(B)" result in B or D when /e is exec'ed?
> 

This is similar to change_onexec() as out lined below. Specifically assuming
change_profile rules are used it matches the sequence

  confinement = aa_getcon(); confinement += B ; aa_change_onexec(confinement)

The only real difference being the the aa_getcon() allows the change to
be flexible to actual confinement at the time of the exec


<< snip >>


>> So in the end the questions are?
>>
>> 1. Do we want to provide access to stacking without rules, or reduced
>> rule requirements.
>> - this can be allowed in the straight stacking without much risk
> 
> I think there is a fair amount of risk since we now have a large number
> of userspace programs that are making policy decisions based on AppArmor
> confinement contexts. In some cases, they're using the query API to have
> the kernel make the decision for them and I assume that the kernel is
> handling that appropriately. However, in other cases, they're making
> their own decisions based on the confinement context and I highly doubt
> that they're all ready for something crazy like
> ":ns:foo//&bar//&:child_ns:baz (mixed)".
> 
> (waiting to chime in on #2 until the question above is answered)
> 

Yes the query API should be handling this correctly.

So there are applications that are handling this decision on their own
and to be blunt I am very interested in breaking them now. This should
not ever have been done, and I am not nor ever have been in favour of
it. Breaking and fixing things now is far better than having to do it
in the future when even more things are doing this.

In the current case if they handle the context as a unique string they
will in fact be safe, in that stacking is an exclusionary operation.
The only side effect is that they cache different permissions for
  A
  B
  A//&B

the net result being they have to prompt for permissions 1 more time
(in this case) than the really have to. However once delegation lands
doing this will be wrong and delegation will not be able to work with
apps that do this.

So like I said better to break the few now, because delegation is
coming and when it does it will cause a lot more problems than stacking
for these apps.