Properly locate credentials in collection caches in mechglue

This commit is contained in:
Robbie Harwood 2017-12-01 13:22:21 -05:00
parent f968274a52
commit f63618cd42
3 changed files with 260 additions and 1 deletions

View File

@ -0,0 +1,147 @@
From 0bd013108403f9cbdccc34d24ea8c188bd6fd13a Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Mon, 20 Nov 2017 14:09:04 -0500
Subject: [PATCH] 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>
Reviewed-by: Simo Sorce <simo@redhat.com>
Merges: #221
(cherry picked from commit 670240a6cd4d5e2ecf13e481621098693cdbaa89)
---
proxy/src/mechglue/gpp_creds.c | 81 +++++++++++++++++++++++++++++------------
proxy/src/mechglue/gss_plugin.h | 2 +-
2 files changed, 59 insertions(+), 24 deletions(-)
diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c
index 3ebd726..187ada7 100644
--- a/proxy/src/mechglue/gpp_creds.c
+++ b/proxy/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_initialize(ctx, ccache, cred.client);
- if (ret != 0) {
+ ret = krb5_cc_default(ctx, &ccache);
+ if (ret)
goto done;
- }
+
+ ret = krb5_cc_initialize(ctx, ccache, cred.client);
+ 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/proxy/src/mechglue/gss_plugin.h b/proxy/src/mechglue/gss_plugin.h
index 333d63c..c0e8870 100644
--- a/proxy/src/mechglue/gss_plugin.h
+++ b/proxy/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);

View File

@ -0,0 +1,107 @@
From 47637312e566d7fecf2bf9c60efe85faab0945cc Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Fri, 17 Nov 2017 13:53:37 -0500
Subject: [PATCH] Separate cred and ccache manipulation in
gpp_store_remote_creds()
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
(cherry picked from commit 221b553bfb4082085d05b40da9a04c1f7e4af533)
---
proxy/src/mechglue/gpp_creds.c | 62 ++++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 23 deletions(-)
diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c
index 6bdff45..3ebd726 100644
--- a/proxy/src/mechglue/gpp_creds.c
+++ b/proxy/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) return ret;
+ 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

View File

@ -1,6 +1,6 @@
Name: gssproxy
Version: 0.7.0
Release: 24%{?dist}
Release: 25%{?dist}
Summary: GSSAPI Proxy
Group: System Environment/Libraries
@ -39,6 +39,8 @@ Patch21: Emit-debug-on-queue-errors.patch
Patch22: Do-not-call-gpm_grab_sock-twice.patch
Patch23: Fix-error-message-handling-in-gp_config_from_dir.patch
Patch24: Only-empty-FILE-ccaches-when-storing-remote-creds.patch
Patch25: Separate-cred-and-ccache-manipulation-in-gpp_store_r.patch
Patch26: Properly-locate-credentials-in-collection-caches-in-.patch
### Dependencies ###
Requires: krb5-libs >= 1.12.0
@ -136,6 +138,9 @@ rm -rf %{buildroot}
%systemd_postun_with_restart gssproxy.service
%changelog
* Fri Dec 01 2017 Robbie Harwood <rharwood@redhat.com> - 0.7.0-25
- Properly locate credentials in collection caches in mechglue
* Tue Oct 31 2017 Robbie Harwood <rharwood@redhat.com> - 0.7.0-24
- Only empty FILE ccaches when storing remote creds