[SSSD] [PATCH] Every time return directory for krb5 cache collection.

Lukas Slebodnik lslebodn at redhat.com
Sat Jun 22 11:55:51 UTC 2013


On (21/06/13 20:45), Jakub Hrozek wrote:
>On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
>> Rewritten patches are attached.
>> 
>> LS
>
>Two nitpicks:
>
>> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
>> +                                           krb5_context ctx,
>> +                                           krb5_principal principal,
>> +                                           const char *ccname)
>> +{
>> +    krb5_error_code kerr;
>> +    krb5_ccache tmp_cc = NULL;
>> +    char *tmp_ccname = NULL;
>> +    char *ret_ccname = NULL;
>> +
>> +    kerr = krb5_cc_set_default_name(ctx, ccname);
>> +    if (kerr != 0) {
>> +        KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
>> +        return NULL;
>> +    }
>> +
>> +    kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
>> +    if (kerr != 0) {
>> +        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
>> +        return NULL;
>> +    }
>> +
>> +    kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
>> +    if (kerr !=0) {
>> +        KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
>> +    }
>> +
>> +    ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
>> +    if (ret_ccname == NULL) {
>> +        DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));
>> +    }
>> +
>> +done:
>
>This done label is unused and GCC emits a warning over that. It should
>be used if krb5_cc_get_full_name() fails.
>
>> +    if (tmp_cc != NULL) {
>> +        kerr = krb5_cc_close(ctx, tmp_cc);
>> +        if (kerr != 0) {
>> +            KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
>> +        }
>> +    }
>> +    krb5_free_string(ctx, tmp_ccname);
>> +
>> +    return ret_ccname;
>> +}
>> +
>
>
>> 
>> +    kerr = krb5_unparse_name(ctx, princ, &full_name);
>> +    if (kerr !=0) {
>> +        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
>> +        goto done;
>> +    }
>> +
>> +    if (0 == strcmp(default_full_name, full_name)) {
>> +        ret = true;
>> +    }
>> +
>
>If you're changing the other patch, can you also add DEBUG message here
>saying what has been compared?
>
>> +done:
>> +    if (default_cc != NULL) {
>> +        kerr = krb5_cc_close(ctx, default_cc);
>> +        if (kerr != 0) {
>> +            KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);
>> +            goto done;
>> +        }
>> +    }
>> +
>> +    /* all functions can be safely called with NULL. */
>> +    krb5_free_principal(ctx, default_princ);
>> +    krb5_free_unparsed_name(ctx, default_full_name);
>> +    krb5_free_unparsed_name(ctx, full_name);
>> +
>> +    return ret;
>> +}
>
>So far I ran a number of tests against IPA. I should still check AD
>provider especially with enterprise principals to see if they still
>work. If they do I'll ack.
>
>But I will also file a ticket for 1.10.1 to move the checks to the
>backend so they are all done on one place as Sumit suggested earlier.
>
I removed checks in former updates. There left only mapping
from collection ccache (DIR:<cc_dir_name>) to ccache
(DIR::<cc_dir_name>/<ticket_name>)

I tried some modifications on backend side (with gdb), but I could not get an aim
1. store collection ccache name stored in sysdb
2. environment variable KRB5CCNAME also contains collection ccache name
3. do not have more ccaches with same principal name

>I don't think it's necessary to delay 1.10.0 any longer for this issue,
>but it's the right thing to do eventually.

Updated patches are attached.

LS
-------------- next part --------------
>From 80c105fde6ab656827fb440d030d3b4a8b453a50 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 29 May 2013 13:15:58 +0200
Subject: [PATCH 1/3] Fix wrong detection of krb5 ccname

DIR:/run/user/1000/krb5cc is valid ccname, but function sss_krb5_cc_file_path
returned NULL in this case.
---
 src/util/sss_krb5.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index 74db98fe9ee4cba858de5b459f0a5540003c63f8..9adb8ff2647fd04a86abef3ef58c015ba3dd21b2 100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -996,7 +996,10 @@ sss_krb5_cc_file_path(const char *full_location)
 #ifdef HAVE_KRB5_DIRCACHE
         case SSS_KRB5_TYPE_DIR:
             /* DIR::/run/user/tkt_foo */
