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.

--
PetrĀ³

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

Reply via email to