[apparmor] [patch] aa.py: change_profile vs. changes_profile

[John Johansen]

On 11/29/2014 05:18 AM, Christian Boltz wrote:
> Hello,
> 
> Am Freitag, 28. November 2014 schrieb Steve Beattie:
>> There's some really wonky behavior for view differences when doing so
>> (with or without the patch applied):
>>
>> --- /tmp/home.ubuntu.tmp.spork.sh 2014-11-28 23:07:50.769388829 -0800
>> +++ /tmp/tmpj6ytsskb    2014-11-28 23:09:56.641365708 -0800
>> @@ -13,9 +15,26 @@
>>
>>
>>    profile /bin/touch flags=(complain) {
>> +    /bin/cat rCx,
>> +    /bin/rm rix,
>> +
>>      #include <abstractions/base>
>>
>>      /bin/touch mr,
>>
>>    }
>>  }
>> +
>> +  profile /bin/cat flags=(complain) {
>> +    #include <abstractions/base>
>> +
>> +    /bin/cat mr,
>> +
>> +  }
>> +
>> +  profile /bin/touch flags=(complain) {
>> +    #include <abstractions/base>
>> +
>> +    /bin/touch mr,
>> +
>> +  }
>>
>> Note how the child profile for /bin/touch is getting re-added .
>> Furthermore, the /bin/cat and /bin/rm rules that get added to the
>> first instantiation of the /bin/touch profile belong in the parent
>> profile, not any of the children. However, the clean diff generates
>> the correct output:
>>
>> --- /tmp/tmpp9dstlgb    2014-11-28 23:37:32.193515557 -0800
>> +++ /tmp/tmpxn5pm9t8    2014-11-28 23:37:32.193515557 -0800
>> @@ -3,6 +3,8 @@
>>    capability dac_read_search,
>>
>>    /bin/bash ix,
>> +  /bin/cat rCx,
>> +  /bin/rm rix,
>>    /bin/touch rCx,
>>    /dev/pts/7 rw,
>>    /dev/tty rw,
>> @@ -11,6 +13,13 @@
>>    /tmp/spork.data a,
>>
>>
>> +  profile /bin/cat flags=(complain) {
>> +    #include <abstractions/base>
>> +
>> +    /bin/cat mr,
>> +
>> +  }
>> +
>>    profile /bin/touch flags=(complain) {
>>      #include <abstractions/base>
>>
>> which is what is written out as well.
> 
> (V)iew changes aka non-clean diff has known issues with child profiles, 
> see https://bugs.launchpad.net/apparmor/+bug/1394788
> 
> However, I'm surprised that it also mixes entries for the main profile 
> into a child profile - I didn't notice that until now.
> 
> Can you please open another bugreport? (if possible/not too difficult, 
> include a reproducer.)
> 
https://bugs.launchpad.net/apparmor/+bug/1404893

> I'm quite sure the problem is in the handling of RE_PROFILE_START in 
> serialize_profile_from_old_profile() which sets in_contained_hat = True 
> only if it finds "profile .*" (aka matches[3]), but doesn't do it 
> without the "profile" keyword. (Does your old profile contain the 
> "profile" keyword for the subprofiles?)
> 
the profile, hat or ^ symbol are required for a subprofile or the
specialized hat subprofile, the parser will reject the profile otherwise.
So knowing that and looking at the above diff output with is using
the profile keyword I would say yes.

However this does bring up and interesting point, if the profile
keyword is required by the re, how is it at handling 'hat' and '^'?

> Just a wild, untested guess (around line 3849):
> 
>             if RE_PROFILE_START.search(line):
>                 matches = RE_PROFILE_START.search(line).groups()
> -                if profile and profile == hat and matches[3]:
> +                if profile and profile == hat:
> -                   hat = matches[3]
> +                   if matches[3]:
> +                        hat = matches[3]
> +                   else:
> +                        hat = matches[1]
>                     in_contained_hat = True
>                     if write_prof_data[profile][hat]['profile']:
>                         pass
> 
> Feedback welcome ;-)
> 
sorry, I am going to stick this into the back of the queue with the
other outstanding aa.py patches