From bc274756c2a41fa80e46035b1507af968637c1a9 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Nov 17 2017 18:18:45 +0000
Subject: [PATCH 1/4] Fix a krb5 error code being returned as GSS major


Signed-off-by: Robbie Harwood <rharwood@redhat.com>

---

diff --git a/src/mechglue/gpp_creds.c b/src/mechglue/gpp_creds.c
index 6bdff45..583c0c7 100644
--- a/src/mechglue/gpp_creds.c
+++ b/src/mechglue/gpp_creds.c
@@ -154,7 +154,7 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
     memset(&cred, 0, sizeof(cred));
 
     ret = krb5_init_context(&ctx);
-    if (ret) return ret;
+    if (ret) goto done;
 
     if (cred_store) {
         for (unsigned i = 0; i < cred_store->count; i++) {

From 6ce002b183f3d31ad4265e915996ced529d382f0 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Nov 17 2017 18:39:51 +0000
Subject: [PATCH 2/4] Fix unused variable


Signed-off-by: Robbie Harwood <rharwood@redhat.com>

---

diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index f9f9258..dd29519 100644
--- a/src/client/gpm_common.c
+++ b/src/client/gpm_common.c
@@ -516,8 +516,6 @@ static uint32_t gpm_next_xid(struct gpm_ctx *gpmctx)
 
 static struct gpm_ctx *gpm_get_ctx(void)
 {
-    int ret;
-
     pthread_once(&gpm_init_once_control, gpm_init_once);
 
     return &gpm_global_ctx;

From 4eef3de0de55e0aaa5a4bb67c48e162b9fe17339 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Nov 17 2017 19:01:20 +0000
Subject: [PATCH 3/4] Separate cred and ccache manipulation in gpp_store_remote_creds()


Signed-off-by: Robbie Harwood <rharwood@redhat.com>

---

diff --git a/src/mechglue/gpp_creds.c b/src/mechglue/gpp_creds.c
index 583c0c7..dac763f 100644
--- a/src/mechglue/gpp_creds.c
+++ b/src/mechglue/gpp_creds.c
@@ -136,6 +136,40 @@ bool gpp_creds_are_equal(gssx_cred *a, gssx_cred *b)
     return true;
 }
 
+static krb5_error_code gpp_construct_cred(gssx_cred *creds, krb5_context ctx,
+                                          krb5_creds *cred, char *cred_name)
+{
+    XDR xdrctx;
+    bool xdrok;
+    krb5_error_code ret = 0;
+
+    memset(cred, 0, sizeof(*cred));
+
+    memcpy(cred_name, creds->desired_name.display_name.octet_string_val,
+           creds->desired_name.display_name.octet_string_len);
+    cred_name[creds->desired_name.display_name.octet_string_len] = '\0';
+
+    ret = krb5_parse_name(ctx, cred_name, &cred->client);
+    if (ret) {
+        return ret;
+    }
+
+    ret = krb5_parse_name(ctx, GPKRB_SRV_NAME, &cred->server);
+    if (ret) {
+        return ret;
+    }
+
+    cred->ticket.data = malloc(GPKRB_MAX_CRED_SIZE);
+    xdrmem_create(&xdrctx, cred->ticket.data, GPKRB_MAX_CRED_SIZE,
+                  XDR_ENCODE);
+    xdrok = xdr_gssx_cred(&xdrctx, creds);
+    if (!xdrok) {
+        return ENOSPC;
+    }
+    cred->ticket.length = xdr_getpos(&xdrctx);
+    return 0;
+}
+
 uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
                                 gss_const_key_value_set_t cred_store,
                                 gssx_cred *creds)
@@ -145,17 +179,18 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
     krb5_creds cred;
     krb5_error_code ret;
     char cred_name[creds->desired_name.display_name.octet_string_len + 1];
-    XDR xdrctx;
-    bool xdrok;
     const char *cc_type;
 
     *min = 0;
 
-    memset(&cred, 0, sizeof(cred));
-
     ret = krb5_init_context(&ctx);
     if (ret) goto done;
 
+    ret = gpp_construct_cred(creds, ctx, &cred, cred_name);
+    if (ret) {
+        goto done;
+    }
+
     if (cred_store) {
         for (unsigned i = 0; i < cred_store->count; i++) {
             if (strcmp(cred_store->elements[i].key, "ccache") == 0) {
@@ -175,25 +210,6 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
         if (ret) goto done;
     }
 
-    memcpy(cred_name, creds->desired_name.display_name.octet_string_val,
-           creds->desired_name.display_name.octet_string_len);
-    cred_name[creds->desired_name.display_name.octet_string_len] = '\0';
-
-    ret = krb5_parse_name(ctx, cred_name, &cred.client);
-    if (ret) goto done;
-
-    ret = krb5_parse_name(ctx, GPKRB_SRV_NAME, &cred.server);
-    if (ret) goto done;
-
-    cred.ticket.data = malloc(GPKRB_MAX_CRED_SIZE);
-    xdrmem_create(&xdrctx, cred.ticket.data, GPKRB_MAX_CRED_SIZE, XDR_ENCODE);
-    xdrok = xdr_gssx_cred(&xdrctx, creds);
-    if (!xdrok) {
-        ret = ENOSPC;
-        goto done;
-    }
-    cred.ticket.length = xdr_getpos(&xdrctx);
-
     cc_type = krb5_cc_get_type(ctx, ccache);
     if (strcmp(cc_type, "FILE") == 0) {
         /* FILE ccaches don't handle updates properly: if they have the same

From 4a0a210ede55cbc81ee1b79158a59e1d5c819cef Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Nov 30 2017 23:24:14 +0000
Subject: [PATCH 4/4] Properly locate credentials in collection caches in mechglue


Previously, we would just put the credentials in the default cache for
a collection type, which lead to some mysterious failures.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>

---

diff --git a/src/mechglue/gpp_creds.c b/src/mechglue/gpp_creds.c
index dac763f..1ac9691 100644
--- a/src/mechglue/gpp_creds.c
+++ b/src/mechglue/gpp_creds.c
@@ -170,7 +170,16 @@ static krb5_error_code gpp_construct_cred(gssx_cred *creds, krb5_context ctx,
     return 0;
 }
 
-uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
+/* Store creds from remote in a local ccache, updating where possible.
+ *
+ * If store_as_default_cred is true, the cred is made default for its
+ * collection, if there is one.  Note that if the ccache is not of a
+ * collection type, the creds will overwrite the ccache.
+ *
+ * If no "ccache" entry is specified in cred_store, the default ccache for a
+ * new context will be used.
+ */
+uint32_t gpp_store_remote_creds(uint32_t *min, bool store_as_default_cred,
                                 gss_const_key_value_set_t cred_store,
                                 gssx_cred *creds)
 {
@@ -179,7 +188,7 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
     krb5_creds cred;
     krb5_error_code ret;
     char cred_name[creds->desired_name.display_name.octet_string_len + 1];
-    const char *cc_type;
+    const char *cc_name;
 
     *min = 0;
 
@@ -191,38 +200,64 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
         goto done;
     }
 
-    if (cred_store) {
-        for (unsigned i = 0; i < cred_store->count; i++) {
-            if (strcmp(cred_store->elements[i].key, "ccache") == 0) {
-                ret = krb5_cc_resolve(ctx, cred_store->elements[i].value,
-                                      &ccache);
-                if (ret) goto done;
-                break;
-            }
+    for (unsigned i = 0; cred_store && i < cred_store->count; i++) {
+        if (strcmp(cred_store->elements[i].key, "ccache") == 0) {
+            /* krb5 creates new ccaches based off the default name. */
+            ret = krb5_cc_set_default_name(ctx,
+                                           cred_store->elements[i].value);
+            if (ret)
+                goto done;
+
+            break;
         }
     }
-    if (!ccache) {
-        if (!default_creds) {
-            ret = ENOMEDIUM;
-            goto done;
-        }
-        ret = krb5_cc_default(ctx, &ccache);
-        if (ret) goto done;
-    }
 
-    cc_type = krb5_cc_get_type(ctx, ccache);
-    if (strcmp(cc_type, "FILE") == 0) {
+    cc_name = krb5_cc_default_name(ctx);
+    if (strncmp(cc_name, "FILE:", 5) == 0 || !strchr(cc_name, ':')) {
         /* FILE ccaches don't handle updates properly: if they have the same
          * principal name, they are blackholed.  We either have to change the
          * name (at which point the file grows forever) or flash the cache on
          * every update. */
+        ret = krb5_cc_default(ctx, &ccache);
+        if (ret)
+            goto done;
+
         ret = krb5_cc_initialize(ctx, ccache, cred.client);
-        if (ret != 0) {
+        if (ret != 0)
+            goto done;
+
+        ret = krb5_cc_store_cred(ctx, ccache, &cred);
+        goto done;
+    }
+
+    ret = krb5_cc_cache_match(ctx, cred.client, &ccache);
+    if (ret == KRB5_CC_NOTFOUND) {
+        /* A new ccache within the collection whose name is based off the
+         * default_name for the context.  krb5_cc_new_unique only accepts the
+         * leading component of a name as a type. */
+        char *cc_type;
+        const char *p;
+
+        p = strchr(cc_name, ':'); /* can't be FILE here */
+        cc_type = strndup(cc_name, p - cc_name);
+        if (!cc_type) {
+            ret = ENOMEM;
             goto done;
         }
+
+        ret = krb5_cc_new_unique(ctx, cc_type, NULL, &ccache);
+        free(cc_type);
     }
+    if (ret)
+        goto done;
 
     ret = krb5_cc_store_cred(ctx, ccache, &cred);
+    if (ret)
+        goto done;
+
+    if (store_as_default_cred) {
+        ret = krb5_cc_switch(ctx, ccache);
+    }
 
 done:
     if (ctx) {
diff --git a/src/mechglue/gss_plugin.h b/src/mechglue/gss_plugin.h
index 333d63c..c0e8870 100644
--- a/src/mechglue/gss_plugin.h
+++ b/src/mechglue/gss_plugin.h
@@ -76,7 +76,7 @@ uint32_t gpp_cred_handle_init(uint32_t *min, bool defcred, const char *ccache,
                               struct gpp_cred_handle **out_handle);
 uint32_t gpp_cred_handle_free(uint32_t *min, struct gpp_cred_handle *handle);
 bool gpp_creds_are_equal(gssx_cred *a, gssx_cred *b);
-uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds,
+uint32_t gpp_store_remote_creds(uint32_t *min, bool store_as_default_cred,
                                 gss_const_key_value_set_t cred_store,
                                 gssx_cred *creds);
 

