From 6c54e99b3d69e1c2e5e10521cdbb4f1abf25b22c Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Thu, 1 Jun 2017 13:12:53 +1000
Subject: [PATCH] ca-add: fix permission issue

The ca-add command pre_callback uses ldap.can_add() to check whether
the user has permission to add CAs.  Alas, the GetEffectiveRights
control used by ldap.can_add() doesn't correctly interpret ACIs with
'targetfilter' constraints, and returns a false-negative for
non-admin users, even when they have the 'System: Add CA'
permission.

To work around this, add the CA object to FreeIPA before attempting
to create the CA in Dogtag.  If the CA creation in Dogtag succeds,
the user then updates the FreeIPA object with the Authority ID and
other authoritative data returned by Dogtag.  If the CA creation in
Dogtag fails, the user cleans up by deleting the newly-created CA
object from FreeIPA.

This modified procedure ensures that the user certainly has the
'System: Add CA' permission before the CA creation in Dogtag is
attempted.  But it also means that the user must have 'write' and
'delete' permission on 'ipaca' objects in FreeIPA, so that it can
complete the object after CA creation in Dogtag, or clean up if that
step fails.  Therefore, update the 'System: Add CA' permission to
confer 'write' and 'delete' access on 'ipaca' objects, as well as
'add' access.

Fixes: https://pagure.io/freeipa/issue/6609
---
 ACI.txt                 |  2 +-
 ipaserver/plugins/ca.py | 46 ++++++++++++++++++++++++++++++----------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 185812a881..1e30dba9cc 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -23,7 +23,7 @@ aci: (targetattr = "automountmapname || description")(targetfilter = "(objectcla
 dn: cn=automount,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=automountmap)")(version 3.0;acl "permission:System: Remove Automount Maps";allow (delete) groupdn = "ldap:///cn=System: Remove Automount Maps,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=cas,cn=ca,dc=ipa,dc=example
-aci: (targetfilter = "(objectclass=ipaca)")(version 3.0;acl "permission:System: Add CA";allow (add) groupdn = "ldap:///cn=System: Add CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetfilter = "(objectclass=ipaca)")(version 3.0;acl "permission:System: Add CA";allow (add,delete,write) groupdn = "ldap:///cn=System: Add CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=cas,cn=ca,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=ipaca)")(version 3.0;acl "permission:System: Delete CA";allow (delete) groupdn = "ldap:///cn=System: Delete CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=cas,cn=ca,dc=ipa,dc=example
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 9bb163dffa..61b8ac5582 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -12,6 +12,7 @@
     LDAPUpdate, LDAPRetrieve, LDAPQuery, pkey_to_value)
 from ipaserver.plugins.cert import ca_enabled_check
 from ipalib import _, ngettext, x509
+from ipapython.dn import DN
 
 
 __doc__ = _("""
@@ -132,7 +133,7 @@ class ca(LDAPObject):
             },
         },
         'System: Add CA': {
-            'ipapermright': {'add'},
+            'ipapermright': {'add', 'delete', 'write'},
             'replaces': [
                 '(target = "ldap:///cn=*,cn=cas,cn=ca,$SUFFIX")(version 3.0;acl "permission:Add CA";allow (add) groupdn = "ldap:///cn=Add CA,cn=permissions,cn=pbac,$SUFFIX";)',
             ],
@@ -231,11 +232,6 @@ class ca_add(LDAPCreate):
     )
 
     def pre_callback(self, ldap, dn, entry, entry_attrs, *keys, **options):
-        ca_enabled_check(self.api)
-        if not ldap.can_add(dn[1:]):
-            raise errors.ACIError(
-                info=_("Insufficient 'add' privilege for entry '%s'.") % dn)
-
         # check for name collision before creating CA in Dogtag
         try:
             api.Object.ca.get_dn_if_exists(keys[-1])
@@ -250,19 +246,37 @@ def pre_callback(self, ldap, dn, entry, entry_attrs, *keys, **options):
                 "Subject DN is already used by CA '%s'"
                 ) % result['result'][0]['cn'][0])
 
-        # Create the CA in Dogtag.
-        with self.api.Backend.ra_lightweight_ca as ca_api:
-            resp = ca_api.create_ca(options['ipacasubjectdn'])
-        entry['ipacaid'] = [resp['id']]
-        entry['ipacaissuerdn'] = [resp['issuerDN']]
-
-        # In the event that the issued certificate's subject DN
-        # differs from what was requested, record the actual DN.
-        #
-        entry['ipacasubjectdn'] = [resp['dn']]
+        # Use dummy values for the unknown MUST attributes;
+        # we will update them later.
+        entry['ipacaid'] = 'DUMMY'
+        entry['ipacaissuerdn'] = 'cn=DUMMY'
         return dn
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        # We got this far, so the CA object in IPA has been created,
+        # which implies that user has 'Add CA' permission.  We can't
+        # use 'can_add' because GetEffectiveRights doesn't
+        # understand ACIs with 'targetfilter' and gives a false
+        # negative.
+
+        # Create the CA in Dogtag.
+        try:
+            with self.api.Backend.ra_lightweight_ca as ca_api:
+                resp = ca_api.create_ca(options['ipacasubjectdn'])
+        except Exception as e:
+            # something went wrong; clean up and re-throw
+            ldap.delete_entry(dn)
+            raise e
+
+        # update the entry with the new info
+        if entry_attrs['ipacaid'][0].lower() != resp['id'].lower():
+            entry_attrs['ipacaid'] = [resp['id']]
+        if DN(entry_attrs['ipacaissuerdn'][0]) != DN(resp['issuerDN']):
+            entry_attrs['ipacaissuerdn'] = [resp['issuerDN']]
+        if DN(entry_attrs['ipacasubjectdn'][0]) != DN(resp['dn']):
+            entry_attrs['ipacasubjectdn'] = [resp['dn']]
+        ldap.update_entry(entry_attrs)
+
         set_certificate_attrs(entry_attrs, options)
         return dn
 
