From f68877af829bf73da8e965c9458a9846d1757038 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: CVE-2022-2031 s4:kdc: Split out a samba_kdc_get_entry_principal() function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider [jsutton@samba.org Adapted entry to entry_ex->entry] [jsutton@samba.org Fixed conflicts caused by superfluous whitespace] --- source4/kdc/db-glue.c | 192 ++++++++++++++++++++++++++++---------------------- 1 file changed, 107 insertions(+), 85 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 45159e6e64d..ac0c206b5c1 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -824,6 +824,101 @@ static bool is_kadmin_changepw(krb5_context context, (principal_comp_strcmp(context, principal, 1, "changepw") == 0); } +static krb5_error_code samba_kdc_get_entry_principal( + krb5_context context, + struct samba_kdc_db_context *kdc_db_ctx, + const char *samAccountName, + enum samba_kdc_ent_type ent_type, + unsigned flags, + krb5_const_principal in_princ, + krb5_principal *out_princ) +{ + struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; + krb5_error_code ret = 0; + + /* + * If we are set to canonicalize, we get back the fixed UPPER + * case realm, and the real username (ie matching LDAP + * samAccountName) + * + * Otherwise, if we are set to enterprise, we + * get back the whole principal as-sent + * + * Finally, if we are not set to canonicalize, we get back the + * fixed UPPER case realm, but the as-sent username + */ + + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { + if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { + /* + * When requested to do so, ensure that the + * both realm values in the principal are set + * to the upper case, canonical realm + */ + ret = smb_krb5_make_principal(context, out_princ, + lpcfg_realm(lp_ctx), "krbtgt", + lpcfg_realm(lp_ctx), NULL); + if (ret) { + return ret; + } + smb_krb5_principal_set_type(context, *out_princ, KRB5_NT_SRV_INST); + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + /* + * this appears to be required regardless of + * the canonicalize flag from the client + */ + ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL) { + ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } else if ((flags & SDB_F_FORCE_CANON) || + ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { + /* + * SDB_F_CANON maps from the canonicalize flag in the + * packet, and has a different meaning between AS-REQ + * and TGS-REQ. We only change the principal in the AS-REQ case + * + * The SDB_F_FORCE_CANON if for new MIT KDC code that wants + * the canonical name in all lookups, and takes care to + * canonicalize only when appropriate. + */ + ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + + /* While we have copied the client principal, tests + * show that Win2k3 returns the 'corrected' realm, not + * the client-specified realm. This code attempts to + * replace the client principal's realm with the one + * we determine from our records */ + + /* this has to be with malloc() */ + ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + return 0; +} + /* * Construct an hdb_entry from a directory entry. */ @@ -913,93 +1008,8 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, userAccountControl |= msDS_User_Account_Control_Computed; } - /* - * If we are set to canonicalize, we get back the fixed UPPER - * case realm, and the real username (ie matching LDAP - * samAccountName) - * - * Otherwise, if we are set to enterprise, we - * get back the whole principal as-sent - * - * Finally, if we are not set to canonicalize, we get back the - * fixed UPPER case realm, but the as-sent username - */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { p->is_krbtgt = true; - - if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { - /* - * When requested to do so, ensure that the - * both realm values in the principal are set - * to the upper case, canonical realm - */ - ret = smb_krb5_make_principal(context, &entry_ex->entry.principal, - lpcfg_realm(lp_ctx), "krbtgt", - lpcfg_realm(lp_ctx), NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - smb_krb5_principal_set_type(context, entry_ex->entry.principal, KRB5_NT_SRV_INST); - } else { - ret = krb5_copy_principal(context, principal, &entry_ex->entry.principal); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - /* - * this appears to be required regardless of - * the canonicalize flag from the client - */ - ret = smb_krb5_principal_set_realm(context, entry_ex->entry.principal, lpcfg_realm(lp_ctx)); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } - - } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && principal == NULL) { - ret = smb_krb5_make_principal(context, &entry_ex->entry.principal, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } else if ((flags & SDB_F_FORCE_CANON) || - ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { - /* - * SDB_F_CANON maps from the canonicalize flag in the - * packet, and has a different meaning between AS-REQ - * and TGS-REQ. We only change the principal in the AS-REQ case - * - * The SDB_F_FORCE_CANON if for new MIT KDC code that wants - * the canonical name in all lookups, and takes care to - * canonicalize only when appropriate. - */ - ret = smb_krb5_make_principal(context, &entry_ex->entry.principal, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - } else { - ret = krb5_copy_principal(context, principal, &entry_ex->entry.principal); - if (ret) { - krb5_clear_error_message(context); - goto out; - } - - /* While we have copied the client principal, tests - * show that Win2k3 returns the 'corrected' realm, not - * the client-specified realm. This code attempts to - * replace the client principal's realm with the one - * we determine from our records */ - - /* this has to be with malloc() */ - ret = smb_krb5_principal_set_realm(context, entry_ex->entry.principal, lpcfg_realm(lp_ctx)); - if (ret) { - krb5_clear_error_message(context); - goto out; - } } /* First try and figure out the flags based on the userAccountControl */ @@ -1185,6 +1195,18 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, } } + ret = samba_kdc_get_entry_principal(context, + kdc_db_ctx, + samAccountName, + ent_type, + flags, + principal, + &entry_ex->entry.principal); + if (ret != 0) { + krb5_clear_error_message(context); + goto out; + } + entry_ex->entry.valid_start = NULL; entry_ex->entry.max_life = malloc(sizeof(*entry_ex->entry.max_life)); -- cgit v1.2.1