[apparmor] [PATCH] fix the fix for network flag when generating policy
John Johansen
john.johansen at canonical.com
Mon Jul 2 21:17:27 UTC 2012
On 07/02/2012 10:33 AM, Steve Beattie wrote:
> On Mon, Jul 02, 2012 at 01:27:34AM -0700, John Johansen wrote:
>> The previous patch to fix policy compilation around the network flag had a
>> serious flaw. The test for the network flag was being applied against both
>> the kernel flags and the cache flags. This means that if either the kernel
>> or the cache did not have the flag set then network mediation would be
>> turned off.
>>
>> Thus if a kernel was booted without the flag, and a cache was generated
>> based on that kernel and then the system was rebooted into a kernel with
>> the network flag present, the parser on generating the new policy would
>> detect the old cache did not support network and turn it off for the
>> new policy as well.
>>
>> This can be fixed by either removing the old cache first or regenerating
>> the cache twice. As the first generation will write that networking is
>> supported in the cache (even though the policy will have it disabled), and
>> the second generation will generate the correct policy.
>>
>> The following patch moves the test so that it is only applied to the kernel
>> flags set.
>>
>> ---
>>
>> === modified file 'parser/parser_main.c'
>> --- parser/parser_main.c 2012-07-01 08:35:05 +0000
>> +++ parser/parser_main.c 2012-07-02 07:49:14 +0000
>> @@ -873,11 +873,6 @@
>> //fprintf(stderr, "flags string: %s\n", flags_string);
>> //fprintf(stderr, "changehat %d\n", flag_changehat_version);
>> }
>> - if (strstr(flags_string, "network"))
>> - kernel_supports_network = 1;
>> - else
>> - kernel_supports_network = 0;
>> -
>> return;
>>
>> fail:
>> @@ -1187,7 +1182,12 @@
>> write_cache = 0;
>> skip_read_cache = 1;
>> return;
>> - }
>> + } else if (strstr(flags_string, "network"))
>> + kernel_supports_network = 1;
>> + else
>> + kernel_supports_network = 0;
>> +
>> +
>
> I'm confused. Don't we detect whether we have network support in
> get_match_string()? Why are we detecting it again?
>
it is confusing, the whole thing needs to be cleaned up/rewritten.
We are only testing for network support in get_match_string for
the case where features is a directory. The legacy case of checking
in the compat interface isn't handled.
The reason I didn't rearrange/rework for this patch is I was looking
for a minimum patch to cover debian bug 679597
> It'd be nice to check for kernel features in one location, it looks like
> we check for network in two locations, mount in one location, change_hat
> versions in a different location...
>
yep I am working on a larger patch to clean this up
More information about the AppArmor
mailing list