[Freeipa-devel] [PATCHES] 0521-0522 - Add managed read permissions to krbtpolicy & Allow anonymous read access to Kerberos realm container name

[Martin Kosek]

On 04/15/2014 03:16 PM, Simo Sorce wrote:
> On Tue, 2014-04-15 at 13:13 +0200, Petr Viktorin wrote:
>> 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?
> 
> It is no secret that an IPA server has a container named after the
> domain. And the REALM name is available unauthenticated from DNS, so
> knowledge of it's existence is given.
> Therefore I see no problem if anonymous can see the container exists, as
> long as no contents (beyond what we already determined need to be) are
> revealed I see no problem.
> 
> Simo.
> 
> 

Patches work fine. The only problem I see that we now expose group password
policies

# ldapsearch -h `hostname` -x -b
'cn=MKOSEK-FEDORA20.TEST,cn=kerberos,dc=mkosek-fedora20,dc=test' cn
...
# MKOSEK-FEDORA20.TEST, kerberos, mkosek-fedora20.test
dn: cn=MKOSEK-FEDORA20.TEST,cn=kerberos,dc=mkosek-fedora20,dc=test
cn: MKOSEK-FEDORA20.TEST

# global_policy, MKOSEK-FEDORA20.TEST, kerberos, mkosek-fedora20.test
dn: cn=global_policy,cn=MKOSEK-FEDORA20.TEST,cn=kerberos,dc=mkosek-fedora20,dc
 =test
cn: global_policy

# ipausers, MKOSEK-FEDORA20.TEST, kerberos, mkosek-fedora20.test
dn: cn=ipausers,cn=MKOSEK-FEDORA20.TEST,cn=kerberos,dc=mkosek-fedora20,dc=test
cn: ipausers
...

It seems suboptimal as we now moved access to group password policy to special
permission and we probably do not want to leak a list of defined group policies.

Question is how to prevent it as the group password policies have nsContainer
OC. I see 2 options:

1) Update pwpolicy plugin to avoid using nsContainer for group password policy
(I do not think it is needed, krbPwdPolicy contains cn attribute

2) Update our container allowing ACI to not display it.

Option 1) would be nice, but I am afraid it would break search in older
versions which expects the nsContainer OC to be there.

As for option 2) I am not sure how best to do it. It would be best to exclude
both cn=etc and cn=kerberos subtrees, but I could not figure out an ACI syntax
to do it.

Both
(target != "ldap:///some=dn")(target != "ldap:///other=dn")
and
(target != "ldap:///some=dn" && target != "ldap:///other=dn")
are not correct. CCing Ludwig to advise.

Alternative is to exclude the the password policies by targetfilter, but I
think excluding whole tree is better.

Martin