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 <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index ab6cbc73f9540375f918fc26859f889265b24182..b3c1f305953d33218b94998f848b5e8e5a44ac9c 100644 (file)
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -925,6 +925,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.
  */
@@ -1017,93 +1112,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->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->principal, KRB5_NT_SRV_INST);
-               } else {
-                       ret = krb5_copy_principal(context, principal, &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->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->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->principal, lpcfg_realm(lp_ctx), samAccountName, NULL);
-               if (ret) {
-                       krb5_clear_error_message(context);
-                       goto out;
-               }
-       } else {
-               ret = krb5_copy_principal(context, principal, &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->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 */
@@ -1296,6 +1306,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->principal);
+       if (ret != 0) {
+               krb5_clear_error_message(context);
+               goto out;
+       }
+
        entry->valid_start = NULL;
 
        entry->max_life = malloc(sizeof(*entry->max_life));