krb5 commit: Honor transited-policy-checked flag in servers

Greg Hudson ghudson at mit.edu
Fri Jan 24 21:15:55 EST 2020


https://github.com/krb5/krb5/commit/a5aa5969bc6ed404b86318b47c38dfc3d3aeb8df
commit a5aa5969bc6ed404b86318b47c38dfc3d3aeb8df
Author: Greg Hudson <ghudson at mit.edu>
Date:   Fri Jan 24 10:25:18 2020 -0500

    Honor transited-policy-checked flag in servers
    
    For consistency with Heimdal and simplicity of server configuration,
    do not check the transited field in krb5_rd_req() if the
    transited-policy-checked flag is set in the ticket.
    
    Add a cross-realm test using the gcred and rdreq harnesses to test
    server transited processing.  Also fix the KDC capaths case so that
    the client actually doesn't know the path to the server realm.  In
    k5test.py, adjust _cfg_merge() to remove keys mapped to None in the
    second dictionary (instead of mapping them to None in the result), so
    that deleting whole sections works.  Remove the corresponding check
    for None in _write_cfg_section() as it is no longer needed.
    
    ticket: 8870 (new)
    tags: pullup
    target_version: 1.18

 src/lib/krb5/krb/rd_req_dec.c |   11 ++++++---
 src/tests/gcred.c             |   10 +++++++-
 src/tests/t_crossrealm.py     |   43 +++++++++++++++++++++++++++++++++++-----
 src/util/k5test.py            |    6 +++-
 4 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/src/lib/krb5/krb/rd_req_dec.c b/src/lib/krb5/krb/rd_req_dec.c
