upgrade: remove ipaCert and key from /etc/httpd/alias

With ipa 4.5+, the RA cert is stored in files in
/var/lib/ipa/ra-agent.{key|pem}. The upgrade code handles
the move from /etc/httpd/alias to the files but does not remove
the private key from /etc/httpd/alias.

The fix calls certutil -F -n ipaCert to remove cert and key,
instead of -D -n ipaCert which removes only the cert.

Fixes: https://pagure.io/freeipa/issue/7329
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>

ipapython/certdb.py

@@ -892,6 +892,32 @@ class NSSDatabase:
     def delete_cert(self, nick):
         self.run_certutil(["-D", "-n", nick])
 
+    def delete_key_only(self, nick):
+        """Delete the key with provided nick
+
+        This commands removes the key but leaves the cert in the DB.
+        """
+        keys = self.list_keys()
+        # keys is a list of tuple(slot, algo, keyid, nickname)
+        for (_slot, _algo, keyid, nickname) in keys:
+            if nickname == nick:
+                # Key is present in the DB, delete the key
+                self.run_certutil(["-F", "-k", keyid])
+                break
+
+    def delete_key_and_cert(self, nick):
+        """Delete a cert and its key from the DB"""
+        try:
+            self.run_certutil(["-F", "-n", nick])
+        except ipautil.CalledProcessError:
+            # Using -F -k instead of -F -n because the latter fails if
+            # the DB contains only the key
+            self.delete_key_only(nick)
+        # Check that cert was deleted
+        for (certname, _flags) in self.list_certs():
+            if certname == nick:
+                self.delete_cert(nick)
+
     def verify_server_cert_validity(self, nickname, hostname):
         """Verify a certificate is valid for a SSL server with given hostname
 

ipaserver/install/plugins/update_ra_cert_store.py

@@ -52,7 +52,7 @@ class update_ra_cert_store(Updater):
 
         # stop tracking the old cert and remove it
         certmonger.stop_tracking(paths.HTTPD_ALIAS_DIR, nickname=ra_nick)
-        certdb.delete_cert(ra_nick)
+        certdb.delete_key_and_cert(ra_nick)
         if os.path.exists(paths.OLD_KRA_AGENT_PEM):
             os.remove(paths.OLD_KRA_AGENT_PEM)
 

ipatests/test_ipapython/test_certdb.py

@@ -5,6 +5,7 @@ import os
 import pytest
 
 from ipapython.certdb import NSSDatabase, TRUSTED_PEER_TRUST_FLAGS
+from ipapython import ipautil
 from ipaplatform.osinfo import osinfo
 
 CERTNICK = 'testcert'
@@ -167,3 +168,77 @@ def test_auto_db():
         assert nssdb.filenames is not None
         assert nssdb.exists()
         nssdb.list_certs()
+
+
+def test_delete_cert_and_key():
+    """Test that delete_cert + delete_key always deletes everything
+
+    Test with a NSSDB that contains:
+    - cert + key
+    - key only
+    - cert only
+    - none of them
+    """
+    cmd = ipautil.run(['mktemp'], capture_output=True)
+    p12file = cmd.output.strip()
+
+    try:
+        with NSSDatabase() as nssdb:
+            nssdb.create_db()
+
+            # 1. Test delete_key_and_cert when cert + key are present
+            # Create a NSS DB with cert + key
+            create_selfsigned(nssdb)
+            # Save both in a p12 file for latter use
+            ipautil.run(
+                [
+                    'pk12util',
+                    '-o', p12file, '-n', CERTNICK, '-d', nssdb.secdir,
+                    '-k', nssdb.pwd_file,
+                    '-w', nssdb.pwd_file
+                ])
+            # Delete cert and key
+            nssdb.delete_key_and_cert(CERTNICK)
+            # make sure that everything was deleted
+            assert len(nssdb.list_keys()) == 0
+            assert len(nssdb.list_certs()) == 0
+
+            # 2. Test delete_key_and_cert when only key is present
+            # Import cert and key then remove cert
+            import_args = [
+                'pk12util',
+                '-i', p12file, '-d', nssdb.secdir,
+                '-k', nssdb.pwd_file,
+                '-w', nssdb.pwd_file]
+            ipautil.run(import_args)
+            nssdb.delete_cert(CERTNICK)
+            # Delete cert and key
+            nssdb.delete_key_and_cert(CERTNICK)
+            # make sure that everything was deleted
+            assert len(nssdb.list_keys()) == 0
+            assert len(nssdb.list_certs()) == 0
+
+            # 3. Test delete_key_and_cert when only cert is present
+            # Import cert and key then remove key
+            ipautil.run(import_args)
+            nssdb.delete_key_only(CERTNICK)
+            # make sure the db contains only the cert
+            assert len(nssdb.list_keys()) == 0
+            assert len(nssdb.list_certs()) == 1
+
+            # Delete cert and key when key is not present
+            nssdb.delete_key_and_cert(CERTNICK)
+            # make sure that everything was deleted
+            assert len(nssdb.list_keys()) == 0
+            assert len(nssdb.list_certs()) == 0
+
+            # 4. Test delete_key_and_cert with a wrong nickname
+            # Import cert and key
+            ipautil.run(import_args)
+            # Delete cert and key
+            nssdb.delete_key_and_cert('wrongnick')
+            # make sure that nothing was deleted
+            assert len(nssdb.list_keys()) == 1
+            assert len(nssdb.list_certs()) == 1
+    finally:
+        os.unlink(p12file)