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>

[jsutton@samba.org Adapted entry to entry_ex->entry]

[jsutton@samba.org Fixed conflicts caused by superfluous whitespace]

1 files 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));