Martin Kosek wrote:
On Mon, 2012-01-23 at 12:20 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Tue, 2012-01-17 at 17:59 -0500, Rob Crittenden wrote:
When deleting an HBAC rule we need to ensure that an SELinux user map
isn't pointing at it. The search for this didn't work well at all.

This patch corrects the search and makes it more specific.

I also tested that it works with the --continue flag of hbacrule-del.

The ticket has instructions on testing.

rob

Works fine. There is just one part that is IMO too complicated:

+            hbacrule = options['seealso']
+            kw = dict(cn=hbacrule, all=True)
               _entries = api.Command.hbacrule_find(None, **kw)['result']
               del options['seealso']
-            if _entries:
-                options['seealso'] = _entries[0]['dn']
+            found = False
+            # look for an exact match. The search may return partial
+            # matches.
+            for entry in _entries:
+                if entry['cn'][0] == hbacrule:
+                    found = True
+                    options['seealso'] = entry['dn']
+            if not found:
+                return dict(count=0, result=[], truncated=False)

I think hbacrule_find(None, cn=HBACRULE) should not return partial
matches, but just the exact match (tried with hbacrule-find
--name=HBACRULE). Then the loop over entries wouldn't be needed.

Couldn't we simply call hbacrule_show since we want just one HBAC rule
with a known primary key?

Martin


hbacrule_show would need to be modified to take a dn, that would be a
way to fix this.

rob

Not sure I see the problem with hbacrule_show. I tested this piece of
code and it worked fine:

selinuxusermap_find:
...
         if 'seealso' in options:
             hbacrule = options['seealso']

             try:
                 hbac = api.Command['hbacrule_show'](hbacrule,
all=True)['result']
                 dn = hbac['dn']
             except errors.NotFound:
                 return dict(count=0, result=[], truncated=False)
             options['seealso'] = dn
...

Martin


Ok, I misunderstood your point. Yes, this is vastly better. Updated patch attached.

rob
>From 31c07fe8450e1732a1f5b843c0c4903489d8b3a1 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 17 Jan 2012 17:54:00 -0500
Subject: [PATCH] Fix deletion of HBAC Rules when there are SELinux user maps
 defined

When deleting an HBAC rule we need to ensure that an SELinux user
map isn't pointing at it. We need to take what is the cn of the HBAC
rule and see if that rule exists, then return the dn to that rule.

The search was not being done properly and wasn't enforcing uniqueness.
It could have returned partial matches as well (so tests for the
search test).

https://fedorahosted.org/freeipa/ticket/2269
---
 ipalib/plugins/hbacrule.py                      |    2 +-
 ipalib/plugins/selinuxusermap.py                |   21 ++++++++++----
 tests/test_xmlrpc/test_selinuxusermap_plugin.py |   35 +++++++++++++++++++++++
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/ipalib/plugins/hbacrule.py b/ipalib/plugins/hbacrule.py
index 0fa44a5903cf35715d0d97acb5aa0a5eb58ddf76..53d25aac697c78d78428c023efba6780d3d46633 100644
--- a/ipalib/plugins/hbacrule.py
+++ b/ipalib/plugins/hbacrule.py
@@ -243,7 +243,7 @@ class hbacrule_del(LDAPDelete):
     msg_summary = _('Deleted HBAC rule "%(value)s"')
 
     def pre_callback(self, ldap, dn, *keys, **options):
-        kw = dict(seealso=dn)
+        kw = dict(seealso=keys[0])
         _entries = api.Command.selinuxusermap_find(None, **kw)
         if _entries['count']:
             raise errors.DependentEntry(key=keys[0], label=self.api.Object['selinuxusermap'].label_singular, dependent=_entries['result'][0]['cn'][0])
diff --git a/ipalib/plugins/selinuxusermap.py b/ipalib/plugins/selinuxusermap.py
index 475376f6efc9a8e17abe7b5553173b2b57142170..ea46b2df9489a8ef5722eaa1a81d1a30153f3043 100644
--- a/ipalib/plugins/selinuxusermap.py
+++ b/ipalib/plugins/selinuxusermap.py
@@ -29,7 +29,9 @@ SELinux User Mapping
 Map IPA users to SELinux users by host.
 
 Hosts, hostgroups, users and groups can be either defined within
