Updated patch attached.  Notably restores/adds revocation behaviour
to host-mod and service-mod.

Thanks,
Fraser

On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:
> On 27/05/15 15:53, Fraser Tweedale wrote:
> >This patch adds supports for multiple user / host certificates.  No
> >schema change is needed ('usercertificate' attribute is already
> >multi-value).  The revoke-previous-cert behaviour of host-mod and
> >user-mod has been removed but revocation behaviour of -del and
> >-disable is preserved.
> >
> >The latest profiles/caacl patchset (0001..0013 v5) depends on this
> >patch for correct cert-request behaviour.
> >
> >There is one design question (or maybe more, let me know): the
> >`--out=FILENAME' option to {host,service} show saves ONE certificate
> >to the named file.  I propose to either:
> >
> >a) write all certs, suffixing suggested filename with either a
> >    sequential numerical index, e.g. "cert.pem" becomes
> >    "cert.pem.1", "cert.pem.2", and so on; or
> >
> >b) as above, but suffix with serial number and, if there are
> >    different issues, some issuer-identifying information.
> >
> >Let me know your thoughts.
> >
> >Thanks,
> >Fraser
> >
> >
> Is there a possible way how to store certificates into one file?
> I read about possibilities to have multiple certs in one .pem file, but I'm
> not cert guru :)
> 
> I personally vote for serial number in case there are multiple certificates,
> if ^ is no possible.
> 
> 
> 1)
> +            if len(certs) > 0:
> 
> please use only,
> if certs:
> 
> 2)
> You need to re-generate API/ACI.txt in this patch
> 
> 3)
> syntax error:
> +        for dercert in certs_der
> 
> 
> 4)
> command
> ipa user-mod ca_user --certificate=<ceritifcate>
> 
> removes the current certificate from the LDAP, by design.
> Should be the old certificate(s) revoked? You removed that part in the code.
> 
> only the --addattr='usercertificate=<cert>' appends new value there
> 
> -- 
> Martin Basti
> 
From 58d301e1c8e6efba68bf6459d51c24c640b67edd Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Wed, 27 May 2015 08:02:08 -0400
Subject: [PATCH] Support multiple host and service certificates

Update the framework to support multiple host and service
certificates.

host-mod and service-mod revoke existing certificates that are not
included in the modified entry.  Using addattr=certificate=... will
result in no certificates being revoked.

The existing behaviour of host-disable, host-del, service-disable
and service-del (revoke existing certificate) is preserved but now
applies to all certificates in the host or service entry.

Part of: http://www.freeipa.org/page/V4/User_Certificates
---
 API.txt                   | 10 ++---
 ipalib/plugins/host.py    | 96 ++++++++++++++++++++++++-----------------------
 ipalib/plugins/service.py | 83 ++++++++++++++++++++++++----------------
 3 files changed, 104 insertions(+), 85 deletions(-)

diff --git a/API.txt b/API.txt
index 
da69f32de5c12c0d85a7d61d9027385aa3c0ee05..3cfcf34939a58d6888de8f0a7a6ef0c7779c993e
 100644
--- a/API.txt
+++ b/API.txt
@@ -1812,7 +1812,7 @@ option: Str('nsosversion', attribute=True, cli_name='os', 
multivalue=False, requ
 option: Flag('random', attribute=False, autofill=True, cli_name='random', 
default=False, multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: Bytes('usercertificate', attribute=True, cli_name='certificate', 
multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, cli_name='certificate', 
multivalue=True, required=False)
 option: Str('userclass', attribute=True, cli_name='class', multivalue=True, 
required=False)
 option: Str('userpassword', attribute=True, cli_name='password', 
multivalue=False, required=False)
 option: Str('version?', exclude='webui')
@@ -1935,7 +1935,7 @@ option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
 option: Int('timelimit?', autofill=False, minvalue=0)
-option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=False, query=True, required=False)
+option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=True, query=True, required=False)
 option: Str('userclass', attribute=True, autofill=False, cli_name='class', 
multivalue=True, query=True, required=False)
 option: Str('userpassword', attribute=True, autofill=False, 
cli_name='password', multivalue=False, query=True, required=False)
 option: Str('version?', exclude='webui')
@@ -1966,7 +1966,7 @@ option: Flag('raw', autofill=True, cli_name='raw', 
default=False, exclude='webui
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Flag('updatedns?', autofill=True, default=False)
-option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=True, required=False)
 option: Str('userclass', attribute=True, autofill=False, cli_name='class', 
multivalue=True, required=False)
 option: Str('userpassword', attribute=True, autofill=False, 
cli_name='password', multivalue=False, required=False)
 option: Str('version?', exclude='webui')
@@ -3584,7 +3584,7 @@ option: Bool('ipakrbrequirespreauth', attribute=False, 
cli_name='requires_pre_au
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: Bytes('usercertificate', attribute=True, cli_name='certificate', 
multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, cli_name='certificate', 
multivalue=True, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
@@ -3702,7 +3702,7 @@ option: Flag('no_members', autofill=True, default=False, 
exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, 
exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, autofill=False, 
cli_name='certificate', multivalue=True, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an 
LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 
c47439743da45b8629d1b2afbd210d87591784ce..13af9dc4c5e4ce00b4d969bcb0789d0034b2bba7
 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -493,7 +493,7 @@ class host(LDAPObject):
             label=_('Random password'),
             flags=('no_create', 'no_update', 'no_search', 'virtual_attribute'),
         ),
-        Bytes('usercertificate?', validate_certificate,
+        Bytes('usercertificate*', validate_certificate,
             cli_name='certificate',
             label=_('Certificate'),
             doc=_('Base-64 encoded server certificate'),
@@ -640,11 +640,11 @@ class host_add(LDAPCreate):
             entry_attrs['userpassword'] = 
ipa_generate_password(characters=host_pwd_chars)
             # save the password so it can be displayed in post_callback
             setattr(context, 'randompassword', entry_attrs['userpassword'])
-        cert = options.get('usercertificate')
-        if cert:
-            cert = x509.normalize_certificate(cert)
+        certs = options.get('usercertificate', [])
+        certs_der = map(x509.normalize_certificate, certs)
+        for cert in certs_der:
             x509.verify_cert_subject(ldap, keys[-1], cert)
-            entry_attrs['usercertificate'] = cert
+        entry_attrs['usercertificate'] = certs_der
         entry_attrs['managedby'] = dn
         entry_attrs['objectclass'].append('ieee802device')
         entry_attrs['objectclass'].append('ipasshhost')
@@ -786,8 +786,7 @@ class host_del(LDAPDelete):
                 entry_attrs = ldap.get_entry(dn, ['usercertificate'])
             except errors.NotFound:
                 self.obj.handle_not_found(*keys)
-            cert = entry_attrs.single_value.get('usercertificate')
-            if cert:
+            for cert in entry_attrs.get('usercertificate', []):
                 cert = x509.normalize_certificate(cert)
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
@@ -864,39 +863,42 @@ class host_mod(LDAPUpdate):
             if 'krbprincipalaux' not in obj_classes:
                 obj_classes.append('krbprincipalaux')
                 entry_attrs['objectclass'] = obj_classes
-        cert = x509.normalize_certificate(entry_attrs.get('usercertificate'))
-        if cert:
-            if self.api.Command.ca_is_enabled()['result']:
-                x509.verify_cert_subject(ldap, keys[-1], cert)
-                entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
-                oldcert = entry_attrs_old.single_value.get('usercertificate')
-                if oldcert:
-                    oldcert = x509.normalize_certificate(oldcert)
+
+        # verify certificates
+        certs = entry_attrs.get('usercertificate', [])
+        certs_der = map(x509.normalize_certificate, certs)
+        for cert in certs_der:
+            x509.verify_cert_subject(ldap, keys[-1], cert)
+
+        # revoke removed certificates
+        if self.api.Command.ca_is_enabled()['result']:
+            entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
+            old_certs = entry_attrs_old.get('usercertificate', [])
+            removed_certs = set(old_certs) - set(certs)
+            removed_certs_der = map(x509.normalize_certificate, removed_certs)
+            for cert in removed_certs_der:
+                try:
+                    serial = unicode(x509.get_serial_number(cert, x509.DER))
                     try:
-                        serial = x509.get_serial_number(oldcert, x509.DER)
-                        serial = unicode(serial)
-                        try:
-                            result = api.Command['cert_show'](serial)['result']
-                            if 'revocation_reason' not in result:
-                                try:
-                                    api.Command['cert_revoke'](
-                                        serial, revocation_reason=4)
-                                except errors.NotImplementedError:
-                                    # some CA's might not implement revoke
-                                    pass
-                        except errors.NotImplementedError:
-                            # some CA's might not implement revoke
-                            pass
-                    except NSPRError, nsprerr:
-                        if nsprerr.errno == -8183:
-                            # If we can't decode the cert them proceed with
-                            # modifying the host.
-                            self.log.info("Problem decoding certificate %s" %
-                                          nsprerr.args[1])
-                        else:
-                            raise nsprerr
-
-            entry_attrs['usercertificate'] = cert
+                        result = api.Command['cert_show'](serial)['result']
+                        if 'revocation_reason' not in result:
+                            try:
+                                api.Command['cert_revoke'](
+                                    serial, revocation_reason=4)
+                            except errors.NotImplementedError:
+                                # some CA's might not implement revoke
+                                pass
+                    except errors.NotImplementedError:
+                        # some CA's might not implement revoke
+                        pass
+                except NSPRError, nsprerr:
+                    if nsprerr.errno == -8183:
+                        # If we can't decode the cert them proceed with
+                        # modifying the host.
+                        self.log.info("Problem decoding certificate %s" %
+                                      nsprerr.args[1])
+                    else:
+                        raise nsprerr
 
         if options.get('random'):
             entry_attrs['userpassword'] = 
ipa_generate_password(characters=host_pwd_chars)
@@ -1148,10 +1150,9 @@ class host_disable(LDAPQuery):
             entry_attrs = ldap.get_entry(dn, ['usercertificate'])
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
-        cert = entry_attrs.single_value.get('usercertificate')
-        if cert:
-            if self.api.Command.ca_is_enabled()['result']:
-                cert = x509.normalize_certificate(cert)
+        if self.api.Command.ca_is_enabled()['result']:
+            certs = entry_attrs.get('usercertificate', [])
+            for cert in map(x509.normalize_certificate, certs):
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
                     try:
@@ -1175,10 +1176,11 @@ class host_disable(LDAPQuery):
                     else:
                         raise nsprerr
 
-            # Remove the usercertificate altogether
-            entry_attrs['usercertificate'] = None
-            ldap.update_entry(entry_attrs)
-            done_work = True
+            if certs:
+                # Remove the usercertificate altogether
+                entry_attrs['usercertificate'] = None
+                ldap.update_entry(entry_attrs)
+                done_work = True
 
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
         if entry_attrs['has_keytab']:
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 
b37dc7b4bf56b69df204fd29e9487f1390197bbe..4699751a71dae2cf535e5f278f27b69426493c9f
 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -437,7 +437,7 @@ class service(LDAPObject):
             primary_key=True,
             normalizer=lambda value: normalize_principal(value),
         ),
-        Bytes('usercertificate?', validate_certificate,
+        Bytes('usercertificate*', validate_certificate,
             cli_name='certificate',
             label=_('Certificate'),
             doc=_('Base-64 encoded server certificate'),
@@ -503,11 +503,11 @@ class service_add(LDAPCreate):
 
         self.obj.validate_ipakrbauthzdata(entry_attrs)
 
-        cert = options.get('usercertificate')
-        if cert:
-            dercert = x509.normalize_certificate(cert)
+        certs = options.get('usercertificate', [])
+        certs_der = map(x509.normalize_certificate, certs)
+        for dercert in certs_der:
             x509.verify_cert_subject(ldap, hostname, dercert)
-            entry_attrs['usercertificate'] = dercert
+        entry_attrs['usercertificate'] = certs_der
 
         if not options.get('force', False):
              # We know the host exists if we've gotten this far but we
@@ -555,9 +555,7 @@ class service_del(LDAPDelete):
                 entry_attrs = ldap.get_entry(dn, ['usercertificate'])
             except errors.NotFound:
                 self.obj.handle_not_found(*keys)
-            cert = entry_attrs.get('usercertificate')
-            if cert:
-                cert = cert[0]
+            for cert in entry_attrs.get('usercertificate', []):
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
                     try:
@@ -597,25 +595,43 @@ class service_mod(LDAPUpdate):
 
         self.obj.validate_ipakrbauthzdata(entry_attrs)
 
-        if 'usercertificate' in options:
-            (service, hostname, realm) = split_principal(keys[-1])
-            cert = options.get('usercertificate')
-            if cert:
-                dercert = x509.normalize_certificate(cert)
-                x509.verify_cert_subject(ldap, hostname, dercert)
+        (service, hostname, realm) = split_principal(keys[-1])
+
+        # verify certificates
+        certs = options.get('usercertificate', [])
+        certs_der = map(x509.normalize_certificate, certs)
+        for dercert in certs_der:
+            x509.verify_cert_subject(ldap, hostname, dercert)
+
+        # revoke removed certificates
+        if self.api.Command.ca_is_enabled()['result']:
+            entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
+            old_certs = entry_attrs_old.get('usercertificate', [])
+            removed_certs = set(old_certs) - set(certs)
+            removed_certs_der = map(x509.normalize_certificate, removed_certs)
+            for cert in removed_certs_der:
                 try:
-                    entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
-                except errors.NotFound:
-                    self.obj.handle_not_found(*keys)
-                if 'usercertificate' in entry_attrs_old:
-                    # FIXME: what to do here? do we revoke the old cert?
-                    fmt = 'entry already has a certificate, serial number: %s' 
% (
-                        
x509.get_serial_number(entry_attrs_old['usercertificate'][0], x509.DER)
-                    )
-                    raise errors.GenericError(format=fmt)
-                entry_attrs['usercertificate'] = dercert
-            else:
-                entry_attrs['usercertificate'] = None
+                    serial = unicode(x509.get_serial_number(cert, x509.DER))
+                    try:
+                        result = api.Command['cert_show'](serial)['result']
+                        if 'revocation_reason' not in result:
+                            try:
+                                api.Command['cert_revoke'](
+                                    serial, revocation_reason=4)
+                            except errors.NotImplementedError:
+                                # some CA's might not implement revoke
+                                pass
+                    except errors.NotImplementedError:
+                        # some CA's might not implement revoke
+                        pass
+                except NSPRError, nsprerr:
+                    if nsprerr.errno == -8183:
+                        # If we can't decode the cert them proceed with
+                        # modifying the host.
+                        self.log.info("Problem decoding certificate %s" %
+                                      nsprerr.args[1])
+                    else:
+                        raise nsprerr
 
         update_krbticketflags(ldap, entry_attrs, attrs_list, options, True)
 
@@ -815,9 +831,9 @@ class service_disable(LDAPQuery):
         # See if we do any work at all here and if not raise an exception
         done_work = False
 
-        if 'usercertificate' in entry_attrs:
-            if self.api.Command.ca_is_enabled()['result']:
-                cert = 
x509.normalize_certificate(entry_attrs.get('usercertificate')[0])
+        if self.api.Command.ca_is_enabled()['result']:
+            certs = entry_attrs.get('usercertificate', [])
+            for cert in map(x509.normalize_certificate, certs):
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
                     try:
@@ -839,10 +855,11 @@ class service_disable(LDAPQuery):
                     else:
                         raise nsprerr
 
-            # Remove the usercertificate altogether
-            entry_attrs['usercertificate'] = None
-            ldap.update_entry(entry_attrs)
-            done_work = True
+            if len(certs) > 0:
+                # Remove the usercertificate altogether
+                entry_attrs['usercertificate'] = None
+                ldap.update_entry(entry_attrs)
+                done_work = True
 
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
         if entry_attrs['has_keytab']:
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to