[Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

[Petr Viktorin]

On 09/25/2012 01:54 PM, Martin Kosek wrote:
> config-mod is capable of changing default SELinux user map order
> and a default SELinux user. Validate the new config values to
> prevent bogus default SELinux users to be assigned to IPA users.
>
> https://fedorahosted.org/freeipa/ticket/2993
>
> ---
> Note: I removed the previous "validate" construct:
> -        validate = dict(options)
> -        validate.update(entry_attrs)
> ... as entry_attrs contains both values set via standard options and *attr.
>
> Martin
>

The patch looks OK, see a comment below.

I found strange behavior in validate_selinuxuser. Perhaps it's material 
for another ticket. This command passes validation:

$ ./ipa config_mod 
--ipaselinuxusermapdefault=unconfined_u:s0-s0:c0.c1023 
--ipaselinuxusermaporder='unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c4,c4:→Why 
is stuff allowed here?'
[...]
   SELinux user map order: 
unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c3.c8,c4:→Why is stuff 
allowed here?
   Default SELinux user: unconfined_u:s0-s0:c0.c1023
   PAC type: MS-PAC

Obviously extra info should not be allowed.
Is "s5-s1" or "c4.c3" valid? Can the first value be higher than the second?
AFAIK (I'm not an expert though), MCS doesn't allow dashes, so "c0-c4" 
should not be allowed. Chains like "c1.c2.c3" also don't look right.

> freeipa-mkosek-313-validate-selinux-users-in-config-mod.patch
>
>
>  From 296eedc7cfd258b9e5eaf4f182b1a9625f5bf1a1 Mon Sep 17 00:00:00 2001
> From: Martin Kosek<mkosek at redhat.com>
> Date: Tue, 25 Sep 2012 13:46:56 +0200
> Subject: [PATCH] Validate SELinux users in config-mod
>
> config-mod is capable of changing default SELinux user map order
> and a default SELinux user. Validate the new config values to
> prevent bogus default SELinux users to be assigned to IPA users.
>
> https://fedorahosted.org/freeipa/ticket/2993
> ---
>   ipalib/plugins/config.py                | 49 +++++++++++++++++++++------------
>   tests/test_xmlrpc/test_config_plugin.py | 44 ++++++++++++++++++++++++-----
>   2 files changed, 69 insertions(+), 24 deletions(-)
>
> diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
> index e02519d5759f4e4a6d6a7075fe896f8b2e69b451..1c62e0d942231fac442ee2c1f31431003c08e283 100644
> --- a/ipalib/plugins/config.py
> +++ b/ipalib/plugins/config.py
[...]
> +                # validate the new user order first
> +                for user in userlist:
> +                    if not user:
> +                        raise errors.ValidationError(name='ipaselinuxusermaporder',
> +                                error=_('A list of SELinux users delimited by $ expected'))
> +

This will only catch empty users (i.e. a "$$", or a "$" at beginning or 
end), right? A specific message like "empty users not allowed" could be 
more helpful.

-- 
Petr³