On 01/06/15 06:40, Fraser Tweedale wrote:
New version of patch; ``{host,service}-show --out=FILE`` now writes
all certs to FILE.  Rebased on latest master.

Thanks,
Fraser

On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:
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

My objections/proposed solutions in attached patch.

* VERSION
* In the previous version normalized values was stored in LDAP, so I added it back. (I dont know why there is no normalization in param settings, but normalization for every certificate is done in callback. I will file a ticket for this) * IMO only normalized certificates should be compared in the old certificates detection

--
Martin Basti

From 9bac1d4826156528f6b976d384502fa1f4aaae40 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 1 Jun 2015 14:37:45 +0200
Subject: [PATCH] review

---
 VERSION                   | 4 ++--
 ipalib/plugins/host.py    | 5 +++--
 ipalib/plugins/service.py | 5 +++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/VERSION b/VERSION
index 07c00d000064a7687497b09524aa821dbcecc88a..24a2913226961a807da49076184a1053c897e748 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: ftweedal - allow multiple host/service certificates
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 2ad1aaada69bd9c6a768e0fd3290217d63b652ea..9ad087e26250d86b15fbe723a98cca278ef29adf 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -874,8 +874,8 @@ class host_mod(LDAPUpdate):
         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)
+            old_certs_der = map(x509.normalize_certificate, old_certs)
+            removed_certs_der = set(old_certs_der) - set(certs_der)
             for cert in removed_certs_der:
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
@@ -899,6 +899,7 @@ class host_mod(LDAPUpdate):
                                       nsprerr.args[1])
                     else:
                         raise nsprerr
+        entry_attrs['usercertificate'] = certs_der
 
         if options.get('random'):
             entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars)
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 668e016bd9f56fa75f791355fb9d8d3bd02f8f1a..c290344cf6c14155ec1b103525ff8642a7a8e2af 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -607,8 +607,8 @@ class service_mod(LDAPUpdate):
         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)
+            old_certs_der = map(x509.normalize_certificate, old_certs)
+            removed_certs_der = set(old_certs_der) - set(certs_der)
             for cert in removed_certs_der:
                 try:
                     serial = unicode(x509.get_serial_number(cert, x509.DER))
@@ -632,6 +632,7 @@ class service_mod(LDAPUpdate):
                                       nsprerr.args[1])
                     else:
                         raise nsprerr
+        entry_attrs['usercertificate'] = certs_der
 
         update_krbticketflags(ldap, entry_attrs, attrs_list, options, True)
 
-- 
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