From deae811366b0e0a41c57ae5ec8662a3eac938a2f Mon Sep 17 00:00:00 2001 From: "Richard E. Silverman" 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 | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/lib/krb5/ccache/ccfns.c b/src/lib/krb5/ccache/ccfns.c index 1084d519..2047ac8b 100644 --- a/src/lib/krb5/ccache/ccfns.c +++ b/src/lib/krb5/ccache/ccfns.c @@ -77,6 +77,26 @@ 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)) { + return cache->ops->store(context, cache, creds); + } else { + krb5_free_cred_contents(context, &discard); + return 0; + } +} + +krb5_error_code KRB5_CALLCONV krb5_cc_store_cred(krb5_context context, krb5_ccache cache, krb5_creds *creds) { @@ -85,7 +105,7 @@ krb5_cc_store_cred(krb5_context context, krb5_ccache cache, 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; /* @@ -100,9 +120,7 @@ krb5_cc_store_cred(krb5_context context, krb5_ccache cache, 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