On 04/25/2014 01:08 PM, Martin Kosek wrote:
On 04/25/2014 01:01 PM, Petr Viktorin wrote:
On 04/24/2014 05:15 PM, Simo Sorce wrote:
On Thu, 2014-04-24 at 16:47 +0200, Martin Kosek wrote:
On 04/24/2014 03:42 PM, Simo Sorce wrote:
On Thu, 2014-04-24 at 15:18 +0200, Martin Kosek wrote:
On 04/24/2014 02:28 PM, Simo Sorce wrote:
On Thu, 2014-04-24 at 14:17 +0200, Martin Kosek wrote:
On 04/24/2014 09:41 AM, Petr Viktorin wrote:
On 04/23/2014 08:56 PM, Simo Sorce wrote:
On Wed, 2014-04-23 at 20:37 +0200, Petr Viktorin wrote:
Admin access to read-only attributes such as ipaUniqueId, memberOf,
krbPrincipalName is provided by the anonymous read ACI, which will go
away. This patch adds a blanket read ACI for these.
I also moved some related ACIs to 20-aci.update.

Previously krbPwdHistory was also readable by admins. I don't think we
want to include that.
Simo, should admins be allowed to read krbExtraData?

Probably not necessary but there is nothing secret in it either.

Simo.

OK. I'm not a fan of hiding things from the admin, so no changes to the
patch
are necessary here.


536:
As we are touching these ACIs, may it is a time to see the blacklist of
attributes that admin cannot write and check if this is still wanted:

ipaUniqueId - OK, generated by DS plugin
memberOf - OK, generated by DS plugin
serverHostName - I did not even find a place where we manipulate it, except
host.py -> remove from blacklist?

What about this one?

not sure, I did not work much on the hosts objects.

Rob? Do you know?

enrolledBy - OK, generated by DS plugin
krbExtraData - OK, generated by DS plugin
krbPrincipalName - why can't admin change it? It is filled by framework, I
would not personally blacklist it

It is changed by the ipa rename plugin when the user uid change, that's
why we prevent the admin from explicitly change it.

krbCanonicalName - same as krbPrincipalName

Ok, in that case krbPrincipalName and krbCanonicalName should stay, right?

They should be accessible by admin, yes.

Note that we are now discussing write access. I.e. what attributes needs to
stay in the admin's global write ACI blacklist and which can be removed ->
admin can write in them.

IIUC, these should be only readable by admin, not writeable.


krbPrincipalAliases - same as krbPrincipalName - we need this removed if we
want to set aliases anyway

Allow this one?

This is one I wanted to eventually get rid of, Alexander seem to have
introduced it, but we discussed in the past of a way to kill it.
I forgot the details thouhg.
Alexander did we open a ticket for this ?

Ok, we may eventually get rid of it, but does it need to be blacklisted in
admins global write ACI?


krbPasswordExpiration - OK, generated by DS plugin
krbLastPwdChange - OK, generated by DS plugin
krbUPEnabled - not used, can we remove it?

What about this one?

We never use it.

I read this as "remove it from global admin write ACI blacklist".


krbTicketPolicyReference - why cannot admin set it?

Unclear why, probably should be able to.

We may want to remove it from blacklist then.

We never used it, but we should probably allow admin to modify

Ok, let us remove it from global admin write ACI blacklist.


krbPwdPolicyReference - why cannot admin set it?

We assign password policy based on groups now, right ?

Yup.

I see no complains, i.e. I read it as "remove it from global admin write ACI
blacklist".



krbPrincipalType - why cannot admin set it?

Unused.

So... allow this one?

we never use it so we do not need to allow admins by default.

My point was to limit admin write ACI blacklist to the bare minimum. Only to
attributes that really needs to be blacklisted as they are operated by DS
plugins - like memberOf and others.

My proposal is to remove it from global admin write ACI blacklist given it is
not used.

Ack, I agree with your intent.

Simo.


If I understand correctly, we want to allow:
- krbPrincipalAliases
- krbPrincipalType
- krbPwdPolicyReference
- krbTicketPolicyReference
- krbUPEnabled
- serverHostName

Here is the patch for that.

The list looks good to me.

Martin, just to make sure: 0536-0537 are good to push, right?

One of the reasons why I wanted to prune the blacklist a bit was to make the
Admin read ACI (Admin read-only attributes) simpler (read - shorter). So please
also update this aci in 536.

Ah, right.

IMO, 536 and 538 can be squashed, but that's up to you.

