Debugging sepolgen-ifgen?

Debugging sepolgen-ifgen?

[Sven Vermeulen]

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...

Wkr,
	Sven Vermeulen

Re: Debugging sepolgen-ifgen?

[Stephen Smalley]

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@redhat.com>
Date:   Wed Oct 9 17:01:35 2013 -0400

    sepolgen did not work with filename transitions.
    
    This patch adds support for it.

Re: Debugging sepolgen-ifgen?

[Daniel J Walsh]

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@redhat.com>
> 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@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
>
I don't see anything obviously wrong with that patch?

Re: Debugging sepolgen-ifgen?

[Stephen Smalley]

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@redhat.com>
>> 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@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>
>>
> 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...

Re: Debugging sepolgen-ifgen?

[Steve Lawrence]

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@redhat.com>
>>> 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@tycho.nsa.gov
>>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>>
>>>
>> 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

Re: Debugging sepolgen-ifgen?

[Daniel J Walsh]

On 08/25/2014 03:18 PM, Steve Lawrence wrote:
> 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@redhat.com>
>>>> 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@tycho.nsa.gov
>>>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>>>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>>>
>>>>
>>> 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
If you remove it and the command succeeds I am happy.  Most of these
changes were made to just stop the
application from blowing up.  If we get better accuracy on matches and
the code does not break, It is a win win.

Re: Debugging sepolgen-ifgen?

[Karl MacMillan]

[-- Attachment #1: Type: text/plain, Size: 3763 bytes --]

FWIW, most of the shift / reduce conflicts are because the grammar really
is ambiguous (since we are trying to parse both the selinux policy language
and the m4 additions on top of that). While in an ideal world those would
be cleaned up so that we would at least choose what to do in the ambiguous
cases it just never seemed worthwhile to me.

Karl


On Tue, Aug 5, 2014 at 9:09 AM, Stephen Smalley <sds@tycho.nsa.gov> 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@redhat.com>
> >> 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@tycho.nsa.gov
> >> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> >> To get help, send an email containing "help" to
> Selinux-request@tycho.nsa.gov.
> >>
> >>
> > 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...
>

[-- Attachment #2: Type: text/html, Size: 5172 bytes --]