From bc274756c2a41fa80e46035b1507af968637c1a9 Mon Sep 17 00:00:00 2001 From: Robbie Harwood 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 --- 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 Date: Nov 17 2017 18:39:51 +0000 Subject: [PATCH 2/4] Fix unused variable Signed-off-by: Robbie Harwood --- 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 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 --- 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 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 --- 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);