[SSSD] [PATCH] KRB5: Check FAST kinit errors using get_tgt_times()

Jakub Hrozek jhrozek at redhat.com
Mon Dec 8 12:58:29 UTC 2014


Hi,

Coverity found an unused variable in the krb5_child code. After reading
the code again after a week it was written, I think we should do some
changes for better readability.

Mostly, fail on error from get_tgt_times() and change the flow so the
main branch (ie topmost indent) will be the success branch. It makes
sense to shortcut on success the first time get_tgt_times() is called to
check existing FAST ccache, but I think the second time, we expect
success, otherwise the forked child failed.
-------------- next part --------------
>From 3ce7cf85d02dbdf3bc405191001b3bf4c3d208c9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 8 Dec 2014 13:29:23 +0100
Subject: [PATCH] KRB5: Check FAST kinit errors using get_tgt_times()

---
 src/providers/krb5/krb5_child.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index e8260c387c152b79c3e24db81ddc7b949cac531b..76a0757f6176d4ad9d049bc8825a64328b19a818 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1670,6 +1670,7 @@ static krb5_error_code get_tgt_times(krb5_context ctx, const char *ccname,
     krberr = krb5_cc_resolve(ctx, ccname, &ccache);
     if (krberr != 0) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_cc_resolve failed.\n");
+        KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, krberr);
         goto done;
     }
 
@@ -1822,7 +1823,6 @@ static krb5_error_code check_fast_ccache(TALLOC_CTX *mem_ctx,
             } while (kerr == -1 && errno == EINTR);
 
             if (kerr > 0) {
-                kerr = EIO;
                 if (WIFEXITED(status)) {
                     kerr = WEXITSTATUS(status);
                     /* Don't blindly fail if the child fails, but check
@@ -1838,26 +1838,28 @@ static krb5_error_code check_fast_ccache(TALLOC_CTX *mem_ctx,
                           fchild_pid);
                 }
             } else {
-                DEBUG(SSSDBG_FUNC_DATA,
-                    "Failed to wait for children %d\n", fchild_pid);
-                kerr = EIO;
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "Failed to wait for child %d\n", fchild_pid);
+                /* Let the code re-check the TGT times and fail if we
+                 * can't find the updated principal */
             }
     }
 
     /* Check the ccache times again. Should be updated ... */
     memset(&tgtt, 0, sizeof(tgtt));
     kerr = get_tgt_times(ctx, ccname, server_princ, client_princ, &tgtt);
-    if (kerr == 0) {
-        if (tgtt.endtime > time(NULL)) {
-            DEBUG(SSSDBG_FUNC_DATA, "FAST TGT was successfully recreated!\n");
-            goto done;
-        } else {
-            kerr = ERR_CREDS_EXPIRED;
-            goto done;
-        }
+    if (kerr != 0) {
+        DEBUG(SSSDBG_OP_FAILURE, "get_tgt_times() failed\n");
+        goto done;
     }
 
-    kerr = 0;
+    if (tgtt.endtime < time(NULL)) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              "Valid FAST TGT not found after attempting to renew it\n");
+        kerr = ERR_CREDS_EXPIRED;
+        goto done;
+    }
+    DEBUG(SSSDBG_FUNC_DATA, "FAST TGT was successfully recreated!\n");
 
 done:
     if (client_princ != NULL) {
-- 
1.9.3



More information about the sssd-devel mailing list