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]

diff --git a/selftest/knownfail.d/kadmin_changepw b/selftest/knownfail.d/kadmin_changepw
deleted file mode 100644 (file)
index 97c1479..0000000
--- 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 5cd8615f6a943c2ead9d25b03f3d0e3d1f64670a..49ab29f115daf66b75e1cf8af8a38a6553be50a5 100644 (file)
--- 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 385c118a0732cb141f2d6336bc493dddfcc064ad..d2d7136608eeca8886f9cb36815ab940bdfb8533 100644 (file)
--- 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) {