I tried, but I couldn't get the commit message to not read like two unrelated changes, which to me is a clear sign they shouldn't be squashed. (I thought about also split moving the ACIs and the modifications, but maybe that'd be too purist.)

I renumbered 0536 to 0538 to keep the order they should be applied in. Entire patchset attached.

--
PetrĀ³

From 92a510d599760d2d3d3e97905eb41050bc6f276f Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 23 Apr 2014 19:09:31 +0200
Subject: [PATCH] test_ldap: Read a publicly accessible attribute when testing
 anonymous bind

The usercertificate attribute is slated to not be readable for
anonymous users. Use associateddomain in $SUFFIX instead.
---
 ipatests/test_ipaserver/test_ldap.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_ipaserver/test_ldap.py b/ipatests/test_ipaserver/test_ldap.py
index 254461c80998bdd0423f0758963c9d1bf896351b..7168a69b0cba038a7ebd805dc3637cce5b587117 100644
--- a/ipatests/test_ipaserver/test_ldap.py
+++ b/ipatests/test_ipaserver/test_ldap.py
@@ -61,11 +61,10 @@ def test_anonymous(self):
         """
         self.conn = ldap2(shared_instance=False, ldap_uri=self.ldapuri)
         self.conn.connect()
-        entry_attrs = self.conn.get_entry(self.dn, ['usercertificate'])
-        cert = entry_attrs.get('usercertificate')
-        cert = cert[0]
-        serial = unicode(x509.get_serial_number(cert, x509.DER))
-        assert serial is not None
+        dn = api.env.basedn
+        entry_attrs = self.conn.get_entry(dn, ['associateddomain'])
+        domain = entry_attrs.single_value['associateddomain']
+        assert domain == api.env.domain
 
     def test_GSSAPI(self):
         """
-- 
1.9.0

From 517ec61710fe4b3320f00b794d4eb94d17d8a6ec Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 26 Mar 2014 17:11:23 +0100
Subject: [PATCH] aci-update: Trim the admin write blacklist

These attributes are removed from the blacklist, which means
high-level admins can now modify them:

- krbPrincipalAliases
- krbPrincipalType
- krbPwdPolicyReference
- krbTicketPolicyReference
- krbUPEnabled
- serverHostName

The intention is to only blacklist password attributes and attributes
that are managed by DS plugins.

Also, move the admin ACIs from ldif and trusts.update to aci.update.
---
 install/share/default-aci.ldif   |  3 ---
 install/updates/20-aci.update    | 13 +++++++++++++
 install/updates/60-trusts.update |  6 ------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/install/share/default-aci.ldif b/install/share/default-aci.ldif
index 490a69de0933f02c9268a8fbd0ed8596cecf801c..480facf3294c593c6a2bcf326e20c32157d6d3c6 100644
--- a/install/share/default-aci.ldif
+++ b/install/share/default-aci.ldif
@@ -5,10 +5,7 @@ dn: $SUFFIX
 add: aci
 aci: (targetfilter = "(&(!(objectClass=ipaToken))(!(objectClass=ipatokenTOTP))(!(objectClass=ipatokenHOTP))(!(objectClass=ipatokenRadiusConfiguration)))")(target != "ldap:///idnsname=*,cn=dns,$SUFFIX";)(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || userPKCS12 || ipaNTHash || ipaNTTrustAuthOutgoing || ipaNTTrustAuthIncoming")(version 3.0; acl "Enable Anonymous access"; allow (read, search, compare) userdn = "ldap:///anyone";;)
 aci: (targetattr = "memberOf || memberHost || memberUser")(version 3.0; acl "No anonymous access to member information"; deny (read,search,compare) userdn != "ldap:///all";;)
-aci: (targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)
 aci: (targetattr = "userpassword || krbprincipalkey || sambalmpassword || sambantpassword")(version 3.0; acl "selfservice:Self can write own password"; allow (write) userdn="ldap:///self";;)
-aci: (targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || ipaNTHash")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)
-aci: (targetfilter = "(objectClass=krbPwdPolicy)")(targetattr = "krbMaxPwdLife || krbMinPwdLife || krbPwdMinDiffChars || krbPwdMinLength || krbPwdHistoryLength")(version 3.0;acl "Admins can write password policies"; allow (read, search, compare, write) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)
 aci: (targetattr = "*")(target = "ldap:///cn=*,ou=SUDOers,$SUFFIX";)(version 3.0; acl "No anonymous access to sudo"; deny (read,search,compare) userdn != "ldap:///all";;)
 
 dn: $SUFFIX
diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index d3a9db2ae1e81ee9a0e8fb73102457bc2ec1826f..2db3bead207eab1f7458e430b5510b66c46bbf4c 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -31,3 +31,16 @@ dn: cn=replicas,cn=ipa,cn=etc,$SUFFIX
 # 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";;)'
+
+# Access for high-level admins
+dn: $SUFFIX
+# Read/write
+remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || krbTicketFlags || ipaUniqueId || memberOf || serverHostName || enrolledBy")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || krbTicketFlags || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+add:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbPasswordExpiration || krbPwdHistory || krbLastPwdChange || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+# Write-only
+remove:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+add:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || ipaNTHash")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+add:aci:'(targetfilter = "(objectClass=krbPwdPolicy)")(targetattr = "krbMaxPwdLife || krbMinPwdLife || krbPwdMinDiffChars || krbPwdMinLength || krbPwdHistoryLength")(version 3.0;acl "Admins can write password policies"; allow (read, search, compare, write) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
diff --git a/install/updates/60-trusts.update b/install/updates/60-trusts.update
index 1fb3cc47c88189df4c45852aba67915c023f676d..77c2104ffa62462634438f7b729cdfd71cd27eb3 100644
--- a/install/updates/60-trusts.update
+++ b/install/updates/60-trusts.update
@@ -35,12 +35,6 @@ dn: $SUFFIX
 add:aci: '(targetattr = "ipaNTHash")(version 3.0; acl "Samba system principals can read and write NT passwords"; allow (read,write) groupdn="ldap:///cn=adtrust agents,cn=sysaccounts,cn=etc,$SUFFIX";)'
 remove:aci: '(targetattr = "ipaNTHash")(version 3.0; acl "Samba system principals can read NT passwords"; allow (read) groupdn="ldap:///cn=adtrust agents,cn=sysaccounts,cn=etc,$SUFFIX";)'
 replace:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || userPKCS12")(version 3.0; acl "Enable Anonymous access"; allow (read, search, compare) userdn = "ldap:///anyone";;)::(target != "ldap:///idnsname=*,cn=dns,$SUFFIX";)(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || userPKCS12 || ipaNTHash")(version 3.0; acl "Enable Anonymous access"; allow (read, search, compare) userdn = "ldap:///anyone";;)'
-remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || krbTicketFlags || ipaUniqueId || memberOf || serverHostName || enrolledBy")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
-remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || krbTicketFlags || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
-remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPrincipalExpiration || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
-remove:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'   
-add:aci:'(targetattr != "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || krbPrincipalName || krbCanonicalName || krbUPEnabled || krbTicketPolicyReference || krbPasswordExpiration || krbPwdPolicyReference || krbPrincipalType || krbPwdHistory || krbLastPwdChange || krbPrincipalAliases || krbExtraData || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount || ipaUniqueId || memberOf || serverHostName || enrolledBy || ipaNTHash")(version 3.0; acl "Admin can manage any entry"; allow (all) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
-replace:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)::(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || ipaNTHash")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
 
 # Add the default PAC type to configuration
 dn: cn=ipaConfig,cn=etc,$SUFFIX
-- 
1.9.0

From da02ad018258f7b45495599c9d8eb04aea6b0d6a Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 25 Apr 2014 13:14:47 +0200
Subject: [PATCH] aci-update: Add ACI for read-only admin attributes

Most admin access is granted with the "Admin can manage any entry" ACI,
but before the global anonymous read ACI is removed, read-only admin
access must be explicitly given.
Add an ACI for read-only attributes.

https://fedorahosted.org/freeipa/ticket/4319
---
 install/updates/20-aci.update | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index 2db3bead207eab1f7458e430b5510b66c46bbf4c..d9dcad2e572ab72ff793c41a4300562caead6c77 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -44,3 +44,5 @@ dn: $SUFFIX
 remove:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
 add:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || ipaNTHash")(version 3.0; acl "Admins can write passwords"; allow (add,delete,write) groupdn="ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
 add:aci:'(targetfilter = "(objectClass=krbPwdPolicy)")(targetattr = "krbMaxPwdLife || krbMinPwdLife || krbPwdMinDiffChars || krbPwdMinLength || krbPwdHistoryLength")(version 3.0;acl "Admins can write password policies"; allow (read, search, compare, write) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
+# Read-only
+add:aci:'(targetattr="ipaUniqueId || memberOf || enrolledBy || krbExtraData || krbPrincipalName || krbCanonicalName || krbPasswordExpiration || krbLastPwdChange || krbLastSuccessfulAuth || krbLastFailedAuth || krbLoginFailedCount")(version 3.0; acl "Admin read-only attributes"; allow (read, search, compare) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX";;)'
-- 
1.9.0

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

Reply via email to