[PATCH] sepolgen: parse gen_tunable as bool

[PATCH] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Currently sepolgen-ifgen parses a gen_tunable statement as interface
and reports:

    Missing interface definition for gen_tunable

Add grammar for gen_tunable statements in the refparser

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index 2e521a0f..be7e7890 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -126,6 +126,7 @@ tokens = (
     'GEN_REQ',
     'TEMPLATE',
     'GEN_CONTEXT',
+    'GEN_TUNABLE',
     #   m4
     'IFELSE',
     'IFDEF',
@@ -192,6 +193,7 @@ reserved = {
     'gen_require' : 'GEN_REQ',
     'template' : 'TEMPLATE',
     'gen_context' : 'GEN_CONTEXT',
+    'gen_tunable' : 'GEN_TUNABLE',
     # M4
     'ifelse' : 'IFELSE',
     'ifndef' : 'IFNDEF',
@@ -518,6 +520,7 @@ def p_policy_stmt(p):
                    | range_transition_def
                    | role_transition_def
                    | bool
+                   | gen_tunable
                    | define
                    | initial_sid
                    | genfscon
@@ -844,6 +847,17 @@ def p_bool(p):
         b.state = False
     p[0] = b
 
+def p_gen_tunable(p):
+    '''gen_tunable : GEN_TUNABLE OPAREN IDENTIFIER COMMA TRUE CPAREN
+                   | GEN_TUNABLE OPAREN IDENTIFIER COMMA FALSE CPAREN'''
+    b = refpolicy.Bool()
+    b.name = p[3]
+    if p[5] == "true":
+        b.state = True
+    else:
+        b.state = False
+    p[0] = b
+
 def p_conditional(p):
     ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
                     | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
-- 
2.26.2

Re: [PATCH] sepolgen: parse gen_tunable as bool

[Stephen Smalley]

On Tue, May 5, 2020 at 3:03 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Currently sepolgen-ifgen parses a gen_tunable statement as interface
> and reports:
>
>     Missing interface definition for gen_tunable
>
> Add grammar for gen_tunable statements in the refparser
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

This looks correct to me but I don't see that warning/error when
running sepolgen-ifgen without this patch.
Is this reproducible?

> ---
>  python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> index 2e521a0f..be7e7890 100644
> --- a/python/sepolgen/src/sepolgen/refparser.py
> +++ b/python/sepolgen/src/sepolgen/refparser.py
> @@ -126,6 +126,7 @@ tokens = (
>      'GEN_REQ',
>      'TEMPLATE',
>      'GEN_CONTEXT',
> +    'GEN_TUNABLE',
>      #   m4
>      'IFELSE',
>      'IFDEF',
> @@ -192,6 +193,7 @@ reserved = {
>      'gen_require' : 'GEN_REQ',
>      'template' : 'TEMPLATE',
>      'gen_context' : 'GEN_CONTEXT',
> +    'gen_tunable' : 'GEN_TUNABLE',
>      # M4
>      'ifelse' : 'IFELSE',
>      'ifndef' : 'IFNDEF',
> @@ -518,6 +520,7 @@ def p_policy_stmt(p):
>                     | range_transition_def
>                     | role_transition_def
>                     | bool
> +                   | gen_tunable
>                     | define
>                     | initial_sid
>                     | genfscon
> @@ -844,6 +847,17 @@ def p_bool(p):
>          b.state = False
>      p[0] = b
>
> +def p_gen_tunable(p):
> +    '''gen_tunable : GEN_TUNABLE OPAREN IDENTIFIER COMMA TRUE CPAREN
> +                   | GEN_TUNABLE OPAREN IDENTIFIER COMMA FALSE CPAREN'''
> +    b = refpolicy.Bool()
> +    b.name = p[3]
> +    if p[5] == "true":
> +        b.state = True
> +    else:
> +        b.state = False
> +    p[0] = b
> +
>  def p_conditional(p):
>      ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
>                      | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
> --
> 2.26.2
>

[PATCH v2 1/3] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Currently sepolgen-ifgen parses a gen_tunable statement as interface
and reports in verbose mode:

    Missing interface definition for gen_tunable

Add grammar for gen_tunable statements in the refparser

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index 2e521a0f..f3e0ae87 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -126,6 +126,7 @@ tokens = (
     'GEN_REQ',
     'TEMPLATE',
     'GEN_CONTEXT',
+    'GEN_TUNABLE',
     #   m4
     'IFELSE',
     'IFDEF',
@@ -192,6 +193,7 @@ reserved = {
     'gen_require' : 'GEN_REQ',
     'template' : 'TEMPLATE',
     'gen_context' : 'GEN_CONTEXT',
+    'gen_tunable' : 'GEN_TUNABLE',
     # M4
     'ifelse' : 'IFELSE',
     'ifndef' : 'IFNDEF',
@@ -518,6 +520,7 @@ def p_policy_stmt(p):
                    | range_transition_def
                    | role_transition_def
                    | bool
+                   | gen_tunable
                    | define
                    | initial_sid
                    | genfscon
@@ -844,6 +847,17 @@ def p_bool(p):
         b.state = False
     p[0] = b
 
+def p_gen_tunable(p):
+    '''gen_tunable : GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA TRUE CPAREN
+                   | GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA FALSE CPAREN'''
+    b = refpolicy.Bool()
+    b.name = p[4]
+    if p[7] == "true":
+        b.state = True
+    else:
+        b.state = False
+    p[0] = b
+
 def p_conditional(p):
     ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
                     | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
-- 
2.27.0.rc2

[PATCH v2 2/3] refparser: add missing newline after error message

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 python/sepolgen/src/sepolgen/refparser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index f3e0ae87..9f850990 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -1148,6 +1148,6 @@ def parse_headers(root, output=None, expand=True, debug=False):
             status.step()
 
     if len(failures):
-        o("failed to parse some headers: %s" % ", ".join(failures))
+        o("failed to parse some headers: %s\n" % ", ".join(failures))
 
     return headers
-- 
2.27.0.rc2

[PATCH v2 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Christian Göttsche]

On a SELinux disabled system `selinux.security_policyvers()` will fail;
do not bailout but use a fallback policy version to check if a binary
policy file with that extension exists.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 python/audit2allow/sepolgen-ifgen | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
index 4a71cda4..48e60f1d 100644
--- a/python/audit2allow/sepolgen-ifgen
+++ b/python/audit2allow/sepolgen-ifgen
@@ -69,7 +69,11 @@ def get_policy():
     p = selinux.selinux_current_policy_path()
     if p and os.path.exists(p):
         return p
-    i = selinux.security_policyvers()
+    try:
+        i = selinux.security_policyvers()
+    except OSError:
+        # SELinux Disabled Machine
+        i = 50 # some high enough default value
     p = selinux.selinux_binary_policy_path() + "." + str(i)
     while i > 0 and not os.path.exists(p):
         i = i - 1
@@ -80,18 +84,16 @@ def get_policy():
 
 
 def get_attrs(policy_path, attr_helper):
+    if not policy_path:
+        policy_path = get_policy()
+    if not policy_path:
+        sys.stderr.write("No installed policy to check\n")
+        return None
+
     try:
-        if not policy_path:
-            policy_path = get_policy()
-        if not policy_path:
-            sys.stderr.write("No installed policy to check\n")
-            return None
         outfile = tempfile.NamedTemporaryFile()
     except IOError as e:
-        sys.stderr.write("could not open attribute output file\n")
-        return None
-    except OSError:
-        # SELinux Disabled Machine
+        sys.stderr.write("could not open attribute output file: %s\n" % e)
         return None
 
     fd = open("/dev/null", "w")
-- 
2.27.0.rc2

Re: [PATCH] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Am Mi., 27. Mai 2020 um 17:04 Uhr schrieb Stephen Smalley
<stephen.smalley.work@gmail.com>:
>
> On Tue, May 5, 2020 at 3:03 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Currently sepolgen-ifgen parses a gen_tunable statement as interface
> > and reports:
> >
> >     Missing interface definition for gen_tunable
> >
> > Add grammar for gen_tunable statements in the refparser
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> This looks correct to me but I don't see that warning/error when
> running sepolgen-ifgen without this patch.
> Is this reproducible?
>

It should be when running in verbose mode. (running against Refpolicy)

> > ---
> >  python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> > index 2e521a0f..be7e7890 100644
> > --- a/python/sepolgen/src/sepolgen/refparser.py
> > +++ b/python/sepolgen/src/sepolgen/refparser.py
> > @@ -126,6 +126,7 @@ tokens = (
> >      'GEN_REQ',
> >      'TEMPLATE',
> >      'GEN_CONTEXT',
> > +    'GEN_TUNABLE',
> >      #   m4
> >      'IFELSE',
> >      'IFDEF',
> > @@ -192,6 +193,7 @@ reserved = {
> >      'gen_require' : 'GEN_REQ',
> >      'template' : 'TEMPLATE',
> >      'gen_context' : 'GEN_CONTEXT',
> > +    'gen_tunable' : 'GEN_TUNABLE',
> >      # M4
> >      'ifelse' : 'IFELSE',
> >      'ifndef' : 'IFNDEF',
> > @@ -518,6 +520,7 @@ def p_policy_stmt(p):
> >                     | range_transition_def
> >                     | role_transition_def
> >                     | bool
> > +                   | gen_tunable
> >                     | define
> >                     | initial_sid
> >                     | genfscon
> > @@ -844,6 +847,17 @@ def p_bool(p):
> >          b.state = False
> >      p[0] = b
> >
> > +def p_gen_tunable(p):
> > +    '''gen_tunable : GEN_TUNABLE OPAREN IDENTIFIER COMMA TRUE CPAREN
> > +                   | GEN_TUNABLE OPAREN IDENTIFIER COMMA FALSE CPAREN'''
> > +    b = refpolicy.Bool()
> > +    b.name = p[3]
> > +    if p[5] == "true":
> > +        b.state = True
> > +    else:
> > +        b.state = False
> > +    p[0] = b
> > +
> >  def p_conditional(p):
> >      ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
> >                      | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
> > --
> > 2.26.2
> >

Re: [PATCH v2 1/3] sepolgen: parse gen_tunable as bool

[Stephen Smalley]

On Thu, May 28, 2020 at 8:52 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Currently sepolgen-ifgen parses a gen_tunable statement as interface
> and reports in verbose mode:
>
>     Missing interface definition for gen_tunable
>
> Add grammar for gen_tunable statements in the refparser
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> index 2e521a0f..f3e0ae87 100644
> --- a/python/sepolgen/src/sepolgen/refparser.py
> +++ b/python/sepolgen/src/sepolgen/refparser.py
> @@ -126,6 +126,7 @@ tokens = (
>      'GEN_REQ',
>      'TEMPLATE',
>      'GEN_CONTEXT',
> +    'GEN_TUNABLE',
>      #   m4
>      'IFELSE',
>      'IFDEF',
> @@ -192,6 +193,7 @@ reserved = {
>      'gen_require' : 'GEN_REQ',
>      'template' : 'TEMPLATE',
>      'gen_context' : 'GEN_CONTEXT',
> +    'gen_tunable' : 'GEN_TUNABLE',
>      # M4
>      'ifelse' : 'IFELSE',
>      'ifndef' : 'IFNDEF',
> @@ -518,6 +520,7 @@ def p_policy_stmt(p):
>                     | range_transition_def
>                     | role_transition_def
>                     | bool
> +                   | gen_tunable
>                     | define
>                     | initial_sid
>                     | genfscon
> @@ -844,6 +847,17 @@ def p_bool(p):
>          b.state = False
>      p[0] = b
>
> +def p_gen_tunable(p):
> +    '''gen_tunable : GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA TRUE CPAREN
> +                   | GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA FALSE CPAREN'''

Looks like you need to also support the case where no quoting is
performed.  Otherwise, I still see syntax errors, e.g.
/usr/share/selinux/refpolicy/include/services/apache.if: Syntax error
on line 35 allow_httpd_$1_script_anon_write [type=IDENTIFIER]

35: gen_tunable(allow_httpd_$1_script_anon_write, false)

Re: [PATCH v2 1/3] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Am Do., 28. Mai 2020 um 16:23 Uhr schrieb Stephen Smalley
<stephen.smalley.work@gmail.com>:
>
> On Thu, May 28, 2020 at 8:52 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Currently sepolgen-ifgen parses a gen_tunable statement as interface
> > and reports in verbose mode:
> >
> >     Missing interface definition for gen_tunable
> >
> > Add grammar for gen_tunable statements in the refparser
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> > index 2e521a0f..f3e0ae87 100644
> > --- a/python/sepolgen/src/sepolgen/refparser.py
> > +++ b/python/sepolgen/src/sepolgen/refparser.py
> > @@ -126,6 +126,7 @@ tokens = (
> >      'GEN_REQ',
> >      'TEMPLATE',
> >      'GEN_CONTEXT',
> > +    'GEN_TUNABLE',
> >      #   m4
> >      'IFELSE',
> >      'IFDEF',
> > @@ -192,6 +193,7 @@ reserved = {
> >      'gen_require' : 'GEN_REQ',
> >      'template' : 'TEMPLATE',
> >      'gen_context' : 'GEN_CONTEXT',
> > +    'gen_tunable' : 'GEN_TUNABLE',
> >      # M4
> >      'ifelse' : 'IFELSE',
> >      'ifndef' : 'IFNDEF',
> > @@ -518,6 +520,7 @@ def p_policy_stmt(p):
> >                     | range_transition_def
> >                     | role_transition_def
> >                     | bool
> > +                   | gen_tunable
> >                     | define
> >                     | initial_sid
> >                     | genfscon
> > @@ -844,6 +847,17 @@ def p_bool(p):
> >          b.state = False
> >      p[0] = b
> >
> > +def p_gen_tunable(p):
> > +    '''gen_tunable : GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA TRUE CPAREN
> > +                   | GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA FALSE CPAREN'''
>
> Looks like you need to also support the case where no quoting is
> performed.  Otherwise, I still see syntax errors, e.g.
> /usr/share/selinux/refpolicy/include/services/apache.if: Syntax error
> on line 35 allow_httpd_$1_script_anon_write [type=IDENTIFIER]
>
> 35: gen_tunable(allow_httpd_$1_script_anon_write, false)

I tried to avoid that by modifying Refpolicy[1], but I can include the
additional grammar.


[1]: https://github.com/SELinuxProject/refpolicy/pull/201

Re: [PATCH v2 2/3] refparser: add missing newline after error message

[Stephen Smalley]

On Thu, May 28, 2020 at 8:52 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

This seems fine although I wonder if we should just do this inside of
the o() definition and have it done for all callers.
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  python/sepolgen/src/sepolgen/refparser.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
> index f3e0ae87..9f850990 100644
> --- a/python/sepolgen/src/sepolgen/refparser.py
> +++ b/python/sepolgen/src/sepolgen/refparser.py
> @@ -1148,6 +1148,6 @@ def parse_headers(root, output=None, expand=True, debug=False):
>              status.step()
>
>      if len(failures):
> -        o("failed to parse some headers: %s" % ", ".join(failures))
> +        o("failed to parse some headers: %s\n" % ", ".join(failures))
>
>      return headers
> --
> 2.27.0.rc2
>

Re: [PATCH v2 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Stephen Smalley]

On Thu, May 28, 2020 at 8:52 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On a SELinux disabled system `selinux.security_policyvers()` will fail;
> do not bailout but use a fallback policy version to check if a binary
> policy file with that extension exists.

Technically we should be using sepol_policy_kern_vers_max() as the
upper bound since this is for the purpose of reading the policy by
sepolgen-ifgen-attr-helper and it requires that the policy version be
known to the version of libsepol against which it was compiled but I
guess there isn't a python wrapper for it.  Not sure why we aren't
just having sepolgen-ifgen-attr-helper itself find the policy file in
which case it could call sepol_policy_kern_vers_max().  Not keen on
hardcoding an upper bound here.

>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  python/audit2allow/sepolgen-ifgen | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
> index 4a71cda4..48e60f1d 100644
> --- a/python/audit2allow/sepolgen-ifgen
> +++ b/python/audit2allow/sepolgen-ifgen
> @@ -69,7 +69,11 @@ def get_policy():
>      p = selinux.selinux_current_policy_path()
>      if p and os.path.exists(p):
>          return p
> -    i = selinux.security_policyvers()
> +    try:
> +        i = selinux.security_policyvers()
> +    except OSError:
> +        # SELinux Disabled Machine
> +        i = 50 # some high enough default value
>      p = selinux.selinux_binary_policy_path() + "." + str(i)
>      while i > 0 and not os.path.exists(p):
>          i = i - 1
> @@ -80,18 +84,16 @@ def get_policy():
>
>
>  def get_attrs(policy_path, attr_helper):
> +    if not policy_path:
> +        policy_path = get_policy()
> +    if not policy_path:
> +        sys.stderr.write("No installed policy to check\n")
> +        return None
> +
>      try:
> -        if not policy_path:
> -            policy_path = get_policy()
> -        if not policy_path:
> -            sys.stderr.write("No installed policy to check\n")
> -            return None
>          outfile = tempfile.NamedTemporaryFile()
>      except IOError as e:
> -        sys.stderr.write("could not open attribute output file\n")
> -        return None
> -    except OSError:
> -        # SELinux Disabled Machine
> +        sys.stderr.write("could not open attribute output file: %s\n" % e)
>          return None
>
>      fd = open("/dev/null", "w")
> --
> 2.27.0.rc2
>

Re: [PATCH v2 1/3] sepolgen: parse gen_tunable as bool

[Stephen Smalley]

On Thu, May 28, 2020 at 10:52 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Am Do., 28. Mai 2020 um 16:23 Uhr schrieb Stephen Smalley
> <stephen.smalley.work@gmail.com>:
> > Looks like you need to also support the case where no quoting is
> > performed.  Otherwise, I still see syntax errors, e.g.
> > /usr/share/selinux/refpolicy/include/services/apache.if: Syntax error
> > on line 35 allow_httpd_$1_script_anon_write [type=IDENTIFIER]
> >
> > 35: gen_tunable(allow_httpd_$1_script_anon_write, false)
>
> I tried to avoid that by modifying Refpolicy[1], but I can include the
> additional grammar.
>
>
> [1]: https://github.com/SELinuxProject/refpolicy/pull/201

Looks like your refpolicy pull request was merged so I guess we can
take this one as is if we don't care about fixing it for older
refpolicy versions.  The third patch in the series still needs to be
reworked or dropped I think as per my comments there.

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

[PATCH v3 1/3] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Currently sepolgen-ifgen parses a gen_tunable statement as interface
and reports in verbose mode:

    Missing interface definition for gen_tunable

Add grammar for gen_tunable statements in the refparser

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3: no changes

 python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index 2e521a0f..f3e0ae87 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -126,6 +126,7 @@ tokens = (
     'GEN_REQ',
     'TEMPLATE',
     'GEN_CONTEXT',
+    'GEN_TUNABLE',
     #   m4
     'IFELSE',
     'IFDEF',
@@ -192,6 +193,7 @@ reserved = {
     'gen_require' : 'GEN_REQ',
     'template' : 'TEMPLATE',
     'gen_context' : 'GEN_CONTEXT',
+    'gen_tunable' : 'GEN_TUNABLE',
     # M4
     'ifelse' : 'IFELSE',
     'ifndef' : 'IFNDEF',
@@ -518,6 +520,7 @@ def p_policy_stmt(p):
                    | range_transition_def
                    | role_transition_def
                    | bool
+                   | gen_tunable
                    | define
                    | initial_sid
                    | genfscon
@@ -844,6 +847,17 @@ def p_bool(p):
         b.state = False
     p[0] = b
 
+def p_gen_tunable(p):
+    '''gen_tunable : GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA TRUE CPAREN
+                   | GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA FALSE CPAREN'''
+    b = refpolicy.Bool()
+    b.name = p[4]
+    if p[7] == "true":
+        b.state = True
+    else:
+        b.state = False
+    p[0] = b
+
 def p_conditional(p):
     ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
                     | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
-- 
2.27.0

[PATCH v3 2/3] refparser: add missing newline after error message

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3: no changes

 python/sepolgen/src/sepolgen/refparser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index f3e0ae87..9f850990 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -1148,6 +1148,6 @@ def parse_headers(root, output=None, expand=True, debug=False):
             status.step()
 
     if len(failures):
-        o("failed to parse some headers: %s" % ", ".join(failures))
+        o("failed to parse some headers: %s\n" % ", ".join(failures))
 
     return headers
-- 
2.27.0

[PATCH v3 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Christian Göttsche]

On a SELinux disabled system the python call
`selinux.security_policyvers()` will fail.

Move the logic to find a binary policy by iterating over appended
version suffixes from the python script `sepolgen-ifgen` to the C
helper `sepolgen-ifgen-attr-helper` to make use of the libsepol
interface `sepol_policy_kern_vers_max()`.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v3: Move the iteration logic from sepolgen-ifgen to 
    sepolgen-ifgen-attr-helper and use sepol_policy_kern_vers_max()
    instead of selinux.security_policyvers(), to work on SELinux
    disabled systems

 python/audit2allow/sepolgen-ifgen             | 26 ++-----------
 .../audit2allow/sepolgen-ifgen-attr-helper.c  | 39 ++++++++++++++++---
 2 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
index 4a71cda4..19c3ee30 100644
--- a/python/audit2allow/sepolgen-ifgen
+++ b/python/audit2allow/sepolgen-ifgen
@@ -27,7 +27,6 @@
 
 
 import sys
-import os
 import tempfile
 import subprocess
 
@@ -65,34 +64,15 @@ def parse_options():
     return options
 
 
-def get_policy():
-    p = selinux.selinux_current_policy_path()
-    if p and os.path.exists(p):
-        return p
-    i = selinux.security_policyvers()
-    p = selinux.selinux_binary_policy_path() + "." + str(i)
-    while i > 0 and not os.path.exists(p):
-        i = i - 1
-        p = selinux.selinux_binary_policy_path() + "." + str(i)
-    if i > 0:
-        return p
-    return None
-
-
 def get_attrs(policy_path, attr_helper):
+    if not policy_path:
+        policy_path = selinux.selinux_binary_policy_path()
+
     try:
-        if not policy_path:
-            policy_path = get_policy()
-        if not policy_path:
-            sys.stderr.write("No installed policy to check\n")
-            return None
         outfile = tempfile.NamedTemporaryFile()
     except IOError as e:
         sys.stderr.write("could not open attribute output file\n")
         return None
-    except OSError:
-        # SELinux Disabled Machine
-        return None
 
     fd = open("/dev/null", "w")
     ret = subprocess.Popen([attr_helper, policy_path, outfile.name], stdout=fd).wait()
diff --git a/python/audit2allow/sepolgen-ifgen-attr-helper.c b/python/audit2allow/sepolgen-ifgen-attr-helper.c
index 1ce37b0d..dab6fb15 100644
--- a/python/audit2allow/sepolgen-ifgen-attr-helper.c
+++ b/python/audit2allow/sepolgen-ifgen-attr-helper.c
@@ -147,13 +147,42 @@ static policydb_t *load_policy(const char *filename)
 	policydb_t *policydb;
 	struct policy_file pf;
 	FILE *fp;
+	char pathname[PATH_MAX];
+	int suffix_ver;
 	int ret;
 
-	fp = fopen(filename, "r");
-	if (fp == NULL) {
-		fprintf(stderr, "Can't open '%s':  %s\n",
-			filename, strerror(errno));
-		return NULL;
+	/*
+	 * First use the pure given path.
+	 * If it does not exist use paths with version suffixes,
+	 * starting from the maximum supported policy version.
+	 */
+	if (access(filename, F_OK) == 0) {
+		fp = fopen(filename, "r");
+		if (fp == NULL) {
+			fprintf(stderr, "Can't open '%s':  %s\n",
+				filename, strerror(errno));
+			return NULL;
+		}
+	} else {
+		for (suffix_ver = sepol_policy_kern_vers_max(); suffix_ver > 0; suffix_ver--) {
+			snprintf(pathname, sizeof(pathname), "%s.%d", filename, suffix_ver);
+
+			if (access(pathname, F_OK) == 0)
+				break;
+		}
+
+		if (suffix_ver <= 0) {
+			fprintf(stderr, "Can't find any policy at '%s'\n",
+				filename);
+			return NULL;
+		}
+
+		fp = fopen(pathname, "r");
+		if (fp == NULL) {
+			fprintf(stderr, "Can't open '%s':  %s\n",
+				pathname, strerror(errno));
+			return NULL;
+		}
 	}
 
 	policy_file_init(&pf);
-- 
2.27.0

Re: [PATCH v3 1/3] sepolgen: parse gen_tunable as bool

[Stephen Smalley]

On Fri, Jun 5, 2020 at 10:49 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Currently sepolgen-ifgen parses a gen_tunable statement as interface
> and reports in verbose mode:
>
>     Missing interface definition for gen_tunable
>
> Add grammar for gen_tunable statements in the refparser
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

NB when previously acked and no changes, you can include the previous
Acked-by line in the re-submission.

Re: [PATCH v3 2/3] refparser: add missing newline after error message

[Stephen Smalley]

On Fri, Jun 5, 2020 at 10:49 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Re: [PATCH v3 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Stephen Smalley]

On Fri, Jun 5, 2020 at 10:49 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On a SELinux disabled system the python call
> `selinux.security_policyvers()` will fail.
>
> Move the logic to find a binary policy by iterating over appended
> version suffixes from the python script `sepolgen-ifgen` to the C
> helper `sepolgen-ifgen-attr-helper` to make use of the libsepol
> interface `sepol_policy_kern_vers_max()`.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

I think there are two problems with this change:
1) It drops the attempt to use /sys/fs/selinux/policy entirely, even
if SELinux-enabled.
2) It will incorrectly try to append version suffixes to a pathname
specified via -p and open those files if the user made a mistake and
specified a non-existent file rather than just reporting an error on
the original user-supplied path.

Instead, switch the helper to take a -p pathname optional argument
with no required argument, and if no pathname was specified, then have
the helper itself try selinux_current_policy_path() and then
selinux_binary_policy_path() + version suffixes.  This will require
linking the helper with libselinux but I don't see that as a problem
since it was already a dependency for the python script.  We don't
have to worry about the helper command line interface being stable
IMHO since it is just an internal helper and not directly used by end
users.

> ---
> v3: Move the iteration logic from sepolgen-ifgen to
>     sepolgen-ifgen-attr-helper and use sepol_policy_kern_vers_max()
>     instead of selinux.security_policyvers(), to work on SELinux
>     disabled systems
>
>  python/audit2allow/sepolgen-ifgen             | 26 ++-----------
>  .../audit2allow/sepolgen-ifgen-attr-helper.c  | 39 ++++++++++++++++---
>  2 files changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
> index 4a71cda4..19c3ee30 100644
> --- a/python/audit2allow/sepolgen-ifgen
> +++ b/python/audit2allow/sepolgen-ifgen
> @@ -27,7 +27,6 @@
>
>
>  import sys
> -import os
>  import tempfile
>  import subprocess
>
> @@ -65,34 +64,15 @@ def parse_options():
>      return options
>
>
> -def get_policy():
> -    p = selinux.selinux_current_policy_path()
> -    if p and os.path.exists(p):
> -        return p
> -    i = selinux.security_policyvers()
> -    p = selinux.selinux_binary_policy_path() + "." + str(i)
> -    while i > 0 and not os.path.exists(p):
> -        i = i - 1
> -        p = selinux.selinux_binary_policy_path() + "." + str(i)
> -    if i > 0:
> -        return p
> -    return None
> -
> -
>  def get_attrs(policy_path, attr_helper):
> +    if not policy_path:
> +        policy_path = selinux.selinux_binary_policy_path()
> +
>      try:
> -        if not policy_path:
> -            policy_path = get_policy()
> -        if not policy_path:
> -            sys.stderr.write("No installed policy to check\n")
> -            return None
>          outfile = tempfile.NamedTemporaryFile()
>      except IOError as e:
>          sys.stderr.write("could not open attribute output file\n")
>          return None
> -    except OSError:
> -        # SELinux Disabled Machine
> -        return None
>
>      fd = open("/dev/null", "w")
>      ret = subprocess.Popen([attr_helper, policy_path, outfile.name], stdout=fd).wait()
> diff --git a/python/audit2allow/sepolgen-ifgen-attr-helper.c b/python/audit2allow/sepolgen-ifgen-attr-helper.c
> index 1ce37b0d..dab6fb15 100644
> --- a/python/audit2allow/sepolgen-ifgen-attr-helper.c
> +++ b/python/audit2allow/sepolgen-ifgen-attr-helper.c
> @@ -147,13 +147,42 @@ static policydb_t *load_policy(const char *filename)
>         policydb_t *policydb;
>         struct policy_file pf;
>         FILE *fp;
> +       char pathname[PATH_MAX];
> +       int suffix_ver;
>         int ret;
>
> -       fp = fopen(filename, "r");
> -       if (fp == NULL) {
> -               fprintf(stderr, "Can't open '%s':  %s\n",
> -                       filename, strerror(errno));
> -               return NULL;
> +       /*
> +        * First use the pure given path.
> +        * If it does not exist use paths with version suffixes,
> +        * starting from the maximum supported policy version.
> +        */
> +       if (access(filename, F_OK) == 0) {
> +               fp = fopen(filename, "r");
> +               if (fp == NULL) {
> +                       fprintf(stderr, "Can't open '%s':  %s\n",
> +                               filename, strerror(errno));
> +                       return NULL;
> +               }
> +       } else {
> +               for (suffix_ver = sepol_policy_kern_vers_max(); suffix_ver > 0; suffix_ver--) {
> +                       snprintf(pathname, sizeof(pathname), "%s.%d", filename, suffix_ver);
> +
> +                       if (access(pathname, F_OK) == 0)
> +                               break;
> +               }
> +
> +               if (suffix_ver <= 0) {
> +                       fprintf(stderr, "Can't find any policy at '%s'\n",
> +                               filename);
> +                       return NULL;
> +               }
> +
> +               fp = fopen(pathname, "r");
> +               if (fp == NULL) {
> +                       fprintf(stderr, "Can't open '%s':  %s\n",
> +                               pathname, strerror(errno));
> +                       return NULL;
> +               }
>         }
>
>         policy_file_init(&pf);
> --
> 2.27.0
>

[PATCH v4 1/3] sepolgen: parse gen_tunable as bool

[Christian Göttsche]

Currently sepolgen-ifgen parses a gen_tunable statement as interface
and reports in verbose mode:

    Missing interface definition for gen_tunable

Add grammar for gen_tunable statements in the refparser

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
v4: no changes
v3: no changes

 python/sepolgen/src/sepolgen/refparser.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index 2e521a0f..f3e0ae87 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -126,6 +126,7 @@ tokens = (
     'GEN_REQ',
     'TEMPLATE',
     'GEN_CONTEXT',
+    'GEN_TUNABLE',
     #   m4
     'IFELSE',
     'IFDEF',
@@ -192,6 +193,7 @@ reserved = {
     'gen_require' : 'GEN_REQ',
     'template' : 'TEMPLATE',
     'gen_context' : 'GEN_CONTEXT',
+    'gen_tunable' : 'GEN_TUNABLE',
     # M4
     'ifelse' : 'IFELSE',
     'ifndef' : 'IFNDEF',
@@ -518,6 +520,7 @@ def p_policy_stmt(p):
                    | range_transition_def
                    | role_transition_def
                    | bool
+                   | gen_tunable
                    | define
                    | initial_sid
                    | genfscon
@@ -844,6 +847,17 @@ def p_bool(p):
         b.state = False
     p[0] = b
 
+def p_gen_tunable(p):
+    '''gen_tunable : GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA TRUE CPAREN
+                   | GEN_TUNABLE OPAREN TICK IDENTIFIER SQUOTE COMMA FALSE CPAREN'''
+    b = refpolicy.Bool()
+    b.name = p[4]
+    if p[7] == "true":
+        b.state = True
+    else:
+        b.state = False
+    p[0] = b
+
 def p_conditional(p):
     ''' conditional : IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE
                     | IF OPAREN cond_expr CPAREN OBRACE interface_stmts CBRACE ELSE OBRACE interface_stmts CBRACE
-- 
2.27.0

[PATCH v4 2/3] refparser: add missing newline after error message

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
v4: no changes
v3: no changes

 python/sepolgen/src/sepolgen/refparser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/sepolgen/src/sepolgen/refparser.py b/python/sepolgen/src/sepolgen/refparser.py
index f3e0ae87..9f850990 100644
--- a/python/sepolgen/src/sepolgen/refparser.py
+++ b/python/sepolgen/src/sepolgen/refparser.py
@@ -1148,6 +1148,6 @@ def parse_headers(root, output=None, expand=True, debug=False):
             status.step()
 
     if len(failures):
-        o("failed to parse some headers: %s" % ", ".join(failures))
+        o("failed to parse some headers: %s\n" % ", ".join(failures))
 
     return headers
-- 
2.27.0

[PATCH v4 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Christian Göttsche]

On a SELinux disabled system the python call
`selinux.security_policyvers()` will fail.

Move the logic to find a binary policy from the python script
`sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
Change the helper command line interface to accept an optional policy
path as second argument.  If not given try the current loaded policy
(`selinux_current_policy_path`) and if running on a SELinux disabled
system iterate over the default store path appending policy versions
starting at the maximum supported policy version
(`sepol_policy_kern_vers_max`).

This changes the helper command line interface from:
    sepolgen-ifgen-attr-helper policy_file out_file
to
    sepolgen-ifgen-attr-helper out_file [policy_file]
and adds a linkage to libselinux.

Export LIBSELINUXA like LIBSEPOLA in the root Makefile

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v4: Improve the behavior on no explicit policy path given:
    - Reorder helper's command line interface
    - Use loaded policy on SELinux enabled systems
v3: Move the iteration logic from sepolgen-ifgen to 
    sepolgen-ifgen-attr-helper and use sepol_policy_kern_vers_max()
    instead of selinux.security_policyvers(), to work on SELinux
    disabled systems

 Makefile                                      |  2 +
 python/audit2allow/Makefile                   |  5 ++-
 python/audit2allow/sepolgen-ifgen             | 28 ++----------
 .../audit2allow/sepolgen-ifgen-attr-helper.c  | 43 +++++++++++++++++--
 4 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/Makefile b/Makefile
index 298cd2b7..caf4cd3c 100644
--- a/Makefile
+++ b/Makefile
@@ -23,12 +23,14 @@ endif
 ifneq ($(DESTDIR),)
 	LIBDIR ?= $(DESTDIR)$(PREFIX)/lib
 	LIBSEPOLA ?= $(LIBDIR)/libsepol.a
+	LIBSELINUXA ?= $(LIBDIR)/libselinux.a
 
 	CFLAGS += -I$(DESTDIR)$(PREFIX)/include
 	LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
 	export CFLAGS
 	export LDFLAGS
 	export LIBSEPOLA
+	export LIBSELINUXA
 endif
 
 all install relabel clean test indent:
diff --git a/python/audit2allow/Makefile b/python/audit2allow/Makefile
index 15db5490..5400586c 100644
--- a/python/audit2allow/Makefile
+++ b/python/audit2allow/Makefile
@@ -15,10 +15,13 @@ CFLAGS ?= -Werror -Wall -W
 ifeq ($(LIBSEPOLA),)
         LDLIBS_LIBSEPOLA := -l:libsepol.a
 endif
+ifeq ($(LIBSELINUXA),)
+        LDLIBS_LIBSELINUXA := -l:libselinux.a
+endif
 
 all: audit2why sepolgen-ifgen-attr-helper
 
-sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o $(LIBSEPOLA)
+sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o $(LIBSEPOLA) $(LIBSELINUXA)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
 
 audit2why:
diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
index 4a71cda4..b7a04c71 100644
--- a/python/audit2allow/sepolgen-ifgen
+++ b/python/audit2allow/sepolgen-ifgen
@@ -27,7 +27,6 @@
 
 
 import sys
-import os
 import tempfile
 import subprocess
 
@@ -65,37 +64,18 @@ def parse_options():
     return options
 
 
-def get_policy():
-    p = selinux.selinux_current_policy_path()
-    if p and os.path.exists(p):
-        return p
-    i = selinux.security_policyvers()
-    p = selinux.selinux_binary_policy_path() + "." + str(i)
-    while i > 0 and not os.path.exists(p):
-        i = i - 1
-        p = selinux.selinux_binary_policy_path() + "." + str(i)
-    if i > 0:
-        return p
-    return None
-
-
 def get_attrs(policy_path, attr_helper):
     try:
-        if not policy_path:
-            policy_path = get_policy()
-        if not policy_path:
-            sys.stderr.write("No installed policy to check\n")
-            return None
         outfile = tempfile.NamedTemporaryFile()
     except IOError as e:
         sys.stderr.write("could not open attribute output file\n")
         return None
-    except OSError:
-        # SELinux Disabled Machine
-        return None
 
     fd = open("/dev/null", "w")
-    ret = subprocess.Popen([attr_helper, policy_path, outfile.name], stdout=fd).wait()
+    if policy_path:
+        ret = subprocess.Popen([attr_helper, outfile.name, policy_path], stdout=fd).wait()
+    else:
+        ret = subprocess.Popen([attr_helper, outfile.name], stdout=fd).wait()
     fd.close()
     if ret != 0:
         sys.stderr.write("could not run attribute helper\n")
diff --git a/python/audit2allow/sepolgen-ifgen-attr-helper.c b/python/audit2allow/sepolgen-ifgen-attr-helper.c
index 1ce37b0d..001ae80b 100644
--- a/python/audit2allow/sepolgen-ifgen-attr-helper.c
+++ b/python/audit2allow/sepolgen-ifgen-attr-helper.c
@@ -26,6 +26,8 @@
 #include <sepol/policydb/avtab.h>
 #include <sepol/policydb/util.h>
 
+#include <selinux/selinux.h>
+
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -147,8 +149,41 @@ static policydb_t *load_policy(const char *filename)
 	policydb_t *policydb;
 	struct policy_file pf;
 	FILE *fp;
+	char pathname[PATH_MAX];
+	int suffix_ver;
 	int ret;
 
+	/* no explicit policy name given, try loaded policy on a SELinux enabled system */
+	if (!filename) {
+		filename = selinux_current_policy_path();
+	}
+
+	/* try bare default policy path */
+	if (!filename && access(selinux_binary_policy_path(), F_OK) == 0) {
+		filename = selinux_binary_policy_path();
+	}
+
+	/*
+	 * Fallback to default store paths with version suffixes,
+	 * starting from the maximum supported policy version.
+	 */
+	if (!filename) {
+		for (suffix_ver = sepol_policy_kern_vers_max(); suffix_ver > 0; suffix_ver--) {
+			snprintf(pathname, sizeof(pathname), "%s.%d", selinux_binary_policy_path(), suffix_ver);
+
+			if (access(pathname, F_OK) == 0) {
+				filename = pathname;
+				break;
+			}
+		}
+
+		if (!filename) {
+			fprintf(stderr, "Can't find any policy at '%s'\n",
+				selinux_binary_policy_path());
+			return NULL;
+		}
+	}
+
 	fp = fopen(filename, "r");
 	if (fp == NULL) {
 		fprintf(stderr, "Can't open '%s':  %s\n",
@@ -188,7 +223,7 @@ static policydb_t *load_policy(const char *filename)
 
 void usage(char *progname)
 {
-	printf("usage: %s policy_file out_file\n", progname);
+	printf("usage: %s out_file [policy_file]\n", progname);
 }
 
 int main(int argc, char **argv)
@@ -197,18 +232,18 @@ int main(int argc, char **argv)
 	struct callback_data cb_data;
 	FILE *fp;
 
-	if (argc != 3) {
+	if (argc != 2 && argc != 3) {
 		usage(argv[0]);
 		return -1;
 	}
 
 	/* Open the policy. */
-	p = load_policy(argv[1]);
+	p = load_policy(argv[2]);
 	if (p == NULL)
 		return -1;
 
 	/* Open the output policy. */
-	fp = fopen(argv[2], "w");
+	fp = fopen(argv[1], "w");
 	if (fp == NULL) {
 		fprintf(stderr, "error opening output file\n");
 		policydb_destroy(p);
-- 
2.27.0

Re: [PATCH v4 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Stephen Smalley]

On Thu, Jun 11, 2020 at 9:54 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On a SELinux disabled system the python call
> `selinux.security_policyvers()` will fail.
>
> Move the logic to find a binary policy from the python script
> `sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
> Change the helper command line interface to accept an optional policy
> path as second argument.  If not given try the current loaded policy
> (`selinux_current_policy_path`) and if running on a SELinux disabled
> system iterate over the default store path appending policy versions
> starting at the maximum supported policy version
> (`sepol_policy_kern_vers_max`).
>
> This changes the helper command line interface from:
>     sepolgen-ifgen-attr-helper policy_file out_file
> to
>     sepolgen-ifgen-attr-helper out_file [policy_file]
> and adds a linkage to libselinux.
>
> Export LIBSELINUXA like LIBSEPOLA in the root Makefile

I don't think you need the static libselinux, just add -lselinux and
use the shared library.
I would have added a -p policy_file optional argument via getopt(3)
but no big deal either way.
I don't think selinux_binary_policy_path() ever returns a complete
pathname so I'm not sure there is any point in testing it first
without a version suffix at all?
You don't need to re-post the first two patches; those are already
queued for merging.

[PATCH v5 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Christian Göttsche]

On a SELinux disabled system the python call
`selinux.security_policyvers()` will fail.

Move the logic to find a binary policy from the python script
`sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
Change the helper command line interface to accept an optional policy
path as second argument.  If not given try the current loaded policy
(`selinux_current_policy_path`) and if running on a SELinux disabled
system iterate over the default store path appending policy versions
starting at the maximum supported policy version
(`sepol_policy_kern_vers_max`).

This changes the helper command line interface from:
    sepolgen-ifgen-attr-helper policy_file out_file
to
    sepolgen-ifgen-attr-helper out_file [policy_file]
and adds a linkage to libselinux.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v5: - Do not check bare selinux_binary_policy_path()
    - Link helper dynamically with libselinux
v4: Improve the behavior on no explicit policy path given:
    - Reorder helper's command line interface
    - Use loaded policy on SELinux enabled systems
v3: Move the iteration logic from sepolgen-ifgen to
    sepolgen-ifgen-attr-helper and use sepol_policy_kern_vers_max()
    instead of selinux.security_policyvers(), to work on SELinux
    disabled systems

 python/audit2allow/Makefile                   |  2 +-
 python/audit2allow/sepolgen-ifgen             | 28 ++------------
 .../audit2allow/sepolgen-ifgen-attr-helper.c  | 38 +++++++++++++++++--
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/python/audit2allow/Makefile b/python/audit2allow/Makefile
index 15db5490..025c282a 100644
--- a/python/audit2allow/Makefile
+++ b/python/audit2allow/Makefile
@@ -18,7 +18,7 @@ endif
 
 all: audit2why sepolgen-ifgen-attr-helper
 
-sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o $(LIBSEPOLA)
+sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o $(LIBSEPOLA) -lselinux
 	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
 
 audit2why:
diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
index 4a71cda4..b7a04c71 100644
--- a/python/audit2allow/sepolgen-ifgen
+++ b/python/audit2allow/sepolgen-ifgen
@@ -27,7 +27,6 @@
 
 
 import sys
-import os
 import tempfile
 import subprocess
 
@@ -65,37 +64,18 @@ def parse_options():
     return options
 
 
-def get_policy():
-    p = selinux.selinux_current_policy_path()
-    if p and os.path.exists(p):
-        return p
-    i = selinux.security_policyvers()
-    p = selinux.selinux_binary_policy_path() + "." + str(i)
-    while i > 0 and not os.path.exists(p):
-        i = i - 1
-        p = selinux.selinux_binary_policy_path() + "." + str(i)
-    if i > 0:
-        return p
-    return None
-
-
 def get_attrs(policy_path, attr_helper):
     try:
-        if not policy_path:
-            policy_path = get_policy()
-        if not policy_path:
-            sys.stderr.write("No installed policy to check\n")
-            return None
         outfile = tempfile.NamedTemporaryFile()
     except IOError as e:
         sys.stderr.write("could not open attribute output file\n")
         return None
-    except OSError:
-        # SELinux Disabled Machine
-        return None
 
     fd = open("/dev/null", "w")
-    ret = subprocess.Popen([attr_helper, policy_path, outfile.name], stdout=fd).wait()
+    if policy_path:
+        ret = subprocess.Popen([attr_helper, outfile.name, policy_path], stdout=fd).wait()
+    else:
+        ret = subprocess.Popen([attr_helper, outfile.name], stdout=fd).wait()
     fd.close()
     if ret != 0:
         sys.stderr.write("could not run attribute helper\n")
diff --git a/python/audit2allow/sepolgen-ifgen-attr-helper.c b/python/audit2allow/sepolgen-ifgen-attr-helper.c
index 1ce37b0d..53f20818 100644
--- a/python/audit2allow/sepolgen-ifgen-attr-helper.c
+++ b/python/audit2allow/sepolgen-ifgen-attr-helper.c
@@ -26,6 +26,8 @@
 #include <sepol/policydb/avtab.h>
 #include <sepol/policydb/util.h>
 
+#include <selinux/selinux.h>
+
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -147,8 +149,36 @@ static policydb_t *load_policy(const char *filename)
 	policydb_t *policydb;
 	struct policy_file pf;
 	FILE *fp;
+	char pathname[PATH_MAX];
+	int suffix_ver;
 	int ret;
 
+	/* no explicit policy name given, try loaded policy on a SELinux enabled system */
+	if (!filename) {
+		filename = selinux_current_policy_path();
+	}
+
+	/*
+	 * Fallback to default store paths with version suffixes,
+	 * starting from the maximum supported policy version.
+	 */
+	if (!filename) {
+		for (suffix_ver = sepol_policy_kern_vers_max(); suffix_ver > 0; suffix_ver--) {
+			snprintf(pathname, sizeof(pathname), "%s.%d", selinux_binary_policy_path(), suffix_ver);
+
+			if (access(pathname, F_OK) == 0) {
+				filename = pathname;
+				break;
+			}
+		}
+
+		if (!filename) {
+			fprintf(stderr, "Can't find any policy at '%s'\n",
+				selinux_binary_policy_path());
+			return NULL;
+		}
+	}
+
 	fp = fopen(filename, "r");
 	if (fp == NULL) {
 		fprintf(stderr, "Can't open '%s':  %s\n",
@@ -188,7 +218,7 @@ static policydb_t *load_policy(const char *filename)
 
 void usage(char *progname)
 {
-	printf("usage: %s policy_file out_file\n", progname);
+	printf("usage: %s out_file [policy_file]\n", progname);
 }
 
 int main(int argc, char **argv)
@@ -197,18 +227,18 @@ int main(int argc, char **argv)
 	struct callback_data cb_data;
 	FILE *fp;
 
-	if (argc != 3) {
+	if (argc != 2 && argc != 3) {
 		usage(argv[0]);
 		return -1;
 	}
 
 	/* Open the policy. */
-	p = load_policy(argv[1]);
+	p = load_policy(argv[2]);
 	if (p == NULL)
 		return -1;
 
 	/* Open the output policy. */
-	fp = fopen(argv[2], "w");
+	fp = fopen(argv[1], "w");
 	if (fp == NULL) {
 		fprintf(stderr, "error opening output file\n");
 		policydb_destroy(p);
-- 
2.27.0

[PATCH v6 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Christian Göttsche]

On a SELinux disabled system the python call
`selinux.security_policyvers()` will fail.

Move the logic to find a binary policy from the python script
`sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
Change the helper command line interface to accept an optional policy
path as second argument.  If not given try the current loaded policy
(`selinux_current_policy_path`) and if running on a SELinux disabled
system iterate over the default store path appending policy versions
starting at the maximum supported policy version
(`sepol_policy_kern_vers_max`).

This changes the helper command line interface from:
    sepolgen-ifgen-attr-helper policy_file out_file
to
    sepolgen-ifgen-attr-helper out_file [policy_file]
and adds a linkage to libselinux.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v6: fix -lselinux usage
v5: - Do not check bare selinux_binary_policy_path()
    - Link helper dynamically with libselinux
v4: Improve the behavior on no explicit policy path given:
    - Reorder helper's command line interface
    - Use loaded policy on SELinux enabled systems
v3: Move the iteration logic from sepolgen-ifgen to
    sepolgen-ifgen-attr-helper and use sepol_policy_kern_vers_max()
    instead of selinux.security_policyvers(), to work on SELinux
    disabled systems

 python/audit2allow/Makefile                   |  2 +-
 python/audit2allow/sepolgen-ifgen             | 28 ++------------
 .../audit2allow/sepolgen-ifgen-attr-helper.c  | 38 +++++++++++++++++--
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/python/audit2allow/Makefile b/python/audit2allow/Makefile
index 15db5490..76bf4e37 100644
--- a/python/audit2allow/Makefile
+++ b/python/audit2allow/Makefile
@@ -19,7 +19,7 @@ endif
 all: audit2why sepolgen-ifgen-attr-helper
 
 sepolgen-ifgen-attr-helper: sepolgen-ifgen-attr-helper.o $(LIBSEPOLA)
-	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
+	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA) -lselinux
 
 audit2why:
 	ln -sf audit2allow audit2why
diff --git a/python/audit2allow/sepolgen-ifgen b/python/audit2allow/sepolgen-ifgen
index 4a71cda4..b7a04c71 100644
--- a/python/audit2allow/sepolgen-ifgen
+++ b/python/audit2allow/sepolgen-ifgen
@@ -27,7 +27,6 @@
 
 
 import sys
-import os
 import tempfile
 import subprocess
 
@@ -65,37 +64,18 @@ def parse_options():
     return options
 
 
-def get_policy():
-    p = selinux.selinux_current_policy_path()
-    if p and os.path.exists(p):
-        return p
-    i = selinux.security_policyvers()
-    p = selinux.selinux_binary_policy_path() + "." + str(i)
-    while i > 0 and not os.path.exists(p):
-        i = i - 1
-        p = selinux.selinux_binary_policy_path() + "." + str(i)
-    if i > 0:
-        return p
-    return None
-
-
 def get_attrs(policy_path, attr_helper):
     try:
-        if not policy_path:
-            policy_path = get_policy()
-        if not policy_path:
-            sys.stderr.write("No installed policy to check\n")
-            return None
         outfile = tempfile.NamedTemporaryFile()
     except IOError as e:
         sys.stderr.write("could not open attribute output file\n")
         return None
-    except OSError:
-        # SELinux Disabled Machine
-        return None
 
     fd = open("/dev/null", "w")
-    ret = subprocess.Popen([attr_helper, policy_path, outfile.name], stdout=fd).wait()
+    if policy_path:
+        ret = subprocess.Popen([attr_helper, outfile.name, policy_path], stdout=fd).wait()
+    else:
+        ret = subprocess.Popen([attr_helper, outfile.name], stdout=fd).wait()
     fd.close()
     if ret != 0:
         sys.stderr.write("could not run attribute helper\n")
diff --git a/python/audit2allow/sepolgen-ifgen-attr-helper.c b/python/audit2allow/sepolgen-ifgen-attr-helper.c
index 1ce37b0d..53f20818 100644
--- a/python/audit2allow/sepolgen-ifgen-attr-helper.c
+++ b/python/audit2allow/sepolgen-ifgen-attr-helper.c
@@ -26,6 +26,8 @@
 #include <sepol/policydb/avtab.h>
 #include <sepol/policydb/util.h>
 
+#include <selinux/selinux.h>
+
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -147,8 +149,36 @@ static policydb_t *load_policy(const char *filename)
 	policydb_t *policydb;
 	struct policy_file pf;
 	FILE *fp;
+	char pathname[PATH_MAX];
+	int suffix_ver;
 	int ret;
 
+	/* no explicit policy name given, try loaded policy on a SELinux enabled system */
+	if (!filename) {
+		filename = selinux_current_policy_path();
+	}
+
+	/*
+	 * Fallback to default store paths with version suffixes,
+	 * starting from the maximum supported policy version.
+	 */
+	if (!filename) {
+		for (suffix_ver = sepol_policy_kern_vers_max(); suffix_ver > 0; suffix_ver--) {
+			snprintf(pathname, sizeof(pathname), "%s.%d", selinux_binary_policy_path(), suffix_ver);
+
+			if (access(pathname, F_OK) == 0) {
+				filename = pathname;
+				break;
+			}
+		}
+
+		if (!filename) {
+			fprintf(stderr, "Can't find any policy at '%s'\n",
+				selinux_binary_policy_path());
+			return NULL;
+		}
+	}
+
 	fp = fopen(filename, "r");
 	if (fp == NULL) {
 		fprintf(stderr, "Can't open '%s':  %s\n",
@@ -188,7 +218,7 @@ static policydb_t *load_policy(const char *filename)
 
 void usage(char *progname)
 {
-	printf("usage: %s policy_file out_file\n", progname);
+	printf("usage: %s out_file [policy_file]\n", progname);
 }
 
 int main(int argc, char **argv)
@@ -197,18 +227,18 @@ int main(int argc, char **argv)
 	struct callback_data cb_data;
 	FILE *fp;
 
-	if (argc != 3) {
+	if (argc != 2 && argc != 3) {
 		usage(argv[0]);
 		return -1;
 	}
 
 	/* Open the policy. */
-	p = load_policy(argv[1]);
+	p = load_policy(argv[2]);
 	if (p == NULL)
 		return -1;
 
 	/* Open the output policy. */
-	fp = fopen(argv[2], "w");
+	fp = fopen(argv[1], "w");
 	if (fp == NULL) {
 		fprintf(stderr, "error opening output file\n");
 		policydb_destroy(p);
-- 
2.27.0

Re: [PATCH v6 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Stephen Smalley]

On Mon, Jun 15, 2020 at 11:08 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On a SELinux disabled system the python call
> `selinux.security_policyvers()` will fail.
>
> Move the logic to find a binary policy from the python script
> `sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
> Change the helper command line interface to accept an optional policy
> path as second argument.  If not given try the current loaded policy
> (`selinux_current_policy_path`) and if running on a SELinux disabled
> system iterate over the default store path appending policy versions
> starting at the maximum supported policy version
> (`sepol_policy_kern_vers_max`).
>
> This changes the helper command line interface from:
>     sepolgen-ifgen-attr-helper policy_file out_file
> to
>     sepolgen-ifgen-attr-helper out_file [policy_file]
> and adds a linkage to libselinux.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Re: [PATCH v6 3/3] sepolgen-ifgen: refactor default policy path retrieval

[Petr Lautrbach]

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

On Mon, Jun 15, 2020 at 12:30:43PM -0400, Stephen Smalley wrote:
> On Mon, Jun 15, 2020 at 11:08 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > On a SELinux disabled system the python call
> > `selinux.security_policyvers()` will fail.
> >
> > Move the logic to find a binary policy from the python script
> > `sepolgen-ifgen` to the C-helper `sepolgen-ifgen-attr-helper`.
> > Change the helper command line interface to accept an optional policy
> > path as second argument.  If not given try the current loaded policy
> > (`selinux_current_policy_path`) and if running on a SELinux disabled
> > system iterate over the default store path appending policy versions
> > starting at the maximum supported policy version
> > (`sepol_policy_kern_vers_max`).
> >
> > This changes the helper command line interface from:
> >     sepolgen-ifgen-attr-helper policy_file out_file
> > to
> >     sepolgen-ifgen-attr-helper out_file [policy_file]
> > and adds a linkage to libselinux.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> 
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> 

All 3 applied. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]