rpms/ipa
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Backport various fixes found by RHEL and upstream tests

Browse Source 
- 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 <abokovoy@redhat.com>
This commit is contained in:
Alexander Bokovoy 2023-11-08 11:50:46 +02:00
parent eb660edcd1
commit cbef046169
4 changed files with 385 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

153
freeipa-4.11-pki-revocation-changes.patch Normal file
View File

@ -0,0 +1,153 @@
From d4271391adc45c781092db0fb89b802743a9dda8 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
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 <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <flo@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
---
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 <rcritten@redhat.com>
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 <flo@redhat.com>
Reviewed-By: Thomas Woerner <twoerner@redhat.com>
---
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

135
freeipa-4.11-pwpolicy-minlength.patch Normal file
View File

@ -0,0 +1,135 @@
From 9b0b723a0e62f18d41be53900ab8a3e710708563 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
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 <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
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

88
freeipa-4.11-py3.12-timezone-changes.patch Normal file
View File

@ -0,0 +1,88 @@
From d9ad56155e76f97ad9326d5c1bcc6e19eea3a0da Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud <flo@redhat.com>
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 <flo@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
---
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

10
freeipa.spec
View File

@ -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 <abokovoy@redhat.com> - 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 <abokovoy@redhat.com> - 4.11.0-6
- Adopt trust to AD code to Samba changes in case SIDs are malformed