From cbef046169e905e34b33cee2fcf435a194729378 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Wed, 8 Nov 2023 11:50:46 +0200 Subject: [PATCH] Backport various fixes found by RHEL and upstream tests - timezone shift in handling certificates (due to py3.12 adaptation) - 'reason' vs 'Reason' in PKI revocation JSON API response - allow removal of minlength attribute from a custom password policy Signed-off-by: Alexander Bokovoy --- freeipa-4.11-pki-revocation-changes.patch | 153 +++++++++++++++++++++ freeipa-4.11-pwpolicy-minlength.patch | 135 ++++++++++++++++++ freeipa-4.11-py3.12-timezone-changes.patch | 88 ++++++++++++ freeipa.spec | 10 +- 4 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 freeipa-4.11-pki-revocation-changes.patch create mode 100644 freeipa-4.11-pwpolicy-minlength.patch create mode 100644 freeipa-4.11-py3.12-timezone-changes.patch diff --git a/freeipa-4.11-pki-revocation-changes.patch b/freeipa-4.11-pki-revocation-changes.patch new file mode 100644 index 0000000..0519ae4 --- /dev/null +++ b/freeipa-4.11-pki-revocation-changes.patch @@ -0,0 +1,153 @@ +From d4271391adc45c781092db0fb89b802743a9dda8 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Mon, 11 Sep 2023 21:37:05 +0000 +Subject: [PATCH 1/2] The PKI JSON API the revocation reason key may be + case-sensitive + +PKI 11.4.0 changed the reason keyword in the REST API from lower-case +to camel-case in https://github.com/dogtagpki/pki/commit/926eb221ce6 + +Use Reason instead of reason as the keyword for revocations +for PKI 11.4.0+ + +Related: https://pagure.io/freeipa/issue/9345 + +Signed-off-by: Rob Crittenden +Reviewed-By: Florence Blanc-Renaud +Reviewed-By: Thomas Woerner +--- + ipaserver/plugins/dogtag.py | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py +index 1c2c51824..0036803c8 100644 +--- a/ipaserver/plugins/dogtag.py ++++ b/ipaserver/plugins/dogtag.py +@@ -274,6 +274,8 @@ if six.PY3: + + logger = logging.getLogger(__name__) + ++pki_version = pki.util.Version(pki.specification_version()) ++ + # These are general status return values used when + # CMSServlet.outputError() is invoked. + CMS_SUCCESS = 0 +@@ -1130,7 +1132,11 @@ class ra(rabase.rabase, RestClient): + serial_number = int(serial_number, 0) + + path = 'agent/certs/{}/revoke'.format(serial_number) +- data = '{{"reason":"{}"}}'.format(reasons[revocation_reason]) ++ if pki_version < pki.util.Version("11.4.0"): ++ keyword = "reason" ++ else: ++ keyword = "Reason" ++ data = '{{"{}":"{}"}}'.format(keyword, reasons[revocation_reason]) + + http_status, _http_headers, http_body = self._ssldo( + 'POST', path, +-- +2.41.0 + + +From 0539d97f3e9d2b7d80549ff08d78fe55afcc2dbb Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Thu, 26 Oct 2023 13:59:21 -0400 +Subject: [PATCH 2/2] WIP: Get the PKI version from the remote to determine the + argument + +Reviewed-By: Florence Blanc-Renaud +Reviewed-By: Thomas Woerner +--- + ipaserver/plugins/dogtag.py | 55 ++++++++++++++++++++++++++++++++----- + 1 file changed, 48 insertions(+), 7 deletions(-) + +diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py +index 0036803c8..7cd51ae58 100644 +--- a/ipaserver/plugins/dogtag.py ++++ b/ipaserver/plugins/dogtag.py +@@ -274,8 +274,6 @@ if six.PY3: + + logger = logging.getLogger(__name__) + +-pki_version = pki.util.Version(pki.specification_version()) +- + # These are general status return values used when + # CMSServlet.outputError() is invoked. + CMS_SUCCESS = 0 +@@ -1059,6 +1057,39 @@ class ra(rabase.rabase, RestClient): + + return cmd_result + ++ def get_pki_version(self): ++ """ ++ Retrieve the version of a remote PKI server. ++ ++ The REST API request is a GET to the info URI: ++ GET /pki/rest/info HTTP/1.1 ++ ++ The response is: {"Version":"11.5.0","Attributes":{"Attribute":[]}} ++ """ ++ path = "/pki/rest/info" ++ logger.debug('%s.get_pki_version()', type(self).__name__) ++ http_status, _http_headers, http_body = self._ssldo( ++ 'GET', path, ++ headers={ ++ 'Content-Type': 'application/json', ++ 'Accept': 'application/json', ++ }, ++ use_session=False, ++ ) ++ if http_status != 200: ++ self.raise_certificate_operation_error('get_pki_version', ++ detail=http_status) ++ ++ try: ++ response = json.loads(ipautil.decode_json(http_body)) ++ except ValueError as e: ++ logger.debug("Response from CA was not valid JSON: %s", e) ++ raise errors.RemoteRetrieveError( ++ reason=_("Response from CA was not valid JSON") ++ ) ++ ++ return response.get('Version') ++ + + def revoke_certificate(self, serial_number, revocation_reason=0): + """ +@@ -1125,6 +1156,20 @@ class ra(rabase.rabase, RestClient): + detail='7 is not a valid revocation reason' + ) + ++ # dogtag changed the argument case for revocation from ++ # "reason" to "Reason" in PKI 11.4.0. Detect that change ++ # based on the remote version and pass the expected value ++ # in. ++ pki_version = pki.util.Version(self.get_pki_version()) ++ if pki_version is None: ++ self.raise_certificate_operation_error('revoke_certificate', ++ detail="Remove version not " ++ "detected") ++ if pki_version < pki.util.Version("11.4.0"): ++ reason = "reason" ++ else: ++ reason = "Reason" ++ + # Convert serial number to integral type from string to properly handle + # radix issues. Note: the int object constructor will properly handle + # large magnitude integral values by returning a Python long type +@@ -1132,11 +1177,7 @@ class ra(rabase.rabase, RestClient): + serial_number = int(serial_number, 0) + + path = 'agent/certs/{}/revoke'.format(serial_number) +- if pki_version < pki.util.Version("11.4.0"): +- keyword = "reason" +- else: +- keyword = "Reason" +- data = '{{"{}":"{}"}}'.format(keyword, reasons[revocation_reason]) ++ data = '{{"{}":"{}"}}'.format(reason, reasons[revocation_reason]) + + http_status, _http_headers, http_body = self._ssldo( + 'POST', path, +-- +2.41.0 + diff --git a/freeipa-4.11-pwpolicy-minlength.patch b/freeipa-4.11-pwpolicy-minlength.patch new file mode 100644 index 0000000..2920d87 --- /dev/null +++ b/freeipa-4.11-pwpolicy-minlength.patch @@ -0,0 +1,135 @@ +From 9b0b723a0e62f18d41be53900ab8a3e710708563 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Thu, 18 May 2023 09:23:32 -0400 +Subject: [PATCH] Allow password policy minlength to be removed like other + values + +This is a side-effect of adding the libpwquality options. It +imposes its own hardcoded minimum password length so some care +was needed to ensure that it isn't set too low. + +So if there are no libpwquality options used then it's fine to +have no minlength in the policy. + +Fixes: https://pagure.io/freeipa/issue/9297 + +Signed-off-by: Rob Crittenden +Reviewed-By: Alexander Bokovoy +Reviewed-By: Florence Blanc-Renaud +--- + ipaserver/plugins/pwpolicy.py | 10 +++-- + ipatests/test_integration/test_pwpolicy.py | 45 +++++++++++++++++++++- + 2 files changed, 50 insertions(+), 5 deletions(-) + +diff --git a/ipaserver/plugins/pwpolicy.py b/ipaserver/plugins/pwpolicy.py +index 5ea3e6b78..15cfef45b 100644 +--- a/ipaserver/plugins/pwpolicy.py ++++ b/ipaserver/plugins/pwpolicy.py +@@ -462,6 +462,7 @@ class pwpolicy(LDAPObject): + return False + + has_pwquality_value = False ++ min_length = 0 + if not add: + if len(keys) > 0: + existing_entry = self.api.Command.pwpolicy_show( +@@ -470,14 +471,15 @@ class pwpolicy(LDAPObject): + existing_entry = self.api.Command.pwpolicy_show( + all=True,)['result'] + existing_entry.update(entry_attrs) +- min_length = int(get_val(existing_entry, 'krbpwdminlength')) +- ++ if existing_entry.get('krbpwdminlength'): ++ min_length = int(get_val(existing_entry, 'krbpwdminlength')) + has_pwquality_value = has_pwquality_set(existing_entry) + else: +- min_length = int(get_val(entry_attrs, 'krbpwdminlength')) ++ if entry_attrs.get('krbpwdminlength'): ++ min_length = int(get_val(entry_attrs, 'krbpwdminlength')) + has_pwquality_value = has_pwquality_set(entry_attrs) + +- if min_length and min_length < 6 and has_pwquality_value: ++ if min_length < 6 and has_pwquality_value: + raise errors.ValidationError( + name='minlength', + error=_('Minimum length must be >= 6 if maxrepeat, ' +diff --git a/ipatests/test_integration/test_pwpolicy.py b/ipatests/test_integration/test_pwpolicy.py +index 41d6e9070..652c95e47 100644 +--- a/ipatests/test_integration/test_pwpolicy.py ++++ b/ipatests/test_integration/test_pwpolicy.py +@@ -36,7 +36,9 @@ class TestPWPolicy(IntegrationTest): + cls.master.run_command(['ipa', 'group-add-member', POLICY, + '--users', USER]) + cls.master.run_command(['ipa', 'pwpolicy-add', POLICY, +- '--priority', '1', '--gracelimit', '-1']) ++ '--priority', '1', ++ '--gracelimit', '-1', ++ '--minlength', '6']) + cls.master.run_command(['ipa', 'passwd', USER], + stdin_text='{password}\n{password}\n'.format( + password=PASSWORD +@@ -92,6 +94,12 @@ class TestPWPolicy(IntegrationTest): + "--minlength", "0", + "--minclasses", "0",], + ) ++ # minlength => 6 is required for any of the libpwquality settings ++ self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--minlength", "6"], ++ raiseonerr=False, ++ ) + + @pytest.fixture + def reset_pwpolicy(self): +@@ -212,6 +220,7 @@ class TestPWPolicy(IntegrationTest): + assert 'Password is too simple' in \ + result.stdout_text + ++ self.reset_password(self.master) + # test with valid password + for valid in ('Passw0rd', 'password1!', 'Password!'): + self.kinit_as_user(self.master, PASSWORD, valid) +@@ -252,6 +261,40 @@ class TestPWPolicy(IntegrationTest): + assert result.returncode != 0 + assert 'minlength' in result.stderr_text + ++ def test_minlength_empty(self, reset_pwpolicy): ++ """Test that the pwpolicy minlength can be blank ++ """ ++ # Ensure it is set to a non-zero value to avoid EmptyModlist ++ self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--minlength", "10",] ++ ) ++ # Enable one of the libpwquality options, removing minlength ++ # should fail. ++ self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--maxrepeat", "4",] ++ ) ++ result = self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--minlength", "",], raiseonerr=False ++ ) ++ assert result.returncode != 0 ++ ++ # Remove the blocking value ++ self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--maxrepeat", "",] ++ ) ++ ++ # Now erase it ++ result = self.master.run_command( ++ ["ipa", "pwpolicy-mod", POLICY, ++ "--minlength", "",] ++ ) ++ assert result.returncode == 0 ++ assert 'minlength' not in result.stderr_text ++ + def test_minlength_add(self): + """Test that adding a new policy with minlength is caught. + """ +-- +2.41.0 + diff --git a/freeipa-4.11-py3.12-timezone-changes.patch b/freeipa-4.11-py3.12-timezone-changes.patch new file mode 100644 index 0000000..b90bac3 --- /dev/null +++ b/freeipa-4.11-py3.12-timezone-changes.patch @@ -0,0 +1,88 @@ +From d9ad56155e76f97ad9326d5c1bcc6e19eea3a0da Mon Sep 17 00:00:00 2001 +From: Florence Blanc-Renaud +Date: Mon, 9 Oct 2023 13:54:17 +0200 +Subject: [PATCH] ipalib: fix the IPACertificate validity dates + +The class IPACertificate builds objects from x509 Certificate +objects and creates the not_valid_before and not_valid_after values +by converting to a timestamp + applying timezone delta to UTC + reading +from the timestamp. This results in applying twice the delta. + +Use a simpler method that replaces the timezone info with UTC in the +datetime object. + +Fixes: https://pagure.io/freeipa/issue/9462 + +Signed-off-by: Florence Blanc-Renaud +Reviewed-By: Rob Crittenden +--- + ipalib/x509.py | 6 ++---- + ipatests/test_ipalib/test_x509.py | 25 +++++++++++++++++++++++++ + 2 files changed, 27 insertions(+), 4 deletions(-) + +diff --git a/ipalib/x509.py b/ipalib/x509.py +index 7396688ae..769d48007 100644 +--- a/ipalib/x509.py ++++ b/ipalib/x509.py +@@ -266,13 +266,11 @@ class IPACertificate(crypto_x509.Certificate): + + @property + def not_valid_before(self): +- return datetime.datetime.fromtimestamp( +- self._cert.not_valid_before.timestamp(), tz=datetime.timezone.utc) ++ return self._cert.not_valid_before.replace(tzinfo=datetime.timezone.utc) + + @property + def not_valid_after(self): +- return datetime.datetime.fromtimestamp( +- self._cert.not_valid_after.timestamp(), tz=datetime.timezone.utc) ++ return self._cert.not_valid_after.replace(tzinfo=datetime.timezone.utc) + + @property + def tbs_certificate_bytes(self): +diff --git a/ipatests/test_ipalib/test_x509.py b/ipatests/test_ipalib/test_x509.py +index c25e8a0b5..74287c84a 100644 +--- a/ipatests/test_ipalib/test_x509.py ++++ b/ipatests/test_ipalib/test_x509.py +@@ -26,6 +26,7 @@ from binascii import hexlify + from configparser import RawConfigParser + import datetime + from io import StringIO ++import os + import pickle + + import pytest +@@ -253,6 +254,30 @@ class test_x509: + b'+\x06\x01\x05\x05\x07\x03\x01' + ) + ++ def test_cert_with_timezone(self): ++ """ ++ Test the not_before and not_after values in a diffent timezone ++ ++ Test for https://pagure.io/freeipa/issue/9462 ++ """ ++ # Store initial timezone, then set to New York ++ tz = os.environ.get('TZ', None) ++ os.environ['TZ'] = 'America/New_York' ++ # Load the cert, extract not before and not after ++ cert = x509.load_pem_x509_certificate(goodcert_headers) ++ not_before = datetime.datetime(2010, 6, 25, 13, 0, 42, 0, ++ datetime.timezone.utc) ++ not_after = datetime.datetime(2015, 6, 25, 13, 0, 42, 0, ++ datetime.timezone.utc) ++ # Reset timezone to previous value ++ if tz: ++ os.environ['TZ'] = tz ++ else: ++ del os.environ['TZ'] ++ # ensure the timezone doesn't mess with not_before and not_after ++ assert cert.not_valid_before == not_before ++ assert cert.not_valid_after == not_after ++ + def test_load_pkcs7_pem(self): + certlist = x509.pkcs7_to_certs(good_pkcs7, datatype=x509.PEM) + assert len(certlist) == 1 +-- +2.41.0 + diff --git a/freeipa.spec b/freeipa.spec index 5393173..b8aee8e 100644 --- a/freeipa.spec +++ b/freeipa.spec @@ -201,7 +201,7 @@ Name: %{package_name} Version: %{IPA_VERSION} -Release: 6%{?rc_version:.%rc_version}%{?dist} +Release: 7%{?rc_version:.%rc_version}%{?dist} Summary: The Identity, Policy and Audit system License: GPL-3.0-or-later @@ -221,6 +221,9 @@ Source2: gpgkey-0E63D716D76AC080A4A33513F40800B6298EB963.asc %endif Patch0001: freeipa-4.11-samba-changes.patch +Patch0002: freeipa-4.11-pki-revocation-changes.patch +Patch0003: freeipa-4.11-py3.12-timezone-changes.patch +Patch0004: freeipa-4.11-pwpolicy-minlength.patch # RHEL spec file only: START: Change branding to IPA and Identity Management # Moved branding logos and background to redhat-logos-ipa-80.4: @@ -1741,6 +1744,11 @@ fi %endif %changelog +* Wed Nov 08 2023 Alexander Bokovoy - 4.11.0-7 +- ipalib: fix the IPACertificate validity dates (python 3.12 compatibility) +- Handle PKI revocation response differences in JSON API +- Allow removal of minimal length from a custom password policy + * Mon Oct 23 2023 Alexander Bokovoy - 4.11.0-6 - Adopt trust to AD code to Samba changes in case SIDs are malformed