Re: Debugging sepolgen-ifgen?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/05/2014 09:09 AM, Stephen Smalley wrote:
> On 08/04/2014 05:44 PM, Daniel J Walsh wrote:
>>
>> On 08/04/2014 01:07 PM, Stephen Smalley wrote:
>>> On 08/02/2014 03:19 PM, Sven Vermeulen wrote:
>>>> Hi all
>>>>
>>>> I've noticed that on my system, for some interfaces, the results in
>>>> /var/lib/sepolgen/interface_info are missing file-specific feedback.
>>>>
>>>> For instance, consider the kernel_rw_kernel_sysctl() interface, which is
>>>> coded as follows:
>>>>
>>>> interface(`kernel_rw_kernel_sysctl',`
>>>>         gen_require(`
>>>>                 type proc_t, sysctl_t, sysctl_kernel_t;
>>>>         ')
>>>>
>>>>         rw_files_pattern($1, { proc_t sysctl_t sysctl_kernel_t }, sysctl_kernel_t)
>>>>
>>>>         list_dirs_pattern($1, { proc_t sysctl_t }, sysctl_kernel_t)
>>>> ')
>>>>
>>>> In the interface_info file, I only find the following metadata about this
>>>> interface:
>>>>
>>>> [InterfaceVector kernel_rw_kernel_sysctl $1:source ]
>>>> $1,sysctl_t,dir,getattr,open,search
>>>> $1,sysctl_kernel_t,dir,getattr,open,search
>>>> $1,proc_t,dir,getattr,open,search
>>>>
>>>> Shouldn't this at least contain something like this?
>>>>
>>>> $1,sysctl_kernel_t,file,write,getattr,lock,open,ioctl,append 
>>>>
>>>> Although not critical, it does result in audit2allow -R to not use
>>>> refpolicy-style interfaces when possible...
>>>>
>>>> How can I debug this? I know the file is generated by sepolgen-ifgen, but
>>>> rerunning doesn't add in any file-related metadata and I'm totally oblivious
>>>> on how the parsing is done...
>>> Not sure about that beyond the -d -v options.
>>> However, this appears to be a regression; despite encountering some syntax errors during parsing,
>>> sepolgen-ifgen from 21030423 generates a more accurate vector:
>>>
>>> [InterfaceVector kernel_rw_kernel_sysctl $1:source ]
>>> $1,sysctl_t,dir,getattr,open,search
>>> $1,sysctl_kernel_t,file,write,getattr,read,lock,open,ioctl,append
>>> $1,sysctl_kernel_t,dir,search,read,lock,ioctl,getattr,open
>>> $1,proc_t,dir,getattr,open,search
>>>
>>> while sepolgen-ifgen from 20131030_4 generates the reduced set you have above.
>>>
>>> Seems to have been broken by:
>>>
>>> commit 17cc87e56b0241688c119f774f103622b002e0ae
>>> Author: Dan Walsh <dwalsh@xxxxxxxxxx>
>>> Date:   Wed Oct 9 17:01:35 2013 -0400
>>>
>>>     sepolgen did not work with filename transitions.
>>>     
>>>     This patch adds support for it.
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Selinux mailing list
>>> Selinux@xxxxxxxxxxxxx
>>> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
>>> To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
>>>
>>>
>> I don't see anything obviously wrong with that patch?
> 
> If I revert that one patch, it works.  I don't know offhand what the
> underlying issue is, but I'd guess you are introducing an ambiguity into
> the grammar.  I do notice that the definitions of IDENTIFIER and
> FILENAME do not match the ones in checkpolicy policy_scan.l; I do not
> know why that is.  I also notice that whereas bison reports no warnings
> on the checkpolicy policy_parse.y grammar, sepolgen-ifgen -d reports 669
> shift/reduce conflicts before your patch and 671 shift/reduce conflicts
> afterward; neither seems very good...

Looking at the patch that seems to have caused the problem, it makes the
following change:

@@ -461,6 +469,7 @@ def p_interface_call_param(p):
 def p_interface_call_param_list(p):
     '''interface_call_param_list : interface_call_param
                                  | interface_call_param_list COMMA
interface_call_param
+                                 | interface_call_param_list COMMA
interface_call_param COMMA interface_call_param_list
     '''
     if len(p) == 2:
         p[0] = [p[1]]

For some reason, it adds a new pattern to the interface_call_param_list
pattern, which is "list, param, list". To me, this doesn't seem
necessary. The first two patterns should cover all combinations of
interface call parameters. And reverting this hunk gives the same
InterfaceVector as with the patch reverted. So it seems like the right fix.

Dan, any idea why this hunk was added?

Thanks,
- Steve
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux