public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
* [rpms/freeipa] f44: Update patches for pr8557 and pr8558
@ 2026-09-19 10:39 Alexander Bokovoy
0 siblings, 0 replies; only message in thread
From: Alexander Bokovoy @ 2026-09-19 10:39 UTC (permalink / raw)
To: git-commits
A new commit has been pushed.
Repo : rpms/freeipa
Branch : f44
Commit : a18e10b40d98f670425c21ecfb78f40f16304ca7
Author : Alexander Bokovoy <abokovoy@redhat.com>
Date : 2026-09-19T13:38:28+03:00
Stats : +1226/-33 in 3 file(s)
URL : https://src.fedoraproject.org/rpms/freeipa/c/a18e10b40d98f670425c21ecfb78f40f16304ca7?branch=f44
Log:
Update patches for pr8557 and pr8558
Update preliminary patches to the final versions for
https://github.com/freeipa/freeipa/pull/8557.patch
https://github.com/freeipa/freeipa/pull/8558.patch
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
---
diff --git a/freeipa-pr-8557.patch b/freeipa-pr-8557.patch
index a3208d5..213ae24 100644
--- a/freeipa-pr-8557.patch
+++ b/freeipa-pr-8557.patch
@@ -1,26 +1,45 @@
-From 483ec603a06d284fd52ea27830ea412bb379165d Mon Sep 17 00:00:00 2001
+From 58ea09f489406fea624d390cee386d85aa66fd6c Mon Sep 17 00:00:00 2001
From: Julien Rische <jrische@redhat.com>
Date: Mon, 7 Sep 2026 11:16:09 +0200
-Subject: [PATCH 1/2] kdb: use "ipaOriginalUid" attr consistantly in PAC trust
- check
+Subject: [PATCH] kdb: rework cross-realm PAC validation and ID override checks
-a9e3fe97336ff942ea21af13a8b8d9adf2a89d37 added a process to cross-check
-AD client principal names against ID override based on SID. But it used
-the POSIX "uid" attribute instead of the "ipaOriginalUid" one.
+The cross-realm PAC validation code had several issues:
-f80e9667f6f6d426cfcdb287c3392e71fbd3e14c intended to fix this problem,
-but only changed the attribute name to be extrated from the fetched ID
-override LDAP entry, not the searched attribute, which results in a
-systematic failure for AD users.
+The ID override check used the POSIX "uid" attribute instead of
+"ipaOriginalUid", and compared it against the ticket cname which is
+the service principal (not the user) in S4U2Self referral TGTs.
-The present commit uses "ipaOriginalUid" for both LDAP search and entry
-attribute queries.
+The PAC cross-validation compared client_name directly against
+account_name using case-sensitive strcmp, which failed for S4U2Self
+referral TGTs where client_name is qualified ("username@userRealm"
+per MS-SFU 3.2.5.1.1) and for legitimate case differences.
+
+This commit reworks the validation to use PAC_CLIENT_INFO client_name
+as the reference for all name-based checks. MIT krb5 validates
+client_name against the correct non-PAC identity source (ticket cname,
+PA-FOR-USER, or PA-S4U-X509-USER) before the KDB plugin runs, making
+it the value IPA will use to identify the user, and the value we must
+cross-validate against canonical PAC attributes to detect alias-based
+identity fraud.
+
+Comparisons now handle the qualified ("user@REALM") and unqualified
+("user") forms of client_name: when both values are qualified, a full
+comparison including the realm is performed; otherwise only the name
+part is compared.
+
+The ID override check now compares ipaOriginalUid against client_name
+instead of the ticket cname, fixing S4U2Self flows.
+
+All comparisons are now case-insensitive, and all validation failures
+return KRB5KDC_ERR_POLICY as mandated by MS-SFU 3.2.5.1.
Signed-off-by: Julien Rische <jrische@redhat.com>
+Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
- daemons/ipa-kdb/ipa_kdb.h | 1 +
- daemons/ipa-kdb/ipa_kdb_mspac_trust.c | 6 ++++--
- 2 files changed, 5 insertions(+), 2 deletions(-)
+ daemons/ipa-kdb/ipa_kdb.h | 1 +
+ daemons/ipa-kdb/ipa_kdb_mspac_private.h | 6 +
+ daemons/ipa-kdb/ipa_kdb_mspac_trust.c | 288 ++++++++++++++----------
+ 3 files changed, 176 insertions(+), 119 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index cbac54fecc9..1c6647b7d25 100644
@@ -34,21 +53,409 @@ index cbac54fecc9..1c6647b7d25 100644
/* Virtual managed ticket flags like "-allow_tix", are always controlled by the
* "nsAccountLock" attribute, such flags should never be set in the database.
+diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
+index c76fb8e59cb..ccdcd6c59dd 100644
+--- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h
++++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
+@@ -23,6 +23,12 @@
+
+ #pragma once
+
++/* This is used as a global maximum length for:
++ * userPrincipalName: no explicit limit
++ * samAccountName: 20 for backward compatibility reasons
++ */
++#define MSPAC_ID_NAME_MAX_LENGTH (2048)
++
+ struct ipadb_mspac {
+ char *flat_domain_name;
+ char *flat_server_name;
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_trust.c b/daemons/ipa-kdb/ipa_kdb_mspac_trust.c
-index a09dc2dfbda..32dca0edbaf 100644
+index a09dc2dfbda..9dfda66337e 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac_trust.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac_trust.c
-@@ -417,7 +417,8 @@ ipadb_check_trust_view_override(krb5_context context,
+@@ -18,6 +18,60 @@
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
++ *
++ *
++ * THREAT MODEL
++ *
++ * If Active Directory does not enforce uniqueness of principal aliases
++ * (UPN), a regular AD user could set their userPrincipalName to
++ * impersonate another user (e.g. "administrator@AD.TEST"). Since IPA
++ * resolves AD user identity from the ticket client name, a forged alias
++ * could lead to privilege escalation.
++ *
++ * However, other PAC attributes (account_name from PAC_LOGON_INFO, SAM
++ * name and SID from UPN_DNS_INFO extended format) always reflect the
++ * canonical AD identity regardless of aliases. Cross-validating these
++ * against client_name detects the inconsistency.
++ *
++ * PAC_CLIENT_INFO client_name
++ *
++ * Before the KDB plugin runs, MIT krb5 validates PAC_CLIENT_INFO
++ * client_name (MS-PAC 2.7) against the appropriate non-PAC identity
++ * source (tgs_policy.c:check_tgs_s4u2self, check_normal_tgs_pac):
++ * - ticket cname for regular TGS
++ * - PA-FOR-USER for S4U2Self
++ * - PA-S4U-X509-USER for certificate-based S4U2Self
++ * This makes client_name the value that IPA will use to identify the
++ * user, and the value we must cross-validate against canonical PAC
++ * attributes to detect alias-based identity fraud.
++ *
++ * client_name is bare ("username") in regular tickets, or qualified
++ * ("username@userRealm") in S4U2Self referral TGTs (MS-SFU 3.2.5.1.1).
++ * The name part (before '@') is used for cross-validation against bare
++ * attributes. When both client_name and the tested attribute are
++ * qualified, a full comparison including the realm is performed.
++ *
++ * CHECKS PERFORMED
++ *
++ * 1. PAC cross-validation: all PAC name attributes (account_name, UPN,
++ * SAM name) must match client_name. UPN extended SID must match the
++ * logon info SID. PAC_REQUESTER_SID is not checked because it may
++ * legitimately differ in S4U delegation scenarios (MS-PAC 2.12).
++ * AD is case-insensitive, so all string comparisons use
++ * case-insensitive functions.
++ *
++ * 2. ID override validation: if a Default Trust View override exists for
++ * the PAC SID, its ipaOriginalUid must match client_name.
++ *
++ * 3. UPN qualification: for enterprise principals, the UPN must be
++ * qualified (contain '@') unless constructed (U flag), per MS-KILE
++ * 3.3.5.2.
++ *
++ * ERROR HANDLING
++ *
++ * All validation failures return KRB5KDC_ERR_POLICY (KDC_ERR_POLICY,
++ * error code 12) as mandated by MS-SFU 3.2.5.1. The status string is
++ * included in the KRB-ERROR e-text field.
+ */
+
+ #include "ipa_kdb.h"
+@@ -40,7 +94,7 @@ struct crossrealm_tgt_info {
+ /* PAC_LOGON_INFO (Type 1) - mandatory
+ * Present since Windows 2000 */
+ bool logon_info_present;
+- const char *account_name;
++ const char *account_name; /* samAccountName, always bare */
+ struct dom_sid logon_info_sid;
+ char *logon_info_sid_string;
+
+@@ -48,15 +102,17 @@ struct crossrealm_tgt_info {
+ * Present since Windows 2000 */
+ bool client_info_present;
+ const char *client_name;
++ bool client_name_is_qualified;
++ size_t unqualified_cn_len; /* length of name part (before '@') */
+
+ /* PAC_UPN_DNS_INFO (Type 12) - optional
+ * Present since Windows Server 2008 */
+ bool upn_dns_info_present;
+- const char *upn;
++ const char *upn; /* always qualified ("user@REALM") */
+ bool upn_is_qualified;
+- bool upn_is_constructed;
+- bool upn_has_sam_and_sid;
+- const char *upn_sam_name;
++ bool upn_is_constructed; /* U flag: UPN built from samAccountName */
++ bool upn_has_sam_and_sid; /* S flag: extended format present */
++ const char *upn_sam_name; /* samAccountName from extended UPN, bare */
+ struct dom_sid upn_sid;
+
+ /* PAC_REQUESTER_SID (Type 18) - optional
+@@ -99,25 +155,25 @@ ipadb_validate_pac_attributes(krb5_context context,
+ /* PAC_LOGON_INFO is mandatory */
+ if (!info->logon_info_present) {
+ *status = "TRUST_PAC_MISSING_LOGON_INFO";
+- return ENOENT;
++ return KRB5KDC_ERR_POLICY;
+ }
+
+ /* Verify account_name was successfully extracted from LOGON_INFO */
+ if (!info->account_name) {
+ *status = "TRUST_PAC_MISSING_ACCOUNT_NAME";
+- return ENOENT;
++ return KRB5KDC_ERR_POLICY;
+ }
+
+ /* PAC_CLIENT_INFO is mandatory */
+ if (!info->client_info_present) {
+ *status = "TRUST_PAC_MISSING_CLIENT_INFO";
+- return ENOENT;
++ return KRB5KDC_ERR_POLICY;
+ }
+
+ /* Verify client_name was successfully extracted from CLIENT_INFO */
+ if (!info->client_name) {
+- *status = "TRUST_PAC_MISSING_CLIENT_NAME";
+- return ENOENT;
++ *status = "TRUST_PAC_CLIENT_NAME_MISSING";
++ return KRB5KDC_ERR_POLICY;
+ }
+
+ return 0;
+@@ -134,7 +190,7 @@ ipadb_extract_crtgt_info(krb5_context context,
+ struct crossrealm_tgt_info *info,
+ const char **status)
+ {
+- krb5_error_code kerr = EINVAL;
++ krb5_error_code kerr = KRB5KDC_ERR_POLICY;
+ krb5_data linfo_blob = {0}, upn_blob = {0}, cinfo_blob = {0};
+ krb5_data req_sid_blob = {0};
+ DATA_BLOB linfo_data, upn_data, cinfo_data, req_sid_data;
+@@ -170,22 +226,22 @@ ipadb_extract_crtgt_info(krb5_context context,
+
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ *status = "TRUST_PAC_CANNOT_PARSE_LOGON_INFO";
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+
+ /* Check if logon_info structure is valid */
+ if (!logon_info.info) {
+ *status = "TRUST_PAC_LOGON_INFO_UNDEFINED";
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+
+ /* Store pointer to account name */
+ info->account_name = logon_info.info->info3.base.account_name.string;
+ if (!info->account_name) {
+- *status = "TRUST_PAC_ACCOUNT_NAME_UNDEFINED";
+- kerr = EINVAL;
++ *status = "TRUST_PAC_ACCOUNT_NAME_MISSING";
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+
+@@ -225,8 +281,18 @@ ipadb_extract_crtgt_info(krb5_context context,
+ (ndr_pull_flags_fn_t)ndr_pull_PAC_INFO);
+ krb5_free_data_contents(context, &cinfo_blob);
+
+- if (NDR_ERR_CODE_IS_SUCCESS(ndr_err))
++ if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
++ const char *at_sign;
++
+ info->client_name = client_info.logon_name.account_name;
++ if (info->client_name) {
++ at_sign = strchr(info->client_name, '@');
++ info->client_name_is_qualified = (at_sign != NULL);
++ info->unqualified_cn_len = at_sign ?
++ (size_t)(at_sign - info->client_name) :
++ strlen(info->client_name);
++ }
++ }
+ }
+
+ /* Extract Type 12 - PRIMARY UPN SOURCE
+@@ -248,14 +314,18 @@ ipadb_extract_crtgt_info(krb5_context context,
+
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ *status = "TRUST_PAC_CANNOT_PARSE_UPN_DNS_INFO";
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+
+ /* Store pointer to UPN string */
+ info->upn = upn_info.upn_dns_info.upn_name;
+- if (info->upn)
+- info->upn_is_qualified = (NULL != strchr(info->upn, '@'));
++ if (!info->upn) {
++ *status = "TRUST_PAC_UPN_MISSING";
++ kerr = KRB5KDC_ERR_POLICY;
++ goto end;
++ }
++ info->upn_is_qualified = (NULL != strchr(info->upn, '@'));
+
+ /* Extract U flag - indicates constructed UPN */
+ info->upn_is_constructed =
+@@ -309,119 +379,100 @@ ipadb_extract_crtgt_info(krb5_context context,
+ return kerr;
+ }
+
+-/* Cross-validate PAC attributes for consistency
+- * All available attributes must match across different PAC buffers.
+- * LOGON_INFO and CLIENT_INFO must be present (asserted).
+- */
++/* Compare a PAC name attribute against client_name (case-insensitive).
++ * If both are qualified, compare fully. Otherwise compare name parts only,
++ * expecting '@' (qualified) or '\0' (bare) after the matched prefix.
++ * Returns true if the names differ. */
++static bool
++client_name_differs(const struct crossrealm_tgt_info *info,
++ const char *tested, bool qualified)
++{
++ return (qualified && info->client_name_is_qualified)
++ ? (0 != strcasecmp(info->client_name, tested))
++ : (0 != strncasecmp(info->client_name, tested,
++ info->unqualified_cn_len)
++ || tested[info->unqualified_cn_len] != (qualified ? '@' : '\0'));
++}
++
++/* Cross-validate PAC name attributes and SIDs against client_name.
++ * See file header for design rationale. */
+ static krb5_error_code
+ ipadb_cross_validate_pac_attributes(krb5_context context,
+ const struct crossrealm_tgt_info *info,
+ const char **status)
+ {
+- krb5_error_code kerr = EINVAL;
+- const char *at_sign = NULL;
+- char *ticket_cname = NULL;
+- size_t name_len;
++ krb5_error_code kerr = KRB5KDC_ERR_POLICY;
+
+ /* Mandatory PAC attributes must have been validated already */
+ assert(info->logon_info_present);
+ assert(info->client_info_present);
+
+- /* Validate UPN matches ticket cname (for enterprise principals) */
+- if (info->is_enterprise_princ && info->upn_dns_info_present) {
+- /* Unparse client principal name (without realm) */
+- kerr = krb5_unparse_name_flags(context, info->client_princ,
+- KRB5_PRINCIPAL_UNPARSE_NO_REALM,
+- &ticket_cname);
+- if (kerr) {
+- *status = "TRUST_CANNOT_UNPARSE_CNAME";
+- goto end;
+- }
+-
+- /* Find the @ sign in UPN */
+- at_sign = strchr(info->upn, '@');
+- if (at_sign) {
+- name_len = at_sign - info->upn;
+-
+- /* Compare UPN name part with ticket cname */
+- if (0 != strncmp(info->upn, ticket_cname, name_len) ||
+- ticket_cname[name_len] != '\0') {
+- *status = "TRUST_PAC_UPN_CNAME_MISMATCH";
+- kerr = EINVAL;
+- goto end;
+- }
+- }
+-
+- krb5_free_unparsed_name(context, ticket_cname);
+- ticket_cname = NULL;
++ /* account_name (bare) vs client_name */
++ if (client_name_differs(info, info->account_name, false)) {
++ *status = "TRUST_PAC_ACCOUNT_NAME_MISMATCH";
++ kerr = KRB5KDC_ERR_POLICY;
++ goto end;
+ }
+
+- /* Cross-validate account names if extended UPN format is available */
+- if (info->upn_has_sam_and_sid) {
+- if (!info->account_name || !info->upn_sam_name ||
+- 0 != strcmp(info->account_name, info->upn_sam_name)) {
+- *status = "TRUST_PAC_ACCOUNT_NAME_MISMATCH";
+- kerr = EINVAL;
++ /* UPN (qualified) vs client_name, enterprise principals only.
++ * For enterprise principals, the AD KDC looks up the user by UPN
++ * (MS-KILE 3.3.5.6.1), so a forged UPN alias could produce a
++ * client_name that differs from account_name. For regular principals
++ * the lookup is by samAccountName, and the UPN may legitimately
++ * differ, so this check would cause false positives. */
++ if (info->is_enterprise_princ && info->upn_dns_info_present) {
++ if (client_name_differs(info, info->upn, true)) {
++ *status = "TRUST_PAC_UPN_MISMATCH";
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+ }
+
+- /* Cross-validate client name matches account name */
+- if (info->client_info_present) {
+- if (!info->client_name || !info->account_name ||
+- 0 != strcmp(info->client_name, info->account_name)) {
+- *status = "TRUST_PAC_CLIENT_NAME_MISMATCH";
+- kerr = EINVAL;
++ /* UPN SAM name (bare) vs client_name */
++ if (info->upn_has_sam_and_sid) {
++ if (!info->upn_sam_name ||
++ client_name_differs(info, info->upn_sam_name, false)) {
++ *status = "TRUST_PAC_UPN_SAM_NAME_MISMATCH";
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+ }
+
+- /* Cross-validate SIDs if extended UPN format is available */
++ /* UPN SID vs logon info SID */
+ if (info->upn_has_sam_and_sid) {
+ if (!dom_sid_check(&info->logon_info_sid,
+ &info->upn_sid, true)) {
+ *status = "TRUST_PAC_UPN_SID_MISMATCH";
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+ }
+
+- /* Cross-validate SID from REQUESTER_SID buffer if present */
+- if (info->requester_sid_present) {
+- if (!dom_sid_check(&info->logon_info_sid,
+- &info->requester_sid, true)) {
+- *status = "TRUST_PAC_REQUESTER_SID_MISMATCH";
+- kerr = EINVAL;
+- goto end;
+- }
+- }
++ /* PAC_REQUESTER_SID is not checked against logon_info_sid because
++ * in S4U delegation scenarios, the requester SID is the service's
++ * SID, not the impersonated user's (MS-PAC 2.12). */
+
+ kerr = 0;
+
+ end:
+- if (ticket_cname)
+- krb5_free_unparsed_name(context, ticket_cname);
+-
+ return kerr;
+ }
+
+-/* Check Default Trust View override username consistency
+- * If a user override exists for this SID in the Default Trust View,
+- * verify that the override username matches the ticket cname.
+- * LOGON_INFO and CLIENT_INFO must be present (asserted).
+- */
++/* If a Default Trust View override exists for this SID, verify that
++ * ipaOriginalUid (always qualified) matches client_name. */
+ static krb5_error_code
+ ipadb_check_trust_view_override(krb5_context context,
+ const struct crossrealm_tgt_info *info,
+ const char **status)
{
struct ipadb_context *ipactx = NULL;
- krb5_error_code kerr = EINVAL;
+- krb5_error_code kerr = EINVAL;
- char *basedn = NULL, *filter = NULL, *attrs[] = {"uid", NULL};
++ krb5_error_code kerr = KRB5KDC_ERR_POLICY;
+ char *basedn = NULL, *filter = NULL;
+ char *attrs[] = {IPA_ORIGINAL_UID_ATTR, NULL};
LDAPMessage *res = NULL, *entry = NULL;
struct berval **uid_values = NULL;
- char *ticket_cname = NULL;
-@@ -478,7 +479,8 @@ ipadb_check_trust_view_override(krb5_context context,
+- char *ticket_cname = NULL;
+ int count;
++ bool mismatch;
+
+ /* Mandatory PAC attributes must have been validated already */
+ assert(info->logon_info_present);
+@@ -470,35 +521,45 @@ ipadb_check_trust_view_override(krb5_context context,
+ goto end;
+ }
+
+- /* Override exists - verify the username matches the ticket cname */
++ /* Override exists - verify the username matches client_name */
+ entry = ldap_first_entry(ipactx->lcontext, res);
+ if (!entry) {
+ *status = "TRUST_CANNOT_READ_OVERRIDE_ENTRY";
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
goto end;
}
@@ -57,4 +464,93 @@ index a09dc2dfbda..32dca0edbaf 100644
+ IPA_ORIGINAL_UID_ATTR);
if (!uid_values || !uid_values[0]) {
*status = "TRUST_OVERRIDE_UID_UNDEFINED";
- kerr = EINVAL;
+- kerr = EINVAL;
++ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+
+- /* Unparse client principal name (without realm) */
+- kerr = krb5_unparse_name_flags(context, info->client_princ,
+- KRB5_PRINCIPAL_UNPARSE_NO_REALM,
+- &ticket_cname);
+- if (kerr) {
+- *status = "TRUST_CANNOT_UNPARSE_CNAME";
+- goto end;
++ /* Compare ipaOriginalUid (always qualified) against client_name.
++ * When client_name is qualified, compare full value including realm.
++ * When bare, compare name part only and verify ipaOriginalUid
++ * continues with '@' at that position. */
++ if (info->client_name_is_qualified) {
++ /* Full comparison: client_name length must match bv_len */
++ mismatch = (strlen(info->client_name) != uid_values[0]->bv_len ||
++ 0 != strncasecmp(info->client_name,
++ uid_values[0]->bv_val,
++ uid_values[0]->bv_len));
++ } else {
++ /* Name part only: unqualified_cn_len must be less than bv_len,
++ * the name part must match, and bv_val must have '@' at that
++ * position */
++ mismatch = (info->unqualified_cn_len >= uid_values[0]->bv_len ||
++ 0 != strncasecmp(info->client_name,
++ uid_values[0]->bv_val,
++ info->unqualified_cn_len) ||
++ uid_values[0]->bv_val[info->unqualified_cn_len] != '@');
+ }
+
+- /* Compare override username with ticket cname */
+- if (0 != strncmp(uid_values[0]->bv_val, ticket_cname,
+- uid_values[0]->bv_len) ||
+- '\0' != ticket_cname[uid_values[0]->bv_len]) {
+- *status = "TRUST_OVERRIDE_UID_CNAME_MISMATCH";
++ if (mismatch) {
++ *status = "TRUST_OVERRIDE_UID_MISMATCH";
+ kerr = KRB5KDC_ERR_POLICY;
+ goto end;
+ }
+@@ -508,8 +569,6 @@ ipadb_check_trust_view_override(krb5_context context,
+ end:
+ if (uid_values)
+ ldap_value_free_len(uid_values);
+- if (ticket_cname)
+- krb5_free_unparsed_name(context, ticket_cname);
+ if (res)
+ ldap_msgfree(res);
+ if (filter)
+@@ -520,10 +579,8 @@ ipadb_check_trust_view_override(krb5_context context,
+ return kerr;
+ }
+
+-/* Check that enterprise principal has qualified UPN in PAC
+- * UPN MUST be qualified if it is NOT constructed (U flag not set).
+- * LOGON_INFO and CLIENT_INFO must be present (asserted).
+- */
++/* For enterprise principals, UPN must be qualified unless constructed
++ * (U flag set), per MS-KILE 3.3.5.2. */
+ static krb5_error_code
+ ipadb_check_upn_qualified(krb5_context context,
+ const struct crossrealm_tgt_info *info,
+@@ -558,14 +615,7 @@ ipadb_check_upn_qualified(krb5_context context,
+ return 0;
+ }
+
+-/* Main function to check trust PAC content
+- *
+- * Performs security checks on trust TGS-REQ:
+- * 1. Cross-validation - verify PAC attributes consistency
+- * 2. User registration - requester SID must exist in Default Trust View
+- * 3. UPN qualification - enterprise principals must have qualified UPN
+- * (if not constructed)
+- */
++/* Entry point for trust PAC content checks. See file header. */
+ krb5_error_code
+ ipadb_check_trust_pac_content(krb5_context context,
+ const krb5_kdc_req *request,
+@@ -574,7 +624,7 @@ ipadb_check_trust_pac_content(krb5_context context,
+ const char **status)
+ {
+ struct crossrealm_tgt_info info;
+- krb5_error_code kerr = EINVAL;
++ krb5_error_code kerr = KRB5KDC_ERR_POLICY;
+
+ /* Initialize PAC check info structure and create talloc context */
+ kerr = ipadb_trust_pac_info_init(&info);
diff --git a/freeipa-pr-8558.patch b/freeipa-pr-8558.patch
index a9aa494..043f07b 100644
--- a/freeipa-pr-8558.patch
+++ b/freeipa-pr-8558.patch
@@ -1,7 +1,7 @@
-From 3c37b5ab9bdb0d1788180b586b5f814535d892cf Mon Sep 17 00:00:00 2001
+From 8f33c1304dec167e844074e89fca857fd3a40df0 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Mon, 7 Sep 2026 16:12:33 +0300
-Subject: [PATCH] Allow fine tuned privilege check
+Subject: [PATCH 1/4] Allow fine tuned privilege check
Let users pass through the privilege check if either
- whole entry read granted ('v' in the GER response)
@@ -10,19 +10,74 @@ Let users pass through the privilege check if either
This is enough to allow LDAPRetrieve to pass, because the rest is
controlled by the explicit ACIs in LDAP.
+Remove permission checks from member-management commands as they apply
+those ACI checks per each individual member (some might fail, some will
+succeed). The partial success is supported and expected by tests.
+
Related: CVE-2026-79678
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
---
- ipaserver/plugins/baseldap.py | 27 ++++++++++++++++++++++-----
- ipaserver/plugins/privilege.py | 29 +++++++++++++++++++++--------
- 2 files changed, 43 insertions(+), 13 deletions(-)
+ ipaserver/plugins/baseldap.py | 98 ++++++++++++++++++++++++++--------
+ ipaserver/plugins/privilege.py | 29 +++++++---
+ 2 files changed, 98 insertions(+), 29 deletions(-)
diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
-index d693c7112..e751dc673 100644
+index d693c7112d5..122a279f2bb 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
-@@ -1054,11 +1054,22 @@ last, after all sets and adds."""),
+@@ -1001,7 +1001,10 @@ class BaseLDAPCommand(Method):
+ enforce_managed_permission_operations = []
+
+ def enforce_managed_permissions(self, *keys, **options):
+- from ipaserver.plugins.privilege import principal_has_privilege
++ # avoid circular import through explicit plugin reference
++ principal_has_privilege = (
++ self.api.packages[0].privilege.principal_has_privilege
++ )
+ mp = getattr(self.obj, 'managed_permissions', None)
+ if not mp:
+ return
+@@ -1024,23 +1027,31 @@ def has_privilege(priv):
+ self.api, op_account, priv)
+ return priv_cache[priv]
+
+- # Effective rights, if needed at all, are fetched once in a single
+- # get-effective-rights round trip shared by every permission below.
+- # probe['res'] is (entry_rights, {attr: rights}) or None (no entry).
+- probe = {}
++ # LDAPDelete and other LDAPMultiQuery commands pass the trailing
++ # primary key as a *list* of pkeys and act on each target entry in
++ # turn (see LDAPDelete.execute). get_dn and the effective-rights probes
++ # below operate on a single entry, so expand the operation into its
++ # individual targets and require the right on every one of them.
++ target_keys = self._expand_target_keys(keys)
+
+- def rights_allow(right, perm, touched):
++ # Effective rights, if needed at all, are fetched at most once per
++ # target in a single get-effective-rights round trip shared by every
++ # permission checked for that target. probes[i]['res'] is
++ # (entry_rights, {attr: rights}) or None (no entry).
++ probes = [{} for _ in target_keys]
++
++ def right_allowed_for(right, perm, touched, probe, tkeys):
+ if right == 'add':
+ # The target does not exist yet and target-scoped/SELFDN add
+ # ACIs cannot be evaluated through a get-effective-rights
+ # template probe, so detect the right with a trial add of the
+ # real target entry (see _can_add_target).
+ if 'add' not in probe:
+- probe['add'] = self._can_add_target(*keys, **options)
++ probe['add'] = self._can_add_target(*tkeys, **options)
+ return probe['add']
+ if 'res' not in probe:
+ probe['res'] = self._probe_effective_rights(
+- mp, enforce_mp, *keys, **options)
++ mp, enforce_mp, *tkeys, **options)
+ res = probe['res']
+ if res is None:
+ # Entry does not exist; let the operation report it properly.
+@@ -1054,11 +1065,31 @@ def rights_allow(right, perm, touched):
if not attrs:
return True
return any('w' in attr_rights.get(a, '') for a in attrs)
@@ -46,10 +101,40 @@ index d693c7112..e751dc673 100644
return True
- return any('r' in attr_rights.get(a, '') for a in attrs)
+ return any('r' in r for r in attr_rights.values())
++
++ def rights_allow(right, perm, touched):
++ # Permit only if the caller may perform the operation on *every*
++ # target entry the command names (a batch delete/retrieve must be
++ # authorised for all of its pkeys).
++ return all(
++ right_allowed_for(right, perm, touched, probe, tkeys)
++ for probe, tkeys in zip(probes, target_keys)
++ )
modified_attrs = None # computed lazily, only for 'write'
for m in mp.keys():
-@@ -1214,8 +1225,14 @@ last, after all sets and adds."""),
+@@ -1106,6 +1137,20 @@ def rights_allow(right, perm, touched):
+ raise errors.ACIError(
+ info=_("not allowed to perform this operation"))
+
++ def _expand_target_keys(self, keys):
++ """Expand an operation's keys into one key-tuple per target entry.
++
++ LDAPDelete and other LDAPMultiQuery commands pass the trailing primary
++ key as a list of pkeys and iterate over it, calling get_dn once per
++ entry (see LDAPDelete.execute). get_dn and the effective-rights probes
++ expect a single pkey, so mirror that expansion here. For ordinary
++ single-target commands the keys are returned unchanged as one tuple.
++ """
++ if keys and self.obj.primary_key and isinstance(
++ keys[-1], (list, tuple)):
++ return [keys[:-1] + (pkey,) for pkey in keys[-1]]
++ return [tuple(keys)]
++
+ def _modified_attributes(self, **options):
+ """Best-effort set of LDAP attributes an update operation changes.
+
+@@ -1214,8 +1259,14 @@ def _probe_effective_rights(self, mp, enforce_mp, *keys, **options):
continue
needed.update(a.lower() for a in perm.get('ipapermdefaultattr', ()))
@@ -65,8 +150,71 @@ index d693c7112..e751dc673 100644
except errors.NotFound:
return None
+@@ -2020,7 +2071,9 @@ class LDAPAddMember(LDAPModMember):
+ member_param_doc = _('%s to add')
+ member_count_out = ('%i member added.', '%i members added.')
+ allow_same = False
+- enforce_managed_permission_operations = ['write']
++ # Member writes are enforced per-member by 389-ds and reported gracefully
++ # in the command's `failed` output, so no API-level gate is applied here
++ # (an up-front ACIError would abort the whole batch instead).
+
+ has_output = (
+ output.Entry('result'),
+@@ -2037,8 +2090,6 @@ class LDAPAddMember(LDAPModMember):
+ has_output_params = global_output_params
+
+ def execute(self, *keys, **options):
+- self.enforce_managed_permissions(*keys, **options)
+-
+ ldap = self.obj.backend
+
+ (member_dns, failed) = self.get_member_dns(**options)
+@@ -2121,7 +2172,9 @@ class LDAPRemoveMember(LDAPModMember):
+ """
+ member_param_doc = _('%s to remove')
+ member_count_out = ('%i member removed.', '%i members removed.')
+- enforce_managed_permission_operations = ['write']
++ # Member writes are enforced per-member by 389-ds and reported gracefully
++ # in the command's `failed` output, so no API-level gate is applied here
++ # (an up-front ACIError would abort the whole batch instead).
+
+ has_output = (
+ output.Entry('result'),
+@@ -2138,8 +2191,6 @@ class LDAPRemoveMember(LDAPModMember):
+ has_output_params = global_output_params
+
+ def execute(self, *keys, **options):
+- self.enforce_managed_permissions(*keys, **options)
+-
+ ldap = self.obj.backend
+
+ (member_dns, failed) = self.get_member_dns(**options)
+@@ -2715,8 +2766,6 @@ def _update_attrs(self, update, entry_attrs):
+ )
+
+ def execute(self, *keys, **options):
+- self.enforce_managed_permissions(*keys, **options)
+-
+ ldap = self.obj.backend
+ try:
+ index = tuple(self.args).index(self.attribute)
+@@ -2725,6 +2774,13 @@ def execute(self, *keys, **options):
+ else:
+ obj_keys = keys[:index]
+
++ # Enforce on the target entry's own keys. For the arg-based variants
++ # the attribute value is a trailing positional arg that is not part of
++ # the entry's DN (see obj_keys), so it must be excluded here: passing
++ # the full keys would make the effective-rights probe's get_dn target a
++ # bogus DN and silently bypass the check.
++ self.enforce_managed_permissions(*obj_keys, **options)
++
+ dn = self.obj.get_dn(*obj_keys, **options)
+ entry_attrs = ldap.make_entry(dn, self.args_options_2_entry(
+ *keys, **options))
diff --git a/ipaserver/plugins/privilege.py b/ipaserver/plugins/privilege.py
-index 47f27f12c..65199adc0 100644
+index 47f27f12c7d..65199adc0d8 100644
--- a/ipaserver/plugins/privilege.py
+++ b/ipaserver/plugins/privilege.py
@@ -92,11 +92,28 @@ def principal_has_privilege(api, principal, privilege):
@@ -113,6 +261,551 @@ index 47f27f12c..65199adc0 100644
# Second try: Check if there is an idoverride for the principal as
# ipaOriginalUid that has the needed privilege.
filter = ldap.make_filter(
---
-2.55.0
+From b1af1d8bbfad25c59073b6197808d57197c5062b Mon Sep 17 00:00:00 2001
+From: Alexander Bokovoy <abokovoy@redhat.com>
+Date: Tue, 8 Sep 2026 09:54:57 +0300
+Subject: [PATCH 2/4] ipatests: fix XMLRPC test expecations
+
+Two classes of behavior changes:
+
+ - permission baseline is now 'read', not 'write'
+ - self-service LDAP ACI is slightly different
+
+Related: CVE-2026-79678
+
+Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
+---
+ .../test_xmlrpc/test_permission_plugin.py | 46 ++++++++++++-------
+ .../test_xmlrpc/test_selfservice_plugin.py | 8 ++--
+ 2 files changed, 33 insertions(+), 21 deletions(-)
+
+diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
+index dbcd1173d7f..84b88e0c764 100644
+--- a/ipatests/test_xmlrpc/test_permission_plugin.py
++++ b/ipatests/test_xmlrpc/test_permission_plugin.py
+@@ -3182,6 +3182,18 @@ class test_permission_bindtype(Declarative):
+ expected=dict(
+ value=permission1,
+ summary=u'Added permission "%s"' % permission1,
++ messages=(
++ {
++ 'message': ('The permission has read rights but no '
++ 'attributes are set.'),
++ 'code': 13032,
++ 'type': 'warning',
++ 'name': 'MissingTargetAttributesinPermission',
++ 'data': {
++ 'right': 'read',
++ }
++ },
++ ),
+ result=dict(
+ dn=permission1_dn,
+ cn=[permission1],
+@@ -3245,13 +3257,13 @@ class test_permission_bindtype(Declarative):
+ summary=u'Modified permission "%s"' % permission1,
+ messages=(
+ {
+- 'message': ('The permission has write rights but no '
++ 'message': ('The permission has read rights but no '
+ 'attributes are set.'),
+ 'code': 13032,
+ 'type': 'warning',
+ 'name': 'MissingTargetAttributesinPermission',
+ 'data': {
+- 'right': 'write',
++ 'right': 'read',
+ }
+ },
+ ),
+@@ -3260,7 +3272,7 @@ class test_permission_bindtype(Declarative):
+ cn=[permission1],
+ objectclass=objectclasses.permission,
+ type=[u'user'],
+- ipapermright=[u'write'],
++ ipapermright=[u'read'],
+ ipapermbindruletype=[u'all'],
+ ipapermissiontype=[u'SYSTEM', u'V2'],
+ ipapermlocation=[users_dn],
+@@ -3272,7 +3284,7 @@ class test_permission_bindtype(Declarative):
+ permission1, users_dn,
+ '(targetfilter = "(objectclass=posixaccount)")' +
+ '(version 3.0;acl "permission:%s";' % permission1 +
+- 'allow (write) userdn = "ldap:///all";)',
++ 'allow (read) userdn = "ldap:///all";)',
+ ),
+
+ dict(
+@@ -3301,7 +3313,7 @@ class test_permission_bindtype(Declarative):
+ dn=permission1_dn,
+ cn=[permission1],
+ type=[u'user'],
+- ipapermright=[u'write'],
++ ipapermright=[u'read'],
+ ipapermbindruletype=[u'all'],
+ objectclass=objectclasses.permission,
+ ipapermissiontype=[u'SYSTEM', u'V2'],
+@@ -3348,13 +3360,13 @@ class test_permission_bindtype(Declarative):
+ summary=u'Modified permission "%s"' % permission1,
+ messages=(
+ {
+- 'message': ('The permission has write rights but no '
++ 'message': ('The permission has read rights but no '
+ 'attributes are set.'),
+ 'code': 13032,
+ 'type': 'warning',
+ 'name': 'MissingTargetAttributesinPermission',
+ 'data': {
+- 'right': 'write',
++ 'right': 'read',
+ }
+ },
+ ),
+@@ -3363,7 +3375,7 @@ class test_permission_bindtype(Declarative):
+ cn=[permission1_renamed],
+ type=[u'user'],
+ objectclass=objectclasses.permission,
+- ipapermright=[u'write'],
++ ipapermright=[u'read'],
+ ipapermbindruletype=[u'all'],
+ ipapermissiontype=[u'SYSTEM', u'V2'],
+ ipapermlocation=[users_dn],
+@@ -3375,7 +3387,7 @@ class test_permission_bindtype(Declarative):
+ permission1_renamed, users_dn,
+ '(targetfilter = "(objectclass=posixaccount)")' +
+ '(version 3.0;acl "permission:%s";' % permission1_renamed +
+- 'allow (write) userdn = "ldap:///all";)',
++ 'allow (read) userdn = "ldap:///all";)',
+ ),
+
+ dict(
+@@ -3391,13 +3403,13 @@ class test_permission_bindtype(Declarative):
+ summary=u'Modified permission "%s"' % permission1_renamed,
+ messages=(
+ {
+- 'message': ('The permission has write rights but no '
++ 'message': ('The permission has read rights but no '
+ 'attributes are set.'),
+ 'code': 13032,
+ 'type': 'warning',
+ 'name': 'MissingTargetAttributesinPermission',
+ 'data': {
+- 'right': 'write',
++ 'right': 'read',
+ }
+ },
+ ),
+@@ -3406,7 +3418,7 @@ class test_permission_bindtype(Declarative):
+ cn=[permission1_renamed],
+ objectclass=objectclasses.permission,
+ type=[u'user'],
+- ipapermright=[u'write'],
++ ipapermright=[u'read'],
+ ipapermbindruletype=[u'permission'],
+ ipapermissiontype=[u'SYSTEM', u'V2'],
+ ipapermlocation=[users_dn],
+@@ -3418,7 +3430,7 @@ class test_permission_bindtype(Declarative):
+ permission1_renamed, users_dn,
+ '(targetfilter = "(objectclass=posixaccount)")' +
+ '(version 3.0;acl "permission:%s";' % permission1_renamed +
+- 'allow (write) groupdn = "ldap:///%s";)' % permission1_renamed_dn,
++ 'allow (read) groupdn = "ldap:///%s";)' % permission1_renamed_dn,
+ ),
+
+ dict(
+@@ -3432,13 +3444,13 @@ class test_permission_bindtype(Declarative):
+ summary=u'Modified permission "%s"' % permission1_renamed,
+ messages=(
+ {
+- 'message': ('The permission has write rights but no '
++ 'message': ('The permission has read rights but no '
+ 'attributes are set.'),
+ 'code': 13032,
+ 'type': 'warning',
+ 'name': 'MissingTargetAttributesinPermission',
+ 'data': {
+- 'right': 'write',
++ 'right': 'read',
+ }
+ },
+ ),
+@@ -3447,7 +3459,7 @@ class test_permission_bindtype(Declarative):
+ cn=[permission1],
+ type=[u'user'],
+ objectclass=objectclasses.permission,
+- ipapermright=[u'write'],
++ ipapermright=[u'read'],
+ ipapermbindruletype=[u'permission'],
+ ipapermissiontype=[u'SYSTEM', u'V2'],
+ ipapermlocation=[users_dn],
+@@ -3459,7 +3471,7 @@ class test_permission_bindtype(Declarative):
+ permission1, users_dn,
+ '(targetfilter = "(objectclass=posixaccount)")' +
+ '(version 3.0;acl "permission:%s";' % permission1 +
+- 'allow (write) groupdn = "ldap:///%s";)' % permission1_dn,
++ 'allow (read) groupdn = "ldap:///%s";)' % permission1_dn,
+ ),
+
+ dict(
+diff --git a/ipatests/test_xmlrpc/test_selfservice_plugin.py b/ipatests/test_xmlrpc/test_selfservice_plugin.py
+index 3ce043e46a4..be86c5a6727 100644
+--- a/ipatests/test_xmlrpc/test_selfservice_plugin.py
++++ b/ipatests/test_xmlrpc/test_selfservice_plugin.py
+@@ -803,7 +803,7 @@ class test_selfservice_cli_add_del(Declarative):
+ r'(version 3.0;acl '
+ r'\22selfservice:selfservice_add_1002\22;'
+ r'allow (write) (userdn = \22ldap:///self\22 '
+- r'and userdn = \22ldap:///all\22;)',
++ r'and userdn = \22ldap:///all\22);)',
+ ),
+ ),
+
+@@ -881,7 +881,7 @@ class test_selfservice_cli_add_del(Declarative):
+ r'(version 3.0;acl '
+ r'\22selfservice:selfservice_add_1005\22;'
+ r'allow (write) (userdn = \22ldap:///self\22 '
+- r'and userdn = \22ldap:///all\22;)',
++ r'and userdn = \22ldap:///all\22);)',
+ ),
+ ),
+
+@@ -1591,7 +1591,7 @@ class test_selfservice_mod_cli(Declarative):
+ r'(version 3.0;acl '
+ r'\22selfservice:%s\22;'
+ r'allow (write) (userdn = \22ldap:///self\22 '
+- r'and userdn = \22ldap:///all\22;)'
++ r'and userdn = \22ldap:///all\22);)'
+ ) % SS_CLI_MOD,
+ ),
+ ),
+@@ -1658,7 +1658,7 @@ class test_selfservice_mod_cli(Declarative):
+ r'(version 3.0;acl '
+ r'\22selfservice:%s\22;'
+ r'allow (write) (userdn = \22ldap:///self\22 '
+- r'and userdn = \22ldap:///all\22;)'
++ r'and userdn = \22ldap:///all\22);)'
+ ) % SS_CLI_MOD,
+ ),
+ ),
+
+From 29ec4764e04c2841d001478fd63ba6ea06d3c7aa Mon Sep 17 00:00:00 2001
+From: Alexander Bokovoy <abokovoy@redhat.com>
+Date: Tue, 8 Sep 2026 12:57:44 +0300
+Subject: [PATCH 3/4] permission enforcement: allow services to define keys to
+ enforce
+
+service_add_smb constructs cifs/hostname out of hostname. We need to
+adjust what the permission check is using for an ADD test per command.
+
+Related: CVE-2026-79678
+
+Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
+---
+ ipaserver/plugins/baseldap.py | 16 ++++++++++++++++
+ ipaserver/plugins/service.py | 7 +++++++
+ 2 files changed, 23 insertions(+)
+
+diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
+index 122a279f2bb..926afb17ec7 100644
+--- a/ipaserver/plugins/baseldap.py
++++ b/ipaserver/plugins/baseldap.py
+@@ -1027,6 +1027,11 @@ def has_privilege(priv):
+ self.api, op_account, priv)
+ return priv_cache[priv]
+
++ # Some commands construct the real target principal/DN only later (in
++ # pre_callback), so let them supply the keys the probes should target
++ # (e.g. service-add-smb builds cifs/<hostname> from a bare hostname).
++ keys = self._enforcement_keys(*keys, **options)
++
+ # LDAPDelete and other LDAPMultiQuery commands pass the trailing
+ # primary key as a *list* of pkeys and act on each target entry in
+ # turn (see LDAPDelete.execute). get_dn and the effective-rights probes
+@@ -1137,6 +1142,17 @@ def rights_allow(right, perm, touched):
+ raise errors.ACIError(
+ info=_("not allowed to perform this operation"))
+
++ def _enforcement_keys(self, *keys, **options):
++ """Keys identifying the target entry for permission enforcement.
++
++ Defaults to the command's own keys. Commands whose real target DN is
++ derived only later -- e.g. service-add-smb, which takes a bare hostname
++ and constructs the ``cifs/<hostname>`` principal in pre_callback --
++ override this so the effective-rights/trial-add probes target the
++ actual entry rather than a DN built from the raw key.
++ """
++ return keys
++
+ def _expand_target_keys(self, keys):
+ """Expand an operation's keys into one key-tuple per target entry.
+
+diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
+index bbe71ddc78d..51ea015f401 100644
+--- a/ipaserver/plugins/service.py
++++ b/ipaserver/plugins/service.py
+@@ -810,6 +810,13 @@ def get_options(self):
+ if check:
+ yield arg
+
++ def _enforcement_keys(self, *keys, **options):
++ # The primary key here is a bare hostname; the real target principal
++ # (cifs/<hostname>) is only built in pre_callback. Enforce the add
++ # against that actual principal so the "Hosts can add own services"
++ # ACI (target krbprincipalname=*/($dn)@$REALM) is matched correctly.
++ return ('cifs/{}'.format(keys[0]),)
++
+ def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
+ *keys, **options):
+ assert isinstance(dn, DN)
+
+From 6eabd3c675a9d09667d6319ac65a6104a66b5f4f Mon Sep 17 00:00:00 2001
+From: Alexander Bokovoy <abokovoy@redhat.com>
+Date: Tue, 8 Sep 2026 18:04:52 +0300
+Subject: [PATCH 4/4] ipa-pwd-extop: allow password expiration override on
+ create
+
+Permit to create accounts with a limited principal password expiration
+but not beyond what Kerberos password policy defines for this account.
+
+Additionally, fix timezone-related bugs in setting password expiration.
+
+Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
+---
+ .../ipa-slapi-plugins/ipa-pwd-extop/common.c | 2 +-
+ .../ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 50 +++++++++++
+ ipapython/ipaldap.py | 21 ++++-
+ ipatests/test_integration/test_commands.py | 86 +++++++++++++++++++
+ 4 files changed, 154 insertions(+), 5 deletions(-)
+
+diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+index 71642540a20..ebae0465b66 100644
+--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
++++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+@@ -799,7 +799,7 @@ int ipapwd_setdate(Slapi_Entry *source, Slapi_Mods *smods, const char *attr,
+ Slapi_Attr *t;
+ bool exists;
+
+- exists = (slapi_entry_attr_find(source, attr, &t) == 0);
++ exists = (source != NULL && slapi_entry_attr_find(source, attr, &t) == 0);
+
+ if (remove) {
+ if (exists) {
+diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+index 4bfd60e7c3f..71ccc5f0712 100644
+--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
++++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+@@ -400,6 +400,40 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
+ goto done;
+ }
+
++ /* If the entry being added already carries a krbPasswordExpiration, treat
++ * it as an explicit request. An explicit request overrides the default
++ * expiration derived by ipapwd_CheckPolicy (which, for an admin-set
++ * password, force-expires the account), but is still capped at the policy
++ * maximum lifetime -- a password must never outlive policy. A
++ * max_pwd_life of 0 means "never expire".
++ *
++ * This is only meaningful on LDAP ADD, where the attribute value is the
++ * request; on a password change it holds the previous expiration and must
++ * not constrain the new one. */
++ {
++ char *req_exp_str;
++ time_t req_exp;
++
++ req_exp_str = slapi_entry_attr_get_charptr(e, "krbPasswordExpiration");
++ req_exp = ipapwd_gentime_to_time_t(req_exp_str);
++ slapi_ch_free_string(&req_exp_str);
++
++ if (req_exp != 0) {
++ time_t policy_max = 0;
++
++ if (pwdop->pwdata.policy.max_pwd_life > 0) {
++ policy_max = pwdop->pwdata.timeNow +
++ pwdop->pwdata.policy.max_pwd_life;
++ }
++
++ if (policy_max != 0 && req_exp > policy_max) {
++ pwdop->pwdata.expireTime = policy_max;
++ } else {
++ pwdop->pwdata.expireTime = req_exp;
++ }
++ }
++ }
++
+ if (is_krb || is_smb || is_ipant) {
+
+ Slapi_Value **svals = NULL;
+@@ -1150,6 +1184,22 @@ static int ipapwd_post_modadd(Slapi_PBlock *pb)
+ }
+ }
+
++ /* The date-setting and grace-time logic below needs the current entry to
++ * detect already-present single-valued attributes (e.g. a client-supplied
++ * krbPasswordExpiration on add) so ipapwd_setdate can REPLACE rather than
++ * append a duplicate value. On add -- and on password changes that skipped
++ * history -- pwdata.target is still NULL, so fetch the just-committed entry
++ * (freed with the mod path's entry at 'done'). */
++ if (pwdop->pwdata.target == NULL) {
++ Slapi_DN *tmp_dn = slapi_sdn_new_dn_byref(pwdop->pwdata.dn);
++ if (tmp_dn) {
++ (void)slapi_search_internal_get_entry(tmp_dn, 0,
++ &pwdop->pwdata.target,
++ ipapwd_plugin_id);
++ slapi_sdn_free(&tmp_dn);
++ }
++ }
++
+ /* we assume that krb attributes are properly updated too if keys were
+ * passed in */
+ if (!pwdop->skip_keys) {
+diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
+index 1e56aad87d8..a3f1319c431 100644
+--- a/ipapython/ipaldap.py
++++ b/ipapython/ipaldap.py
+@@ -23,7 +23,7 @@
+ import errno
+ import logging
+ import time
+-from datetime import datetime
++from datetime import datetime, timezone
+ from decimal import Decimal
+ from copy import deepcopy
+ import contextlib
+@@ -972,6 +972,20 @@ def get_attribute_single_value(self, name_or_oid):
+
+ return None
+
++ @staticmethod
++ def _encode_datetime(val):
++ """Translate a datetime to an LDAP GeneralizedTime string.
++
++ The GeneralizedTime format used here (``...Z``) denotes UTC, so a
++ timezone-aware value must be converted to UTC first; otherwise its
++ wall-clock components would be emitted verbatim and mislabelled as
++ UTC. A naive datetime is assumed to already be in UTC (the convention
++ used throughout IPA).
++ """
++ if val.tzinfo is not None:
++ val = val.astimezone(timezone.utc)
++ return val.strftime(LDAP_GENERALIZED_TIME_FORMAT)
++
+ def encode(self, val):
+ """
+ Encode attribute value to LDAP representation (str/bytes).
+@@ -999,7 +1013,7 @@ def encode(self, val):
+ dct = dict((k, self.encode(v)) for k, v in val.items())
+ return dct
+ elif isinstance(val, datetime):
+- return val.strftime(LDAP_GENERALIZED_TIME_FORMAT).encode('utf-8')
++ return self._encode_datetime(val).encode('utf-8')
+ elif isinstance(val, x509.IPACertificate):
+ return val.public_bytes(x509.Encoding.DER)
+ elif val is None:
+@@ -1389,8 +1403,7 @@ def make_filter_from_attr(
+ value = u'\\'.join(
+ value[i:i+2] for i in six.moves.range(-2, len(value), 2))
+ elif isinstance(value, datetime):
+- value = value.strftime(
+- LDAP_GENERALIZED_TIME_FORMAT)
++ value = cls._encode_datetime(value)
+ value = ldap.filter.escape_filter_chars(value)
+ else:
+ value = str(value)
+diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
+index 4d221abc208..d370b49a05b 100644
+--- a/ipatests/test_integration/test_commands.py
++++ b/ipatests/test_integration/test_commands.py
+@@ -471,6 +471,92 @@ def test_change_sysaccount_password_issue7561(self):
+ tasks.ldappasswd_sysaccount_change(sysuser, original_passwd,
+ new_passwd, master)
+
++ def test_user_add_password_expiration_bounded_by_policy(self):
++ """user-add with --password and an explicit --password-expiration.
++
++ The ipa-pwd-extop DS plugin must:
++ * store a single value of the single-valued krbPasswordExpiration
++ attribute (it used to append a second, policy-derived value on add,
++ failing the operation with a schema violation), and
++ * honor an explicitly requested expiration but bound it by the
++ effective password policy, so a password never outlives policy.
++ """
++ master = self.master
++ tasks.kinit_admin(master)
++ base_dn = str(master.domain.basedn)
++
++ # Effective global policy maximum password lifetime, in seconds
++ # (krbMaxPwdLife is stored in seconds; 0 means "never expire").
++ result = master.run_command(['ipa', 'pwpolicy-show', '--raw'])
++ match = re.search(r'krbmaxpwdlife:\s*(\d+)', result.stdout_text)
++ max_life_secs = int(match.group(1)) if match else 0
++
++ def add_user(user, exp_dt):
++ master.run_command(
++ ['ipa', 'user-add', user, '--first', user, '--last', user,
++ '--password', '--password-expiration',
++ exp_dt.strftime('%Y%m%d%H%M%SZ')],
++ stdin_text='Secret123\nSecret123\n')
++
++ def stored_expirations(user):
++ result = tasks.ldapsearch_dm(
++ master,
++ 'uid={user},cn=users,cn=accounts,{base_dn}'.format(
++ user=user, base_dn=base_dn),
++ ['krbpasswordexpiration'],
++ scope='base')
++ values = re.findall(r'krbpasswordexpiration: (\S+)',
++ result.stdout_text.lower())
++ return [datetime.strptime(v.strip().upper(), '%Y%m%d%H%M%SZ')
++ for v in values]
++
++ # A request comfortably within policy must be honored verbatim.
++ within_user = 'pwdexpwithin'
++ if max_life_secs:
++ within_delta = timedelta(seconds=min(max_life_secs // 2, 86400))
++ else:
++ within_delta = timedelta(days=1)
++ requested = datetime.utcnow() + within_delta
++ # second precision only: krbPasswordExpiration has no sub-second part
++ requested = requested.replace(microsecond=0)
++ try:
++ add_user(within_user, requested)
++ stored = stored_expirations(within_user)
++ assert len(stored) == 1, (
++ 'krbPasswordExpiration must be single-valued, got %r' % stored)
++ assert abs((stored[0] - requested).total_seconds()) < 120, (
++ 'within-policy request %s not honored, stored %s'
++ % (requested, stored[0]))
++ finally:
++ master.run_command(['ipa', 'user-del', within_user],
++ raiseonerr=False)
++
++ # A request far beyond policy must be capped at the policy maximum
++ # (only meaningful when the policy actually sets a maximum lifetime).
++ if max_life_secs:
++ over_user = 'pwdexpover'
++ requested = (datetime.utcnow()
++ + timedelta(seconds=max_life_secs)
++ + timedelta(days=3650))
++ requested = requested.replace(microsecond=0)
++ try:
++ add_user(over_user, requested)
++ stored = stored_expirations(over_user)
++ assert len(stored) == 1, (
++ 'krbPasswordExpiration must be single-valued, got %r'
++ % stored)
++ assert stored[0] < requested - timedelta(days=1), (
++ 'beyond-policy request was not capped, stored %s'
++ % stored[0])
++ cap = (datetime.utcnow() + timedelta(seconds=max_life_secs)
++ + timedelta(days=1))
++ assert stored[0] <= cap, (
++ 'stored expiration %s exceeds policy cap %s'
++ % (stored[0], cap))
++ finally:
++ master.run_command(['ipa', 'user-del', over_user],
++ raiseonerr=False)
++
+ def get_krbinfo(self, user):
+ base_dn = str(self.master.domain.basedn)
+ result = tasks.ldapsearch_dm(
diff --git a/freeipa.spec b/freeipa.spec
index 0364fa9..e1d54a6 100644
--- a/freeipa.spec
+++ b/freeipa.spec
@@ -211,7 +211,7 @@
Name: %{package_name}
Version: %{IPA_VERSION}
-Release: 1.2%{?rc_version:.%rc_version}%{?dist}
+Release: 2%{?rc_version:.%rc_version}%{?dist}
Summary: The Identity, Policy and Audit system
License: GPL-3.0-or-later
@@ -1976,6 +1976,10 @@ fi
%endif
%changelog
+* Sat Sep 19 2026 Alexander Bokovoy <abokovoy@redhat.com> - 4.13.4-2
+- Update post-4.13.4-release patches
+- Rebuild against Samba 4.25.0-RC2
+
* Mon Sep 07 2026 Alexander Bokovoy <abokovoy@redhat.com> - 4.13.4-1.2
- Fine-tune privilege checks (upstream PR 8558)
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-09-19 10:39 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 10:39 [rpms/freeipa] f44: Update patches for pr8557 and pr8558 Alexander Bokovoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox