krb5 commit: Do not always canonicalize enterprise principals

[Greg Hudson]

https://github.com/krb5/krb5/commit/3f5955631a2056f8ec4d1ce73d9681fa7da061c2
commit 3f5955631a2056f8ec4d1ce73d9681fa7da061c2
Author: Isaac Boukris <iboukris at gmail.com>
Date:   Sat Nov 2 13:32:32 2019 +0100

    Do not always canonicalize enterprise principals
    
    When processing an AS request in the KDC, do not assume
    KRB5_KDB_FLAG_CANONICALIZE for enterprise client names.  This change
    allows the KDB module to only canonicalize enterprise client names if
    the canonicalize flag was set on the request, as Windows does.  The
    KDB module may check the principal type and apply canonicalization as
    appropriate.
    
    [ghudson at mit.edu: edited comments; rewrote commit message]
    
    ticket: 8858 (new)

 src/include/kdb.h   |   21 ++++++++++++---------
 src/kdc/do_as_req.c |    9 ++++-----
 src/tests/t_kdb.py  |   12 ++++++++++++
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/include/kdb.h b/src/include/kdb.h
index a632de9..fc9400b 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -1057,15 +1057,18 @@ typedef struct _kdb_vftabl {
      * in-realm alias, fill in a different value for entries->princ than the
      * one requested.
      *
-     * A module can return out-of-realm referrals if KRB5_KDB_FLAG_CANONICALIZE
-     * is set.  For AS request clients (KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY is
-     * also set), the module should do so by simply filling in an out-of-realm
-     * name in entries->princ and setting all other fields to NULL.  Otherwise,
-     * the module should return the entry for the cross-realm TGS of the
-     * referred-to realm.  For TGS referals, the module can also include
-     * tl-data of type KRB5_TL_SERVER_REFERRAL containing ASN.1-encoded Windows
-     * referral data as documented in draft-ietf-krb-wg-kerberos-referrals-11
-     * appendix A; this will be returned to the client as encrypted padata.
+     * A module can return a referral to another realm if
+     * KRB5_KDB_FLAG_CANONICALIZE is set, or if
+     * KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY is set and search_for->type is
+     * KRB5_NT_ENTERPRISE_PRINCIPAL.  If KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY is
+     * set, the module should return a referral by simply filling in an
+     * out-of-realm name in (*entry)->princ and setting all other fields to
+     * NULL.  Otherwise, the module should return the entry for the cross-realm
+     * TGS of the referred-to realm.  For TGS referals, the module can also
+     * include tl-data of type KRB5_TL_SERVER_REFERRAL containing ASN.1-encoded
+     * Windows referral data as documented in
+     * draft-ietf-krb-wg-kerberos-referrals-11 appendix A; this will be
+     * returned to the client as encrypted padata.
      */
     krb5_error_code (*get_principal)(krb5_context kcontext,
                                      krb5_const_principal search_for,
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 5da8abd..fcff99f 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -596,15 +596,14 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
      * of cross realm TGS entries.
      */
     setflag(state->c_flags, KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY);
-    /*
-     * Note that according to the referrals draft we should
-     * always canonicalize enterprise principal names.
-     */
+    /* Enterprise principals are implicitly alias-ok. */
     if (isflagset(state->request->kdc_options, KDC_OPT_CANONICALIZE) ||
         state->request->client->type == KRB5_NT_ENTERPRISE_PRINCIPAL) {
-        setflag(state->c_flags, KRB5_KDB_FLAG_CANONICALIZE);
         setflag(state->c_flags, KRB5_KDB_FLAG_ALIAS_OK);
     }
+    if (isflagset(state->request->kdc_options, KDC_OPT_CANONICALIZE)) {
+        setflag(state->c_flags, KRB5_KDB_FLAG_CANONICALIZE);
+    }
     if (include_pac_p(kdc_context, state->request)) {
         setflag(state->c_flags, KRB5_KDB_FLAG_INCLUDE_PAC);
     }
diff --git a/src/tests/t_kdb.py b/src/tests/t_kdb.py
index cc5d2fc..7271fcb 100755
--- a/src/tests/t_kdb.py
+++ b/src/tests/t_kdb.py
@@ -340,11 +340,14 @@ ldap_modify('dn: krbPrincipalName=canon at KRBTEST.COM,cn=t1,cn=krb5\n'
             'changetype: modify\n'
             'add: krbPrincipalName\n'
             'krbPrincipalName: alias at KRBTEST.COM\n'
+            'krbPrincipalName: ent at abc@KRBTEST.COM\n'
             '-\n'
             'add: krbCanonicalName\n'
             'krbCanonicalName: canon at KRBTEST.COM\n')
 realm.run([kadminl, 'getprinc', 'alias'],
           expected_msg='Principal: canon at KRBTEST.COM\n')
+realm.run([kadminl, 'getprinc', 'ent\@abc'],
+          expected_msg='Principal: canon at KRBTEST.COM\n')
 realm.run([kadminl, 'getprinc', 'canon'],
           expected_msg='Principal: canon at KRBTEST.COM\n')
 realm.run([kvno, 'alias', 'canon'])
@@ -389,6 +392,15 @@ realm.run([kadminl, 'modprinc', '+requires_preauth', 'canon'])
 realm.kinit('canon', password('canon'))
 realm.kinit('alias', password('canon'), ['-C'])
 
+# Test enterprise alias with and without canonicalization.
+realm.kinit('ent at abc', password('canon'), ['-E', '-C'])
+realm.run([kvno, 'alias'])
+realm.klist('canon at KRBTEST.COM', 'alias at KRBTEST.COM')
+
+realm.kinit('ent at abc', password('canon'), ['-E'])
+realm.run([kvno, 'alias'])
+realm.klist('ent\@abc at KRBTEST.COM', 'alias at KRBTEST.COM')
+
 # Test client name canonicalization in non-krbtgt AS reply
 realm.kinit('alias', password('canon'), ['-C', '-S', 'kadmin/changepw'])