[29943] in CVS-changelog-for-Kerberos-V5

home help back first fref pref prev next nref lref last post

krb5 commit: Don't cache referral and alternate TGT replies

daemon@ATHENA.MIT.EDU (Greg Hudson)
Wed Apr 26 13:28:43 2017

Date: Wed, 26 Apr 2017 13:28:37 -0400
From: Greg Hudson <ghudson@mit.edu>
Message-Id: <201704261728.v3QHSbjY031976@drugstore.mit.edu>
To: cvs-krb5@mit.edu
Reply-To: krbdev@mit.edu
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cvs-krb5-bounces@mit.edu

https://github.com/krb5/krb5/commit/1dc619624421002b1e64d3b8c7e270508381b3e6
commit 1dc619624421002b1e64d3b8c7e270508381b3e6
Author: Greg Hudson <ghudson@mit.edu>
Date:   Tue Apr 25 17:05:23 2017 -0400

    Don't cache referral and alternate TGT replies
    
    During a TGS request, if we get a TGT response that we didn't directly
    ask for (a referral TGT or an alternate TGT), don't cache it.  It
    would have limited value in the cache as similar operations won't look
    for that TGT.  If the overall TGS operation fails and is repeated, we
    could wind up caching the same entry multiple times, which doesn't
    work well with our current ccache implementations.
    
    ticket: 8579

 src/lib/krb5/krb/get_creds.c |   13 +++----------
 src/tests/t_crossrealm.py    |   35 +++++++++++++++++++++++++++++++++++
 src/tests/t_referral.py      |   11 ++++++++---
 3 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/src/lib/krb5/krb/get_creds.c b/src/lib/krb5/krb/get_creds.c
index 110abeb..8890ee1 100644
--- a/src/lib/krb5/krb/get_creds.c
+++ b/src/lib/krb5/krb/get_creds.c
@@ -576,14 +576,6 @@ step_referrals(krb5_context context, krb5_tkt_creds_context ctx)
     }
 
     if (ctx->referral_count == 1) {
-        /* Cache the referral TGT only if it's from the local realm.
-         * Make sure to note the associated authdata, if any. */
-        code = krb5_copy_authdata(context, ctx->authdata,
-                                  &ctx->reply_creds->authdata);
-        if (code != 0)
-            return code;
-        (void) krb5_cc_store_cred(context, ctx->ccache, ctx->reply_creds);
-
         /* The authdata in this TGT will be copied into subsequent TGTs or the
          * final credentials, so we don't need to request it again. */
         krb5_free_authdata(context, ctx->in_creds->authdata);
@@ -934,8 +926,9 @@ step_get_tgt(krb5_context context, krb5_tkt_creds_context ctx)
         /* See where we wound up on the path (or off it). */
         path_realm = find_realm_in_path(context, ctx, tgt_realm);
         if (path_realm != NULL) {
-            /* We got a realm on the expected path, so we can cache it. */
-            (void) krb5_cc_store_cred(context, ctx->ccache, ctx->cur_tgt);
+            /* Only cache the TGT if we asked for it, to avoid duplicates. */
+            if (path_realm == ctx->next_realm)
+                (void)krb5_cc_store_cred(context, ctx->ccache, ctx->cur_tgt);
             if (path_realm == ctx->last_realm) {
                 /* We received a TGT for the target realm. */
                 TRACE_TKT_CREDS_TARGET_TGT(context, ctx->cur_tgt->server);
diff --git a/src/tests/t_crossrealm.py b/src/tests/t_crossrealm.py
index 1fa4879..e7ddb05 100755
--- a/src/tests/t_crossrealm.py
+++ b/src/tests/t_crossrealm.py
@@ -33,10 +33,39 @@ def stop(*realms):
         r.stop()
 
 
+# Verify that the princs appear as the service principals in the klist
+# output for the realm r, in order.
+def check_klist(r, princs):
+    out = r.run([klist])
+    count = 0
+    seen_header = False
+    for l in out.split('\n'):
+        if l.startswith('Valid starting'):
+            seen_header = True
+            continue
+        if not seen_header or l == '':
+            continue
+        if count >= len(princs):
+            fail('too many entries in klist output')
+        svcprinc = l.split()[4]
+        if svcprinc != princs[count]:
+            fail('saw service princ %s in klist output, expected %s' %
+                 (svcprinc, princs[count]))
+        count += 1
+    if count != len(princs):
+        fail('not enough entries in klist output')
+
+
+def tgt(r1, r2):
+    return 'krbtgt/%s@%s' % (r1.realm, r2.realm)
+
+
 # Basic two-realm test with cross TGTs in both directions.
 r1, r2 = cross_realms(2)
 test_kvno(r1, r2.host_princ, 'basic r1->r2')
+check_klist(r1, (tgt(r1, r1), tgt(r2, r1), r2.host_princ))
 test_kvno(r2, r1.host_princ, 'basic r2->r1')
+check_klist(r2, (tgt(r2, r2), tgt(r1, r2), r1.host_princ))
 stop(r1, r2)
 
 # Test the KDC domain walk for hierarchically arranged realms.  The
@@ -47,6 +76,7 @@ r1, r2, r3 = cross_realms(3, xtgts=((0,1), (1,2)),
                           args=({'realm': 'A.X'}, {'realm': 'X'},
                                 {'realm': 'B.X'}))
 test_kvno(r1, r3.host_princ, 'KDC domain walk')
+check_klist(r1, (tgt(r1, r1), r3.host_princ))
 stop(r1, r2, r3)
 
 # Test client capaths.  The client in A will ask for a cross TGT to D,
@@ -62,6 +92,8 @@ r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
                                     {'realm': 'D', 'krb5_conf': capaths}))
 r1client = r1.special_env('client', False, krb5_conf=capaths)
 test_kvno(r1, r4.host_princ, 'client capaths', r1client)
+check_klist(r1, (tgt(r1, r1), tgt(r2, r1), tgt(r3, r2), tgt(r4, r3),
+                 r4.host_princ))
 stop(r1, r2, r3, r4)
 
 # Test KDC capaths.  The KDCs for A and B have appropriate capaths
@@ -74,6 +106,7 @@ r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
                                     {'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))
 stop(r1, r2, r3, r4)
 
 # Test transited error.  The KDC for C does not recognize B as an
@@ -85,6 +118,7 @@ r1, r2, r3 = cross_realms(3, xtgts=((0,1), (1,2)),
                                 {'realm': 'B'}, {'realm': 'C'}))
 r1.run([kvno, r3.host_princ], expected_code=1,
        expected_msg='KDC policy rejects request')
+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
@@ -98,6 +132,7 @@ r1, r2, r3, r4 = cross_realms(4, xtgts=((0,1), (1,2), (2,3)),
                                     {'realm': 'D'}))
 r1.run([kvno, r4.host_princ], expected_code=1,
        expected_msg='Illegal cross-realm ticket')
+check_klist(r1, (tgt(r1, r1), tgt(r4, r3)))
 stop(r1, r2, r3, r4)
 
 success('Cross-realm tests')
diff --git a/src/tests/t_referral.py b/src/tests/t_referral.py
index 9765116..138fc36 100755
--- a/src/tests/t_referral.py
+++ b/src/tests/t_referral.py
@@ -17,13 +17,18 @@ os.rename(realm.ccache, savefile)
 # Get credentials and check that we got a referral to REFREALM.
 def testref(realm, nametype):
     shutil.copyfile(savefile, realm.ccache)
-    realm.run(['./gcred', nametype, 'a/x.d'])
-    realm.klist(realm.user_princ, 'a/x.d@REFREALM')
+    realm.run(['./gcred', nametype, 'a/x.d@'])
+    out = realm.run([klist]).split('\n')
+    if len(out) != 8:
+        fail('unexpected number of lines in klist output')
+    if out[5].split()[4] != 'a/x.d@' or out[6].split()[4] != 'a/x.d@REFREALM':
+        fail('unexpected service principals in klist output')
+    exit(0)
 
 # Get credentials and check that we get an error, not a referral.
 def testfail(realm, nametype):
     shutil.copyfile(savefile, realm.ccache)
-    realm.run(['./gcred', nametype, 'a/x.d'], expected_code=1,
+    realm.run(['./gcred', nametype, 'a/x.d@'], expected_code=1,
               expected_msg='not found in Kerberos database')
 
 # Create a modified KDC environment and restart the KDC.
_______________________________________________
cvs-krb5 mailing list
cvs-krb5@mit.edu
https://mailman.mit.edu/mailman/listinfo/cvs-krb5

home help back first fref pref prev next nref lref last post