Adjust sepolgen grammar to support allowxperm, et. al.

Adjust sepolgen grammar to support allowxperm, et. al.

[chris.lindee]

Provide basic support for allowxperm, auditallowxperm, dontauditxperm and neverallowxperm.  While I would prefer additional changes to help avoid the presence of magic numbers (e.g. a new macro, much like interface, but for recursively defining named xperm numbers), this patch set is sufficient for my and - hopefully - the majority of the community's needs.

In particular, this change will keep /usr/bin/sepolgen-ifgen from spewing errors on the following policy every time selinux-policy-targeted gets updated: https://github.com/openzfs/zfs/pull/13271/files#diff-70b325e496b997b3c4a5a9f0aacee16343b82e07a8ed8220304ccb5f6504a582

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 python/sepolgen/src/sepolgen/refpolicy.py | 18 ++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)

[PATCH 1/2] sepolgen: Update refparser to handle xperm

[chris.lindee]

From: Chris Lindee <chris.lindee+github@gmail.com>

Extend the grammar to support `allowxperm`, et. al. directives, which
were added in policy version 30 to give more granular control.  This
commit adds basic support for the syntax, copying heavily from the
grammar for `allowperm`, et. al.

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 80 +++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index e611637f..1d801f41 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -67,6 +67,7 @@ tokens = (
     'FILENAME',
     'IDENTIFIER',
     'NUMBER',
+    'XNUMBER',
     'PATH',
     'IPV6_ADDR',
     # reserved words
@@ -112,6 +113,10 @@ tokens = (
     'DONTAUDIT',
     'AUDITALLOW',
     'NEVERALLOW',
+    'ALLOWXPERM',
+    'DONTAUDITXPERM',
+    'AUDITALLOWXPERM',
+    'NEVERALLOWXPERM',
     'PERMISSIVE',
     'TYPEBOUNDS',
     'TYPE_TRANSITION',
@@ -179,6 +184,10 @@ reserved = {
     'dontaudit' : 'DONTAUDIT',
     'auditallow' : 'AUDITALLOW',
     'neverallow' : 'NEVERALLOW',
+    'allowxperm' : 'ALLOWXPERM',
+    'dontauditxperm' : 'DONTAUDITXPERM',
+    'auditallowxperm' : 'AUDITALLOWXPERM',
+    'neverallowxperm' : 'NEVERALLOWXPERM',
     'permissive' : 'PERMISSIVE',
     'typebounds' : 'TYPEBOUNDS',
     'type_transition' : 'TYPE_TRANSITION',
@@ -231,6 +240,12 @@ t_PATH      = r'/[a-zA-Z0-9)_\.\*/\$]*'
 t_ignore    = " \t"
 
 # More complex tokens
+def t_XNUMBER(t):
+    r'0x[0-9A-Fa-f]+'
+    # Turn hexadecimal into integer
+    t.value = int(t.value, 16)
+    return t
+
 def t_IPV6_ADDR(t):
     r'[a-fA-F0-9]{0,4}:[a-fA-F0-9]{0,4}:([a-fA-F0-9]|:)*'
     # This is a function simply to force it sooner into
@@ -505,6 +520,7 @@ def p_policy(p):
 def p_policy_stmt(p):
     '''policy_stmt : gen_require
                    | avrule_def
+                   | avextrule_def
                    | typerule_def
                    | typebound_def
                    | typeattribute_def
@@ -810,6 +826,26 @@ def p_avrule_def(p):
     a.perms = p[6]
     p[0] = a
 
+def p_avextrule_def(p):
+    '''avextrule_def : ALLOWXPERM names names COLON names identifier xperm_set SEMI
+                     | DONTAUDITXPERM names names COLON names identifier xperm_set SEMI
+                     | AUDITALLOWXPERM names names COLON names identifier xperm_set SEMI
+                     | NEVERALLOWXPERM names names COLON names identifier xperm_set SEMI
+    '''
+    a = refpolicy.AVExtRule()
+    if p[1] == 'dontauditxperm':
+        a.rule_type = refpolicy.AVExtRule.DONTAUDITXPERM
+    elif p[1] == 'auditallowxperm':
+        a.rule_type = refpolicy.AVExtRule.AUDITALLOWXPERM
+    elif p[1] == 'neverallowxperm':
+        a.rule_type = refpolicy.AVExtRule.NEVERALLOWXPERM
+    a.src_types = p[2]
+    a.tgt_types = p[3]
+    a.obj_classes = p[5]
+    a.operation = p[6]
+    a.xperms = p[7]
+    p[0] = a
+
 def p_typerule_def(p):
     '''typerule_def : TYPE_TRANSITION names names COLON names IDENTIFIER SEMI
                     | TYPE_TRANSITION names names COLON names IDENTIFIER FILENAME SEMI
@@ -987,6 +1023,50 @@ def p_optional_semi(p):
                    | empty'''
     pass
 
+def p_xperm_set(p):
+    '''xperm_set : nested_xperm_set
+                 | TILDE nested_xperm_set
+                 | xperm_set_base
+                 | TILDE xperm_set_base
+    '''
+    p[0] = p[-1]
+    if len(p) == 3:
+        p[0].compliment = True
+
+def p_nested_xperm_set(p):
+    '''nested_xperm_set : OBRACE nested_xperm_list CBRACE
+    '''
+    p[0] = p[2]
+
+def p_nested_xperm_list(p):
+    '''nested_xperm_list : nested_xperm_element
+                         | nested_xperm_list nested_xperm_element
+    '''
+    p[0] = p[1]
+    if len(p) == 3:
+        p[0].extend(p[2])
+
+def p_nested_xperm_element(p):
+    '''nested_xperm_element : xperm_set_base
+                            | nested_xperm_set
+    '''
+    p[0] = p[1]
+
+def p_xperm_set_base(p):
+    '''xperm_set_base : xperm_number
+                      | xperm_number MINUS xperm_number
+    '''
+    p[0] = refpolicy.XpermSet()
+    if len(p) == 2:
+        p[0].add(p[1])
+    else:
+        p[0].add(p[1], p[3])
+
+def p_xperm_number(p):
+    '''xperm_number : NUMBER
+                    | XNUMBER
+    '''
+    p[0] = int(p[1])
 
 #
 # Interface to the parser
-- 
2.37.1

[PATCH 2/2] sepolgen: Support named xperms

[chris.lindee]

From: Chris Lindee <chris.lindee+github@gmail.com>

The `allowxperm` et. al. directives take a magical integer for one of
the fields, which hinders readability.  This commit adds support for
basic names in place of a number or group of numbers.

Notably, this does not support recursive definition of names, as that
would require a larger grammar re-write to avoid parsing conflicts.

Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 18 ++++++++++++++++--
 python/sepolgen/src/sepolgen/refpolicy.py | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index 1d801f41..4d74b342 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -349,6 +349,7 @@ def p_statement(p):
     '''statement : interface
                  | template
                  | obj_perm_set
+                 | obj_xperm_set
                  | policy
                  | policy_module_stmt
                  | module_stmt
@@ -502,7 +503,15 @@ def p_obj_perm_set(p):
     s = refpolicy.ObjPermSet(p[4])
     s.perms = p[8]
     p[0] = s
-    
+
+def p_obj_xperm_set(p):
+    'obj_xperm_set : DEFINE OPAREN TICK IDENTIFIER SQUOTE COMMA TICK xperm_set_base SQUOTE CPAREN'
+    ids = refpolicy.XpermIdentifierDict()
+    ids.set(p[4], p[8])
+
+    p[0] = refpolicy.ObjPermSet(p[4])
+    p[0].perms = set(p[8])
+
 #
 # Basic SELinux policy language
 #
@@ -1049,8 +1058,13 @@ def p_nested_xperm_list(p):
 def p_nested_xperm_element(p):
     '''nested_xperm_element : xperm_set_base
                             | nested_xperm_set
+                            | IDENTIFIER
     '''
-    p[0] = p[1]
+    if isinstance(p[1], refpolicy.XpermSet()):
+        p[0] = p[1]
+    else:
+        ids = refpolicy.XpermIdentifierDict()
+        p[0] = ids.get(p[1])
 
 def p_xperm_set_base(p):
     '''xperm_set_base : xperm_number
diff --git a/python/sepolgen/src/sepolgen/refpolicy.py b/python/sepolgen/src/sepolgen/refpolicy.py
index 3e907e91..07d622d2 100644
--- a/python/sepolgen/src/sepolgen/refpolicy.py
+++ b/python/sepolgen/src/sepolgen/refpolicy.py
@@ -413,6 +413,24 @@ class XpermSet():
 
         return "%s{ %s }" % (compl, " ".join(vals))
 
+class XpermIdentifierDict(dict):
+    """Extended permission set identifier mapping.
+
+    This singleton class holds the mappings between named
+    extended permission and their numberic value.
+    """
+    def __new__(cls):
+        if not hasattr(cls, 'instance'):
+            cls.instance = super(XpermIdentifierDict, cls).__new__(cls)
+        return cls.instance
+
+    def set(self, key, value):
+        # TODO: warn about redefiniition
+        self[key] = value
+
+    def get(self, key):
+        return self[key]
+
 # Basic statements
 
 class TypeAttribute(Leaf):
-- 
2.37.1

Re: Adjust sepolgen grammar to support allowxperm, et. al.

[James Carter]

On Sun, Jul 31, 2022 at 10:07 PM <chris.lindee@gmail.com> wrote:
>
> Provide basic support for allowxperm, auditallowxperm, dontauditxperm and neverallowxperm.  While I would prefer additional changes to help avoid the presence of magic numbers (e.g. a new macro, much like interface, but for recursively defining named xperm numbers), this patch set is sufficient for my and - hopefully - the majority of the community's needs.
>
> In particular, this change will keep /usr/bin/sepolgen-ifgen from spewing errors on the following policy every time selinux-policy-targeted gets updated: https://github.com/openzfs/zfs/pull/13271/files#diff-70b325e496b997b3c4a5a9f0aacee16343b82e07a8ed8220304ccb5f6504a582
>
> Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>

I have been looking at these two patches. Either there is a problem in
them or there is a problem elsewhere. I can't tell.

What I did to test them was to modify
python/sepolgen/tests/test_refparser.py and add an interface to the
interface_example string.

This is what I added:

define(`XPERM1',`0x9901')
interface(`test_extended_permissions',`
    gen_require(`
        type device_t;
    ')
        allow $1 device_t:chr_file {ioctl};
        allowxperm $1 device_t:chr_file ioctl 0x9910;
        dontauditxperm $1 device_t:chr_file ioctl 0x9911;
        auditallowxperm $1 device_t:chr_file ioctl 0x9912;
        neverallowxperm $1 device_t:chr_file ioctl 0x9913;
        allowxperm $1 device_t:chr_file ioctl { 0x9914 };
        allowxperm $1 device_t:chr_file ioctl { 0x9915 0x9916 };
        allowxperm $1 device_t:chr_file ioctl { 0x9917-0x9919 };
        allowxperm $1 device_t:chr_file ioctl XPERM1;
')

I then uncommented the "refpolicy.print_tree(h)" line in test_refparser.py.

If I comment out the definition of XPERM1 and every xperm rule except
the first four (the first allowxperm, dontauditxperm, auditallowxperm,
and neverallowxperm) then the tree will printed out (although the
rules are printed in reverse order). There is one weirdness in that
the operation is printed out as "['ioctl']", but that is not due to
anything in your patches.

If any xperm rule with curly brackets is used or the definition of
XPERM1, then the test_extended_permissions() interface is not printed
out. It appears that parsing is abandoned as soon as one of those
rules is encountered without an error being given. I literally typed
random characters on the line after the definition of XPERM1 and no
error was given.

I don't see any obvious error in what you have done and it is very
possible that the problem is elsewhere. The xperm stuff that is in
sepolgen is strange. In access.py, xperms are stored in the
AccessVector class as a dictionary with the operation being used as
the key. In refpolicy.py, xperms have their own class, AVExtRule, with
the operation being one field and the xperms being another field that
is an XpermSet.

At this point, your patches might remove the parsing errors, but that
will only give a false sense that the xperms are actually being
handled, when they are not.

I am not sure where to go from here. I don't know the python code very well.

Thanks,
Jim


> ---
>  python/sepolgen/src/sepolgen/refparser.py | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  python/sepolgen/src/sepolgen/refpolicy.py | 18 ++++++++++++++++++
>  2 files changed, 113 insertions(+), 1 deletion(-)
>
>

Re: [PATCH 1/2] sepolgen: Update refparser to handle xperm

[Christian Göttsche]

On Mon, 1 Aug 2022 at 03:57, <chris.lindee@gmail.com> wrote:
>
> From: Chris Lindee <chris.lindee+github@gmail.com>
>
> Extend the grammar to support `allowxperm`, et. al. directives, which
> were added in policy version 30 to give more granular control.  This
> commit adds basic support for the syntax, copying heavily from the
> grammar for `allowperm`, et. al.

Looks good to me; two comments inline.


> Signed-off-by: Chris Lindee <chris.lindee+github@gmail.com>
> ---
>  python/sepolgen/src/sepolgen/refparser.py | 80 +++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> index e611637f..1d801f41 100644
> --- a/python/sepolgen/src/sepolgen/refparser.py
> +++ b/python/sepolgen/src/sepolgen/refparser.py
> @@ -67,6 +67,7 @@ tokens = (
>      'FILENAME',
>      'IDENTIFIER',
>      'NUMBER',
> +    'XNUMBER',
>      'PATH',
>      'IPV6_ADDR',
>      # reserved words
> @@ -112,6 +113,10 @@ tokens = (
>      'DONTAUDIT',
>      'AUDITALLOW',
>      'NEVERALLOW',
> +    'ALLOWXPERM',
> +    'DONTAUDITXPERM',
> +    'AUDITALLOWXPERM',
> +    'NEVERALLOWXPERM',
>      'PERMISSIVE',
>      'TYPEBOUNDS',
>      'TYPE_TRANSITION',
> @@ -179,6 +184,10 @@ reserved = {
>      'dontaudit' : 'DONTAUDIT',
>      'auditallow' : 'AUDITALLOW',
>      'neverallow' : 'NEVERALLOW',
> +    'allowxperm' : 'ALLOWXPERM',
> +    'dontauditxperm' : 'DONTAUDITXPERM',
> +    'auditallowxperm' : 'AUDITALLOWXPERM',
> +    'neverallowxperm' : 'NEVERALLOWXPERM',
>      'permissive' : 'PERMISSIVE',
>      'typebounds' : 'TYPEBOUNDS',
>      'type_transition' : 'TYPE_TRANSITION',
> @@ -231,6 +240,12 @@ t_PATH      = r'/[a-zA-Z0-9)_\.\*/\$]*'
>  t_ignore    = " \t"
>
>  # More complex tokens
> +def t_XNUMBER(t):
> +    r'0x[0-9A-Fa-f]+'
> +    # Turn hexadecimal into integer
> +    t.value = int(t.value, 16)
> +    return t
> +
>  def t_IPV6_ADDR(t):
>      r'[a-fA-F0-9]{0,4}:[a-fA-F0-9]{0,4}:([a-fA-F0-9]|:)*'
>      # This is a function simply to force it sooner into
> @@ -505,6 +520,7 @@ def p_policy(p):
>  def p_policy_stmt(p):
>      '''policy_stmt : gen_require
>                     | avrule_def
> +                   | avextrule_def
>                     | typerule_def
>                     | typebound_def
>                     | typeattribute_def
> @@ -810,6 +826,26 @@ def p_avrule_def(p):
>      a.perms = p[6]
>      p[0] = a
>
> +def p_avextrule_def(p):
> +    '''avextrule_def : ALLOWXPERM names names COLON names identifier xperm_set SEMI
> +                     | DONTAUDITXPERM names names COLON names identifier xperm_set SEMI
> +                     | AUDITALLOWXPERM names names COLON names identifier xperm_set SEMI
> +                     | NEVERALLOWXPERM names names COLON names identifier xperm_set SEMI
> +    '''
> +    a = refpolicy.AVExtRule()
> +    if p[1] == 'dontauditxperm':
> +        a.rule_type = refpolicy.AVExtRule.DONTAUDITXPERM
> +    elif p[1] == 'auditallowxperm':
> +        a.rule_type = refpolicy.AVExtRule.AUDITALLOWXPERM
> +    elif p[1] == 'neverallowxperm':
> +        a.rule_type = refpolicy.AVExtRule.NEVERALLOWXPERM
> +    a.src_types = p[2]
> +    a.tgt_types = p[3]
> +    a.obj_classes = p[5]
> +    a.operation = p[6]
> +    a.xperms = p[7]
> +    p[0] = a
> +
>  def p_typerule_def(p):
>      '''typerule_def : TYPE_TRANSITION names names COLON names IDENTIFIER SEMI
>                      | TYPE_TRANSITION names names COLON names IDENTIFIER FILENAME SEMI
> @@ -987,6 +1023,50 @@ def p_optional_semi(p):
>                     | empty'''
>      pass
>
> +def p_xperm_set(p):
> +    '''xperm_set : nested_xperm_set
> +                 | TILDE nested_xperm_set
> +                 | xperm_set_base
> +                 | TILDE xperm_set_base

maybe include IDENTIFER as option to accept

    allowxperm $1 $2:$3 ioctl $4;

> +    '''
> +    p[0] = p[-1]
> +    if len(p) == 3:
> +        p[0].compliment = True
> +
> +def p_nested_xperm_set(p):
> +    '''nested_xperm_set : OBRACE nested_xperm_list CBRACE
> +    '''
> +    p[0] = p[2]
> +
> +def p_nested_xperm_list(p):
> +    '''nested_xperm_list : nested_xperm_element
> +                         | nested_xperm_list nested_xperm_element
> +    '''
> +    p[0] = p[1]
> +    if len(p) == 3:
> +        p[0].extend(p[2])
> +
> +def p_nested_xperm_element(p):
> +    '''nested_xperm_element : xperm_set_base
> +                            | nested_xperm_set
> +    '''
> +    p[0] = p[1]
> +
> +def p_xperm_set_base(p):
> +    '''xperm_set_base : xperm_number
> +                      | xperm_number MINUS xperm_number
> +    '''
> +    p[0] = refpolicy.XpermSet()
> +    if len(p) == 2:
> +        p[0].add(p[1])
> +    else:
> +        p[0].add(p[1], p[3])

Single numbers might also be enclosed in braces, so maybe add an option

    OBRACE xperm_number CBRACE

and parsing it via

    elif p[1] == '{':
        p[0].add(p[2])

> +
> +def p_xperm_number(p):
> +    '''xperm_number : NUMBER
> +                    | XNUMBER
> +    '''
> +    p[0] = int(p[1])
>
>  #
>  # Interface to the parser
> --
> 2.37.1
>