-            if (residual[0] == ':') return residual+1;
+            if (residual[0] == ':') {
+                ++residual;
+            }
+            return residual;
 #endif
         case SSS_KRB5_TYPE_UNKNOWN:
             break;
-- 
1.8.1.4

-------------- next part --------------
>From 31d9f6c7df59f3c84ea2cbf65d74d653de06ce62 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 29 May 2013 09:57:38 +0200
Subject: [PATCH 2/3] Every time return directory for krb5 cache collection.

Function krb5_cc_get_full_name is called only as a way to validate that,
we have the right cache. Instead of returned name, location will be returned
from function cc_dir_cache_for_princ.

https://fedorahosted.org/sssd/ticket/1936
---
 src/providers/krb5/krb5_child.c | 59 ++++++++++++++++++++++++++++++++++++++++-
 src/providers/krb5/krb5_utils.c |  5 +++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index e9fbfbeafe774bc82f8be4976cc1b5a67a6acc5b..354d280514514e3283ff03ec5d534e24bc00ae36 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1080,12 +1080,58 @@ done:
 
 }
 
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
+                                           krb5_context ctx,
+                                           krb5_principal principal,
+                                           const char *ccname)
+{
+    krb5_error_code kerr;
+    krb5_ccache tmp_cc = NULL;
+    char *tmp_ccname = NULL;
+    char *ret_ccname = NULL;
+
+    kerr = krb5_cc_set_default_name(ctx, ccname);
+    if (kerr != 0) {
+        KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
+        return NULL;
+    }
+
+    kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
+    if (kerr != 0) {
+        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
+        return NULL;
+    }
+
+    kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
+    if (kerr !=0) {
+        KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
+        goto done;
+    }
+
+    ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
+    if (ret_ccname == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));
+    }
+
+done:
+    if (tmp_cc != NULL) {
+        kerr = krb5_cc_close(ctx, tmp_cc);
+        if (kerr != 0) {
+            KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);
+        }
+    }
+    krb5_free_string(ctx, tmp_ccname);
+
+    return ret_ccname;
+}
+
 static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
                                         const char *password)
 {
     const char *realm_name;
     int realm_length;
     krb5_error_code kerr;
+    char *cc_name;
 
 
     kerr = sss_krb5_get_init_creds_opt_set_expire_callback(kr->ctx, kr->options,
@@ -1100,6 +1146,17 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
 
     DEBUG(SSSDBG_TRACE_FUNC,
           ("Attempting kinit for realm [%s]\n",realm_name));
+
+    /* If kr->ccname is cache collection (DIR:/...), we want to work
+     * directly with file ccache (DIR::/...), but cache collection
+     * should be returned bactk to back end.
+     */
+    cc_name = get_ccache_name_by_principal(kr->pd, kr->ctx, kr->princ,
+                                           kr->ccname);
+    if (cc_name == NULL) {
+        cc_name = kr->ccname;
+    }
+
     kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
                                         discard_const(password),
                                         sss_krb5_prompter, kr, 0,
@@ -1134,7 +1191,7 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
     /* Use the updated principal in the creds in case canonicalized */
     kerr = create_ccache(kr->uid, kr->gid, kr->ctx,
                          kr->creds ? kr->creds->client : kr->princ,
-                         kr->ccname, kr->creds);
+                         cc_name, kr->creds);
     if (kerr != 0) {
         KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
         goto done;
diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
index 1883d785e6746c45e51c8b0fabe01afbad688d6d..3f16faa7fb238bbb9801029beed66293cd873c15 100644
--- a/src/providers/krb5/krb5_utils.c
+++ b/src/providers/krb5/krb5_utils.c
@@ -1164,6 +1164,9 @@ cc_dir_cache_for_princ(TALLOC_CTX *mem_ctx, const char *location,
         return NULL;
     }
 
+    /* This function is called only as a way to validate that,
+     * we have the right cache
+     */
     krberr = krb5_cc_get_full_name(context, ccache, &name);
     if (ccache) krb5_cc_close(context, ccache);
     krb5_free_context(context);
@@ -1173,7 +1176,7 @@ cc_dir_cache_for_princ(TALLOC_CTX *mem_ctx, const char *location,
         return NULL;
     }
 
-    return talloc_strdup(mem_ctx, name);
+    return talloc_strdup(mem_ctx, location);
 }
 
 errno_t
-- 
1.8.1.4

-------------- next part --------------
>From ff27655baac9613ba516feb22fed71bec68ef67a Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 19 Jun 2013 18:47:25 +0200
Subject: [PATCH 3/3] Do not switch to credentials everytime.

If user decide to kinit as another user we do not want to switch back
to user ccache at another login. We will switch to new ccache if and only
if default principal name is the same as current principal name, or there is
not any default ccache.

https://fedorahosted.org/sssd/ticket/1936
---
 src/providers/krb5/krb5_child.c | 73 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 69 insertions(+), 4 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 354d280514514e3283ff03ec5d534e24bc00ae36..f2256e547f04a8cd038e113acbf92f86127ffb04 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -436,6 +436,69 @@ done:
     return kerr;
 }
 
