Re: [PATCH 2/3] python/sepolicy: Stop rejecting aliases in sepolicy commands

[Vit Mojzis <vmojzis@redhat.com>]

On 21. 10. 18 11:20, Nicolas Iooss wrote:> On Sun, Oct 21, 2018 at 11:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>>
>> On Tue, Oct 16, 2018 at 12:05 PM Vit Mojzis <vmojzis@redhat.com> wrote:
>>>
>>> Fix CheckDomain and CheckPortType classes to properly deal with aliases.
>>>
>>> Resolves:
>>>     https://bugzilla.redhat.com/show_bug.cgi?id=1600009
>>>
>>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>>> ---
>>>   python/sepolicy/sepolicy.py          |  8 +++-----
>>>   python/sepolicy/sepolicy/__init__.py | 10 +++++++++-
>>>   2 files changed, 12 insertions(+), 6 deletions(-)
>>>
>> [...]
>>> diff --git a/python/sepolicy/sepolicy/__init__.py b/python/sepolicy/sepolicy/__init__.py
>>> index 8484b28c..0da3917b 100644
>>> --- a/python/sepolicy/sepolicy/__init__.py
>>> +++ b/python/sepolicy/sepolicy/__init__.py
>>> @@ -447,6 +447,14 @@ def get_file_types(setype):
>>>       return mpaths
>>>
>>>
>>> +# determine if entered type is an alias
>>> +# and return corresponding type name
>>> +def get_real_type_name(name):
>>> +    try:
>>> +        return next(info(TYPE, name))["name"]
>>> +    except (RuntimeError, StopIteration):
>>> +        return None
>>> +
>>
>> When I first read the function comment, I did not understand whether
>> the returned value was a kind of SELinux type (like something which
>> would say "a file type", "a port type") or the SELinux type which is
>> aliased. It is the second interpretation which is implemented.
>> Moreover, this comment seems to be better in a function docstring. I
>> suggest changing this into the following:
>>
>> def get_real_type_name(name):
>>      """Return the real name of a type:
>>
>>      * If 'name' refers to a type, return the same name.
>>      * If 'name' refers to a type alias, return the corresponding name.
>>      * Otherwise return None.
>>      """
>>
>> What do you think of this suggestion?
>>
>> Moreover, when is RuntimeError triggered, in the exception that are caught?

Catching RuntimeError ensures compatibility with setools3, which
triggers this exception instead of returning empty list.

> 
> Also, if name is empty, sepolicy.info(TYPE, name) returns all
> available types, so get_real_type_name("") returns the first type in
> the list of types defined in the policy. I saw that your patches use
> "if setype and sepolicy.get_real_type_name(setype) not in
> self.valid_types" in many places (which looks good). To increase the
> robustness of the Python code, I would also add a test in
> get_real_type_name as well, like:
> 
>      if not name:
>          return None

Good point, thank you.

> 
> Nicolas
>