CVE-2022-2031 s4:kdc: Fix canonicalisation of kadmin/changepw principal

Since this principal goes through the samba_kdc_fetch_server() path,
setting the canonicalisation flag would cause the principal to be
replaced with the sAMAccountName; this meant requests to
kadmin/changepw@REALM would result in a ticket to krbtgt@REALM. Now we
properly handle canonicalisation for the kadmin/changepw principal.

View with 'git show -b'.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047

Pair-Programmed-With: Andreas Schneider <asn@samba.org>
Signed-off-by: Andreas Schneider <asn@samba.org>
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; removed MIT KDC
 1.20-specific knownfails]

3 files changed, 46 insertions, 41 deletions

diff --git a/selftest/knownfail.d/kadmin_changepw b/selftest/knownfail.d/kadmin_changepw
deleted file mode 100644
index 97c14793ea5..00000000000
--- a/selftest/knownfail.d/kadmin_changepw
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password
diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc
index 5cd8615f6a9..49ab29f115d 100644
--- a/selftest/knownfail_heimdal_kdc
+++ b/selftest/knownfail_heimdal_kdc
@@ -274,8 +274,6 @@
 #
 # Kpasswd tests
 #
-^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc
-^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_from_rodc.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_lifetime.ad_dc
 ^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 385c118a073..d2d7136608e 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -830,6 +830,7 @@ static krb5_error_code samba_kdc_get_entry_principal(
 		const char *samAccountName,
 		enum samba_kdc_ent_type ent_type,
 		unsigned flags,
+		bool is_kadmin_changepw,
 		krb5_const_principal in_princ,
 		krb5_principal *out_princ)
 {
@@ -849,46 +850,52 @@ static krb5_error_code samba_kdc_get_entry_principal(
 	 * fixed UPPER case realm, but the as-sent username
 	 */
 
-	if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) {
-		/*
-		 * When requested to do so, ensure that the
-		 * both realm values in the principal are set
-		 * to the upper case, canonical realm
-		 */
-		code = smb_krb5_make_principal(context,
-					       out_princ,
-					       lpcfg_realm(lp_ctx),
-					       "krbtgt",
-					       lpcfg_realm(lp_ctx),
-					       NULL);
-		if (code != 0) {
-			return code;
-		}
-		smb_krb5_principal_set_type(context,
-					    *out_princ,
-					    KRB5_NT_SRV_INST);
+	/*
+	 * We need to ensure that the kadmin/changepw principal isn't able to
+	 * issue krbtgt tickets, even if canonicalization is turned on.
+	 */
+	if (!is_kadmin_changepw) {
+		if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) {
+			/*
+			 * When requested to do so, ensure that the
+			 * both realm values in the principal are set
+			 * to the upper case, canonical realm
+			 */
+			code = smb_krb5_make_principal(context,
+						       out_princ,
+						       lpcfg_realm(lp_ctx),
+						       "krbtgt",
+						       lpcfg_realm(lp_ctx),
+						       NULL);
+			if (code != 0) {
+				return code;
+			}
+			smb_krb5_principal_set_type(context,
+						    *out_princ,
+						    KRB5_NT_SRV_INST);
 
-		return 0;
-	}
+			return 0;
+		}
 
-	if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) ||
-	    (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) {
-		/*
-		 * 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.
-		 */
-		code = smb_krb5_make_principal(context,
-					      out_princ,
-					      lpcfg_realm(lp_ctx),
-					      samAccountName,
-					      NULL);
-		return code;
+		if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) ||
+		    (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) {
+			/*
+			 * 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.
+			 */
+			code = smb_krb5_make_principal(context,
+						      out_princ,
+						      lpcfg_realm(lp_ctx),
+						      samAccountName,
+						      NULL);
+			return code;
+		}
 	}
 
 	/*
@@ -1194,6 +1201,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
 					    samAccountName,
 					    ent_type,
 					    flags,
+					    entry_ex->entry.flags.change_pw,
 					    principal,
 					    &entry_ex->entry.principal);
 	if (ret != 0) {