index 72bc8fe..bc7fac4 100644
--- a/src/lib/krb5/krb/rd_req_dec.c
+++ b/src/lib/krb5/krb/rd_req_dec.c
@@ -564,16 +564,19 @@ rd_req_decoded_opt(krb5_context context, krb5_auth_context *auth_context,
     {
         krb5_data      * realm;
         krb5_transited * trans;
+        krb5_flags       flags;
 
         realm = &req->ticket->enc_part2->client->realm;
         trans = &(req->ticket->enc_part2->transited);
+        flags = req->ticket->enc_part2->flags;
 
         /*
-         * If the transited list is not empty, then check that all realms
-         * transited are within the hierarchy between the client's realm
-         * and the local realm.
+         * If the transited list is not empty and the KDC hasn't checked it,
+         * then check that all realms transited are within the hierarchy
+         * between the client's realm and the local realm.
          */
-        if (trans->tr_contents.length > 0 && trans->tr_contents.data[0]) {
+        if (!(flags & TKT_FLG_TRANSIT_POLICY_CHECKED) &&
+            trans->tr_contents.length > 0 && trans->tr_contents.data[0]) {
             retval = krb5_check_transited_list(context, &(trans->tr_contents),
                                                realm, &server->realm);
         }
diff --git a/src/tests/gcred.c b/src/tests/gcred.c
index cac524c..1efb933 100644
--- a/src/tests/gcred.c
+++ b/src/tests/gcred.c
@@ -33,7 +33,7 @@
 /*
  * This program is intended to be run from a python script as:
  *
- *     gcred nametype princname
+ *     gcred [-f] [-t] nametype princname
  *
  * where nametype is one of "unknown", "principal", "srv-inst", and "srv-hst",
  * and princname is the name of the service principal.  gcred acquires
@@ -41,6 +41,9 @@
  * the server principal name of the obtained credentials to stdout and exits
  * with status 0.  On failure, gcred displays the error message for the failed
  * operation to stderr and exits with status 1.
+ *
+ * The -f and -t flags set the KRB5_GC_FORWARDABLE and KRB5_GC_NO_TRANSIT_CHECK
+ * options respectively.
  */
 
 #include "k5-int.h"
@@ -73,11 +76,14 @@ main(int argc, char **argv)
 
     check(krb5_init_context(&ctx));
 
-    while ((c = getopt(argc, argv, "f")) != -1) {
+    while ((c = getopt(argc, argv, "ft")) != -1) {
         switch (c) {
         case 'f':
             options |= KRB5_GC_FORWARDABLE;
             break;
+        case 't':
+            options |= KRB5_GC_NO_TRANSIT_CHECK;
+            break;
         default:
             abort();
         }
diff --git a/src/tests/t_crossrealm.py b/src/tests/t_crossrealm.py
index 73259c3..fa7fd26 100755
--- a/src/tests/t_crossrealm.py
+++ b/src/tests/t_crossrealm.py
@@ -107,8 +107,9 @@ r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
                                     {'realm': 'B', 'krb5_conf': capaths},
                                     {'realm': 'C', 'krb5_conf': capaths},
                                     {'realm': 'D', 'krb5_conf': capaths}))
-test_kvno(r1, r4.host_princ, 'KDC capaths')
-check_klist(r1, (tgt(r1, r1), tgt(r4, r3), r4.host_princ))
+r1client = r1.special_env('client', False, krb5_conf={'capaths': None})
+test_kvno(r1, r4.host_princ, 'KDC capaths', r1client)
+check_klist(r1, (tgt(r1, r1), r4.host_princ))
 stop(r1, r2, r3, r4)
 
 # A capaths value of '.' should enforce direct cross-realm, with no
@@ -135,9 +136,39 @@ r1.run([kvno, r3.host_princ], expected_code=1,
 check_klist(r1, (tgt(r1, r1), tgt(r3, r2)))
 stop(r1, r2, r3)
 
-# Test a different kind of transited error.  The KDC for D does not
-# recognize B as an intermediate realm for A->C, so it refuses to
-# verify the krbtgt/C at B ticket in the TGS AP-REQ.
+# Test server transited checking.  The KDC for C recognizes B as an
+# intermediate realm for A->C, but the server environment does not.
+# The server should honor the ticket if the transited-policy-checked
+# flag is set, but not if it isn't.  (It is only possible for our KDC
+# to issue a ticket without the transited-policy-checked flag with
+# reject_bad_transit=false.)
+mark('server transited checking')
+capaths = {'capaths': {'A': {'C': 'B'}}}
+noreject = {'realms': {'$realm': {'reject_bad_transit': 'false'}}}
+r1, r2, r3 = cross_realms(3, xtgts=((0,1), (1,2)),
+                          args=({'realm': 'A', 'krb5_conf': capaths},
+                                {'realm': 'B'},
+                                {'realm': 'C', 'krb5_conf': capaths,
+                                 'kdc_conf': noreject}))
+r3server = r3.special_env('server', False, krb5_conf={'capaths': None})
+# Process a ticket with the transited-policy-checked flag set.
+shutil.copy(r1.ccache, r1.ccache + '.copy')
+r1.run(['./gcred', 'principal', r3.host_princ])
+os.rename(r1.ccache, r3.ccache)
+r3.run(['./rdreq', r3.host_princ], env=r3server, expected_msg='0 success')
+# Try again with the transited-policy-checked flag unset.
+os.rename(r1.ccache + '.copy', r1.ccache)
+r1.run(['./gcred', '-t', 'principal', r3.host_princ])
+os.rename(r1.ccache, r3.ccache)
+r3.run(['./rdreq', r3.host_princ], env=r3server,
+       expected_msg='43 Illegal cross-realm ticket')
+stop(r1, r2, r3)
+
+# Test a four-realm scenario.  This test used to result in an "Illegal
+# cross-realm ticket" error as the KDC for D would refuse to process
+# the cross-realm ticket from C.  Now that we honor the
+# transited-policy-checked flag in krb5_rd_req(), it instead issues a
+# policy error as in the three-realm scenario.
 mark('transited error (four realms)')
 capaths = {'capaths': {'A': {'D': ['B', 'C'], 'C': 'B'}, 'B': {'D': 'C'}}}
 r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
@@ -146,7 +177,7 @@ r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
                                     {'realm': 'C', 'krb5_conf': capaths},
                                     {'realm': 'D'}))
 r1.run([kvno, r4.host_princ], expected_code=1,
-       expected_msg='Illegal cross-realm ticket')
+       expected_msg='KDC policy rejects request')
 check_klist(r1, (tgt(r1, r1), tgt(r4, r3)))
 stop(r1, r2, r3, r4)
 
diff --git a/src/util/k5test.py b/src/util/k5test.py
index 2b0e111..442a4e4 100644
--- a/src/util/k5test.py
+++ b/src/util/k5test.py
@@ -635,7 +635,9 @@ def _cfg_merge(cfg1, cfg2):
         return cfg2
     result = cfg1.copy()
     for key, value2 in cfg2.items():
-        if value2 is None or key not in result:
+        if value2 is None:
+            result.pop(key, None)
+        elif key not in result:
             result[key] = value2
         else:
             value1 = result[key]
@@ -960,7 +962,7 @@ class K5Realm(object):
                 # A string value yields a straightforward variable setting.
                 value = self._subst_cfg_value(value)
                 file.write('%s%s = %s\n' % (indent, name, value))
-            elif value is not None:
+            else:
                 raise TypeError()
 
     def _subst_cfg_value(self, value):


More information about the cvs-krb5 mailing list