Re: [PATCH v2] semanage, sepolicy: list also ports not attributed with port_type

[Petr Lautrbach <plautrba@redhat.com>]

Topi Miettinen <toiwoton@gmail.com> writes:

> On 6.6.2023 13.01, Petr Lautrbach wrote:
>> Topi Miettinen <toiwoton@gmail.com> writes:
>> 
>>> For `semanage port -l` and `sepolicy network -t type`, show also ports
>>> which are not attributed with `port_type`. Such ports may exist in
>>> custom policies and even the attribute `port_type` may not be defined.
>>>
>>> This fixes the following error with `semanage port -l` (and similar
>>> error with `sepolicy network -t type`):
>>>
>>> Traceback (most recent call last):
>>>    File "/usr/sbin/semanage", line 975, in <module>
>>>      do_parser()
>>>    File "/usr/sbin/semanage", line 947, in do_parser
>>>      args.func(args)
>>>    File "/usr/sbin/semanage", line 441, in handlePort
>>>      OBJECT = object_dict['port'](args)
>>>               ^^^^^^^^^^^^^^^^^^^^^^^^^
>>>    File "/usr/lib/python3/dist-packages/seobject.py", line 1057, in __init__
>>>      self.valid_types = list(list(sepolicy.info(sepolicy.ATTRIBUTE, "port_type"))[0]["types"])
>>>                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
>>> IndexError: list index out of range
>>>
>>> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
>>>
>>> ---
>>>
>>> v2: fix other cases and use better version courtesy of Petr Lautrbach
>>> ---
>>>   python/semanage/semanage-bash-completion.sh | 2 +-
>>>   python/semanage/seobject.py                 | 2 +-
>>>   python/sepolicy/sepolicy-bash-completion.sh | 2 +-
>>>   python/sepolicy/sepolicy/__init__.py        | 2 +-
>>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/python/semanage/semanage-bash-completion.sh b/python/semanage/semanage-bash-completion.sh
>>> index d0dd139f..1e3f6f9d 100644
>>> --- a/python/semanage/semanage-bash-completion.sh
>>> +++ b/python/semanage/semanage-bash-completion.sh
>>> @@ -37,7 +37,7 @@ __get_all_types () {
>>>       seinfo -t 2> /dev/null | tail -n +3
>>>   }
>>>   __get_all_port_types () {
>>> -    seinfo -aport_type -x 2>/dev/null | tail -n +2
>>> +    sepolicy network -l
>>>   }
>> 
>> I support this change but it could have a side effect on distributions.
>> E.g. in Fedora we ship semanage bash completion in
>> policycoreutils-python-utils while sepolicy in policycoreutils-devel. On
>> the other hand seinfo is in setools-console package which is not required by
>> policycoreutils-python-utils so completions would not work anyway.
>> 
>>  From upstream POV, it improves the situation so unless there's any other
>> objection from other distribution maintainers I would not block it..
>
> If you prefer, it's also possible to continue to use seinfo with:
>
> seinfo --portcon 2>/dev/null | sed -n 
> 's/^\s\+portcon\s\+\S\+\s\+\S\+\s\+[^:]\+:[^:]\+:\([^:]\+\):\S\+$/\1/gp'
>

`sepolicy network -l` definitely looks better so I'd stick with it.

btw `seinfo --portcon` generates duplicates on Fedora:

$ seinfo --portcon 2>/dev/null | sed -n 's/^\s\+portcon\s\+\S\+\s\+\S\+\s\+[^:]\+:[^:]\+:\([^:]\+\):\S\+$/\1/gp' | wc -l              
663

$ seinfo --portcon 2>/dev/null | sed -n 's/^\s\+portcon\s\+\S\+\s\+\S\+\s\+[^:]\+:[^:]\+:\([^:]\+\):\S\+$/\1/gp' | sort | uniq | wc -l
308




>
>>>   __get_all_domains () {
>>>       seinfo -adomain -x 2>/dev/null | tail -n +2
>>> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
>>> index d82da494..21a6fc91 100644
>>> --- a/python/semanage/seobject.py
>>> +++ b/python/semanage/seobject.py
>>> @@ -1055,7 +1055,7 @@ class portRecords(semanageRecords):
>>>       def __init__(self, args = None):
>>>           semanageRecords.__init__(self, args)
>>>           try:
>>> -            self.valid_types = list(list(sepolicy.info(sepolicy.ATTRIBUTE, "port_type"))[0]["types"])
>>> +            self.valid_types = [x["type"] for x in list(list(sepolicy.info(sepolicy.PORT)))]
>> 
>> I know it's suggested by me. But looking on to it I see repeating list()
>> which is unnecessary. sepolicy.info() returns a generator and so the new
>> list could be constructed directly from it:
>> 
>> [x["type"] for x in sepolicy.info(sepolicy.PORT)]
>
> Thanks!
>
>> 
>> 
>>>           except RuntimeError:
>>>               pass
>>>   
>>> diff --git a/python/sepolicy/sepolicy-bash-completion.sh b/python/sepolicy/sepolicy-bash-completion.sh
>>> index 13638e4d..467333b8 100644
>>> --- a/python/sepolicy/sepolicy-bash-completion.sh
>>> +++ b/python/sepolicy/sepolicy-bash-completion.sh
>>> @@ -52,7 +52,7 @@ __get_all_classes () {
>>>       seinfo -c 2> /dev/null | tail -n +2
>>>   }
>>>   __get_all_port_types () {
>>> -    seinfo -aport_type -x 2> /dev/null | tail -n +2
>>> +    sepolicy network -l
>>>   }
>> 
>> Here the change does not have any side effect and improves the
>> functionality
>
> It's also possible to use the seinfo | sed version here too.
>
>> 
>>>   __get_all_domain_types () {
>>>       seinfo -adomain -x 2> /dev/null | tail -n +2
>>> diff --git a/python/sepolicy/sepolicy/__init__.py b/python/sepolicy/sepolicy/__init__.py
>>> index c177cdfc..76ac7797 100644
>>> --- a/python/sepolicy/sepolicy/__init__.py
>>> +++ b/python/sepolicy/sepolicy/__init__.py
>>> @@ -989,7 +989,7 @@ def get_all_port_types():
>>>       global port_types
>>>       if port_types:
>>>           return port_types
>>> -    port_types = list(sorted(info(ATTRIBUTE, "port_type"))[0]["types"])
>>> +    port_types = [x["type"] for x in list(list(info(PORT)))]
>> 
>> [x["type"] for x in info(PORT)]
>> 
>>>       return port_types
>>>   
>>>   
>>> -- 
>>> 2.39.2
>>