[apparmor] query_label regression test failures

[John Johansen]

On 06/25/2015 03:57 PM, Steve Beattie wrote:
> On Thu, Jun 25, 2015 at 04:30:46PM -0500, Tyler Hicks wrote:
>> On 2015-06-25 13:55:47, Tyler Hicks wrote:
>>> On 2015-06-25 01:21:39, Steve Beattie wrote:
>>>> Hi,
>>>>
>>>> When running the apparmor regression tests on wily with the trunk of
>>>> the userspace tools, I'm getting the following two failures in the
>>>> query_label test:
>>>>
>>>> Error: query_label failed. Test 'QUERY file (all base perms #1)' was expected to 'pass'. Reason for failure 'FAIL: the access should not be allowed and should be audited'
>>>> Error: query_label failed. Test 'QUERY file (all base perms #2)' was expected to 'pass'. Reason for failure 'FAIL: the access should not be allowed and should be audited'
>>>
>>> Note that the test passes when we run them against the wily apparmor
>>> userspace (2.9.2-0ubuntu1). Seems to be something broken specifically in
>>> trunk.
>>  
>> The tests start failing after r3081:
>>
>>   http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3081
>>
>> That patch defined values for AA_MAY_* perms, in apparmor.h, related to
>> file operations:
>>
>>   http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/view/head:/libraries/libapparmor/include/sys/apparmor.h#L34
>>
>> The query_label.c binary already defined many of the macros:
>>
>>   http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/view/head:/tests/regression/apparmor/query_label.c#L22
>>
>> The problem is that the new macros in apparmor.h do not match the old
>> macros in query_label.c. Which ones are correct? I assume that the
>> apparmor.h ones are correct but would like confirmation before I go
>> modify the query_label.c test program.
> 
> Right, running the query_label test compiled against the trunk
> definitions but with the 2.9.2-0ubuntu1 parser fails in the same way.
> 
> Note that changed definition of the AA_MAY_* perms also causes
> compilation of the link_subset test to generate a number of warnings,
> due to link_subset.c defining them differently than in apparmor.h:
> 
>   http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/view/head:/tests/regression/apparmor/link_subset.c#L18
> 
> I was working on a patch to address the warnings, but it becomes
> difficult to work in both a USE_SYSTEM environment where 2.9 libraries
> are available and against the different trunk definitions. I
> didn't want to merely protect with #ifndef AA_MAY_*, because
> link_subset.c defines some macros that aren't defined in either
> libraries/libapparmor/include/sys/apparmor.h or parser/immunix.h:
> http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/view/head:/parser/immunix.h#L25
> 
> One of the questions I have is that with rev 3081, the macro definitions
> become part of the library API, which means that it gets harder to
> change them in the future. Are we sure we want that? (We don't have any
> releases out there with them visible in the header yet.)
> 
It is already being exposed by the kernel and dbus's use. The other types
can be queried, but there just isn't any library help so you need to know
how to do it/interpret the returned values.

I am fine with keeping the defines internal to apparmor but how would you
propose we export the different permission information being currently
returned by the kernel.