Skip to content

Commit

Permalink
ca-add: fix permission issue
Browse files Browse the repository at this point in the history
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.

The GetEffectiveRights problem is being tracked upstream as
https://pagure.io/389-ds-base/issue/49278.  When that ticket has
been fixed, this workaround can and should be reverted.

Fixes: https://pagure.io/freeipa/issue/6609
  • Loading branch information
frasertweedale committed Jun 2, 2017
1 parent 48b7e83 commit 3e13670
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 17 deletions.
2 changes: 1 addition & 1 deletion ACI.txt
Expand Up @@ -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
Expand Down
50 changes: 34 additions & 16 deletions ipaserver/plugins/ca.py
Expand Up @@ -15,6 +15,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__ = _("""
Expand Down Expand Up @@ -135,7 +136,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";)',
],
Expand Down Expand Up @@ -234,11 +235,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 that DN only includes standard naming attributes
dn_attrs = {
ava.attr.lower()
Expand Down Expand Up @@ -271,19 +267,41 @@ 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.
#
# When https://pagure.io/389-ds-base/issue/49278 has been
# fixed we can go back to using 'can_add' in the
# pre_callback.

# 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

Expand Down

0 comments on commit 3e13670

Please sign in to comment.