-the rule or it may point to an existing HBAC rule.
+the rule or it may point to an existing HBAC rule. When using
+--hbacrule option to selinuxusermap-find an exact match is made on the
+HBAC rule name, so only one or zero entries will be returned.
 
 EXAMPLES:
 
@@ -54,6 +56,9 @@ EXAMPLES:
  Enable a named rule:
    ipa selinuxusermap-enable test1
 
+ Find a rule referencing a specific HBAC rule:
+   ipa selinuxusermap-find --hbacrule=allow_some
+
  Remove a named rule:
    ipa selinuxusermap-del john_unconfined
 
@@ -299,11 +304,15 @@ class selinuxusermap_find(LDAPSearch):
     def execute(self, *args, **options):
         # If searching on hbacrule we need to find the uuid to search on
         if 'seealso' in options:
-            kw = dict(cn=options['seealso'], all=True)
-            _entries = api.Command.hbacrule_find(None, **kw)['result']
-            del options['seealso']
-            if _entries:
-                options['seealso'] = _entries[0]['dn']
+            hbacrule = options['seealso']
+
+            try:
+                hbac = api.Command['hbacrule_show'](hbacrule,
+all=True)['result']
+                dn = hbac['dn']
+            except errors.NotFound:
+                return dict(count=0, result=[], truncated=False)
+            options['seealso'] = dn
 
         return super(selinuxusermap_find, self).execute(*args, **options)
 
diff --git a/tests/test_xmlrpc/test_selinuxusermap_plugin.py b/tests/test_xmlrpc/test_selinuxusermap_plugin.py
index 368037dbef59695c9600f84489b79d7df12c8629..2fdccf3ef423f1af9d6e6f8ecb601e54aef9a2de 100644
--- a/tests/test_xmlrpc/test_selinuxusermap_plugin.py
+++ b/tests/test_xmlrpc/test_selinuxusermap_plugin.py
@@ -36,6 +36,7 @@ host1 = u'testhost1.%s' % api.env.domain
 hostdn1 = DN(('fqdn',host1),('cn','computers'),('cn','accounts'),
          api.env.basedn)
 hbacrule1 = u'testhbacrule1'
+hbacrule2 = u'testhbacrule12'
 
 fuzzy_selinuxusermapdn = Fuzzy(
     'ipauniqueid=[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12},%s,%s' % (api.env.container_selinux, api.env.basedn)
@@ -51,6 +52,7 @@ class test_selinuxusermap(Declarative):
         ('user_del', [user1], {}),
         ('host_del', [host1], {}),
         ('hbacrule_del', [hbacrule1], {}),
+        ('hbacrule_del', [hbacrule2], {}),
     ]
 
     tests = [
@@ -310,6 +312,26 @@ class test_selinuxusermap(Declarative):
         ),
 
 
+        dict(
+            desc='Create HBAC rule %r' % hbacrule2,
+            command=(
+                'hbacrule_add', [hbacrule2], {}
+            ),
+            expected=dict(
+                value=hbacrule2,
+                summary=u'Added HBAC rule "%s"' % hbacrule2,
+                result=dict(
+                    cn=[hbacrule2],
+                    objectclass=objectclasses.hbacrule,
+                    ipauniqueid=[fuzzy_uuid],
+                    accessruletype=[u'allow'],
+                    ipaenabledflag=[u'TRUE'],
+                    dn=fuzzy_hbacruledn,
+                ),
+            ),
+        ),
+
+
         ###############
         # Fill out rule with members and/or pointers to HBAC rules
         dict(
@@ -542,6 +564,19 @@ class test_selinuxusermap(Declarative):
         ),
 
 
+        # This tests selinuxusermap-find --hbacrule=<foo> returns an
+        # exact match
+        dict(
+            desc='Try to delete similarly named HBAC rule %r' % hbacrule2,
+            command=('hbacrule_del', [hbacrule2], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=hbacrule2,
+                summary=u'Deleted HBAC rule "%s"' % hbacrule2,
+            )
+        ),
+
+
         # Test clean up
         dict(
             desc='Delete %r' % rule1,
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to