[apparmor] query_label regression test failures
john.johansen at canonical.com
Mon Jul 13 19:07:16 UTC 2015
On 07/13/2015 01:40 AM, Steve Beattie wrote:
> On Sat, Jul 11, 2015 at 12:04:37PM -0700, John Johansen wrote:
>> So after further investigation there are a couple of problems.
>> 1. The test is using the wrong defines: It is using the defines from the
>> parser for the packed dfa permissions. This set of permissions is not
>> meant to be exposed to the outside world
>> 2. The kernel is using the wrong mapping function for the permissions
>> in the file class. This results in partially exposing the packed
>> permissions, but even then it doesn't fully line up with the packed
>> permissions, and is not correct for several of the potential permissions.
>> Attached is a patch that fixes the test, and moves the two tests that
>> fail due to the kernel to xpass.
>> === modified file 'tests/regression/apparmor/query_label.c'
>> --- tests/regression/apparmor/query_label.c 2015-05-28 19:48:46 +0000
>> +++ tests/regression/apparmor/query_label.c 2015-07-10 20:45:07 +0000
>> @@ -35,28 +35,68 @@
>> #define AA_MAY_APPEND (1 << 3)
>> +#ifndef AA_MAY_CREATE
>> +#define AA_MAY_CREATE (1 << 4)
> The reason I haven't submitted a patch that fixes the macro
> redefinition compilation warnings in the link_subset test in the same
> way as you do here is that, at least for that test, one of the
> definitions that it copied from the parser, AA_MAY_MOUNT, is not
> exported through the libapparmor headers. I'm not sure if that's a
> change or not, because it had defined all its macros without including
> external headers.
So the linksubset test is an entirely different beast than here. It is
not looking at or consuming any permission output from the kernel.
Instead it is trying to recreate all valid permission combinations,
that a link rule could be involved in, and checking that the subset
check is being done properly for that combination.
We can freely rename/redefine the macros in link subset.
> The problem this highlights is that if, in the future, the macro
> definitions get changed *and* possibly some of them disappear a
> la AA_MAY_MOUNT, some of the definitions will get pulled from the
> libapparmor exported header *and some won't* and then it's possible
> that multiple macro definitions might map to the same value, which
> is almost certainly problematic.
yes, I'd like to avoid that
> And for Tyler's benefit, hiding the definitions behind a function call
> doesn't really help if the value definitions can go away with
> libapparmor changes and default values are used.
well you can add more functions to get default values but ...
> That said, I don't have a good solution for how to verify that all
> the macro definitions are internally consistent and don't define
> multiple macros to the same value. The only way I can come up with is
> to not define the macros at all in the tests, and rely solely on the
> libapparmor macro definitions, and then if some of those go away due to
> interface changes, the tests and other consumers break at compile time.
Right there is no good solution. We need to be very careful with what
we name things and what we export
More information about the AppArmor