Rob Crittenden wrote:
Martin Kosek wrote:
On Thu, 2011-10-06 at 14:05 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Wed, 2011-10-05 at 17:18 -0400, Rob Crittenden wrote:
The aci prefix was missing in the description of the three dns acis
which made them not show up when viewing their permission entries.

rob

This works fine, but it is just a part of a solution. DNS related
privileges miss memberof attribute for the DNS permissions and thus the
permissions are not listed:

# ipa permission-show "add dns entries"
Permission name: add dns entries
Permissions: add
Type: dnsrecord
Granted to Privilege: DNS Administrators, DNS Servers

# ipa privilege-show "DNS Administrators"
Privilege name: DNS Administrators
Description: DNS Administrators
<<< Missing permissions

I think the reason is that the permissions are in a wrong order in the
LDIF and are created before the privilege itself. When member links are
being created for DNS permissions, the memberof plugin cannot add
memberof attributes for the privilege since it does not exist yet. This
is the main issue that the BZ bug complains about.

Martin


There are two problems:

1. The acis lacked a prefix so they didn't appear as permissions

2. The permission was added before the privilege so the memberof values
weren't being calculated.

This fixes it for new installs and adds an update to fix up existing
installs.

rob

It works fine when doing upgrade. However, when running a clean install,
I get these errors:

# ipa-server-install --setup-dns
...
[9/13]: publish CA cert
[10/13]: creating a keytab for httpd
[11/13]: configuring SELinux for httpd
[12/13]: restarting httpd
[13/13]: configuring httpd to start on boot
done configuring httpd.
Applying LDAP updates
root : ERROR Add failure Object class violation: missing required
attribute "objectclass"
root : ERROR Add failure Object class violation: missing required
attribute "objectclass"
root : ERROR Add failure Object class violation: missing required
attribute "objectclass"
Restarting IPA to initialize updates before performing deletes:
[1/2]: stopping directory server
[2/2]: starting directory server
done configuring dirsrv.
Restarting the directory server
Restarting the KDC
Restarting the web server
Configuring named:
[1/9]: adding DNS container
[2/9]: setting up our zone
[3/9]: setting up reverse zone
[4/9]: setting up our own record
[5/9]: setting up kerberos principal
[6/9]: setting up named.conf
[7/9]: restarting named
[8/9]: configuring named to start on boot
[9/9]: changing resolv.conf to point to ourselves
done configuring named.
==============================================================================

Setup complete

Do you hit this too? Permissions and privileges member attributes were
OK though.

Martin


Bah, ok. We only create these permissions when dns is installed so I'll
need to find some way to optionally add this.

rob

I needed to add a new type to the updater to only add new values if the entry exists.

rob
From 5b42fd757840c088323f589cfb26b4607b892958 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Wed, 5 Oct 2011 17:16:05 -0400
Subject: [PATCH] Fix DNS permissions and membership in privileges

This resolves two issues:

1. The DNS acis lacked a prefix so weren't tied to permissions
2. The permissions were added before the privileges so the member
   values weren't calculated properly

For updates we need to add in the members and recalculate memberof via
a DS task.

https://fedorahosted.org/freeipa/ticket/1898
---
 install/share/dns.ldif               |   46 +++++++++++++++++-----------------
 install/tools/man/ipa-ldap-updater.1 |    1 +
 install/updates/40-delegation.update |    6 ++++
 install/updates/40-dns.update        |   22 ++++++++++++++++
 install/updates/Makefile.am          |    1 +
 ipaserver/install/ldapupdate.py      |   16 ++++++++++-
 6 files changed, 67 insertions(+), 25 deletions(-)
 create mode 100644 install/updates/40-dns.update

diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index dc79222..1ffadb5 100644
--- a/install/share/dns.ldif
+++ b/install/share/dns.ldif
@@ -4,6 +4,29 @@ objectClass: nsContainer
 objectClass: top
 cn: dns
 
+dn: $SUFFIX
+changetype: modify
+add: aci
+aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:add dns entries";allow (add) groupdn = "ldap:///cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX";)
+aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:remove dns entries";allow (delete) groupdn = "ldap:///cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX";)
+aci: (targetattr = "idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || aaaarecord || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:update dns entries";allow (write) groupdn = "ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX";)
+
+dn: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
+changetype: add
+objectClass: top
+objectClass: groupofnames
+objectClass: nestedgroup
+cn: DNS Administrators
+description: DNS Administrators
+
+dn: cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX
+changetype: add
+objectClass: top
+objectClass: groupofnames
+objectClass: nestedgroup
+cn: DNS Servers
+description: DNS Servers
+
 dn: cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX
 changetype: add
 objectClass: groupofnames
@@ -30,26 +53,3 @@ cn: update dns entries
 description: Update DNS entries
 member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
 member: cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX
-
-dn: $SUFFIX
-changetype: modify
-add: aci
-aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Add DNS entries";allow (add) groupdn = "ldap:///cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX";)
-aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Remove DNS entries";allow (delete) groupdn = "ldap:///cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX";)
-aci: (targetattr = "idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || aaaarecord || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Update DNS entries";allow (write) groupdn = "ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX";)
-
-dn: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
-changetype: add
-objectClass: top
-objectClass: groupofnames
-objectClass: nestedgroup
-cn: DNS Administrators
-description: DNS Administrators
-
-dn: cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX
-changetype: add
-objectClass: top
-objectClass: groupofnames
-objectClass: nestedgroup
-cn: DNS Servers
-description: DNS Servers
diff --git a/install/tools/man/ipa-ldap-updater.1 b/install/tools/man/ipa-ldap-updater.1
index ed140b3..d896a1b 100644
--- a/install/tools/man/ipa-ldap-updater.1
+++ b/install/tools/man/ipa-ldap-updater.1
@@ -40,6 +40,7 @@ There are 7 keywords:
     * deleteentry: remove the entry
     * replace: replace an existing value, format is old: new
     * addifnew: add a new attribute and value only if the attribute doesn't already exist. Only works with single\-value attributes.
