[krbdev.mit.edu #8579] duplicate caching of some cross-realm TGTs

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[krbdev.mit.edu #8579] duplicate caching of some cross-realm TGTs

Greg Hudson via RT
>From 6ab3a05d5e3a6ffa211b91b172f737a262239fd0 Mon Sep 17 00:00:00 2001
From: "Richard E. Silverman" <[hidden email]>
Date: Tue, 18 Apr 2017 22:20:50 -0400
Subject: [PATCH] Fix repeated caching of certain credentials.

Under certain circumstances, we would get duplicate caching of cross-realm
tickets. krb5_cc_store_cred() had two problems:

1) It avoided duplicate caching the second time it (potentially) stored a
   credential, but not the first time (which is where this was happening).

2) The method it used to avoid duplicates was flawed anyway. It called
   krb5_cc_remove_cred() before storing, to first remove any matching
   credentials. This seems like an odd approach to begin with... but
   worse: krb5_cc_remove_cred() is actually no-op for the file ccache
   type! (see cc_file.c:fcc_remove_cred()).

Fix this by storing the credential only if there is no matching one
already in the ccache.

I'm not at all sure that this is the right approach, as there are a number
of questions: Was the omission of the first duplicate check intentional?
Was there some reason for using the remove-then-add approach? Some other
cross-realm TGTs aren't cached at all, and we don't get such duplicate
under other circumstances -- so is the duplicate checking supposed to
happen elsewhere? Etc.
---
 src/lib/krb5/ccache/ccfns.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/lib/krb5/ccache/ccfns.c b/src/lib/krb5/ccache/ccfns.c
index 1084d519..94ac1121 100644
--- a/src/lib/krb5/ccache/ccfns.c
+++ b/src/lib/krb5/ccache/ccfns.c
@@ -77,6 +77,25 @@ krb5_cc_close(krb5_context context, krb5_ccache cache)
 }
 
 krb5_error_code KRB5_CALLCONV
+krb5_cc_store_cred_nodup(krb5_context, krb5_ccache, krb5_creds *);
+
+krb5_error_code KRB5_CALLCONV
+krb5_cc_store_cred_nodup(krb5_context context, krb5_ccache cache,
+                         krb5_creds *creds)
+{
+    krb5_creds discard;
+
+    /* Store this credential only if there is no matching one already in
+       the cache. */
+    if (krb5_cc_retrieve_cred(context, cache, KRB5_TC_MATCH_AUTHDATA, creds,
+                              &discard)) {
+        TRACE_CC_STORE(context, cache, creds);
+        return cache->ops->store(context, cache, creds);
+    } else
+        return 0;
+}
+
+krb5_error_code KRB5_CALLCONV
 krb5_cc_store_cred(krb5_context context, krb5_ccache cache,
                    krb5_creds *creds)
 {
@@ -84,8 +103,7 @@ krb5_cc_store_cred(krb5_context context, krb5_ccache cache,
     krb5_ticket *tkt;
     krb5_principal s1, s2;
 
-    TRACE_CC_STORE(context, cache, creds);
-    ret = cache->ops->store(context, cache, creds);
+    ret = krb5_cc_store_cred_nodup(context, cache, creds);
     if (ret) return ret;
 
     /*
@@ -99,10 +117,7 @@ krb5_cc_store_cred(krb5_context context, krb5_ccache cache,
     s2 = tkt->server;
     if (!krb5_principal_compare(context, s1, s2)) {
         creds->server = s2;
-        TRACE_CC_STORE_TKT(context, cache, creds);
-        /* remove any dups */
-        krb5_cc_remove_cred(context, cache, KRB5_TC_MATCH_AUTHDATA, creds);
-        ret = cache->ops->store(context, cache, creds);
+        ret = krb5_cc_store_cred_nodup(context, cache, creds);
         creds->server = s1;
     }
     krb5_free_ticket(context, tkt);
--
2.11.1


_______________________________________________
krb5-bugs mailing list
[hidden email]
https://mailman.mit.edu/mailman/listinfo/krb5-bugs