From 9ab75cae3691ee2226e45e2ed06b60bb684d5414 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Wed, 9 May 2018 11:16:03 -0400
Subject: [PATCH 1/2] Revoke existing certificate when a new one is issued by
 IPA CA

Revocation was broken by commit a931d3edc00f7578223df2afeebdf2da3dd85a68
because addattr is not evaluated.

https://pagure.io/freeipa/issue/7482

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 ipaserver/plugins/cert.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index db624357af..b5351a7cfb 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -898,7 +898,7 @@ def execute(self, csr, all=False, raw=False, chain=False, **kw):
         store = profile['result']['ipacertprofilestoreissued'][0] == 'TRUE'
         if store and 'certificate' in result:
             cert = result.get('certificate')
-            kwargs = dict(addattr=u'usercertificate={}'.format(cert))
+            kwargs = dict(usercertificate=cert)
             # note: we call different commands for the different
             # principal types because handling of 'userCertificate'
             # vs. 'userCertificate;binary' varies by plugin.

From 15f6b5ce063be027813ca5fb474207df2d617f0e Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Wed, 9 May 2018 15:31:55 -0400
Subject: [PATCH 2/2] Test that service/host certificates are revoked on
 renewal

Also add a full skip test if the IPA CA is not configured.

https://pagure.io/freeipa/issue/7482

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 ipatests/test_xmlrpc/test_cert_plugin.py | 72 +++++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index 68f5b16670..246b42438a 100644
--- a/ipatests/test_xmlrpc/test_cert_plugin.py
+++ b/ipatests/test_xmlrpc/test_cert_plugin.py
@@ -33,6 +33,7 @@
 from ipapython.certdb import NSSDatabase
 from ipapython.dn import DN
 from ipapython.ipautil import run
+from ipaserver.install import cainstance
 from ipatests.test_xmlrpc.testcert import subject_base
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
@@ -78,6 +79,7 @@ def is_db_configured():
 class BaseCert(XMLRPC_test):
     host_fqdn = u'ipatestcert.%s' % api.env.domain
     service_princ = u'test/%s@%s' % (host_fqdn, api.env.realm)
+    host_princ = u'host/%s@%s' % (host_fqdn, api.env.realm)
 
     @classmethod
     def setup_class(cls):
@@ -88,6 +90,11 @@ def setup_class(cls):
         if 'cert_show' not in api.Command:
             raise unittest.SkipTest('cert_show not registered')
 
+        ca = cainstance.CAInstance(
+                 api.env.realm, host_name=api.env.host)
+        if not ca.is_configured():
+            raise unittest.SkipTest('CA is not configured')
+
         is_db_configured()
 
     def setup(self):
@@ -199,11 +206,10 @@ def test_0007_service_show(self):
         """
         res = api.Command['service_show'](self.service_princ)['result']
 
-        # Both the old and the new certs should be listed as certificates now
-        certs_encoded = (
-            base64.b64encode(usercert) for usercert in res['usercertificate']
-        )
-        assert set(certs_encoded) == set([cert, newcert])
+        # It should no longer match our old cert
+        assert base64.b64encode(res['usercertificate'][0]) != cert
+        # And it should match the new one
+        assert base64.b64encode(res['usercertificate'][0]) == newcert
 
     def test_0008_cert_show(self):
         """
@@ -484,6 +490,62 @@ def revoke_cert(self, reason):
         # remove host
         assert 'result' in api.Command['host_del'](self.host_fqdn)
 
+    # replace an existing cert and verify that the old one is revoked
+    def test_renew_host_cert(self):
+        assert 'result' in api.Command['host_add'](self.host_fqdn, force=True)
+
+        # generate CSR, request certificate, obtain serial number
+        self.csr = self.generateCSR(str(self.subject))
+        res = api.Command['cert_request'](self.csr,
+                                          principal=self.host_princ,
+                                          add=True, all=True)['result']
+        serial_number = res['serial_number']
+
+        res = api.Command['cert_request'](self.csr,
+                                          principal=self.host_princ,
+                                          add=True, all=True)['result']
+
+        new_serial_number = res['serial_number']
+
+        assert(serial_number != new_serial_number)
+
+        # verify that certificate is revoked with correct reason
+        res2 = api.Command['cert_show'](serial_number, all=True)['result']
+        assert res2['revoked']
+        assert res2['revocation_reason'] == 4  # SUPERSEDED
+
+        # remove host
+        assert 'result' in api.Command['host_del'](self.host_fqdn)
+
+    # replace an existing cert and verify that the old one is revoked
+    def test_renew_service_cert(self):
+        assert 'result' in api.Command['host_add'](self.host_fqdn, force=True)
+        assert 'result' in api.Command['service_add'](
+            self.service_princ, force=True)
+
+        # generate CSR, request certificate, obtain serial number
+        self.csr = self.generateCSR(str(self.subject))
+        res = api.Command['cert_request'](self.csr,
+                                          principal=self.service_princ,
+                                          add=True, all=True)['result']
+        serial_number = res['serial_number']
+
+        res = api.Command['cert_request'](self.csr,
+                                          principal=self.service_princ,
+                                          add=True, all=True)['result']
+
+        new_serial_number = res['serial_number']
+
+        assert(serial_number != new_serial_number)
+
+        # verify that certificate is revoked with correct reason
+        res2 = api.Command['cert_show'](serial_number, all=True)['result']
+        assert res2['revoked']
+        assert res2['revocation_reason'] == 4  # SUPERSEDED
+
+        # remove host
+        assert 'result' in api.Command['host_del'](self.host_fqdn)
+
     def test_revoke_with_reason_0(self):
         self.revoke_cert(0)
 
