SELinux Archive on lore.kernel.org
 help / Atom feed
* [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options
@ 2019-02-06 19:43 Petr Lautrbach
  2019-02-06 19:43 ` [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive Petr Lautrbach
  2019-02-07 21:46 ` [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Nicolas Iooss
  0 siblings, 2 replies; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-06 19:43 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

Previous code traceback-ed when one of the mentioned option was used without
any argument as this state was not handled by the argument parser.

action='store' stores arguments as a list while the original
action='store_const' used str therefore the particular interfaces in
moduleRecords are changed to be compatible with both.

Fixes:
^_^ semanage module -a
Traceback (most recent call last):
  File "/usr/sbin/semanage", line 963, in <module>
    do_parser()
  File "/usr/sbin/semanage", line 942, in do_parser
    args.func(args)
  File "/usr/sbin/semanage", line 608, in handleModule
    OBJECT.add(args.module_name, args.priority)
  File "/usr/lib/python3.7/site-packages/seobject.py", line 402, in add
    if not os.path.exists(file):
  File "/usr/lib64/python3.7/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 python/semanage/semanage    | 25 ++++++++++++-------------
 python/semanage/seobject.py | 10 ++++++++--
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index 6afeac14..9b737fa8 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -609,14 +609,14 @@ def setupInterfaceParser(subparsers):
 
 def handleModule(args):
     OBJECT = seobject.moduleRecords(args)
-    if args.action == "add":
-        OBJECT.add(args.module_name, args.priority)
-    if args.action == "enable":
-        OBJECT.set_enabled(args.module_name, True)
-    if args.action == "disable":
-        OBJECT.set_enabled(args.module_name, False)
-    if args.action == "remove":
-        OBJECT.delete(args.module_name, args.priority)
+    if args.action_add:
+        OBJECT.add(args.action_add, args.priority)
+    if args.action_enable:
+        OBJECT.set_enabled(args.action_enable, True)
+    if args.action_disable:
+        OBJECT.set_enabled(args.action_disable, False)
+    if args.action_remove:
+        OBJECT.delete(args.action_remove, args.priority)
     if args.action == "deleteall":
         OBJECT.deleteall()
     if args.action == "list":
@@ -635,14 +635,13 @@ def setupModuleParser(subparsers):
     parser_add_priority(moduleParser, "module")
 
     mgroup = moduleParser.add_mutually_exclusive_group(required=True)
-    parser_add_add(mgroup, "module")
     parser_add_list(mgroup, "module")
     parser_add_extract(mgroup, "module")
     parser_add_deleteall(mgroup, "module")
-    mgroup.add_argument('-r', '--remove', dest='action', action='store_const', const='remove', help=_("Remove a module"))
-    mgroup.add_argument('-d', '--disable', dest='action', action='store_const', const='disable', help=_("Disable a module"))
-    mgroup.add_argument('-e', '--enable', dest='action', action='store_const', const='enable', help=_("Enable a module"))
-    moduleParser.add_argument('module_name', nargs='?', default=None, help=_('Name of the module to act on'))
+    mgroup.add_argument('-a', '--add', dest='action_add', action='store', nargs=1, metavar='module_name', help=_("Add a module"))
+    mgroup.add_argument('-r', '--remove', dest='action_remove', action='store', nargs='+', metavar='module_name', help=_("Remove a module"))
+    mgroup.add_argument('-d', '--disable', dest='action_disable', action='store', nargs='+', metavar='module_name', help=_("Disable a module"))
+    mgroup.add_argument('-e', '--enable', dest='action_enable', action='store', nargs='+', metavar='module_name', help=_("Enable a module"))
     moduleParser.set_defaults(func=handleModule)
 
 
diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index 556d3ba5..cd2d3457 100644
--- a/python/semanage/seobject.py
+++ b/python/semanage/seobject.py
@@ -401,6 +401,8 @@ class moduleRecords(semanageRecords):
             print("%-25s %-9s %-5s %s" % (t[0], t[2], t[3], disabled))
 
     def add(self, file, priority):
+        if type(file) == list:
+            file = file[0]
         if not os.path.exists(file):
             raise ValueError(_("Module does not exist: %s ") % file)
 
@@ -413,7 +415,9 @@ class moduleRecords(semanageRecords):
             self.commit()
 
     def set_enabled(self, module, enable):
-        for m in module.split():
+        if type(module) == str:
+            module = module.split()
+        for m in module:
             rc, key = semanage_module_key_create(self.sh)
             if rc < 0:
                 raise ValueError(_("Could not create module key"))
@@ -435,7 +439,9 @@ class moduleRecords(semanageRecords):
         if rc < 0:
             raise ValueError(_("Invalid priority %d (needs to be between 1 and 999)") % priority)
 
-        for m in module.split():
+        if type(module) == str:
+            module = module.split()
+        for m in module:
             rc = semanage_module_remove(self.sh, m)
             if rc < 0 and rc != -2:
                 raise ValueError(_("Could not remove module %s (remove failed)") % m)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive
  2019-02-06 19:43 [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach
@ 2019-02-06 19:43 ` Petr Lautrbach
  2019-02-07 21:47   ` Nicolas Iooss
  2019-02-07 21:46 ` [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Nicolas Iooss
  1 sibling, 1 reply; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-06 19:43 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

This method prints a usage message including the message to the standard error
and terminates the program with a status code of 2.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 python/semanage/semanage | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index 9b737fa8..a9fcb319 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -743,9 +743,7 @@ def handlePermissive(args):
         if args.action == "delete":
             OBJECT.delete(args.type)
     else:
-        args.parser.print_usage(sys.stderr)
-        sys.stderr.write(_('semanage permissive: error: the following argument is required: type\n'))
-        sys.exit(1)
+        args.parser.error(message=_('semanage permissive: error: the following argument is required: type\n'))
 
 
 def setupPermissiveParser(subparsers):
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options
  2019-02-06 19:43 [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach
  2019-02-06 19:43 ` [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive Petr Lautrbach
@ 2019-02-07 21:46 ` Nicolas Iooss
  2019-02-15 14:28   ` Petr Lautrbach
  1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Iooss @ 2019-02-07 21:46 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Wed, Feb 6, 2019 at 8:43 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> Previous code traceback-ed when one of the mentioned option was used without
> any argument as this state was not handled by the argument parser.
>
> action='store' stores arguments as a list while the original
> action='store_const' used str therefore the particular interfaces in
> moduleRecords are changed to be compatible with both.
>
> Fixes:
> ^_^ semanage module -a
> Traceback (most recent call last):
>   File "/usr/sbin/semanage", line 963, in <module>
>     do_parser()
>   File "/usr/sbin/semanage", line 942, in do_parser
>     args.func(args)
>   File "/usr/sbin/semanage", line 608, in handleModule
>     OBJECT.add(args.module_name, args.priority)
>   File "/usr/lib/python3.7/site-packages/seobject.py", line 402, in add
>     if not os.path.exists(file):
>   File "/usr/lib64/python3.7/genericpath.py", line 19, in exists
>     os.stat(path)
> TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>

Nice bug :) Nevertheless "if type(module) == str" troubles me because
I except a function to only accept one kind of arguments (either a
list of strings or a string, but not both). Moreover this is
Python3-only code and semanage's shebang does not specify a Python
version (the Python2-equivalent code would have been "if
isinstance(module, basestring)").

I would prefer if the new code looked like this (that I have not tested):

    def set_enabled(self, modules, enable):
        for item_modules in modules:
            for m in item_modules.split():
                # ...

Moreover the "file = file[0]" in moduleRecords.add() looks strange
without a context, which is in handleModule(). I would prefer if this
operation occurred in semanage, where it is clear that args.action_add
always has a single item (because « action='store', nargs=1 »):

    if args.action_add:
        OBJECT.add(args.action_add[0], args.priority)

Nicolas

PS: As setools is now Python3-only and seobject.py requires it, it
seems to be a good time to update the shebang to "#!/usr/bin/python3
-Es".

> ---
>  python/semanage/semanage    | 25 ++++++++++++-------------
>  python/semanage/seobject.py | 10 ++++++++--
>  2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/python/semanage/semanage b/python/semanage/semanage
> index 6afeac14..9b737fa8 100644
> --- a/python/semanage/semanage
> +++ b/python/semanage/semanage
> @@ -609,14 +609,14 @@ def setupInterfaceParser(subparsers):
>
>  def handleModule(args):
>      OBJECT = seobject.moduleRecords(args)
> -    if args.action == "add":
> -        OBJECT.add(args.module_name, args.priority)
> -    if args.action == "enable":
> -        OBJECT.set_enabled(args.module_name, True)
> -    if args.action == "disable":
> -        OBJECT.set_enabled(args.module_name, False)
> -    if args.action == "remove":
> -        OBJECT.delete(args.module_name, args.priority)
> +    if args.action_add:
> +        OBJECT.add(args.action_add, args.priority)
> +    if args.action_enable:
> +        OBJECT.set_enabled(args.action_enable, True)
> +    if args.action_disable:
> +        OBJECT.set_enabled(args.action_disable, False)
> +    if args.action_remove:
> +        OBJECT.delete(args.action_remove, args.priority)
>      if args.action == "deleteall":
>          OBJECT.deleteall()
>      if args.action == "list":
> @@ -635,14 +635,13 @@ def setupModuleParser(subparsers):
>      parser_add_priority(moduleParser, "module")
>
>      mgroup = moduleParser.add_mutually_exclusive_group(required=True)
> -    parser_add_add(mgroup, "module")
>      parser_add_list(mgroup, "module")
>      parser_add_extract(mgroup, "module")
>      parser_add_deleteall(mgroup, "module")
> -    mgroup.add_argument('-r', '--remove', dest='action', action='store_const', const='remove', help=_("Remove a module"))
> -    mgroup.add_argument('-d', '--disable', dest='action', action='store_const', const='disable', help=_("Disable a module"))
> -    mgroup.add_argument('-e', '--enable', dest='action', action='store_const', const='enable', help=_("Enable a module"))
> -    moduleParser.add_argument('module_name', nargs='?', default=None, help=_('Name of the module to act on'))
> +    mgroup.add_argument('-a', '--add', dest='action_add', action='store', nargs=1, metavar='module_name', help=_("Add a module"))
> +    mgroup.add_argument('-r', '--remove', dest='action_remove', action='store', nargs='+', metavar='module_name', help=_("Remove a module"))
> +    mgroup.add_argument('-d', '--disable', dest='action_disable', action='store', nargs='+', metavar='module_name', help=_("Disable a module"))
> +    mgroup.add_argument('-e', '--enable', dest='action_enable', action='store', nargs='+', metavar='module_name', help=_("Enable a module"))
>      moduleParser.set_defaults(func=handleModule)
>
>
> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> index 556d3ba5..cd2d3457 100644
> --- a/python/semanage/seobject.py
> +++ b/python/semanage/seobject.py
> @@ -401,6 +401,8 @@ class moduleRecords(semanageRecords):
>              print("%-25s %-9s %-5s %s" % (t[0], t[2], t[3], disabled))
>
>      def add(self, file, priority):
> +        if type(file) == list:
> +            file = file[0]
>          if not os.path.exists(file):
>              raise ValueError(_("Module does not exist: %s ") % file)
>
> @@ -413,7 +415,9 @@ class moduleRecords(semanageRecords):
>              self.commit()
>
>      def set_enabled(self, module, enable):
> -        for m in module.split():
> +        if type(module) == str:
> +            module = module.split()
> +        for m in module:
>              rc, key = semanage_module_key_create(self.sh)
>              if rc < 0:
>                  raise ValueError(_("Could not create module key"))
> @@ -435,7 +439,9 @@ class moduleRecords(semanageRecords):
>          if rc < 0:
>              raise ValueError(_("Invalid priority %d (needs to be between 1 and 999)") % priority)
>
> -        for m in module.split():
> +        if type(module) == str:
> +            module = module.split()
> +        for m in module:
>              rc = semanage_module_remove(self.sh, m)
>              if rc < 0 and rc != -2:
>                  raise ValueError(_("Could not remove module %s (remove failed)") % m)
> --
> 2.20.1
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive
  2019-02-06 19:43 ` [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive Petr Lautrbach
@ 2019-02-07 21:47   ` Nicolas Iooss
  2019-02-10 16:51     ` Nicolas Iooss
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Iooss @ 2019-02-07 21:47 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Wed, Feb 6, 2019 at 8:43 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>
> This method prints a usage message including the message to the standard error
> and terminates the program with a status code of 2.
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>

For this patch:
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

> ---
>  python/semanage/semanage | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/python/semanage/semanage b/python/semanage/semanage
> index 9b737fa8..a9fcb319 100644
> --- a/python/semanage/semanage
> +++ b/python/semanage/semanage
> @@ -743,9 +743,7 @@ def handlePermissive(args):
>          if args.action == "delete":
>              OBJECT.delete(args.type)
>      else:
> -        args.parser.print_usage(sys.stderr)
> -        sys.stderr.write(_('semanage permissive: error: the following argument is required: type\n'))
> -        sys.exit(1)
> +        args.parser.error(message=_('semanage permissive: error: the following argument is required: type\n'))
>
>
>  def setupPermissiveParser(subparsers):
> --
> 2.20.1
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive
  2019-02-07 21:47   ` Nicolas Iooss
@ 2019-02-10 16:51     ` Nicolas Iooss
  0 siblings, 0 replies; 9+ messages in thread
From: Nicolas Iooss @ 2019-02-10 16:51 UTC (permalink / raw)
  To: Petr Lautrbach; +Cc: selinux

On Thu, Feb 7, 2019 at 10:47 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Wed, Feb 6, 2019 at 8:43 PM Petr Lautrbach <plautrba@redhat.com> wrote:
> >
> > This method prints a usage message including the message to the standard error
> > and terminates the program with a status code of 2.
> >
> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>
> For this patch:
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Merged.

Nicolas

> > ---
> >  python/semanage/semanage | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/python/semanage/semanage b/python/semanage/semanage
> > index 9b737fa8..a9fcb319 100644
> > --- a/python/semanage/semanage
> > +++ b/python/semanage/semanage
> > @@ -743,9 +743,7 @@ def handlePermissive(args):
> >          if args.action == "delete":
> >              OBJECT.delete(args.type)
> >      else:
> > -        args.parser.print_usage(sys.stderr)
> > -        sys.stderr.write(_('semanage permissive: error: the following argument is required: type\n'))
> > -        sys.exit(1)
> > +        args.parser.error(message=_('semanage permissive: error: the following argument is required: type\n'))
> >
> >
> >  def setupPermissiveParser(subparsers):
> > --
> > 2.20.1
> >


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options
  2019-02-07 21:46 ` [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Nicolas Iooss
@ 2019-02-15 14:28   ` Petr Lautrbach
  2019-02-15 16:00     ` [PATCH v2 1/3] python/semanage: Drop python shebang from seobject.py Petr Lautrbach
  0 siblings, 1 reply; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-15 14:28 UTC (permalink / raw)
  To: selinux; +Cc: Nicolas Iooss

Nicolas Iooss <nicolas.iooss@m4x.org> writes:

> On Wed, Feb 6, 2019 at 8:43 PM Petr Lautrbach <plautrba@redhat.com> wrote:
>>
>> Previous code traceback-ed when one of the mentioned option was used without
>> any argument as this state was not handled by the argument parser.
>>
>> action='store' stores arguments as a list while the original
>> action='store_const' used str therefore the particular interfaces in
>> moduleRecords are changed to be compatible with both.
>>
>> Fixes:
>> ^_^ semanage module -a
>> Traceback (most recent call last):
>>   File "/usr/sbin/semanage", line 963, in <module>
>>     do_parser()
>>   File "/usr/sbin/semanage", line 942, in do_parser
>>     args.func(args)
>>   File "/usr/sbin/semanage", line 608, in handleModule
>>     OBJECT.add(args.module_name, args.priority)
>>   File "/usr/lib/python3.7/site-packages/seobject.py", line 402, in add
>>     if not os.path.exists(file):
>>   File "/usr/lib64/python3.7/genericpath.py", line 19, in exists
>>     os.stat(path)
>> TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType
>>
>> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>
> Nice bug :) Nevertheless "if type(module) == str" troubles me because
> I except a function to only accept one kind of arguments (either a
> list of strings or a string, but not both).

The idea was to have backward compatible code. Originally, semanage used
to send a string, while the new code sends a list. I didn't want to
break 3rd party and I wanted to avoid converting of the list from
action_enable to string and the converting it back to list in
moduleRecords.set_enabled()


> Moreover this is
> Python3-only code and semanage's shebang does not specify a Python
> version (the Python2-equivalent code would have been "if
> isinstance(module, basestring)").

Works for me:

> python2
Python 2.7.15 (default, Feb  2 2019, 16:04:51) 
[GCC 9.0.1 20190129 (Red Hat 9.0.1-0.2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> type("name")
<type 'str'>
>>> type(["name"])
<type 'list'>
>>> type("name") == str
True
>>> type(["name"]) == str
False


https://docs.python.org/2/library/functions.html#type


>
> I would prefer if the new code looked like this (that I have not tested):
>
>     def set_enabled(self, modules, enable):
>         for item_modules in modules:
>             for m in item_modules.split():
>                 # ...

Or just convert action_enable to string before it's sent to
moduleRecords.set_enabled():

--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -612,9 +612,9 @@ def handleModule(args):
     if args.action_add:
         OBJECT.add(args.action_add, args.priority)
     if args.action_enable:
-        OBJECT.set_enabled(args.action_enable, True)
+        OBJECT.set_enabled(args.action_enable.join(" "), True)
     if args.action_disable:
-        OBJECT.set_enabled(args.action_disable, False)
+        OBJECT.set_enabled(args.action_disable.join(" "), False)
     if args.action_remove:
         OBJECT.delete(args.action_remove, args.priority)



> Moreover the "file = file[0]" in moduleRecords.add() looks strange
> without a context, which is in handleModule(). I would prefer if this
> operation occurred in semanage, where it is clear that args.action_add
> always has a single item (because « action='store', nargs=1 »):
>
>     if args.action_add:
>         OBJECT.add(args.action_add[0], args.priority)

Good idea.

>
> Nicolas
>
> PS: As setools is now Python3-only and seobject.py requires it, it
> seems to be a good time to update the shebang to "#!/usr/bin/python3
> -Es".

seobject.py is just a module which is not supposed to be entry point.
The shebang does not make sense here and should be removed.

But I'd change at least semanage and chcat as they both directly imports
seobject.

Or rather change the whole project to python3 by default.



>
>> ---
>>  python/semanage/semanage    | 25 ++++++++++++-------------
>>  python/semanage/seobject.py | 10 ++++++++--
>>  2 files changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/python/semanage/semanage b/python/semanage/semanage
>> index 6afeac14..9b737fa8 100644
>> --- a/python/semanage/semanage
>> +++ b/python/semanage/semanage
>> @@ -609,14 +609,14 @@ def setupInterfaceParser(subparsers):
>>
>>  def handleModule(args):
>>      OBJECT = seobject.moduleRecords(args)
>> -    if args.action == "add":
>> -        OBJECT.add(args.module_name, args.priority)
>> -    if args.action == "enable":
>> -        OBJECT.set_enabled(args.module_name, True)
>> -    if args.action == "disable":
>> -        OBJECT.set_enabled(args.module_name, False)
>> -    if args.action == "remove":
>> -        OBJECT.delete(args.module_name, args.priority)
>> +    if args.action_add:
>> +        OBJECT.add(args.action_add, args.priority)
>> +    if args.action_enable:
>> +        OBJECT.set_enabled(args.action_enable, True)
>> +    if args.action_disable:
>> +        OBJECT.set_enabled(args.action_disable, False)
>> +    if args.action_remove:
>> +        OBJECT.delete(args.action_remove, args.priority)
>>      if args.action == "deleteall":
>>          OBJECT.deleteall()
>>      if args.action == "list":
>> @@ -635,14 +635,13 @@ def setupModuleParser(subparsers):
>>      parser_add_priority(moduleParser, "module")
>>
>>      mgroup = moduleParser.add_mutually_exclusive_group(required=True)
>> -    parser_add_add(mgroup, "module")
>>      parser_add_list(mgroup, "module")
>>      parser_add_extract(mgroup, "module")
>>      parser_add_deleteall(mgroup, "module")
>> -    mgroup.add_argument('-r', '--remove', dest='action', action='store_const', const='remove', help=_("Remove a module"))
>> -    mgroup.add_argument('-d', '--disable', dest='action', action='store_const', const='disable', help=_("Disable a module"))
>> -    mgroup.add_argument('-e', '--enable', dest='action', action='store_const', const='enable', help=_("Enable a module"))
>> -    moduleParser.add_argument('module_name', nargs='?', default=None, help=_('Name of the module to act on'))
>> +    mgroup.add_argument('-a', '--add', dest='action_add', action='store', nargs=1, metavar='module_name', help=_("Add a module"))
>> +    mgroup.add_argument('-r', '--remove', dest='action_remove', action='store', nargs='+', metavar='module_name', help=_("Remove a module"))
>> +    mgroup.add_argument('-d', '--disable', dest='action_disable', action='store', nargs='+', metavar='module_name', help=_("Disable a module"))
>> +    mgroup.add_argument('-e', '--enable', dest='action_enable', action='store', nargs='+', metavar='module_name', help=_("Enable a module"))
>>      moduleParser.set_defaults(func=handleModule)
>>
>>
>> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
>> index 556d3ba5..cd2d3457 100644
>> --- a/python/semanage/seobject.py
>> +++ b/python/semanage/seobject.py
>> @@ -401,6 +401,8 @@ class moduleRecords(semanageRecords):
>>              print("%-25s %-9s %-5s %s" % (t[0], t[2], t[3], disabled))
>>
>>      def add(self, file, priority):
>> +        if type(file) == list:
>> +            file = file[0]
>>          if not os.path.exists(file):
>>              raise ValueError(_("Module does not exist: %s ") % file)
>>
>> @@ -413,7 +415,9 @@ class moduleRecords(semanageRecords):
>>              self.commit()
>>
>>      def set_enabled(self, module, enable):
>> -        for m in module.split():
>> +        if type(module) == str:
>> +            module = module.split()
>> +        for m in module:
>>              rc, key = semanage_module_key_create(self.sh)
>>              if rc < 0:
>>                  raise ValueError(_("Could not create module key"))
>> @@ -435,7 +439,9 @@ class moduleRecords(semanageRecords):
>>          if rc < 0:
>>              raise ValueError(_("Invalid priority %d (needs to be between 1 and 999)") % priority)
>>
>> -        for m in module.split():
>> +        if type(module) == str:
>> +            module = module.split()
>> +        for m in module:
>>              rc = semanage_module_remove(self.sh, m)
>>              if rc < 0 and rc != -2:
>>                  raise ValueError(_("Could not remove module %s (remove failed)") % m)
>> --
>> 2.20.1
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] python/semanage: Drop python shebang from seobject.py
  2019-02-15 14:28   ` Petr Lautrbach
@ 2019-02-15 16:00     ` Petr Lautrbach
  2019-02-15 16:00       ` [PATCH v2 2/3] python/semanage: Update semanage to use python3 Petr Lautrbach
  2019-02-15 16:00       ` [PATCH v2 3/3] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach
  0 siblings, 2 replies; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-15 16:00 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

seobject.py is not supposed to be used as entrypoint therefore the shebang is
unnecessary. It also doesn't need execute bits.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 python/semanage/Makefile    | 2 +-
 python/semanage/seobject.py | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/python/semanage/Makefile b/python/semanage/Makefile
index 2e262ef8..37065268 100644
--- a/python/semanage/Makefile
+++ b/python/semanage/Makefile
@@ -27,7 +27,7 @@ install: all
 		fi ; \
 	done
 	test -d $(DESTDIR)/$(PACKAGEDIR) || install -m 755 -d $(DESTDIR)/$(PACKAGEDIR)
-	install -m 755 seobject.py $(DESTDIR)/$(PACKAGEDIR)
+	install -m 644 seobject.py $(DESTDIR)/$(PACKAGEDIR)
 	-mkdir -p $(DESTDIR)$(BASHCOMPLETIONDIR)
 	install -m 644 $(BASHCOMPLETIONS) $(DESTDIR)$(BASHCOMPLETIONDIR)/semanage
 
diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index b31a90c1..13fdf531 100644
--- a/python/semanage/seobject.py
+++ b/python/semanage/seobject.py
@@ -1,4 +1,3 @@
-#! /usr/bin/python -Es
 # Copyright (C) 2005-2013 Red Hat
 # see file 'COPYING' for use and warranty information
 #
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] python/semanage: Update semanage to use python3
  2019-02-15 16:00     ` [PATCH v2 1/3] python/semanage: Drop python shebang from seobject.py Petr Lautrbach
@ 2019-02-15 16:00       ` Petr Lautrbach
  2019-02-15 16:00       ` [PATCH v2 3/3] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach
  1 sibling, 0 replies; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-15 16:00 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

semanage uses seobject which uses setools which is python 3 only.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 python/semanage/semanage | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index 4b544bfc..18191c13 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -1,4 +1,4 @@
-#! /usr/bin/python -Es
+#! /usr/bin/python3 -Es
 # Copyright (C) 2012-2013 Red Hat
 # AUTHOR: Miroslav Grepl <mgrepl@redhat.com>
 # AUTHOR: David Quigley <selinux@davequigley.com>
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] python/semanage module: Fix handling of -a/-e/-d/-r options
  2019-02-15 16:00     ` [PATCH v2 1/3] python/semanage: Drop python shebang from seobject.py Petr Lautrbach
  2019-02-15 16:00       ` [PATCH v2 2/3] python/semanage: Update semanage to use python3 Petr Lautrbach
@ 2019-02-15 16:00       ` Petr Lautrbach
  1 sibling, 0 replies; 9+ messages in thread
From: Petr Lautrbach @ 2019-02-15 16:00 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

Previous code traceback-ed when one of the mentioned option was used without
any argument as this state was not handled by the argument parser.

action='store' stores arguments as a list while the original
action='store_const' used str therefore it's needed to convert list to str
before it's sent to moduleRecords class.

Fixes:
^_^ semanage module -a
Traceback (most recent call last):
  File "/usr/sbin/semanage", line 963, in <module>
    do_parser()
  File "/usr/sbin/semanage", line 942, in do_parser
    args.func(args)
  File "/usr/sbin/semanage", line 608, in handleModule
    OBJECT.add(args.module_name, args.priority)
  File "/usr/lib/python3.7/site-packages/seobject.py", line 402, in add
    if not os.path.exists(file):
  File "/usr/lib64/python3.7/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 python/semanage/semanage | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/python/semanage/semanage b/python/semanage/semanage
index 18191c13..d6d68248 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -609,14 +609,14 @@ def setupInterfaceParser(subparsers):
 
 def handleModule(args):
     OBJECT = seobject.moduleRecords(args)
-    if args.action == "add":
-        OBJECT.add(args.module_name, args.priority)
-    if args.action == "enable":
-        OBJECT.set_enabled(args.module_name, True)
-    if args.action == "disable":
-        OBJECT.set_enabled(args.module_name, False)
-    if args.action == "remove":
-        OBJECT.delete(args.module_name, args.priority)
+    if args.action_add:
+        OBJECT.add(args.action_add[0], args.priority)
+    if args.action_enable:
+        OBJECT.set_enabled(" ".join(args.action_enable), True)
+    if args.action_disable:
+        OBJECT.set_enabled(" ".join(args.action_disable), False)
+    if args.action_remove:
+        OBJECT.delete(" ".join(args.action_remove), args.priority)
     if args.action == "deleteall":
         OBJECT.deleteall()
     if args.action == "list":
@@ -635,14 +635,13 @@ def setupModuleParser(subparsers):
     parser_add_priority(moduleParser, "module")
 
     mgroup = moduleParser.add_mutually_exclusive_group(required=True)
-    parser_add_add(mgroup, "module")
     parser_add_list(mgroup, "module")
     parser_add_extract(mgroup, "module")
     parser_add_deleteall(mgroup, "module")
-    mgroup.add_argument('-r', '--remove', dest='action', action='store_const', const='remove', help=_("Remove a module"))
-    mgroup.add_argument('-d', '--disable', dest='action', action='store_const', const='disable', help=_("Disable a module"))
-    mgroup.add_argument('-e', '--enable', dest='action', action='store_const', const='enable', help=_("Enable a module"))
-    moduleParser.add_argument('module_name', nargs='?', default=None, help=_('Name of the module to act on'))
+    mgroup.add_argument('-a', '--add', dest='action_add', action='store', nargs=1, metavar='module_name', help=_("Add a module"))
+    mgroup.add_argument('-r', '--remove', dest='action_remove', action='store', nargs='+', metavar='module_name', help=_("Remove a module"))
+    mgroup.add_argument('-d', '--disable', dest='action_disable', action='store', nargs='+', metavar='module_name', help=_("Disable a module"))
+    mgroup.add_argument('-e', '--enable', dest='action_enable', action='store', nargs='+', metavar='module_name', help=_("Enable a module"))
     moduleParser.set_defaults(func=handleModule)
 
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 19:43 [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach
2019-02-06 19:43 ` [PATCH 2/2] python/semanage: Use standard argparse.error() method in handlePermissive Petr Lautrbach
2019-02-07 21:47   ` Nicolas Iooss
2019-02-10 16:51     ` Nicolas Iooss
2019-02-07 21:46 ` [PATCH 1/2] python/semanage module: Fix handling of -a/-e/-d/-r options Nicolas Iooss
2019-02-15 14:28   ` Petr Lautrbach
2019-02-15 16:00     ` [PATCH v2 1/3] python/semanage: Drop python shebang from seobject.py Petr Lautrbach
2019-02-15 16:00       ` [PATCH v2 2/3] python/semanage: Update semanage to use python3 Petr Lautrbach
2019-02-15 16:00       ` [PATCH v2 3/3] python/semanage module: Fix handling of -a/-e/-d/-r options Petr Lautrbach

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org selinux@archiver.kernel.org
	public-inbox-index selinux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox