From 8ee3779ded64ff55c3981fb8c2db50cdcd3abc5b Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud 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 Reviewed-By: Christian Heimes --- 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 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 --- 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 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: