On Fri, Jun 5, 2020 at 10:49 AM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
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
>
