[apparmor] PATCH [2/6] - Fix capability log parsing

[John Johansen]

On 09/09/2010 04:25 PM, Steve Beattie wrote:
> On Thu, Sep 09, 2010 at 03:36:18PM -0700, Steve Beattie wrote:
>> On Thu, Sep 09, 2010 at 08:34:40AM -0700, John Johansen wrote:
>>> The capability operation picked up the capability and capname fields.
>>> capability is reported by LSM_AUDIT and is just the capability number.
>>> capname is reported by the apparmor module and is the name the kernel
>>> knows the capability as.
>>>
>>> For now just use capname and silently drop capability when it is found.
>>
>> Actually, I mildly take back my ACK here...
>>
>>> Index: libapparmor/src/scanner.l
>>> ===================================================================
>>> --- libapparmor.orig/src/scanner.l	2010-09-09 07:18:32.214193401 -0700
>>> +++ libapparmor/src/scanner.l	2010-09-09 07:40:32.814193400 -0700
>>> @@ -125,7 +125,6 @@
>>>  old_on			"on"
>>>  old_xattr		"xattr"
>>>  old_change		"change"
>>> -old_capability		"capability"
>>>  old_syscall		"syscall"
>>>  old_link		"link"
>>>  old_fork		"fork"
>>> @@ -159,6 +158,8 @@
>>>  key_fsuid		"fsuid"
>>>  key_ouid		"ouid"
>>>  key_comm		"comm"
>>> +key_capability		"capability"
>>> +key_capname		"capname"
>>>  audit			"audit"
>>>  
>>>  /* syslog tokens */
>>> @@ -303,7 +304,7 @@
>>>  {old_extended}		{ return(TOK_OLD_EXTENDED); }
>>>  {old_on}		{ return(TOK_OLD_ON); }
>>>  {old_change}		{ return(TOK_OLD_CHANGE); }
>>> -{old_capability}	{ BEGIN(sub_id); return(TOK_OLD_CAPABILITY); }
>>> +{key_capability}	{ BEGIN(sub_id); return(TOK_KEY_CAPABILITY); }
>>>  {old_syscall}		{ return(TOK_OLD_SYSCALL); }
>>>  {old_fork}		{ return(TOK_OLD_FORK); }
>>>  {old_child}		{ return(TOK_OLD_CHILD); }
>>> @@ -343,6 +344,8 @@
>>>  {key_fsuid}		{ return(TOK_KEY_FSUID); }
>>>  {key_ouid}		{ return(TOK_KEY_OUID); }
>>>  {key_comm}		{ return(TOK_KEY_COMM); }
>>> +{key_capability}	{ return(TOK_KEY_CAPABILITY); }
>>> +{key_capname}		{ return(TOK_KEY_CAPNAME); }
>>>  
>>>  {syslog_kernel}		{ BEGIN(dmesg_timestamp); return(TOK_SYSLOG_KERNEL); }
>>>  {syslog_month}		{ yylval->t_str = strdup(yytext); return(TOK_DATE_MONTH); }
>>
>> The pattern {key_capabilty} is now listed twice, thanks to the addition
>> in the last chunk. This works out somewhat okay as the first rule
>> will always match (and then jump into the sub_id state which can also
>> handle =ID pairs). The reason for the sub_id behavior is the old message
>> style capability keyword.
>>
>> I think we should have the pattern keyword just once, and have it (for
>> the time being) jump to the sub_id state. Either way, it'll be mildly
>> confusing to untangle when we rip out the old style message support.
> 
> This added patch passes with the testcases I provided:
> 
NAK I think this will be just as confusing, this thing is a mess, and I 
tried to do what I did to leave a little documentation.  The following patches applies on top, of the previous set and converts the second {key_capability} into a comment for the time being, while documenting why its there and what needs to be done in the future.


Index: libapparmor/src/scanner.l
===================================================================
--- libapparmor.orig/src/scanner.l	2010-09-09 08:41:10.504193401 -0700
+++ libapparmor/src/scanner.l	2010-09-09 16:15:43.854193511 -0700
@@ -347,9 +348,21 @@
 {key_fsuid}		{ return(TOK_KEY_FSUID); }
 {key_ouid}		{ return(TOK_KEY_OUID); }
 {key_comm}		{ return(TOK_KEY_COMM); }
-{key_capability}	{ return(TOK_KEY_CAPABILITY); }
+ /* This key_capability entry is here to document, what should be.
+  * currently the capability token is handled by the old set of rules above
+  * it should be handled here, but there is no good way to combine them
+  * that doesn't require more work than it is worth atm because of the
+  * switch to sub_id in the old scanner rules.
+  * The switch to sub_id causes the new rule set in the grammar to need to
+  * accept a TOK_ID instead of TOK_DIGITS, which it should be.
+  * once the old rules and old scanning is ripped out this scanner rule
+  * should be activated and the corresponding rule in the grammar should
+  * be updated to use TOK_DIGITS
+  * {key_capability}	{ return(TOK_KEY_CAPABILITY); }
+  */
 {key_capname}		{ return(TOK_KEY_CAPNAME); }
 {key_offset}		{ return(TOK_KEY_OFFSET); }
 {key_target}		{ return(TOK_KEY_TARGET); }