[Freeipa-devel] [PATCH] 0032 Validate sudo RunAsUser/RunAsGroup arguments

Alexander Bokovoy abokovoy at redhat.com
Wed Dec 7 11:49:49 UTC 2011


On Fri, 02 Dec 2011, Rob Crittenden wrote:
> Alexander Bokovoy wrote:
> >Hi,
> >
> >FreeIPA SUDO rules use --usercat/--groupcat to specify that rule
> >applies to all users or groups. Thus, sudorule-add-runasuser and
> >sudorule-add-runasgroup accept specific groups and users and do not
> >accept ALL reserved word.
> >
> >The patch validates user and group passed to these commands and
> >reports appropriate errors when these are ALL or all arguments
> >are empty.
> >
> >Ticket #1496
> >https://fedorahosted.org/freeipa/ticket/1496
> >
> >One thing I'm not sure about is blocking all variants of the reserved
> >word 'ALL'. The patch blocks them all due to the fact that most likely
> >any of 'all', 'All', 'ALL', 'aLL', and so on are mistyping but there
> >are might be valid cases when group or user is called 'all'.
> 
> Then runasuser check reports runas-group as the attribute name, I
> think it should still be runas-user even though it is a group of
> users.
Ok. Changed.


> Other member commands don't consider it an error to provide any
> actual members, it treats it as a no-op. We should probably be
> consistent.
Don't understand. Did you mean 'to not provide any actual members'?

In case you did, attached patch removes remaining checks for 
runas_{user,group) to be False. 


> It would probably be better to return the value as passed in by the
> user rather than user[0].value.
The issue here is that names come to the callback already as DNs from 
LDAPAddMember's execute() method. Strictly speaking it is already 
different to what user has entered as we do expansion by default to 
add $SUFFIX and appropriate container.

In the updated patch I tried to reduce DN to something reasonable by 
relying on known containers and only showing full DN for cases when 
these are not users/groups containers.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 4bf337aad4ab228efb34b6b553c2856de63cb245 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Mon, 14 Nov 2011 11:23:19 +0200
Subject: [PATCH] Validate sudo RunAsUser/RunAsGroup arguments

FreeIPA SUDO rules use --usercat/--groupcat to specify that rule
applies to all users or groups. Thus, sudorule-add-runasuser and
sudorule-add-runasgroup accept specific groups and users and do not
accept ALL reserved word.

The patch validates user and group passed to these commands and
reports appropriate errors when these are ALL.

Ticket #1496
https://fedorahosted.org/freeipa/ticket/1496
---
 ipalib/plugins/sudorule.py |   60 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py
index 93ca03f0170d922b91eff45ec2f42871336973f1..31b72d860f0dafc34e00432edaa4da69a809e2f8 100644
--- a/ipalib/plugins/sudorule.py
+++ b/ipalib/plugins/sudorule.py
@@ -472,6 +472,18 @@ class sudorule_remove_host(LDAPRemoveMember):
 
 api.register(sudorule_remove_host)
 
+def relative_to_container(env, cn_name, dn):
+    try:
+        cn_ = 'container_%s' % (cn_name)
+        if cn_ in env:
+            cn = DN(env[cn_])+DN(env.basedn)
+        else:
+            return dn
+    except:
+        return dn
+    if dn.endswith(cn):
+        return dn[0].value
+    return dn
 
 class sudorule_add_runasuser(LDAPAddMember):
     __doc__ = _('Add users and groups for Sudo to execute as.')
@@ -479,6 +491,33 @@ class sudorule_add_runasuser(LDAPAddMember):
     member_attributes = ['ipasudorunas']
     member_count_out = ('%i object added.', '%i objects added.')
 
+    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+        def check_validity(runas):
+            v = unicode(runas[0].value)
+            if v.upper() == u'ALL':
+                return False
+            return True
+
+        if 'ipasudorunas' in entry_attrs:
+            # Map group and user options. If option value is missing, runas_* will
+            # be set to False. The ordering is group, user and values are
+            # DNs already so we'll use DN class for extracting actual CN.
+            (runas_group, runas_user) = map(lambda x: len(x)>0 and x[0], entry_attrs['ipasudorunas'].values())
+            if runas_group:
+                group = DN(runas_group)
+                if not check_validity(group):
+                    raise errors.ValidationError(name='runas-user',
+                            error=unicode(_("RunAsUser does not accept '%s' as a group name")) %
+                                    (relative_to_container(self.api.env, 'group', group)))
+            if runas_user:
+                user = DN(runas_user)
+                if not check_validity(user):
+                    raise errors.ValidationError(name='runas-user',
+                            error=unicode(_("RunAsUser does not accept '%s' as a user name")) %
+                                    (relative_to_container(self.api.env, 'user', user)))
+
+        return dn
+
     def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options):
         completed_external = 0
         # Sift through the user failures. We assume that these are all
@@ -547,6 +586,27 @@ class sudorule_add_runasgroup(LDAPAddMember):
     member_attributes = ['ipasudorunasgroup']
     member_count_out = ('%i object added.', '%i objects added.')
 
+    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+        def check_validity(runas):
+            v = unicode(runas[0].value)
+            if v.upper() == u'ALL':
+                return False
+            return True
+
+        if 'ipasudorunasgroup' in entry_attrs:
+            # Extract group DN. If it is empty, runas_group will be set to False.
+            # We'll use DN class to extract actual CN we are interested in.
+            runas_group = entry_attrs['ipasudorunasgroup']['group']
+            runas_group = len(runas_group)>0 and runas_group[0]
+            if runas_group:
+                group = DN(runas_group)
+                if not check_validity(group):
+                    raise errors.ValidationError(name='runas-group',
+                            error=unicode(_("RunAsGroup does not accept '%s' as a group name")) %
+                                    (relative_to_container(self.api.env, 'group', group)))
+
+        return dn
+
     def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options):
         completed_external = 0
         # Sift through the group failures. We assume that these are all
-- 
1.7.7.3



More information about the Freeipa-devel mailing list