On 04/15/2014 09:43 AM, Martin Kosek wrote:
On 04/15/2014 09:38 AM, Martin Kosek wrote:
On 04/14/2014 07:18 PM, Simo Sorce wrote:
On Mon, 2014-04-14 at 18:54 +0200, Petr Viktorin wrote:
Hello,

The first patch adds default read permissions to krbtpolicy. Since the
plugin manages entries in two trees, there are two permissions. Since
two permissions are needed to cover krbtpolicy, it can't be used as a
permission's --type.
The permissions are added to a new privilege, 'Kerberos Ticket Policy
Readers'.

The second patch adds an ACI for reading the Kerberos realm name. Since
client enrollment won't work without this, I don't see a reason for
having it managed by a permission.


LGTM

Simo.


521 breaks a unit test:

======================================================================
FAIL: test_permission[37]: permission_find: Search for u'Testperm_RN' using
--subtree
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
     self.test(*self.arg)
   File "/root/freeipa-master/ipatests/test_xmlrpc/xmlrpc_test.py", line 301, in
<lambda>
     func = lambda: self.check(nice, **test)
   File "/root/freeipa-master/ipatests/test_xmlrpc/xmlrpc_test.py", line 319, in
check
     self.check_output(nice, cmd, args, options, expected, extra_check)
   File "/root/freeipa-master/ipatests/test_xmlrpc/xmlrpc_test.py", line 359, in
check_output
     assert_deepequal(expected, got, nice)
   File "/root/freeipa-master/ipatests/util.py", line 344, in assert_deepequal
     assert_deepequal(e_sub, g_sub, doc, stack + (key,))
   File "/root/freeipa-master/ipatests/util.py", line 352, in assert_deepequal
     VALUE % (doc, expected, got, stack)
AssertionError: assert_deepequal: expected != got.
   test_permission[37]: permission_find: Search for u'Testperm_RN' using 
--subtree
   expected = 1
   got = 2
   path = ('count',)

Thanks for the catch, tests updated.

Otherwise it works fine (krbtpolicy-show for user cannot be tested yet as we
miss permissions for users).

Right; I don't think this permission by itself should allow access to users. Correct me if that's wrong.

I created a users permission for testing:
ipa permission-add 'allow reading user objectclass' --type user --right={read,search,compare} --attrs objectclass --bind all

/me hit Send too soon.

Although 522 works functionally and client now discovers the IPA server, there
is no path from SUFFIX to cn=REALM for anonymous users.

I would personally change the ACI to

(targetattr = "cn || objectclass")(targetfilter =
"(|(objectclass=krbrealmcontainer)(objectclass=krbcontainer))")(version 3.0;acl
"Anonymous read access to Kerberos container";allow (read,compare,search)
userdn = "ldap:///anyone";;)'

and put it to cn=kerberos,$SUFFIX (which is of krbcontainer objectclass).

Right, that's necessary for UIs to list the container.
Simo, are you okay with this?


--
PetrĀ³
From d7dff344118643a891e1e073978a4fd517feba72 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 26 Mar 2014 17:11:23 +0100
Subject: [PATCH] Add managed read permissions to krbtpolicy

Unlike other objects, the ticket policy is stored in different
subtrees: global policy in cn=kerberos and per-user policy in
cn=users,cn=accounts.
Add two permissions, one for each location.

Also, modify tests so that adding new permissions in cn=users
doesn't cause failures.

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
---
 install/updates/40-delegation.update           |  7 +++++
 ipalib/plugins/krbtpolicy.py                   | 38 ++++++++++++++++++++++++-
 ipatests/test_xmlrpc/test_permission_plugin.py | 39 ++++++++++++++++++++++++--
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update
index 27e605789ba152ac61796217ca12a603958931c1..6ab849bf86d129ef93472e970705b117147f0818 100644
--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -408,3 +408,10 @@ dn: cn=Password Policy Readers,cn=privileges,cn=pbac,$SUFFIX
 default:objectClass: top
 default:cn: Password Policy Readers
 default:description: Read password policies
+
+dn: cn=Kerberos Ticket Policy Readers,cn=privileges,cn=pbac,$SUFFIX
+default:objectClass: nestedgroup
+default:objectClass: groupofnames
+default:objectClass: top
+default:cn: Kerberos Ticket Policy Readers
+default:description: Read global and per-user Kerberos ticket policy
diff --git a/ipalib/plugins/krbtpolicy.py b/ipalib/plugins/krbtpolicy.py
index a05583dfb4d3067a1a7f1e097859eac26c3be2ae..4ae676dc5b7ece54c57c9d99afea92ca397b36be 100644
--- a/ipalib/plugins/krbtpolicy.py
+++ b/ipalib/plugins/krbtpolicy.py
@@ -75,8 +75,44 @@ class krbtpolicy(LDAPObject):
     object_name = _('kerberos ticket policy settings')
     default_attributes = ['krbmaxticketlife', 'krbmaxrenewableage']
     limit_object_classes = ['krbticketpolicyaux']
