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

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 72bc8fe97..bc7fac455 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 cac524c51..1efb9339f 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 73259c30f..fa7fd2604 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@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 2b0e11106..442a4e4f7 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):