+#ifdef HAVE_KRB5_DIRCACHE
+static bool need_switch_to_principal(krb5_context ctx, krb5_principal princ)
+{
+    krb5_error_code kerr;
+    krb5_ccache default_cc = NULL;
+    krb5_principal default_princ = NULL;
+    char *default_full_name = NULL;
+    char *full_name = NULL;
+    bool ret = false;
+
+    kerr = krb5_cc_default(ctx, &default_cc);
+    if (kerr !=0) {
+        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
+        goto done;
+    }
+
+    kerr = krb5_cc_get_principal(ctx, default_cc, &default_princ);
+    if (kerr == KRB5_FCC_NOFILE) {
+        /* There is not any default cache. */
+        ret = true;
+        goto done;
+    } else if (kerr != 0) {
+        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
+        goto done;
+    }
+
+    kerr = krb5_unparse_name(ctx, default_princ, &default_full_name);
+    if (kerr !=0) {
+        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
+        goto done;
+    }
+
+    kerr = krb5_unparse_name(ctx, princ, &full_name);
+    if (kerr !=0) {
+        KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);
+        goto done;
+    }
+
+    DEBUG(SSSDBG_FUNC_DATA,
+          ("Comparing default principal [%s] and new principal [%s].\n",
+           default_full_name, full_name));
+    if (0 == strcmp(default_full_name, full_name)) {
+        ret = true;
+    }
+
+done:
+    if (default_cc != NULL) {
+        kerr = krb5_cc_close(ctx, default_cc);
+        if (kerr != 0) {
+            KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);
+            goto done;
+        }
+    }
+
+    /* all functions can be safely called with NULL. */
+    krb5_free_principal(ctx, default_princ);
+    krb5_free_unparsed_name(ctx, default_full_name);
+    krb5_free_unparsed_name(ctx, full_name);
+
+    return ret;
+}
+#endif /* HAVE_KRB5_DIRCACHE */
+
 static krb5_error_code
 store_creds_in_ccache(krb5_context ctx, krb5_principal princ,
                       krb5_ccache cc, krb5_creds *creds)
@@ -466,10 +529,12 @@ store_creds_in_ccache(krb5_context ctx, krb5_principal princ,
     }
 
 #ifdef HAVE_KRB5_DIRCACHE
-    kerr = krb5_cc_switch(ctx, cc);
-    if (kerr != 0) {
-        KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);
-        goto done;
+    if (need_switch_to_principal(ctx, princ)) {
+        kerr = krb5_cc_switch(ctx, cc);
+        if (kerr != 0) {
+            KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);
+            goto done;
+        }
     }
 #endif /* HAVE_KRB5_DIRCACHE */
 
-- 
1.8.1.4



More information about the sssd-devel mailing list