Nicolas Iooss <nicolas.iooss@xxxxxxx> writes: > On Thu, Dec 20, 2018 at 4:14 PM Petr Lautrbach <plautrba@xxxxxxxxxx> 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@xxxxxxxxxx> >> --- >> 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
