[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