[Freeipa-devel] [PATCHES] 0521-0522 - Add managed read permissions to krbtpolicy & Allow anonymous read access to Kerberos realm container name
Martin Kosek
mkosek at redhat.com
Wed Apr 16 14:19:03 UTC 2014
On 04/16/2014 12:34 PM, Petr Viktorin wrote:
> On 04/16/2014 12:07 PM, Petr Viktorin wrote:
>> On 04/16/2014 07:48 AM, Martin Kosek wrote:
>>> On 04/15/2014 06:10 PM, Ludwig Krispenz wrote:
>>>>
>>>> On 04/15/2014 05:45 PM, Ludwig Krispenz wrote:
>>>>>
>>>>> On 04/15/2014 05:10 PM, Martin Kosek wrote:
>>>>>> On 04/15/2014 05:08 PM, Simo Sorce wrote:
>>>>>>> On Tue, 2014-04-15 at 16:48 +0200, Martin Kosek wrote:
>>>>>>>> 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
>>>>>>> It's like an LDAP filter but no quite so maybe:
>>>>>>> (&(target != "ldap:///some=dn")(target != "ldap:///other=dn")) ?
>>>>>>>
>>>>>>> HTH,
>>>>>>> Simo.
>>>>>>>
>>>>>> Try again...
>>>>>>
>>>>>> [15/Apr/2014:17:15:01 +0200] NSACLPlugin - ACL Syntax
>>>>>> Error(-5):(targetfilter=\22(objectclass=nsContainer)\22)(&(target !=
>>>>>> \22ldap:///cn=etc,dc=mkosek-fedora20,dc=test\22)(target !=
>>>>>> \22ldap:///cn=etc,dc=mkosek-fedora20,dc=test\22))(targetattr=\22objectclass
>>>>>>
>>>>>> ||
>>>>>> cn\22)(version 3.0; acl \22Anonymous read access to containers\22;
>>>>>> allow(read,
>>>>>> search, compare) userdn = \22ldap:///anyone\22;)
>>>>>>
>>>>>> Martin
>>>>> (target!="ldap:///some=dn || ldap:///other=dn") is accepted, but in a
>>>>> quick
>>>>> test also didn't work as expected. Need to investigate a bit more
>>>> looks like we do not handle multiple targets at all. Although in the
>>>> bind rules
>>>> logical operations of binddns and bindgroups are handled, so that
>>>> would be an
>>>> enhancement :-(
>>>>
>>>> so I think you either have to move to denies for the two subtrees and
>>>> a general
>>>> allow for the parent tree, which is not nice or use targetfilter if
>>>> possible.
>>>
>>> Ok, thanks for investigation. Petr, it seems we will simply need to
>>> update the container ACI and exclude krbPwdPolicy objectclass in the
>>> target filter.
>>>
>>> Martin
>>
>> Patch here.
>> It doesn't handle updating from unreleased code, so if you're upgrading
>> from recent master, remove the previous "Anonymous read access to
>> containers" ACI.
>
>
> Actually, we might want to delay pushing this patch; It'll need some more
> modifications to allow containers in cn=etc.
Ok. Given you plan to address the krbPwdPolicy separately, I see no problem
with accepting patches 521.2 and 522.2.
ACK, pushed both to master.
Martin
More information about the Freeipa-devel
mailing list