From 331ea51ffb9a2540ce5f4e280384c0637ed470eb Mon Sep 17 00:00:00 2001 From: liningjie Date: Sun, 27 Oct 2024 09:31:58 +0800 Subject: [PATCH] Fix CVE-2024-3183 --- CVE-2024-3183.patch | 615 ++++++++++++++++++++++++++++++++++++++++++++ freeipa.spec | 6 +- 2 files changed, 620 insertions(+), 1 deletion(-) create mode 100644 CVE-2024-3183.patch diff --git a/CVE-2024-3183.patch b/CVE-2024-3183.patch new file mode 100644 index 0000000..396744d --- /dev/null +++ b/CVE-2024-3183.patch @@ -0,0 +1,615 @@ +From 3793bc6d9b167da65e4718a2681794aba0257fe5 Mon Sep 17 00:00:00 2001 +From: Julien Rische +Date: Mon, 25 Mar 2024 18:25:52 +0200 +Subject: [PATCH] kdb: apply combinatorial logic for ticket flags + +The initial design for ticket flags was implementing this logic: +* If a ticket policy is defined for the principal entry, use flags from + this policy if they are set. Otherwise, use default ticket flags. +* If no ticket policy is defined for the principal entry, but there is a + global one, use flags from the global ticket policy if they are set. + Otherwise, use default ticket flags. +* If no policy (principal nor global) is defined, use default ticket + flags. + +However, this logic was broken by a1165ffb which introduced creation of +a principal-level ticket policy in case the ticket flag set is modified. +This was typically the case for the -allow_tix flag, which was set +virtually by the KDB driver when a user was locked until they initialize +their password on first kinit pre-authentication. + +This was causing multiple issues, which are mitigated by the new +approach: + +Now flags from each level are combined together. There flags like ++requires_preauth which are set systematically by the KDB diver, as +well as -allow_tix which is set based on the value of "nsAccountLock". +This commit also adds the implicit -allow_svr ticket flag for user +principals to protect users against Kerberoast-type attacks. None of +these flags are stored in the LDAP database, they are hard-coded in the +KDB driver. + +In addition to these "virtual" ticket flags, flags from both global and +principal ticket policies are applied (if these policies exist). + +Principal ticket policies are not supported for hosts and services, but +this is only an HTTP API limitation. The "krbTicketPolicyAux" object +class is supported for all account types. This is required for ticket +flags like +ok_to_auth_as_delegate. Such flags can be set using "ipa +host-mod" and "ipa serivce-mod", or using kadmin's "modprinc". + +It is possible to ignore flags from the global ticket policy or default +flags like -allow_svr for a user principal by setting the +"final_user_tkt_flags" string attribute to "true" in kadmin. In this +case, any ticket flag can be configured in the principal ticket policy, +except requires_preauth and allow_tix. + +When in IPA setup mode (using the "ipa-setup-override-restrictions" KDB +argument), all the system described above is disabled and ticket flags +are written in the principal ticket policy as they are provided. This is +required to initialize the Kerberos LDAP container during IPA server +installation. + +This fixes CVE-2024-3183 + +Signed-off-by: Julien Rische +--- + daemons/ipa-kdb/ipa_kdb.h | 43 ++++ + daemons/ipa-kdb/ipa_kdb_principals.c | 353 +++++++++++++++++++++++---- + util/ipa_krb5.c | 18 ++ + util/ipa_krb5.h | 4 + + 4 files changed, 365 insertions(+), 53 deletions(-) + +diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h +index 59484d8bb..1a4c71300 100644 +--- a/daemons/ipa-kdb/ipa_kdb.h ++++ b/daemons/ipa-kdb/ipa_kdb.h +@@ -94,6 +94,34 @@ + #define IPA_KRB_AUTHZ_DATA_ATTR "ipaKrbAuthzData" + #define IPA_USER_AUTH_TYPE "ipaUserAuthType" + ++/* Virtual managed ticket flags like "-allow_tix", are always controlled by the ++ * "nsAccountLock" attribute, such flags should never be set in the database. ++ * The following expression combine all of them, and is used to filter them ++ * out. */ ++#define IPA_KDB_TKTFLAGS_VIRTUAL_MANAGED_ALL (KRB5_KDB_DISALLOW_ALL_TIX) ++ ++/* Virtual static ticket flags are hard-coded in the KDB driver. */ ++/* Virtual static mandatory flags are set systematically and implicitly for all ++ * principals. They are filtered out from database ticket flags updates. ++ * (However, "KRB5_KDB_REQUIRES_PRE_AUTH" can still be unset by the ++ * "KDC:Disable Default Preauth for SPNs" global setting) */ ++#define IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_MANDATORY (KRB5_KDB_REQUIRES_PRE_AUTH) ++/* Virtual static default ticket flags are implicitly set for user and non-user ++ * (SPN) principals, and not stored in the database. ++ * (Except if the "IPA_KDB_STRATTR_FINAL_TKTFLAGS" string attribute is "true" ++ * the principal) */ ++/* Virtual static default user ticket flags are set for users only. The ++ * "-allow_svr" flag is set to protect them from CVE-2024-3183. */ ++#define IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_DEFAULTS_USER (KRB5_KDB_DISALLOW_SVR) ++#define IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_DEFAULTS_SPN (0) ++ ++/* If this string attribute is set to "true", then only the virtual managed and ++ * virtual static mandatory ticket flags are applied and filtered out from ++ * database read and write operations for the concerned user principal. ++ * Configurable principal ticket flags are applied, but not the configurable ++ * global ticket policy flags. */ ++#define IPA_KDB_STRATTR_FINAL_USER_TKTFLAGS "final_user_tkt_flags" ++ + struct ipadb_mspac; + struct dom_sid; + +@@ -187,6 +215,21 @@ struct ipadb_e_data { + struct dom_sid *sid; + }; + ++inline static krb5_error_code ++ipadb_get_edata(krb5_db_entry *entry, struct ipadb_e_data **ied) ++{ ++ struct ipadb_e_data *in_ied; ++ ++ in_ied = (struct ipadb_e_data *)entry->e_data; ++ if (!in_ied || in_ied->magic != IPA_E_DATA_MAGIC) ++ return EINVAL; ++ ++ if (ied) ++ *ied = in_ied; ++ ++ return 0; ++} ++ + struct ipadb_context *ipadb_get_context(krb5_context kcontext); + int ipadb_get_connection(struct ipadb_context *ipactx); + +diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c +index 139f091aa..4d95fba5b 100644 +--- a/daemons/ipa-kdb/ipa_kdb_principals.c ++++ b/daemons/ipa-kdb/ipa_kdb_principals.c +@@ -765,9 +765,12 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, + "krbTicketFlags", &result); + if (ret == 0) { + entry->attributes = result; +- } else { +- *polmask |= TKTFLAGS_BIT; + } ++ /* Since principal, global policy, and virtual ticket flags are combined, ++ * they must always be resolved, except if we are in IPA setup mode (because ++ * ticket policies and virtual ticket flags are irrelevant in this case). */ ++ if (!ipactx->override_restrictions) ++ *polmask |= TKTFLAGS_BIT; + + ret = ipadb_ldap_attr_to_int(lcontext, lentry, + "krbMaxTicketLife", &result); +@@ -971,7 +974,12 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, + goto done; + } + if (ret == 0) { +- ied->ipa_user = true; ++ if (1 == krb5_princ_size(kcontext, entry->princ)) { ++ /* A principal must be a POSIX account AND have only one element to ++ * be considered a user (this is to filter out CIFS principals). */ ++ ied->ipa_user = true; ++ } ++ + ret = ipadb_ldap_attr_to_str(lcontext, lentry, + "uid", &uidstring); + if (ret != 0 && ret != ENOENT) { +@@ -1339,23 +1347,150 @@ done: + return ret; + } + +-static krb5_flags maybe_require_preauth(struct ipadb_context *ipactx, +- krb5_db_entry *entry) ++static krb5_error_code ++are_final_tktflags(struct ipadb_context *ipactx, krb5_db_entry *entry, ++ bool *final_tktflags) + { +- const struct ipadb_global_config *config; ++ krb5_error_code kerr; + struct ipadb_e_data *ied; ++ char *str = NULL; ++ bool in_final_tktflags = false; + +- config = ipadb_get_global_config(ipactx); +- if (config && config->disable_preauth_for_spns) { +- ied = (struct ipadb_e_data *)entry->e_data; +- if (ied && ied->ipa_user != true) { +- /* not a user, assume SPN */ +- return 0; +- } ++ kerr = ipadb_get_edata(entry, &ied); ++ if (kerr) ++ goto end; ++ ++ if (!ied->ipa_user) { ++ kerr = 0; ++ goto end; ++ } ++ ++ kerr = krb5_dbe_get_string(ipactx->kcontext, entry, ++ IPA_KDB_STRATTR_FINAL_USER_TKTFLAGS, &str); ++ if (kerr) ++ goto end; ++ ++ in_final_tktflags = str && ipa_krb5_parse_bool(str); ++ ++end: ++ if (final_tktflags) ++ *final_tktflags = in_final_tktflags; ++ ++ krb5_dbe_free_string(ipactx->kcontext, str); ++ return kerr; ++} ++ ++static krb5_error_code ++add_virtual_static_tktflags(struct ipadb_context *ipactx, krb5_db_entry *entry, ++ krb5_flags *tktflags) ++{ ++ krb5_error_code kerr; ++ krb5_flags vsflg; ++ bool final_tktflags; ++ const struct ipadb_global_config *gcfg; ++ struct ipadb_e_data *ied; ++ ++ vsflg = IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_MANDATORY; ++ ++ kerr = ipadb_get_edata(entry, &ied); ++ if (kerr) ++ goto end; ++ ++ kerr = are_final_tktflags(ipactx, entry, &final_tktflags); ++ if (kerr) ++ goto end; ++ ++ /* In practice, principal ticket flags cannot be final for SPNs. */ ++ if (!final_tktflags) ++ vsflg |= ied->ipa_user ? IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_DEFAULTS_USER ++ : IPA_KDB_TKTFLAGS_VIRTUAL_STATIC_DEFAULTS_SPN; ++ ++ if (!ied->ipa_user) { ++ gcfg = ipadb_get_global_config(ipactx); ++ if (gcfg && gcfg->disable_preauth_for_spns) ++ vsflg &= ~KRB5_KDB_REQUIRES_PRE_AUTH; ++ } ++ ++ if (tktflags) ++ *tktflags |= vsflg; ++ ++end: ++ return kerr; ++} ++ ++static krb5_error_code ++get_virtual_static_tktflags_mask(struct ipadb_context *ipactx, ++ krb5_db_entry *entry, krb5_flags *mask) ++{ ++ krb5_error_code kerr; ++ krb5_flags flags = IPA_KDB_TKTFLAGS_VIRTUAL_MANAGED_ALL; ++ ++ kerr = add_virtual_static_tktflags(ipactx, entry, &flags); ++ if (kerr) ++ goto end; ++ ++ if (mask) ++ *mask = ~flags; ++ ++ kerr = 0; ++ ++end: ++ return kerr; ++} ++ ++/* Add ticket flags from the global ticket policy if it exists, otherwise ++ * succeed. If the global ticket policy is set, the "exists" parameter is set to ++ * true. */ ++static krb5_error_code ++add_global_ticket_policy_flags(struct ipadb_context *ipactx, ++ bool *gtpol_exists, krb5_flags *tktflags) ++{ ++ krb5_error_code kerr; ++ char *policy_dn; ++ char *tktflags_attr[] = { "krbticketflags", NULL }; ++ LDAPMessage *res = NULL, *first; ++ int ec, ldap_tktflags; ++ bool in_gtpol_exists = false; ++ ++ ec = asprintf(&policy_dn, "cn=%s,cn=kerberos,%s", ipactx->realm, ++ ipactx->base); ++ if (-1 == ec) { ++ kerr = ENOMEM; ++ goto end; + } + +- /* By default require preauth for all principals */ +- return KRB5_KDB_REQUIRES_PRE_AUTH; ++ kerr = ipadb_simple_search(ipactx, policy_dn, LDAP_SCOPE_BASE, ++ "(objectclass=krbticketpolicyaux)", ++ tktflags_attr, &res); ++ if (kerr) { ++ if (KRB5_KDB_NOENTRY == kerr) ++ kerr = 0; ++ goto end; ++ } ++ ++ first = ldap_first_entry(ipactx->lcontext, res); ++ if (!first) { ++ kerr = 0; ++ goto end; ++ } ++ ++ in_gtpol_exists = true; ++ ++ ec = ipadb_ldap_attr_to_int(ipactx->lcontext, first, "krbticketflags", ++ &ldap_tktflags); ++ if (0 == ec && tktflags) { ++ *tktflags |= (krb5_flags)ldap_tktflags; ++ } ++ ++ kerr = 0; ++ ++end: ++ if (gtpol_exists) ++ *gtpol_exists = in_gtpol_exists; ++ ++ ldap_msgfree(res); ++ free(policy_dn); ++ return kerr; + } + + static krb5_error_code ipadb_fetch_tktpolicy(krb5_context kcontext, +@@ -1368,6 +1503,7 @@ static krb5_error_code ipadb_fetch_tktpolicy(krb5_context kcontext, + char *policy_dn = NULL; + LDAPMessage *res = NULL; + LDAPMessage *first; ++ bool final_tktflags, has_local_tktpolicy = true; + int result; + int ret; + +@@ -1376,12 +1512,18 @@ static krb5_error_code ipadb_fetch_tktpolicy(krb5_context kcontext, + return KRB5_KDB_DBNOTINITED; + } + ++ kerr = are_final_tktflags(ipactx, entry, &final_tktflags); ++ if (kerr) ++ goto done; ++ + ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, + "krbticketpolicyreference", &policy_dn); + switch (ret) { + case 0: + break; + case ENOENT: ++ /* If no principal ticket policy, fallback to the global one. */ ++ has_local_tktpolicy = false; + ret = asprintf(&policy_dn, "cn=%s,cn=kerberos,%s", + ipactx->realm, ipactx->base); + if (ret == -1) { +@@ -1425,12 +1567,13 @@ static krb5_error_code ipadb_fetch_tktpolicy(krb5_context kcontext, + } + } + if (polmask & TKTFLAGS_BIT) { +- ret = ipadb_ldap_attr_to_int(ipactx->lcontext, first, +- "krbticketflags", &result); +- if (ret == 0) { +- entry->attributes |= result; +- } else { +- entry->attributes |= maybe_require_preauth(ipactx, entry); ++ /* If global ticket policy is being applied, set flags only if ++ * user principal ticket flags are not final. */ ++ if (has_local_tktpolicy || !final_tktflags) { ++ ret = ipadb_ldap_attr_to_int(ipactx->lcontext, first, ++ "krbticketflags", &result); ++ if (ret == 0) ++ entry->attributes |= result; + } + } + +@@ -1454,13 +1597,27 @@ static krb5_error_code ipadb_fetch_tktpolicy(krb5_context kcontext, + if (polmask & MAXRENEWABLEAGE_BIT) { + entry->max_renewable_life = 604800; + } +- if (polmask & TKTFLAGS_BIT) { +- entry->attributes |= maybe_require_preauth(ipactx, entry); +- } + + kerr = 0; + } + ++ if (polmask & TKTFLAGS_BIT) { ++ /* If the principal ticket flags were applied, then flags from the ++ * global ticket policy has to be applied atop of them if user principal ++ * ticket flags are not final. */ ++ if (has_local_tktpolicy && !final_tktflags) { ++ kerr = add_global_ticket_policy_flags(ipactx, NULL, ++ &entry->attributes); ++ if (kerr) ++ goto done; ++ } ++ ++ /* Virtual static ticket flags are set regardless of database content */ ++ kerr = add_virtual_static_tktflags(ipactx, entry, &entry->attributes); ++ if (kerr) ++ goto done; ++ } ++ + done: + ldap_msgfree(res); + free(policy_dn); +@@ -2044,6 +2201,36 @@ static void ipadb_mods_free_tip(struct ipadb_mods *imods) + imods->tip--; + } + ++/* Use LDAP REPLACE operation to remove an attribute. ++ * Contrary to the DELETE operation, it will not fail if the attribute does not ++ * exist. */ ++static krb5_error_code ++ipadb_ldap_replace_remove(struct ipadb_mods *imods, char *attribute) ++{ ++ krb5_error_code kerr; ++ LDAPMod *m = NULL; ++ ++ kerr = ipadb_mods_new(imods, &m); ++ if (kerr) ++ return kerr; ++ ++ m->mod_op = LDAP_MOD_REPLACE; ++ m->mod_type = strdup(attribute); ++ if (!m->mod_type) { ++ kerr = ENOMEM; ++ goto end; ++ } ++ ++ m->mod_values = NULL; ++ ++ kerr = 0; ++ ++end: ++ if (kerr) ++ ipadb_mods_free_tip(imods); ++ return kerr; ++} ++ + static krb5_error_code ipadb_get_ldap_mod_str(struct ipadb_mods *imods, + char *attribute, char *value, + int mod_op) +@@ -2460,6 +2647,93 @@ static krb5_error_code ipadb_get_ldap_mod_auth_ind(krb5_context kcontext, + return ret; + } + ++static krb5_error_code ++update_tktflags(krb5_context kcontext, struct ipadb_mods *imods, ++ krb5_db_entry *entry, int mod_op) ++{ ++ krb5_error_code kerr; ++ struct ipadb_context *ipactx; ++ struct ipadb_e_data *ied; ++ bool final_tktflags; ++ krb5_flags tktflags_mask; ++ int tktflags; ++ ++ ipactx = ipadb_get_context(kcontext); ++ if (!ipactx) { ++ kerr = KRB5_KDB_DBNOTINITED; ++ goto end; ++ } ++ ++ if (ipactx->override_restrictions) { ++ /* In IPA setup mode, IPA edata might not be available. In this mode, ++ * ticket flags are written as they are provided. */ ++ tktflags = (int)entry->attributes; ++ } else { ++ kerr = ipadb_get_edata(entry, &ied); ++ if (kerr) ++ goto end; ++ ++ kerr = get_virtual_static_tktflags_mask(ipactx, entry, &tktflags_mask); ++ if (kerr) ++ goto end; ++ ++ kerr = are_final_tktflags(ipactx, entry, &final_tktflags); ++ if (kerr) ++ goto end; ++ ++ /* Flags from the global ticket policy are filtered out only if the user ++ * principal flags are not final. */ ++ if (!final_tktflags) { ++ krb5_flags gbl_tktflags = 0; ++ ++ kerr = add_global_ticket_policy_flags(ipactx, NULL, &gbl_tktflags); ++ if (kerr) ++ goto end; ++ ++ tktflags_mask &= ~gbl_tktflags; ++ } ++ ++ tktflags = (int)(entry->attributes & tktflags_mask); ++ ++ if (LDAP_MOD_REPLACE == mod_op && ied && !ied->has_tktpolaux) { ++ if (0 == tktflags) { ++ /* No point initializing principal ticket policy if there are no ++ * flags left after filtering out virtual and global ticket ++ * policy ones. */ ++ kerr = 0; ++ goto end; ++ } ++ ++ /* if the object does not have the krbTicketPolicyAux class ++ * we need to add it or this will fail, only for modifications. ++ * We always add this objectclass by default when doing an add ++ * from scratch. */ ++ kerr = ipadb_get_ldap_mod_str(imods, "objectclass", ++ "krbTicketPolicyAux", LDAP_MOD_ADD); ++ if (kerr) ++ goto end; ++ } ++ } ++ ++ if (tktflags != 0) { ++ kerr = ipadb_get_ldap_mod_int(imods, "krbTicketFlags", tktflags, ++ mod_op); ++ if (kerr) ++ goto end; ++ } else if (LDAP_MOD_REPLACE == mod_op) { ++ /* If the principal is not being created, and there are no custom ticket ++ * flags to be set, remove the "krbTicketFlags" attribute. */ ++ kerr = ipadb_ldap_replace_remove(imods, "krbTicketFlags"); ++ if (kerr) ++ goto end; ++ } ++ ++ kerr = 0; ++ ++end: ++ return kerr; ++} ++ + static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, + struct ipadb_mods *imods, + krb5_db_entry *entry, +@@ -2535,36 +2809,9 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, + + /* KADM5_ATTRIBUTES */ + if (entry->mask & KMASK_ATTRIBUTES) { +- /* if the object does not have the krbTicketPolicyAux class +- * we need to add it or this will fail, only for modifications. +- * We always add this objectclass by default when doing an add +- * from scratch. */ +- if ((mod_op == LDAP_MOD_REPLACE) && entry->e_data) { +- struct ipadb_e_data *ied; +- +- ied = (struct ipadb_e_data *)entry->e_data; +- if (ied->magic != IPA_E_DATA_MAGIC) { +- kerr = EINVAL; +- goto done; +- } +- +- if (!ied->has_tktpolaux) { +- kerr = ipadb_get_ldap_mod_str(imods, "objectclass", +- "krbTicketPolicyAux", +- LDAP_MOD_ADD); +- if (kerr) { +- goto done; +- } +- } +- } +- +- kerr = ipadb_get_ldap_mod_int(imods, +- "krbTicketFlags", +- (int)entry->attributes, +- mod_op); +- if (kerr) { ++ kerr = update_tktflags(kcontext, imods, entry, mod_op); ++ if (kerr) + goto done; +- } + } + + /* KADM5_MAX_LIFE */ +diff --git a/util/ipa_krb5.c b/util/ipa_krb5.c +index 1ba6d25ee..2e663c506 100644 +--- a/util/ipa_krb5.c ++++ b/util/ipa_krb5.c +@@ -38,6 +38,12 @@ const char *ipapwd_password_max_len_errmsg = \ + TOSTR(IPAPWD_PASSWORD_MAX_LEN) \ + " chars)!"; + ++/* Case-insensitive string values to by parsed as boolean true */ ++static const char *const conf_yes[] = { ++ "y", "yes", "true", "t", "1", "on", ++ NULL, ++}; ++ + /* Salt types */ + #define KRB5P_SALT_SIZE 16 + +@@ -1237,3 +1243,15 @@ done: + } + return ret; + } ++ ++bool ipa_krb5_parse_bool(const char *str) ++{ ++ const char *const *p; ++ ++ for (p = conf_yes; *p; p++) { ++ if (!strcasecmp(*p, str)) ++ return true; ++ } ++ ++ return false; ++} +diff --git a/util/ipa_krb5.h b/util/ipa_krb5.h +index 7d2ebae98..d0280940a 100644 +--- a/util/ipa_krb5.h ++++ b/util/ipa_krb5.h +@@ -174,3 +174,7 @@ static inline bool + krb5_ts_after(krb5_timestamp a, krb5_timestamp b) { + return (uint32_t)a > (uint32_t)b; + } ++ ++/* Implement boolean string parsing function from MIT krb5: ++ * src/lib/krb5/krb/libdef_parse.c:_krb5_conf_boolean() */ ++bool ipa_krb5_parse_bool(const char *str); +-- +2.27.0 + diff --git a/freeipa.spec b/freeipa.spec index e21d816..ea67d63 100644 --- a/freeipa.spec +++ b/freeipa.spec @@ -94,7 +94,7 @@ Name: %{package_name} Version: %{IPA_VERSION} -Release: 6 +Release: 7 Summary: The Identity, Policy and Audit system License: GPLv3+ @@ -105,6 +105,7 @@ Source1: openEuler-platform.tar.gz Patch0001: adapt-freeipa-to-openEuler.patch Patch0002: modify-the-utils-interface.patch Patch0003: Check-the-HTTP-Referer-header-on-all-requests.patch +Patch0004: CVE-2024-3183.patch # For the timestamp trick in patch application BuildRequires: diffstat @@ -1525,6 +1526,9 @@ fi %changelog +* Sun Oct 27 2024 liningjie - 4.9.3-7 +- Fix CVE-2024-3183 + * Thu Apr 25 2024 lilong - 4.9.3-6 - backport fix CVE-2023-5455 -- Gitee