From 8ee3779ded64ff55c3981fb8c2db50cdcd3abc5b Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Nov 30 2018 14:20:59 +0000
Subject: ipa upgrade: handle double-encoded certificates


Issue is linked to the ticket
 #3477 LDAP upload CA cert sometimes double-encodes the value
In old FreeIPA releases (< 3.2), the upgrade plugin was encoding twice
the value of the certificate in cn=cacert,cn=ipa,cn=etc,$BASEDN.

The fix for 3477 is only partial as it prevents double-encoding when a
new cert is uploaded but does not fix wrong values already present in LDAP.

With this commit, the code first tries to read a der cert. If it fails,
it logs a debug message and re-writes the value caCertificate;binary
to repair the entry.

Fixes https://pagure.io/freeipa/issue/7775
Signed-off-by: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Christian Heimes <cheimes@redhat.com>

---

diff --git a/ipaserver/install/plugins/upload_cacrt.py b/ipaserver/install/plugins/upload_cacrt.py
index 85c67e7..763da1e 100644
--- a/ipaserver/install/plugins/upload_cacrt.py
+++ b/ipaserver/install/plugins/upload_cacrt.py
@@ -115,7 +115,18 @@ class update_upload_cacrt(Updater):
                 entry.single_value['cACertificate;binary'] = ca_cert
                 ldap.add_entry(entry)
             else:
-                if b'' in entry['cACertificate;binary']:
+                force_write = False
+                try:
+                    _cert_bin = entry['cACertificate;binary']
+                except ValueError:
+                    # BZ 1644874
+                    # sometimes the cert is badly stored, twice encoded
+                    # force write to fix the value
+                    logger.debug('Fixing the value of cACertificate;binary '
+                                 'in entry %s', entry.dn)
+                    force_write = True
+
+                if force_write or b'' in entry['cACertificate;binary']:
                     entry.single_value['cACertificate;binary'] = ca_cert
                     ldap.update_entry(entry)
 

From 2b0f3a1abb9067a0a5ba8e59762bc41dc51608e2 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Nov 30 2018 14:20:59 +0000
Subject: ipatests: add upgrade test for double-encoded cacert


Create a test for upgrade with the following scenario:
- install master
- write a double-encoded cert in the entry
cn=cacert,,cn=ipa,cn=etc,$basedn
to simulate bug 7775
- call ipa-server-upgrade
- check that the upgrade fixed the value

The upgrade should finish successfully and repair
the double-encoded cert.

Related to https://pagure.io/freeipa/issue/7775

Reviewed-By: Christian Heimes <cheimes@redhat.com>

---

diff --git a/ipatests/test_integration/test_upgrade.py b/ipatests/test_integration/test_upgrade.py
index cbf5f39..e0175bc 100644
--- a/ipatests/test_integration/test_upgrade.py
+++ b/ipatests/test_integration/test_upgrade.py
@@ -6,6 +6,9 @@
 Module provides tests to verify that the upgrade script works.
 """
 
+import base64
+from cryptography.hazmat.primitives import serialization
+from ipapython.dn import DN
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.pytest_ipa.integration import tasks
 
@@ -21,3 +24,35 @@ class TestUpgrade(IntegrationTest):
         assert ("DN: cn=Schema Compatibility,cn=plugins,cn=config does not \
                 exists or haven't been updated" not in cmd.stdout_text)
         assert cmd.returncode == 0
+
+    def test_double_encoded_cacert(self):
+        """Test for BZ 1644874
+
+        In old IPA version, the entry cn=CAcert,cn=ipa,cn=etc,$basedn
+        could contain a double-encoded cert, which leads to ipa-server-upgrade
+        failure.
+        Force a double-encoded value then call upgrade to check the fix.
+        """
+        # Read the current entry from LDAP
+        ldap = self.master.ldap_connect()
+        basedn = self.master.domain.basedn  # pylint: disable=no-member
+        dn = DN(('cn', 'CAcert'), ('cn', 'ipa'), ('cn', 'etc'), basedn)
+        entry = ldap.get_entry(dn)  # pylint: disable=no-member
+        # Extract the certificate as DER then double-encode
+        cacert = entry['cacertificate;binary'][0]
+        cacert_der = cacert.public_bytes(serialization.Encoding.DER)
+        cacert_b64 = base64.b64encode(cacert_der)
+        # overwrite the value with double-encoded cert
+        entry.single_value['cACertificate;binary'] = cacert_b64
+        ldap.update_entry(entry)  # pylint: disable=no-member
+
+        # try the upgrade
+        self.master.run_command(['ipa-server-upgrade'])
+
+        # read the value after upgrade, should be fixed
+        entry = ldap.get_entry(dn)  # pylint: disable=no-member
+        try:
+            _cacert = entry['cacertificate;binary']
+        except ValueError:
+            raise AssertionError('%s contains a double-encoded cert'
+                                 % entry.dn)

From 2a299c786f93e67446d5fd227fe14884b4e0d293 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
Date: Dec 06 2018 10:37:26 +0000
Subject: ipatests: fix TestUpgrade::test_double_encoded_cacert


The test is using a stale ldap connection to the master
(obtained before calling upgrade, and the upgrade stops
and starts 389-ds, breaking the connection).

The fix re-connects before using the ldap handle.

Related to https://pagure.io/freeipa/issue/7775

---

diff --git a/ipatests/test_integration/test_upgrade.py b/ipatests/test_integration/test_upgrade.py
index e0175bc..5cc890e 100644
--- a/ipatests/test_integration/test_upgrade.py
+++ b/ipatests/test_integration/test_upgrade.py
@@ -49,6 +49,8 @@ class TestUpgrade(IntegrationTest):
         # try the upgrade
         self.master.run_command(['ipa-server-upgrade'])
 
+        # reconnect to the master (upgrade stops 389-ds)
+        ldap = self.master.ldap_connect()
         # read the value after upgrade, should be fixed
         entry = ldap.get_entry(dn)  # pylint: disable=no-member
         try: