| Message ID | 20181220151420.30878-4-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/4] python/semanage: move valid_types initialisations to class constructors | expand |
On Thu, Dec 20, 2018 at 4:14 PM Petr Lautrbach <plautrba@redhat.com> wrote: > > load_store_policy() allows to (re)load SELinux policy based on a store name. It > is useful when SELinux is disabled and default policy is not installed; or when > a user wants to query or manipulate another policy. > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1558861 > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > --- > python/sepolicy/sepolicy/__init__.py | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/python/sepolicy/sepolicy/__init__.py b/python/sepolicy/sepolicy/__init__.py > index fbeb731d..b69a6b94 100644 > --- a/python/sepolicy/sepolicy/__init__.py > +++ b/python/sepolicy/sepolicy/__init__.py > @@ -129,6 +129,13 @@ def get_installed_policy(root="/"): > pass > raise ValueError(_("No SELinux Policy installed")) > > +def get_store_policy(store, root="/"): > + try: > + policies = glob.glob("%s%s/policy/policy.*" % (selinux.selinux_path(), store)) > + policies.sort() > + return policies[-1] > + except: > + return None Hi, I agree this function is useful. Nevertheless the sorting order seems to be fragile because '100' < '99', so the policy filename needs to be parsed in order to extract the version as an integer and sort according to it. Moreover its second parameter ("root") is not used and I would rather avoid adding new bare excepts to the code base. I suggest the following implementation of this function: def get_store_policy(store): """Get the path to the policy file located in the given store name""" def policy_sortkey(policy_path): # Parse the extension of a policy path which looks like .../policy/policy.31 extension = policy_path.rsplit('/policy.', 1)[1] try: return int(extension), policy_path except ValueError: # Fallback with sorting on the full path return 0, policy_path policies = glob.glob("%s%s/policy/policy.*" % (selinux.selinux_path(), store)) if not policies: return None # Return the policy with the higher version number policies.sort(key=policy_sortkey) return policies[-1] if policies else None It is more complex but fixes the issues I have identified. If you want to keep "root", it may be possible to use it with both "glob.glob("%s/%s/%s/policy/policy.*" % (root, selinux.selinux_path(), store))" and "return os.path.realpath(policies[-1]) if policies else None" (in order to simplify double-slashes into a single "/" character). What do you think of this? Anyway, thanks for the patches! Nicolas
Nicolas Iooss <nicolas.iooss@m4x.org> writes: > On Thu, Dec 20, 2018 at 4:14 PM Petr Lautrbach <plautrba@redhat.com> wrote: >> >> load_store_policy() allows to (re)load SELinux policy based on a store name. It >> is useful when SELinux is disabled and default policy is not installed; or when >> a user wants to query or manipulate another policy. >> >> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1558861 >> >> Signed-off-by: Petr Lautrbach <plautrba@redhat.com> >> --- >> python/sepolicy/sepolicy/__init__.py | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/python/sepolicy/sepolicy/__init__.py b/python/sepolicy/sepolicy/__init__.py >> index fbeb731d..b69a6b94 100644 >> --- a/python/sepolicy/sepolicy/__init__.py >> +++ b/python/sepolicy/sepolicy/__init__.py >> @@ -129,6 +129,13 @@ def get_installed_policy(root="/"): >> pass >> raise ValueError(_("No SELinux Policy installed")) >> >> +def get_store_policy(store, root="/"): >> + try: >> + policies = glob.glob("%s%s/policy/policy.*" % (selinux.selinux_path(), store)) >> + policies.sort() >> + return policies[-1] >> + except: >> + return None > > Hi, I agree this function is useful. Nevertheless the sorting order > seems to be fragile because '100' < '99', so the policy filename needs > to be parsed in order to extract the version as an integer and sort > according to it. Moreover its second parameter ("root") is not used > and I would rather avoid adding new bare excepts to the code base. > > I suggest the following implementation of this function: > > def get_store_policy(store): > """Get the path to the policy file located in the given store name""" > def policy_sortkey(policy_path): > # Parse the extension of a policy path which looks like > .../policy/policy.31 > extension = policy_path.rsplit('/policy.', 1)[1] > try: > return int(extension), policy_path > except ValueError: > # Fallback with sorting on the full path > return 0, policy_path > policies = glob.glob("%s%s/policy/policy.*" % > (selinux.selinux_path(), store)) > if not policies: > return None > # Return the policy with the higher version number > policies.sort(key=policy_sortkey) > return policies[-1] if policies else None > > It is more complex but fixes the issues I have identified. If you want > to keep "root", it may be possible to use it with both > "glob.glob("%s/%s/%s/policy/policy.*" % (root, selinux.selinux_path(), > store))" and "return os.path.realpath(policies[-1]) if policies else > None" (in order to simplify double-slashes into a single "/" > character). What do you think of this? > It looks good to me. I'll only move policy_sortkey out of this function and use it in also get_installed_policy() as this function use the original sort method. Petr
diff --git a/python/sepolicy/sepolicy/__init__.py b/python/sepolicy/sepolicy/__init__.py index fbeb731d..b69a6b94 100644 --- a/python/sepolicy/sepolicy/__init__.py +++ b/python/sepolicy/sepolicy/__init__.py @@ -129,6 +129,13 @@ def get_installed_policy(root="/"): pass raise ValueError(_("No SELinux Policy installed")) +def get_store_policy(store, root="/"): + try: + policies = glob.glob("%s%s/policy/policy.*" % (selinux.selinux_path(), store)) + policies.sort() + return policies[-1] + except: + return None def policy(policy_file): global all_domains @@ -156,6 +163,11 @@ def policy(policy_file): except: raise ValueError(_("Failed to read %s policy file") % policy_file) +def load_store_policy(store): + policy_file = get_store_policy(store) + if not policy_file: + return None + policy(policy_file) try: policy_file = get_installed_policy()
load_store_policy() allows to (re)load SELinux policy based on a store name. It is useful when SELinux is disabled and default policy is not installed; or when a user wants to query or manipulate another policy. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1558861 Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- python/sepolicy/sepolicy/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+)