Re: policycoreutils, sepolgen (sepolgen-ifgen) issues on Debian

[Joshua Brindle <method@manicmethod.com>]

Manoj Srivastava wrote:
> On Wed, Sep 16 2009, Joshua Brindle wrote:
>
>> Manoj Srivastava wrote:
>>> On Mon, Aug 17 2009, Christopher J. PeBenito wrote:
>>>
>>>
>>>> On Fri, 2009-08-14 at 11:50 -0500, Manoj Srivastava wrote:
>>>>
>>>>> On Fri, Aug 14 2009, Manoj Srivastava wrote:
>>>>>
>>>>>
>>>>>>           I am running into an issue with sepolgen on Debian. Debian ships
>>>>>>    more than one  version of the refpolicy, a default one, and a
>>>>>>    MLS enabled one. So, the include files live in either
>>>>>>    /usr/share/selinux/{default,mls}/include
>>>>>>
>>>>>>           sepolgen (in src/sepolgen/defaults.py) sets refpolicy_devel() to
>>>>>>    a single location -- and thus, only one version of the security policy
>>>>>>    may be supported. So, sepolgen-ifgen from policycoreutils can only work
>>>>>>    with one policy, which may not be the one installed on the target
>>>>>>    machine. Could this be made configurable, somehow? As far as I can
>>>>>>    see, sepolgen's python library does not offer any way to set the value.
>>>>>>
>>>>>>           It would be nice if the location of the include directory could
>>>>>>    be looked for from a PATH like variable setting, to make it easier for
>>>>>>    distributions to ship more than one policy, or for end users to
>>>>>>    experiment with other policies without have to overwrite the single
>>>>>>    default.
>>>>>>
>>>>>           Well, here is a kind of proof-of-concept patch (python is not my
>>>>>    strong suit), and I have only tested in that it allows the package to
>>>>>    compile, and the following code works:
>>>>>
>>>> [...]
>>>>
>>>>>    def refpolicy_makefile():
>>>>> -    return refpolicy_devel() + "/Makefile"
>>>>> +    chooser = PathChoooser("/etc/selinux/sepolgen.conf")
>>>>> +    return chooser("Makefile")
>>>>>
>>>>>    def headers():
>>>>> -    return refpolicy_devel() + "/include"
>>>>> -
>>>>> +    chooser = PathChoooser("/etc/selinux/sepolgen.conf")
>>>>> +    return chooser("include")
>>>>> +
>>>>>
>>>> Why are you making another config file rather than just get the policy
>>>> name from /etc/selinux/config via selinux_getpolicytype()?
>>>>
>>>           This will work well for Debian, since the development files are
>>>    installed under "/usr/share/selinux/" in a subdirectory named after the
>>>    policy. I was not sure that this convention was followed in other
>>>    distributions, though. While I am not certain, google implies that in
>>>    fedora policy type is targeted, but the devel files do not live in
>>>    /usr/share/selinux/targeted.[0]. Given that, perhaps it is better to
>>>    let the user provide guidance about how to map the policy type to a
>>>    directory?
>>>
>>>           Also, I must confess I had forgotten about this call.
>>>
>>>           However, a patch with this is trivial, so an alternate patch
>>>    follows. (Not sure this will work for fedora, so caveat emptor)
>>>
>>>           manoj
>>> [0] http://docs.fedoraproject.org/selinux-user-guide/f11/en-US/chap-Security-Enhanced_Linux-Working_with_SELinux.html
>>>
>>> --8<---------------cut here---------------start------------->8---
>>>
>>> If the user installs a policy whose development files do not live under
>>> /usr/share/selinux/devel/include, sepolgen wqould not work. Debian, for
>>> instance, installs under:
>>> /usr/share/selinux/{default,mls}/include
>>>
>>> This patch uses selinux_getpolicytype() to determine the policy type, and
>>> assumes that there is one-on-one correspondence between policytype and
>>> the directory the development files live in.
>>>
>>> Signed-off-by: Manoj Srivastava<srivasta@debian.org>
>>> ---
>>>    src/sepolgen/defaults.py |    4 +++-
>>>    src/sepolgen/module.py   |    2 +-
>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/sepolgen/defaults.py b/src/sepolgen/defaults.py
>>> index 45ce61a..85e5fb0 100644
>>> --- a/src/sepolgen/defaults.py
>>> +++ b/src/sepolgen/defaults.py
>>> @@ -21,6 +21,8 @@
>>>    Various default settings, including file and directory locations.
>>>    """
>>>
>>> +import selinux
>>> +
>>>    def data_dir():
>>>        return "/var/lib/sepolgen"
>>>
>>> @@ -31,7 +33,7 @@ def interface_info():
>>>        return data_dir() + "/interface_info"
>>>
>>>    def refpolicy_devel():
>>> -    return "/usr/share/selinux/devel"
>>> +    return "/usr/share/selinux/" + selinux.selinux_getpolicytype()[1]
>>>
>>>    def refpolicy_makefile():
>>>        return refpolicy_devel() + "/Makefile"
>>> diff --git a/src/sepolgen/module.py b/src/sepolgen/module.py
>>> index edd24c6..355c9b8 100644
>>> --- a/src/sepolgen/module.py
>>> +++ b/src/sepolgen/module.py
>>> @@ -120,7 +120,7 @@ class ModuleCompiler:
>>>            self.semodule_package = "/usr/bin/semodule_package"
>>>            self.output = output
>>>            self.last_output = ""
>>> -        self.refpol_makefile = "/usr/share/selinux/devel/Makefile"
>>> +        self.refpol_makefile = "/usr/share/selinux/" + selinux.selinux_getpolicytype()[1]  + "/Makefile"
>>>            self.make = "/usr/bin/make"
>>>
>>>        def o(self, str):
>>>
>> This will break Fedora/RHEL AFAIK. I don't necessarily like that RH
>> has interface files in /usr/share/selinux/devel rather than
>> /usr/share/selinux/<policy>/devel or similar but we can't break them.
>
>          Yes, I know it breaks RHEL, which is why I would prefer the
>   other patch I sent in first (re-attached below); this uses a
>   configuration file instead.
>
>> Dan, any chance you could change the location of the interface files?
>
>          Or that would work.
>
>          manoj
>
> --8<---------------cut here---------------start------------->8---
> import defaults
>
> # The following looks for /etc/selinux/sepolgen.conf that
> # does not exist
> print defaults.refpolicy_makefile()
> print defaults.headers()
>
> # Create a configuration file
> testfd = open("/tmp/pathchooser.conf", "w")
> print>>testfd, "# This is a comment"
> print>>testfd, "  # An empty line will follow"
> print>>testfd, ""
> print>>testfd, "SELINUX_DEVEL_PATH = /:/etc:/usr/share/selinux/default:/usr/share/selinux/mls:/usr/share/selinux/devel"
> print>>testfd, "FOO= bar:baz"
> testfd.close()
>
> # Specify a non default config file, that has /etc in it
> chooser = defaults.PathChoooser("/tmp/pathchooser.conf")
> print chooser("passwd")
> --8<---------------cut here---------------end--------------->8---
>          manoj
>
> Signed-off-by: Enrico Zini<enrico@debian.org>
> Signed-off-by: Manoj Srivastava<srivasta@debian.org>
> ---
>   src/sepolgen/defaults.py |   47 +++++++++++++++++++++++++++++++++++++++------
>   1 files changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/src/sepolgen/defaults.py b/src/sepolgen/defaults.py
> index 45ce61a..906c058 100644
> --- a/src/sepolgen/defaults.py
> +++ b/src/sepolgen/defaults.py
> @@ -1,6 +1,6 @@
>   # Authors: Karl MacMillan<kmacmillan@mentalrootkit.com>
>   #
> -# Copyright (C) 2006 Red Hat
> +# Copyright (C) 2006 Red Hat
>   # see file 'COPYING' for use and warranty information
>   #
>   # This program is free software; you can redistribute it and/or
> @@ -17,6 +17,40 @@
>   # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>   #
>
> +import os
> +import re
> +
> +# Select the correct location for the development files based on a
> +# path variable (optionally read from a configuration file)
> +class PathChoooser(object):
> +    def __init__(self, pathname):
> +        self.config = dict()
> +        if not os.path.exists(pathname):
> +            self.config_pathname = "(defaults)"
> +            self.config["SELINUX_DEVEL_PATH"] = "/usr/share/selinux/default:/usr/share/selinux/mls:/usr/share/selinux/devel"
> +            return
> +        self.config_pathname = pathname
> +        ignore = re.compile(r"^\s*(?:#.+)?$")
> +        consider = re.compile(r"^\s*(\w+)\s*=\s*(.+?)\s*$")
> +        for lineno, line in enumerate(open(pathname)):
> +            if ignore.match(line): continue
> +            mo = consider.match(line)
> +            if not mo:
> +                raise ValueError, "%s:%d: line is not in key = value format" % (pathname, lineno+1)
> +            self.config[mo.group(1)] = mo.group(2)
> +
> +    # We're only exporting one useful function, so why not be a function
> +    def __call__(self, testfilename, pathset="SELINUX_DEVEL_PATH"):
> +        paths = self.config.get(pathset, None)
> +        if paths is None:
> +            raise ValueError, "%s was not in %s" % (pathset, self.config_pathname)
> +        paths = paths.split(":")
> +        for p in paths:
> +            target = os.path.join(p, testfilename)
> +            if os.path.exists(target): return target
> +        return os.path.join(paths[0], testfilename)
> +
> +
>   """
>   Various default settings, including file and directory locations.
>   """
> @@ -30,12 +64,11 @@ def perm_map():
>   def interface_info():
>       return data_dir() + "/interface_info"
>
> -def refpolicy_devel():
> -    return "/usr/share/selinux/devel"
> -
>   def refpolicy_makefile():
> -    return refpolicy_devel() + "/Makefile"
> +    chooser = PathChoooser("/etc/selinux/sepolgen.conf")
> +    return chooser("Makefile")
>
>   def headers():
> -    return refpolicy_devel() + "/include"
> -
> +    chooser = PathChoooser("/etc/selinux/sepolgen.conf")
> +    return chooser("include")
> +

I'm not a fan of fixing things by making them configurable. This really 
should just use a standard location of /usr/share/selinux/<type>/devel.

Would you carry the previous patch that changed the path until RHEL and 
Fedora get updated then we can merge it upstream?

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.