+    # permission_filter_objectclasses is deliberately missing,
+    # so it is not possible to create a permission of `--type krbtpolicy`.
+    # This is because we need two permissions to cover both global and per-user
+    # policies.
+    managed_permissions = {
+        'System: Read Default Kerberos Ticket Policy': {
+            'non_object': True,
+            'replaces_global_anonymous_aci': True,
+            'ipapermtargetfilter': ['(objectclass=krbticketpolicyaux)'],
+            'ipapermlocation': DN(container_dn, api.env.basedn),
+            'ipapermbindruletype': 'permission',
+            'ipapermright': {'read', 'search', 'compare'},
+            'ipapermdefaultattr': {
+                'krbdefaultencsalttypes', 'krbmaxrenewableage',
+                'krbmaxticketlife', 'krbsupportedencsalttypes',
+                'objectclass',
+            },
+            'default_privileges': {
+                'Kerberos Ticket Policy Readers',
+            },
+        },
+        'System: Read User Kerberos Ticket Policy': {
+            'non_object': True,
+            'replaces_global_anonymous_aci': True,
+            'ipapermlocation': DN(api.env.container_user, api.env.basedn),
+            'ipapermtargetfilter': ['(objectclass=krbticketpolicyaux)'],
+            'ipapermbindruletype': 'permission',
+            'ipapermright': {'read', 'search', 'compare'},
+            'ipapermdefaultattr': {
+                'krbmaxrenewableage', 'krbmaxticketlife',
+            },
+            'default_privileges': {
+                'Kerberos Ticket Policy Readers',
+            },
+        },
+    }
 
-    label=_('Kerberos Ticket Policy')
+    label = _('Kerberos Ticket Policy')
     label_singular = _('Kerberos Ticket Policy')
 
     takes_params = (
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index d593dd98670334d2ef696f4b632d00943c9862d6..54e8d57dd9dc4c31863aaf7416e9d7f56e749c79 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -100,6 +100,7 @@
 groups_dn = DN(api.env.container_group, api.env.basedn)
 etc_dn = DN('cn=etc', api.env.basedn)
 nonexistent_dn = DN('cn=does not exist', api.env.basedn)
+admin_dn = DN('uid=admin', users_dn)
 
 
 def verify_permission_aci(name, dn, acistring):
@@ -1117,9 +1118,42 @@ class test_permission(Declarative):
         ),
 
         dict(
+            desc='Change subtree of %r to admin' % permission1_renamed_ucase,
+            command=(
+                'permission_mod', [permission1_renamed_ucase],
+                dict(ipapermlocation=admin_dn)
+            ),
+            expected=dict(
+                value=permission1_renamed_ucase,
+                summary=u'Modified permission "%s"' % permission1_renamed_ucase,
+                result=dict(
+                    dn=permission1_renamed_ucase_dn,
+                    cn=[permission1_renamed_ucase],
+                    objectclass=objectclasses.permission,
+                    member_privilege=[privilege1],
+                    ipapermlocation=[admin_dn],
+                    ipapermright=[u'write'],
+                    memberof=[u'ipausers'],
+                    attrs=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                ),
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1_renamed_ucase, admin_dn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=ipausers', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1_renamed_ucase +
+            'allow (write) groupdn = "ldap:///%s";;)' %
+                permission1_renamed_ucase_dn,
+        ),
+
+        dict(
             desc='Search for %r using --subtree' % permission1_renamed_ucase,
             command=('permission_find', [],
-                     {'ipapermlocation': u'ldap:///%s' % users_dn}),
+                     {'ipapermlocation': u'ldap:///%s' % admin_dn}),
             expected=dict(
                 count=1,
                 truncated=False,
@@ -1130,13 +1164,12 @@ class test_permission(Declarative):
                         'cn':[permission1_renamed_ucase],
                         'objectclass': objectclasses.permission,
                         'member_privilege':[privilege1],
-                        'ipapermlocation': [users_dn],
+                        'ipapermlocation': [admin_dn],
                         'ipapermright':[u'write'],
                         'memberof':[u'ipausers'],
                         'attrs': [u'sn'],
                         'ipapermbindruletype': [u'permission'],
                         'ipapermissiontype': [u'SYSTEM', u'V2'],
-                        'ipapermlocation': [users_dn],
                     },
                 ],
             ),
-- 
1.9.0

From f19cabc2d26b04757e84f06390e34b04eb441a46 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 26 Mar 2014 17:11:23 +0100
Subject: [PATCH] Allow anonymous read access to Kerberos containers

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
---
 install/updates/20-aci.update | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index e9e1fe9db4d9c594ae0485c6f7cec8a668a8ff92..9d8a6392f2ffbfb52dd6725ed4008b29bc164b03 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -24,3 +24,7 @@ dn: $SUFFIX
 # Read access to containers
 dn: $SUFFIX
 add:aci:'(targetfilter="(objectclass=nsContainer)")(target!="ldap:///cn=etc,$SUFFIX";)(targetattr="objectclass || cn")(version 3.0; acl "Anonymous read access to containers"; allow(read, search, compare) userdn = "ldap:///anyone";;)'
+
+# Read access to Kerberos container (cn=kerberos) and realm containers (cn=$REALM,cn=kerberos)
+dn: cn=kerberos,$SUFFIX
+add:aci:'(targetattr = "cn || objectclass")(targetfilter = "(|(objectclass=krbrealmcontainer)(objectclass=krbcontainer))")(version 3.0;acl "Anonymous read access to Kerberos containers";allow (read,compare,search) userdn = "ldap:///anyone";;)'
-- 
1.9.0

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

Reply via email to