+    * addifexist: add a new attribute and value only if the entry exists. This is used to update optional entries.
 
 Values is a comma\-separated field so multi\-values may be added at one time. Double or single quotes may be put around individual values that contain embedded commas.
 
diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update
index 66c62ed..a235211 100644
--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -262,3 +262,9 @@ add:member: 'cn=admins,cn=groups,cn=accounts,$SUFFIX'
 # Don't allow admins to update enrolledBy
 dn: $SUFFIX
 replace:aci:'(targetattr = "enrolledby || objectclass")(target = "ldap:///fqdn=*,cn=computers,cn=accounts,$SUFFIX";)(version 3.0;acl "permission:Enroll a host";allow (write) groupdn = "ldap:///cn=Enroll a host,cn=permissions,cn=pbac,$SUFFIX";)::(targetattr = "objectclass")(target = "ldap:///fqdn=*,cn=computers,cn=accounts,$SUFFIX";)(version 3.0;acl "permission:Enroll a host";allow (write) groupdn = "ldap:///cn=Enroll a host,cn=permissions,cn=pbac,$SUFFIX";)'
+
+# The original DNS permissions lacked the tag.
+dn: $SUFFIX
+replace:aci:'(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Add DNS entries";allow (add) groupdn = "ldap:///cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX";)::(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:add dns entries";allow (add) groupdn = "ldap:///cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX";)'
+replace:aci:'(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Remove DNS entries";allow (delete) groupdn = "ldap:///cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX";)::(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:remove dns entries";allow (delete) groupdn = "ldap:///cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX";)'
+replace:aci:'(targetattr = "idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || aaaarecord || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "Update DNS entries";allow (write) groupdn = "ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX";)::(targetattr = "idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || aaaarecord || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX";)(version 3.0;acl "permission:update dns entries";allow (write) groupdn = "ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX";)'
diff --git a/install/updates/40-dns.update b/install/updates/40-dns.update
new file mode 100644
index 0000000..c5915ce
--- /dev/null
+++ b/install/updates/40-dns.update
@@ -0,0 +1,22 @@
+# Add missing member values to attach permissions to their respective
+# privileges and run a memberOf task.
+dn: cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX
+add:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
+add:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
+
+dn: cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX
+add:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
+add:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
+
+dn: cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX
+add:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
+add:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
+
+dn: cn=Update PBAC memberOf $TIME, cn=memberof task, cn=tasks, cn=config
+add: objectClass: top
+add: objectClass: extensibleObject
+add: cn: IPA PBAC memberOf $TIME
+add: basedn: 'cn=privileges,cn=pbac,$SUFFIX'
+add: filter: (objectclass=*)
+add: ttl: 10
+
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index bf4d9af..99b7c56 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -19,6 +19,7 @@ app_DATA =				\
 	20-winsync_index.update		\
 	21-replicas_container.update	\
 	40-delegation.update		\
+	40-dns.update			\
 	45-roles.update			\
 	50-lockout-policy.update	\
 	50-groupuuid.update		\
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index f2f416b..140a9cd 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -267,7 +267,7 @@ class LDAPUpdate:
     def parse_update_file(self, data, all_updates, dn_list):
         """Parse the update file into a dictonary of lists and apply the update
            for each DN in the file."""
-        valid_keywords = ["default", "add", "remove", "only", "deleteentry", "replace", "addifnew"]
+        valid_keywords = ["default", "add", "remove", "only", "deleteentry", "replace", "addifnew", "addifexist"]
         update = {}
         d = ""
         index = ""
@@ -533,6 +533,14 @@ class LDAPUpdate:
                         e.append(v)
                         logging.debug('addifnew: set %s to %s', k, e)
                         entry.setValues(k, e)
+                elif utype == 'addifexist':
+                    logging.debug("addifexist: '%s' to %s, current value %s", v, k, e)
+                    # Only add the attribute if the entry doesn't exist. We
+                    # determine this based on whether it has an objectclass
+                    if entry.getValues('objectclass'):
+                        e.append(v)
+                        logging.debug('addifexist: set %s to %s', k, e)
+                        entry.setValues(k, e)
                 elif utype == 'only':
                     logging.debug("only: set %s to '%s', current value %s", k, v, e)
                     if only.get(k):
@@ -645,7 +653,11 @@ class LDAPUpdate:
             # entry.orig_data = {}
             try:
                 if self.live_run:
-                    self.conn.addEntry(entry.dn, entry.toTupleList())
+                    if len(entry.toTupleList()) > 0:
+                        # addifexist may result in an entry with only a 
+                        # dn defined. In that case there is nothing to do.
+                        # It means the entry doesn't exist, so skip it.
+                        self.conn.addEntry(entry.dn, entry.toTupleList())
                 self.modified = True
             except Exception, e:
                 logging.error("Add failure %s", e)
-- 
1.7.6

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

Reply via email to