diff --git a/SOURCES/Add-Client-ID-to-debug-messages.patch b/SOURCES/Add-Client-ID-to-debug-messages.patch new file mode 100644 index 0000000..63e6a08 --- /dev/null +++ b/SOURCES/Add-Client-ID-to-debug-messages.patch @@ -0,0 +1,148 @@ +From 9e9b8597c09605438a6d880b6df5aaa4696f4b21 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 25 May 2017 15:22:37 -0400 +Subject: [PATCH] Add Client ID to debug messages + +This allows to sort out which debug message belongs to which client when +multiple clients are preforming operations at the same time. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood + +Resolves: #189 +Merges: #191 +(cherry picked from commit 2f158fe4d39c11589d214d3d602c6d10411052dc) +--- + proxy/src/gp_debug.c | 28 +++++++++++++++++++++++++++- + proxy/src/gp_debug.h | 1 + + proxy/src/gp_proxy.h | 1 + + proxy/src/gp_socket.c | 5 +++++ + proxy/src/gp_workers.c | 6 ++++++ + proxy/src/gssproxy.c | 4 ++++ + 6 files changed, 44 insertions(+), 1 deletion(-) + +diff --git a/proxy/src/gp_debug.c b/proxy/src/gp_debug.c +index 3029574..4a141fc 100644 +--- a/proxy/src/gp_debug.c ++++ b/proxy/src/gp_debug.c +@@ -64,6 +64,32 @@ const char *gp_debug_timestamp(void) + return buffer; + } + ++/* thread local connection/client id */ ++static __thread int cid; ++ ++void gp_debug_set_conn_id(int id) ++{ ++ cid = id; ++} ++ ++static const char*gp_debug_conn_id(void) ++{ ++ static __thread char buffer[18]; ++ static __thread int last_cid = 0; ++ ++ if (cid == 0) { ++ buffer[0] = '\0'; ++ return buffer; ++ } ++ ++ if (last_cid == cid) return buffer; ++ ++ (void)snprintf(buffer, 17, "[CID %d]", cid); ++ buffer[17] = '\0'; ++ last_cid = cid; ++ return buffer; ++} ++ + void gp_debug_printf(const char *format, ...) + { + va_list varargs; +@@ -76,7 +102,7 @@ void gp_debug_time_printf(const char *format, ...) + { + va_list varargs; + +- fprintf(stderr, "%s", gp_debug_timestamp()); ++ fprintf(stderr, "%s%s", gp_debug_conn_id(), gp_debug_timestamp()); + + va_start(varargs, format); + vfprintf(stderr, format, varargs); +diff --git a/proxy/src/gp_debug.h b/proxy/src/gp_debug.h +index d3420b0..1c2f8a3 100644 +--- a/proxy/src/gp_debug.h ++++ b/proxy/src/gp_debug.h +@@ -14,6 +14,7 @@ int gp_debug_args(int level); + void gp_debug_toggle(int); + void gp_debug_printf(const char *format, ...); + void gp_debug_time_printf(const char *format, ...); ++void gp_debug_set_conn_id(int id); + + #define GPDEBUG(...) do { \ + if (gp_debug) { \ +diff --git a/proxy/src/gp_proxy.h b/proxy/src/gp_proxy.h +index 971a7b6..55ab83c 100644 +--- a/proxy/src/gp_proxy.h ++++ b/proxy/src/gp_proxy.h +@@ -113,6 +113,7 @@ void gp_socket_send_data(verto_ctx *vctx, struct gp_conn *conn, + struct gp_creds *gp_conn_get_creds(struct gp_conn *conn); + uid_t gp_conn_get_uid(struct gp_conn *conn); + const char *gp_conn_get_socket(struct gp_conn *conn); ++int gp_conn_get_cid(struct gp_conn *conn); + bool gp_selinux_ctx_equal(SELINUX_CTX ctx1, SELINUX_CTX ctx2); + bool gp_conn_check_selinux(struct gp_conn *conn, SELINUX_CTX ctx); + +diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c +index 29b6a44..5064e51 100644 +--- a/proxy/src/gp_socket.c ++++ b/proxy/src/gp_socket.c +@@ -103,6 +103,11 @@ const char *gp_conn_get_socket(struct gp_conn *conn) + return conn->sock_ctx->socket; + } + ++int gp_conn_get_cid(struct gp_conn *conn) ++{ ++ return conn->us.sd; ++} ++ + void gp_conn_free(struct gp_conn *conn) + { + if (!conn) return; +diff --git a/proxy/src/gp_workers.c b/proxy/src/gp_workers.c +index c089b54..d37e57c 100644 +--- a/proxy/src/gp_workers.c ++++ b/proxy/src/gp_workers.c +@@ -357,6 +357,9 @@ static void *gp_worker_main(void *pvt) + + while (!t->pool->shutdown) { + ++ /* initialize debug client id to 0 until work is scheduled */ ++ gp_debug_set_conn_id(0); ++ + /* ======> COND_MUTEX */ + pthread_mutex_lock(&t->cond_mutex); + while (t->query == NULL) { +@@ -374,6 +377,9 @@ static void *gp_worker_main(void *pvt) + /* <====== COND_MUTEX */ + pthread_mutex_unlock(&t->cond_mutex); + ++ /* set client id before hndling requests */ ++ gp_debug_set_conn_id(gp_conn_get_cid(q->conn)); ++ + /* handle the client request */ + gp_handle_query(t->pool, q); + +diff --git a/proxy/src/gssproxy.c b/proxy/src/gssproxy.c +index 5c5937d..94a6a61 100644 +--- a/proxy/src/gssproxy.c ++++ b/proxy/src/gssproxy.c +@@ -159,6 +159,10 @@ int main(int argc, const char *argv[]) + int wait_fd; + int ret = -1; + ++ /* initialize debug client id to 0 in the main thread */ ++ /* we do this early, before any code starts using debug statements */ ++ gp_debug_set_conn_id(0); ++ + struct poptOption long_options[] = { + POPT_AUTOHELP + {"daemon", 'D', POPT_ARG_NONE, &opt_daemon, 0, \ diff --git a/SOURCES/Appease-gcc-7-s-fallthrough-detection.patch b/SOURCES/Appease-gcc-7-s-fallthrough-detection.patch new file mode 100644 index 0000000..024c200 --- /dev/null +++ b/SOURCES/Appease-gcc-7-s-fallthrough-detection.patch @@ -0,0 +1,27 @@ +From f62ece3a9655c6507f40e03a36e28ecbba16f744 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Fri, 19 May 2017 12:18:03 -0400 +Subject: [PATCH] Appease gcc-7's fallthrough detection + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #188 +(cherry picked from commit fdb56f8a0b6a5fd6a2072a525e228596c264883e) +--- + proxy/src/gp_export.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/proxy/src/gp_export.c b/proxy/src/gp_export.c +index 3a927c9..f44da50 100644 +--- a/proxy/src/gp_export.c ++++ b/proxy/src/gp_export.c +@@ -888,7 +888,8 @@ static uint32_t gp_export_creds_linux(uint32_t *min, gss_name_t name, + if (res != NULL) { + break; + } +- /* fall through as ret == NULL is equivalent to ENOENT */ ++ /* ret == NULL is equivalent to ENOENT */ ++ /* fall through */ + case ENOENT: + case ESRCH: + free(pwbuf); diff --git a/SOURCES/Conditionally-reload-kernel-interface-on-SIGHUP.patch b/SOURCES/Conditionally-reload-kernel-interface-on-SIGHUP.patch new file mode 100644 index 0000000..4495bc1 --- /dev/null +++ b/SOURCES/Conditionally-reload-kernel-interface-on-SIGHUP.patch @@ -0,0 +1,59 @@ +From 70d500d0dcc26ff3a87c1dd4bd6ab0c591ed4cf9 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Mon, 26 Jun 2017 17:04:16 -0400 +Subject: [PATCH] Conditionally reload kernel interface on SIGHUP + +Signed-off-by: Alexander Scheel +[rharwood@redhat.com: Simplified logic, rewrote commit message] +Reviewed-by: Robbie Harwood +Resolves: #193 +Merges: #201 +(cherry picked from commit 938af94b8f33c227dcdd6a53ed42de418578d29d) +--- + proxy/src/gp_init.c | 4 +++- + proxy/src/gssproxy.c | 3 +++ + 2 files changed, 6 insertions(+), 1 deletion(-) + +diff --git a/proxy/src/gp_init.c b/proxy/src/gp_init.c +index e69934d..6a2b047 100644 +--- a/proxy/src/gp_init.c ++++ b/proxy/src/gp_init.c +@@ -145,6 +145,7 @@ void init_proc_nfsd(struct gp_config *cfg) + char buf[] = "1"; + bool enabled = false; + int fd, ret; ++ static int poked = 0; + + /* check first if any service enabled kernel support */ + for (int i = 0; i < cfg->num_svcs; i++) { +@@ -154,7 +155,7 @@ void init_proc_nfsd(struct gp_config *cfg) + } + } + +- if (!enabled) { ++ if (!enabled || poked) { + return; + } + +@@ -173,6 +174,7 @@ void init_proc_nfsd(struct gp_config *cfg) + LINUX_PROC_USE_GSS_PROXY_FILE, ret, gp_strerror(ret)); + } + ++ poked = 1; + close(fd); + if (ret != 0) { + goto fail; +diff --git a/proxy/src/gssproxy.c b/proxy/src/gssproxy.c +index 94a6a61..5fc4f8d 100644 +--- a/proxy/src/gssproxy.c ++++ b/proxy/src/gssproxy.c +@@ -140,6 +140,9 @@ static void hup_handler(verto_ctx *vctx, verto_ev *ev UNUSED) + exit(ret); + } + ++ /* conditionally reload kernel interface */ ++ init_proc_nfsd(gpctx->config); ++ + free_config(&old_config); + + GPDEBUG("New config loaded successfully.\n"); diff --git a/SOURCES/Do-not-call-gpm_grab_sock-twice.patch b/SOURCES/Do-not-call-gpm_grab_sock-twice.patch new file mode 100644 index 0000000..5a5852c --- /dev/null +++ b/SOURCES/Do-not-call-gpm_grab_sock-twice.patch @@ -0,0 +1,59 @@ +From 32578afb817f20446d888326814b52a8f3d6c0fe Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 26 Oct 2017 16:59:18 -0400 +Subject: [PATCH] Do not call gpm_grab_sock() twice + +In the gpm_get_ctx() call, we unnecessarily call gpm_grab_sock() which +would cause the lock to be held by one thread and never released. We +already call gpm_grab_sock() as the first thing after gpm_get_ctx() in +gpm_make_call(), plus gpm_make_call() properly releases the socket +once done. + +This corrects the deadlock fix in +461a5fa9f91a2753ebeef6323a64239c35e2f250, which incorrectly released +the lock we wanted to grab. This caused the socket to not be locked +to our thread. Another thread could come along and change the global +ctx while we were still using the socket from another thread, causing +concurrency issues as only one request can be in flight on any given +socket at the same time. + +In special cases where the "thread" uid/gid changes (like in +rpc.gssd), we end up closing the socket while we are still waiting for +an answer from the server, causing additional issues and confusion. + +[rharwood@redhat.com: squashed 2 commits; minor edits accordingly] +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #218 +(cherry picked from commit 8590c5dbc6fa07d0c366df23b982a4b6b9ffc259) +--- + proxy/src/client/gpm_common.c | 9 +++------ + 1 file changed, 3 insertions(+), 6 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 69f4741..2133618 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -152,7 +152,9 @@ static int gpm_grab_sock(struct gpm_ctx *gpmctx) + ret = gpm_open_socket(gpmctx); + } + +- pthread_mutex_unlock(&gpmctx->lock); ++ if (ret) { ++ pthread_mutex_unlock(&gpmctx->lock); ++ } + return ret; + } + +@@ -304,11 +306,6 @@ static struct gpm_ctx *gpm_get_ctx(void) + + pthread_once(&gpm_init_once_control, gpm_init_once); + +- ret = gpm_grab_sock(&gpm_global_ctx); +- if (ret) { +- return NULL; +- } +- + return &gpm_global_ctx; + } + diff --git a/SOURCES/Emit-debug-on-queue-errors.patch b/SOURCES/Emit-debug-on-queue-errors.patch new file mode 100644 index 0000000..c9593c4 --- /dev/null +++ b/SOURCES/Emit-debug-on-queue-errors.patch @@ -0,0 +1,26 @@ +From 25ad523713559f8c875377383ffbd6f7b367d95e Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 26 Oct 2017 11:47:54 -0400 +Subject: [PATCH] Emit debug on queue errors + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #218 +(cherry picked from commit af666affbd4735ba437e3d89d9e22984a556ed16) +--- + proxy/src/gp_workers.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/proxy/src/gp_workers.c b/proxy/src/gp_workers.c +index 2a33c21..18f38f6 100644 +--- a/proxy/src/gp_workers.c ++++ b/proxy/src/gp_workers.c +@@ -314,6 +314,8 @@ static void gp_handle_reply(verto_ctx *vctx, verto_ev *ev) + case GP_QUERY_IN: + /* ?! fallback and kill client conn */ + case GP_QUERY_ERR: ++ GPDEBUGN(3, "[status] Handling query error, terminating CID %d.\n", ++ gp_conn_get_cid(q->conn)); + gp_conn_free(q->conn); + gp_query_free(q, true); + break; diff --git a/SOURCES/Fix-error-checking-on-get_impersonator_fallback.patch b/SOURCES/Fix-error-checking-on-get_impersonator_fallback.patch new file mode 100644 index 0000000..7e17ad5 --- /dev/null +++ b/SOURCES/Fix-error-checking-on-get_impersonator_fallback.patch @@ -0,0 +1,28 @@ +From f21071fb3662824698b61d384b1144657a508043 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 15 Mar 2017 14:57:57 -0400 +Subject: [PATCH] Fix error checking on get_impersonator_fallback() + +Separate commit to ease backporting. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #173 +(cherry picked from commit 25e31ebccde7f0d98480b6a99962fef61dd251b4) +--- + proxy/src/gp_creds.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c +index e05ad01..fdc6bdf 100644 +--- a/proxy/src/gp_creds.c ++++ b/proxy/src/gp_creds.c +@@ -885,7 +885,7 @@ static uint32_t get_impersonator_name(uint32_t *min, gss_cred_id_t cred, + * release that supports this call */ + ret_maj = get_impersonator_fallback(&ret_min, cred, impersonator); + if (ret_maj == GSS_S_FAILURE) { +- if (ret_min == KRB5_CC_NOTFOUND) { ++ if (ret_min == (uint32_t)KRB5_CC_NOTFOUND) { + ret_min = ENOENT; + ret_maj = GSS_S_COMPLETE; + } diff --git a/SOURCES/Fix-error-handling-in-gp_config_from_dir.patch b/SOURCES/Fix-error-handling-in-gp_config_from_dir.patch new file mode 100644 index 0000000..a3c43ae --- /dev/null +++ b/SOURCES/Fix-error-handling-in-gp_config_from_dir.patch @@ -0,0 +1,50 @@ +From 8603c619ca9bc923534d83ee432ddd756f285d4c Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Wed, 12 Jul 2017 09:26:52 -0400 +Subject: [PATCH] Fix error handling in gp_config_from_dir + +Signed-off-by: Alexander Scheel +[rharwood@redhat.com: c99, refactor some existing code] +Reviewed-by: Robbie Harwood +Merges: #204 +(cherry picked from commit eb880e93ed4a48c67ac27b4d5194f0f7786da83e) +--- + proxy/src/gp_config.c | 24 ++++++++++++++---------- + 1 file changed, 14 insertions(+), 10 deletions(-) + +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index 8fd60a3..07f7c8d 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -798,17 +798,21 @@ static int gp_config_from_dir(const char *config_dir, + &error_list, + NULL); + if (ret) { +- if (error_list) { +- uint32_t i; +- uint32_t len = ref_array_getlen(error_list, &i); +- for (i = 0; i < len; i++) { +- GPDEBUG("Error when reading config directory: %s\n", +- (const char *) ref_array_get(error_list, i, NULL)); +- } +- ref_array_destroy(error_list); +- } else { +- GPDEBUG("Error when reading config directory number: %d\n", ret); ++ uint32_t len; ++ ++ if (!error_list) { ++ GPAUDIT("Error when reading config directory number: %d\n", ret); ++ return ret; + } ++ ++ len = ref_array_len(error_list); ++ for (uint32_t i = 0; i < len; i++) { ++ /* libini has an unfixable bug where error strings are (char **) */ ++ GPAUDIT("Error when reading config directory: %s\n", ++ *(char **)ref_array_get(error_list, i, NULL)); ++ } ++ ++ ref_array_destroy(error_list); + return ret; + } + diff --git a/SOURCES/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch b/SOURCES/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch new file mode 100644 index 0000000..f51d589 --- /dev/null +++ b/SOURCES/Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch @@ -0,0 +1,61 @@ +From accc0e772b126d251ca853a080b25466ebc528a3 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 11:24:39 -0500 +Subject: [PATCH] Fix error handling in gpm_send_buffer/gpm_recv_buffer + +Signed-off-by: Alexander Scheel +Reviewed-by: Robbie Harwood +Merges: #213 +[rharwood@redhat.com: commit message formatting, copyright update] +(cherry picked from commit f2530fc280dd84e6abc0f5475e261aa0d2ee2a21) +--- + proxy/src/client/gpm_common.c | 18 ++++++------------ + 1 file changed, 6 insertions(+), 12 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index b14e846..0d314fa 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -1,4 +1,4 @@ +-/* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */ ++/* Copyright (C) 2011,2017 the GSS-PROXY contributors, see COPYING for license */ + + #include "gssapi_gpm.h" + #include +@@ -415,10 +415,7 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + ret = 0; + + done: +- if (ret) { +- /* on errors we can only close the fd and return */ +- gpm_close_socket(gpmctx); +- } ++ /* we only need to return as gpm_retry_socket closes the socket */ + return ret; + } + +@@ -488,9 +485,10 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx, + + done: + if (ret) { +- /* on errors we can only close the fd and return */ +- gpm_close_socket(gpmctx); +- gpm_epoll_close(gpmctx); ++ /* on errors, free the buffer to prevent calling ++ * xdr_destroy(&xdr_reply_ctx); */ ++ free(*buffer); ++ *buffer = NULL; + } + return ret; + } +@@ -560,10 +558,6 @@ static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer, + /* Close and reopen socket before trying again */ + ret = gpm_retry_socket(gpmctx); + +- /* Free buffer and set it to NULL to prevent free(xdr_reply_ctx) */ +- free(*recv_buffer); +- *recv_buffer = NULL; +- + if (ret != 0) + return ret; + ret = ETIMEDOUT; diff --git a/SOURCES/Fix-error-message-handling-in-gp_config_from_dir.patch b/SOURCES/Fix-error-message-handling-in-gp_config_from_dir.patch new file mode 100644 index 0000000..6eaf495 --- /dev/null +++ b/SOURCES/Fix-error-message-handling-in-gp_config_from_dir.patch @@ -0,0 +1,53 @@ +From 298fb49c32d9bf709f14445c1848a3b2419cd3fd Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Fri, 27 Oct 2017 14:39:35 -0400 +Subject: [PATCH] Fix error message handling in gp_config_from_dir() + +Resolves a potential double free if we ever get both a return value +and error message back from ini_config_augment(). + +Commit c0d85387fc38f9554d601ec2ddb111031a694387 exposes a misbehavior +in libini wherein merge failures are presented as nonfatal errors. +Paper around this. + +Signed-off-by: Robbie Harwood +(cherry picked from commit 49708ddde8c58d8197e1f7dfc2b2d097c6b278d5) +--- + proxy/src/gp_config.c | 17 ++++++++++------- + 1 file changed, 10 insertions(+), 7 deletions(-) + +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index cd057a0..cb13b46 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -798,20 +798,23 @@ static int gp_config_from_dir(const char *config_dir, + &error_list, + NULL); + if (error_list) { +- uint32_t len; +- len = ref_array_len(error_list); ++ uint32_t len = ref_array_len(error_list); + for (uint32_t i = 0; i < len; i++) { + /* libini has an unfixable bug where error strings are (char **) */ +- GPAUDIT("Error when reading config directory: %s\n", +- *(char **)ref_array_get(error_list, i, NULL)); ++ char *errmsg = *(char **)ref_array_get(error_list, i, NULL); ++ ++ /* libini reports pattern match failure as (non-fatal) error ++ * https://pagure.io/SSSD/ding-libs/issue/3182 */ ++ if (strstr(errmsg, "did not match provided patterns. Skipping")) { ++ continue; ++ } ++ ++ GPAUDIT("Error when reading config directory: %s\n", errmsg); + } + ref_array_destroy(error_list); + } +- + if (ret && ret != EEXIST) { + GPERROR("Error when reading config directory number: %d\n", ret); +- +- ref_array_destroy(error_list); + return ret; + } + diff --git a/SOURCES/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch b/SOURCES/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch new file mode 100644 index 0000000..7c40244 --- /dev/null +++ b/SOURCES/Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch @@ -0,0 +1,79 @@ +From 8e9756f3cf0d03e5ca43f5212e3d4245ad1d9a00 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 11:16:42 -0500 +Subject: [PATCH] Fix handling of non-EPOLLIN/EPOLLOUT events + +Signed-off-by: Alexander Scheel +Reviewed-by: Robbie Harwood +Merges: #213 +(cherry picked from commit b8f5b2f75612a11753cf742ee0477b98df8e6b02) +--- + proxy/src/client/gpm_common.c | 49 ++++++++++++++++++++++++++++++------------- + 1 file changed, 35 insertions(+), 14 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 7d1158e..b14e846 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -283,26 +283,47 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) { + gpm_epoll_close(gpmctx); + } else if (epoll_ret == 1 && events[0].data.fd == gpmctx->timerfd) { + /* Got an event which is only our timer */ +- ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t)); +- if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { +- /* In the case when reading from the timer failed, don't hide the +- * timer error behind ETIMEDOUT such that it isn't retried */ +- ret = errno; ++ if ((events[0].events & EPOLLIN) == 0) { ++ /* We got an event which was not EPOLLIN; assume this is an error, ++ * and exit with EBADF: epoll_wait said timerfd had an event, ++ * but that event is not an EPOLIN event. */ ++ ret = EBADF; + } else { +- /* If ret == 0, then we definitely timed out. Else, if ret == -1 +- * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird +- * edge case where epoll thinks the timer can be read, but it +- * is blocking more; treat it like a TIMEOUT and retry, as +- * nothing around us would handle EAGAIN from timer and retry +- * it. */ +- ret = ETIMEDOUT; ++ ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t)); ++ if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { ++ /* In the case when reading from the timer failed, don't hide the ++ * timer error behind ETIMEDOUT such that it isn't retried */ ++ ret = errno; ++ } else { ++ /* If ret == 0, then we definitely timed out. Else, if ret == -1 ++ * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird ++ * edge case where epoll thinks the timer can be read, but it ++ * is blocking more; treat it like a TIMEOUT and retry, as ++ * nothing around us would handle EAGAIN from timer and retry ++ * it. */ ++ ret = ETIMEDOUT; ++ } + } + gpm_epoll_close(gpmctx); + } else { + /* If ret == 2, then we ignore the timerfd; that way if the next + * operation cannot be performed immediately, we timeout and retry. +- * If ret == 1 and data.fd == gpmctx->fd, return 0. */ +- ret = 0; ++ * Always check the returned event of the socket fd. */ ++ int fd_index = 0; ++ if (epoll_ret == 2 && events[fd_index].data.fd != gpmctx->fd) { ++ fd_index = 1; ++ } ++ ++ if ((events[fd_index].events & event_flags) == 0) { ++ /* We cannot call EPOLLIN/EPOLLOUT at this time; assume that this ++ * is a fatal error; return with EBADFD to distinguish from ++ * EBADF in timer_fd case. */ ++ ret = EBADFD; ++ gpm_epoll_close(gpmctx); ++ } else { ++ /* We definintely got a EPOLLIN/EPOLLOUT event; return success. */ ++ ret = 0; ++ } + } + + epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_DEL, gpmctx->fd, NULL); diff --git a/SOURCES/Fix-memory-leak.patch b/SOURCES/Fix-memory-leak.patch new file mode 100644 index 0000000..540c1ef --- /dev/null +++ b/SOURCES/Fix-memory-leak.patch @@ -0,0 +1,25 @@ +From abcd9ae04b1c3f9f0ebb72bd48737b08d5d7fe65 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 25 May 2017 21:35:37 -0400 +Subject: [PATCH] Fix memory leak + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Related-to: #176 +(cherry picked from commit 69a73d85eb3e70fdc7501794d5fd11a73a1d20fa) +--- + proxy/src/gp_export.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/proxy/src/gp_export.c b/proxy/src/gp_export.c +index f44da50..4e081df 100644 +--- a/proxy/src/gp_export.c ++++ b/proxy/src/gp_export.c +@@ -381,6 +381,7 @@ uint32_t gp_export_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + + done: + *min = ret_min; ++ gss_release_buffer(&ret_min, &token); + gss_release_name(&ret_min, &name); + gss_release_oid_set(&ret_min, &mechanisms); + return ret_maj; diff --git a/SOURCES/Fix-mismatched-sign-comparisons.patch b/SOURCES/Fix-mismatched-sign-comparisons.patch new file mode 100644 index 0000000..675b7e7 --- /dev/null +++ b/SOURCES/Fix-mismatched-sign-comparisons.patch @@ -0,0 +1,741 @@ +From a68b8b418bfc42c628fee605cc52dca92ab410c9 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 15 Mar 2017 14:52:08 -0400 +Subject: [PATCH] Fix mismatched sign comparisons + +We are c99, so also migrate to `for`-loop initializers where possible for +clarity. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #173 +(cherry picked from commit 377e92c7ead312c530b233a1e023493ecde033d6) +--- + proxy/src/client/gpm_acquire_cred.c | 11 +++------- + proxy/src/client/gpm_common.c | 4 ++-- + proxy/src/client/gpm_import_and_canon_name.c | 7 +++--- + proxy/src/client/gpm_indicate_mechs.c | 33 ++++++++++++---------------- + proxy/src/gp_common.h | 3 +-- + proxy/src/gp_config.c | 9 +++----- + proxy/src/gp_conv.c | 6 ++--- + proxy/src/gp_creds.c | 3 +-- + proxy/src/gp_export.c | 9 +++----- + proxy/src/gp_rpc_acquire_cred.c | 5 ++--- + proxy/src/gp_rpc_debug.c | 26 +++++++++++----------- + proxy/src/gp_rpc_indicate_mechs.c | 15 +++++-------- + proxy/src/gp_socket.c | 4 ++-- + proxy/src/gp_util.c | 4 ++-- + proxy/tests/t_utils.c | 4 ++-- + 15 files changed, 58 insertions(+), 85 deletions(-) + +diff --git a/proxy/src/client/gpm_acquire_cred.c b/proxy/src/client/gpm_acquire_cred.c +index 632973d..8e30e1d 100644 +--- a/proxy/src/client/gpm_acquire_cred.c ++++ b/proxy/src/client/gpm_acquire_cred.c +@@ -6,8 +6,6 @@ static int gpmint_cred_to_actual_mechs(gssx_cred *c, gss_OID_set *a) + { + gssx_cred_element *e; + gss_OID_set m = GSS_C_NO_OID_SET; +- int i; +- + + if (c->elements.elements_len) { + +@@ -22,7 +20,7 @@ static int gpmint_cred_to_actual_mechs(gssx_cred *c, gss_OID_set *a) + return ENOMEM; + } + +- for (i = 0; i < c->elements.elements_len; i++) { ++ for (unsigned i = 0; i < c->elements.elements_len; i++) { + e = &c->elements.elements_val[i]; + + m->elements[i].elements = gp_memdup(e->mech.octet_string_val, +@@ -280,7 +278,6 @@ OM_uint32 gpm_inquire_cred(OM_uint32 *minor_status, + uint32_t ret_maj = GSS_S_COMPLETE; + uint32_t life; + int cu; +- int i; + + if (!cred) { + *minor_status = 0; +@@ -308,8 +305,7 @@ OM_uint32 gpm_inquire_cred(OM_uint32 *minor_status, + life = GSS_C_INDEFINITE; + cu = -1; + +- for (i = 0; i < cred->elements.elements_len; i++) { +- ++ for (unsigned i = 0; i < cred->elements.elements_len; i++) { + e = &cred->elements.elements_val[i]; + + switch (e->cred_usage) { +@@ -402,7 +398,7 @@ OM_uint32 gpm_inquire_cred_by_mech(OM_uint32 *minor_status, + gss_OID_desc tmp_oid; + uint32_t ret_min = 0; + uint32_t ret_maj = GSS_S_COMPLETE; +- int i; ++ unsigned i; + + if (!cred) { + *minor_status = 0; +@@ -414,7 +410,6 @@ OM_uint32 gpm_inquire_cred_by_mech(OM_uint32 *minor_status, + } + + for (i = 0; i < cred->elements.elements_len; i++) { +- + e = &cred->elements.elements_val[i]; + gp_conv_gssx_to_oid(&e->mech, &tmp_oid); + if (!gss_oid_equal(&tmp_oid, mech_type)) { +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 030765a..8c96986 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -166,7 +166,7 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + char *buffer, uint32_t length) + { + uint32_t size; +- size_t wn; ++ ssize_t wn; + size_t pos; + bool retry; + int ret; +@@ -232,7 +232,7 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx, + char *buffer, uint32_t *length) + { + uint32_t size; +- size_t rn; ++ ssize_t rn; + size_t pos; + int ret; + +diff --git a/proxy/src/client/gpm_import_and_canon_name.c b/proxy/src/client/gpm_import_and_canon_name.c +index 83d0736..70149a3 100644 +--- a/proxy/src/client/gpm_import_and_canon_name.c ++++ b/proxy/src/client/gpm_import_and_canon_name.c +@@ -275,7 +275,6 @@ OM_uint32 gpm_inquire_name(OM_uint32 *minor_status, + { + gss_buffer_set_t xattrs = GSS_C_NO_BUFFER_SET; + int ret; +- int i; + + *minor_status = 0; + +@@ -306,13 +305,13 @@ OM_uint32 gpm_inquire_name(OM_uint32 *minor_status, + *minor_status = ENOMEM; + return GSS_S_FAILURE; + } +- for (i = 0; i < xattrs->count; i++) { ++ for (unsigned i = 0; i < xattrs->count; i++) { + ret = gp_copy_gssx_to_buffer( + &name->name_attributes.name_attributes_val[i].attr, + &xattrs->elements[i]); + if (ret) { +- for (--i; i >= 0; i--) { +- free(xattrs->elements[i].value); ++ for (; i > 0; i--) { ++ free(xattrs->elements[i-1].value); + } + free(xattrs->elements); + free(xattrs); +diff --git a/proxy/src/client/gpm_indicate_mechs.c b/proxy/src/client/gpm_indicate_mechs.c +index d4df923..b019a96 100644 +--- a/proxy/src/client/gpm_indicate_mechs.c ++++ b/proxy/src/client/gpm_indicate_mechs.c +@@ -51,7 +51,6 @@ static uint32_t gpm_copy_gss_OID_set(uint32_t *minor_status, + gss_OID_set n; + uint32_t ret_maj; + uint32_t ret_min; +- int i; + + ret_maj = gss_create_empty_oid_set(&ret_min, &n); + if (ret_maj) { +@@ -59,7 +58,7 @@ static uint32_t gpm_copy_gss_OID_set(uint32_t *minor_status, + return ret_maj; + } + +- for (i = 0; i < oldset->count; i++) { ++ for (size_t i = 0; i < oldset->count; i++) { + ret_maj = gss_add_oid_set_member(&ret_min, &oldset->elements[i], &n); + if (ret_maj) { + *minor_status = ret_min; +@@ -124,7 +123,6 @@ static void gpmint_indicate_mechs(void) + uint32_t ret_min; + uint32_t ret_maj = 0; + int ret = 0; +- int i; + + memset(arg, 0, sizeof(gssx_arg_indicate_mechs)); + memset(res, 0, sizeof(gssx_res_indicate_mechs)); +@@ -158,7 +156,7 @@ static void gpmint_indicate_mechs(void) + goto done; + } + +- for (i = 0; i < res->mechs.mechs_len; i++) { ++ for (unsigned i = 0; i < res->mechs.mechs_len; i++) { + mi = &res->mechs.mechs_val[i]; + gi = &global_mechs.info[i]; + +@@ -222,7 +220,7 @@ static void gpmint_indicate_mechs(void) + goto done; + } + +- for (i = 0; i < res->mech_attr_descs.mech_attr_descs_len; i++) { ++ for (unsigned i = 0; i < res->mech_attr_descs.mech_attr_descs_len; i++) { + ma = &res->mech_attr_descs.mech_attr_descs_val[i]; + ga = &global_mechs.desc[i]; + +@@ -249,7 +247,7 @@ static void gpmint_indicate_mechs(void) + + done: + if (ret || ret_maj) { +- for (i = 0; i < global_mechs.desc_len; i++) { ++ for (unsigned i = 0; i < global_mechs.desc_len; i++) { + ga = &global_mechs.desc[i]; + gss_release_oid(&discard, &ga->attr); + gss_release_buffer(&discard, ga->name); +@@ -258,7 +256,7 @@ done: + } + free(global_mechs.desc); + global_mechs.desc = NULL; +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + gi = &global_mechs.info[i]; + gss_release_oid(&discard, &gi->mech); + gss_release_oid_set(&discard, &gi->name_types); +@@ -335,7 +333,6 @@ OM_uint32 gpm_inquire_names_for_mech(OM_uint32 *minor_status, + { + uint32_t ret_min; + uint32_t ret_maj; +- int i; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -351,7 +348,7 @@ OM_uint32 gpm_inquire_names_for_mech(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + if (!gpm_equal_oids(global_mechs.info[i].mech, mech_type)) { + continue; + } +@@ -375,7 +372,6 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status, + uint32_t discard; + gss_OID name_type = GSS_C_NO_OID; + int present; +- int i; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -402,7 +398,7 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status, + goto done; + } + +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + ret_maj = gss_test_oid_set_member(&ret_min, name_type, + global_mechs.info[i].name_types, + &present); +@@ -439,7 +435,6 @@ OM_uint32 gpm_inquire_attrs_for_mech(OM_uint32 *minor_status, + uint32_t ret_min; + uint32_t ret_maj; + uint32_t discard; +- int i; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -451,7 +446,7 @@ OM_uint32 gpm_inquire_attrs_for_mech(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + if (!gpm_equal_oids(global_mechs.info[i].mech, mech)) { + continue; + } +@@ -495,7 +490,6 @@ OM_uint32 gpm_inquire_saslname_for_mech(OM_uint32 *minor_status, + uint32_t ret_min; + uint32_t ret_maj; + uint32_t discard; +- int i; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -511,7 +505,7 @@ OM_uint32 gpm_inquire_saslname_for_mech(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + if (!gpm_equal_oids(global_mechs.info[i].mech, desired_mech)) { + continue; + } +@@ -554,7 +548,6 @@ OM_uint32 gpm_display_mech_attr(OM_uint32 *minor_status, + uint32_t ret_min; + uint32_t ret_maj; + uint32_t discard; +- int i; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -570,7 +563,7 @@ OM_uint32 gpm_display_mech_attr(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- for (i = 0; i < global_mechs.desc_len; i++) { ++ for (unsigned i = 0; i < global_mechs.desc_len; i++) { + if (!gpm_equal_oids(global_mechs.desc[i].attr, mech_attr)) { + continue; + } +@@ -614,7 +607,6 @@ OM_uint32 gpm_indicate_mechs_by_attrs(OM_uint32 *minor_status, + uint32_t ret_maj; + uint32_t discard; + int present; +- int i, j; + + if (!minor_status) { + return GSS_S_CALL_INACCESSIBLE_WRITE; +@@ -636,8 +628,9 @@ OM_uint32 gpm_indicate_mechs_by_attrs(OM_uint32 *minor_status, + return ret_maj; + } + +- for (i = 0; i < global_mechs.info_len; i++) { ++ for (unsigned i = 0; i < global_mechs.info_len; i++) { + if (desired_mech_attrs != GSS_C_NO_OID_SET) { ++ unsigned j; + for (j = 0; j < desired_mech_attrs->count; j++) { + ret_maj = gss_test_oid_set_member(&ret_min, + &desired_mech_attrs->elements[j], +@@ -657,6 +650,7 @@ OM_uint32 gpm_indicate_mechs_by_attrs(OM_uint32 *minor_status, + } + } + if (except_mech_attrs != GSS_C_NO_OID_SET) { ++ unsigned j; + for (j = 0; j < except_mech_attrs->count; j++) { + ret_maj = gss_test_oid_set_member(&ret_min, + &except_mech_attrs->elements[j], +@@ -676,6 +670,7 @@ OM_uint32 gpm_indicate_mechs_by_attrs(OM_uint32 *minor_status, + } + } + if (critical_mech_attrs != GSS_C_NO_OID_SET) { ++ unsigned j; + for (j = 0; j < critical_mech_attrs->count; j++) { + ret_maj = gss_test_oid_set_member(&ret_min, + &critical_mech_attrs->elements[j], +diff --git a/proxy/src/gp_common.h b/proxy/src/gp_common.h +index edc23b4..4f76e58 100644 +--- a/proxy/src/gp_common.h ++++ b/proxy/src/gp_common.h +@@ -104,9 +104,8 @@ union gp_rpc_res { + #define gp_options_find(res, opts, name, len) \ + do { \ + struct gssx_option *_v; \ +- int _o; \ + res = NULL; \ +- for (_o = 0; _o < opts.options_len; _o++) { \ ++ for (unsigned _o = 0; _o < opts.options_len; _o++) { \ + _v = &opts.options_val[_o]; \ + if (gp_option_name_match(_v, name, len)) { \ + res = _v; \ +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index 5c1ca02..a671333 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -57,11 +57,9 @@ static void free_str_array(const char ***a, int *count) + + void free_cred_store_elements(gss_key_value_set_desc *cs) + { +- int i; +- + if (!cs->elements) return; + +- for (i = 0; i < cs->count; i++) { ++ for (unsigned i = 0; i < cs->count; i++) { + safefree(cs->elements[i].key); + safefree(cs->elements[i].value); + } +@@ -146,7 +144,7 @@ static int get_krb5_mech_cfg(struct gp_service *svc, + &count, &strings); + if (ret == 0) { + const char *p; +- size_t len; ++ ssize_t len; + char *key; + + svc->krb5.store.elements = +@@ -698,7 +696,6 @@ struct gp_creds_handle *gp_service_get_creds_handle(struct gp_service *svc) + void free_config(struct gp_config **cfg) + { + struct gp_config *config = *cfg; +- uint32_t i; + + if (!config) { + return; +@@ -709,7 +706,7 @@ void free_config(struct gp_config **cfg) + free(config->socket_name); + free(config->proxy_user); + +- for (i=0; i < config->num_svcs; i++) { ++ for (int i = 0; i < config->num_svcs; i++) { + gp_service_free(config->svcs[i]); + safefree(config->svcs[i]); + } +diff --git a/proxy/src/gp_conv.c b/proxy/src/gp_conv.c +index 71d6d9d..b874b06 100644 +--- a/proxy/src/gp_conv.c ++++ b/proxy/src/gp_conv.c +@@ -599,7 +599,6 @@ done: + int gp_conv_gssx_to_oid_set(gssx_OID_set *in, gss_OID_set *out) + { + gss_OID_set o; +- int i; + + if (in->gssx_OID_set_len == 0) { + *out = GSS_C_NO_OID_SET; +@@ -618,7 +617,7 @@ int gp_conv_gssx_to_oid_set(gssx_OID_set *in, gss_OID_set *out) + return ENOMEM; + } + +- for (i = 0; i < o->count; i++) { ++ for (size_t i = 0; i < o->count; i++) { + o->elements[i].elements = + gp_memdup(in->gssx_OID_set_val[i].octet_string_val, + in->gssx_OID_set_val[i].octet_string_len); +@@ -641,7 +640,6 @@ int gp_conv_gssx_to_oid_set(gssx_OID_set *in, gss_OID_set *out) + int gp_conv_oid_set_to_gssx(gss_OID_set in, gssx_OID_set *out) + { + int ret; +- int i; + + if (in->count == 0) { + return 0; +@@ -653,7 +651,7 @@ int gp_conv_oid_set_to_gssx(gss_OID_set in, gssx_OID_set *out) + return ENOMEM; + } + +- for (i = 0; i < in->count; i++) { ++ for (size_t i = 0; i < in->count; i++) { + ret = gp_conv_octet_string(in->elements[i].length, + in->elements[i].elements, + &out->gssx_OID_set_val[i]); +diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c +index 6570b06..e05ad01 100644 +--- a/proxy/src/gp_creds.c ++++ b/proxy/src/gp_creds.c +@@ -312,7 +312,6 @@ static int gp_get_cred_environment(struct gp_call_ctx *gpcall, + int k_num = -1; + int ck_num = -1; + int cc_num = -1; +- int d; + + memset(cs, 0, sizeof(gss_key_value_set_desc)); + +@@ -419,7 +418,7 @@ static int gp_get_cred_environment(struct gp_call_ctx *gpcall, + ret = ENOMEM; + goto done; + } +- for (d = 0; d < svc->krb5.store.count; d++) { ++ for (unsigned d = 0; d < svc->krb5.store.count; d++) { + if (strcmp(svc->krb5.store.elements[d].key, "client_keytab") == 0) { + ck_num = cs->count; + } else if (strcmp(svc->krb5.store.elements[d].key, "keytab") == 0) { +diff --git a/proxy/src/gp_export.c b/proxy/src/gp_export.c +index 12b8d5f..3a927c9 100644 +--- a/proxy/src/gp_export.c ++++ b/proxy/src/gp_export.c +@@ -288,7 +288,6 @@ uint32_t gp_export_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + uint32_t acceptor_lifetime = 0; + struct gssx_cred_element *el; + int ret; +- int i, j; + struct gp_creds_handle *handle = NULL; + gss_buffer_desc token = GSS_C_EMPTY_BUFFER; + +@@ -314,8 +313,7 @@ uint32_t gp_export_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + } + out->elements.elements_len = mechanisms->count; + +- for (i = 0, j = 0; i < mechanisms->count; i++, j++) { +- ++ for (unsigned i = 0, j = 0; i < mechanisms->count; i++, j++) { + el = &out->elements.elements_val[j]; + + ret_maj = gss_inquire_cred_by_mech(&ret_min, *in, +@@ -399,11 +397,10 @@ static void gp_set_cred_options(gssx_cred *cred, gss_cred_id_t gss_cred) + krb5_enctype *ktypes; + bool no_ci_flags = false; + uint32_t maj, min; +- int i, j; + +- for (i = 0; i < cred->elements.elements_len; i++) { ++ for (unsigned i = 0; i < cred->elements.elements_len; i++) { + ce = &cred->elements.elements_val[i]; +- for (j = 0; j < ce->options.options_len; j++) { ++ for (unsigned j = 0; j < ce->options.options_len; j++) { + op = &ce->options.options_val[j]; + if ((op->option.octet_string_len == + sizeof(KRB5_SET_ALLOWED_ENCTYPE)) && +diff --git a/proxy/src/gp_rpc_acquire_cred.c b/proxy/src/gp_rpc_acquire_cred.c +index e9c7d56..fcb4fbe 100644 +--- a/proxy/src/gp_rpc_acquire_cred.c ++++ b/proxy/src/gp_rpc_acquire_cred.c +@@ -20,7 +20,6 @@ int gp_acquire_cred(struct gp_call_ctx *gpcall, + gss_cred_id_t *add_out_cred = NULL; + int acquire_type = ACQ_NORMAL; + int ret; +- int i; + + aca = &arg->acquire_cred; + acr = &res->acquire_cred; +@@ -63,7 +62,7 @@ int gp_acquire_cred(struct gp_call_ctx *gpcall, + goto done; + } + +- for (i = 0; i < desired_mechs->count; i++) { ++ for (unsigned i = 0; i < desired_mechs->count; i++) { + desired_mech = &desired_mechs->elements[i]; + + if (!gp_creds_allowed_mech(gpcall, desired_mech)) { +@@ -93,7 +92,7 @@ int gp_acquire_cred(struct gp_call_ctx *gpcall, + + cred_usage = gp_conv_gssx_to_cred_usage(aca->cred_usage); + +- for (i = 0; i < use_mechs->count; i++) { ++ for (unsigned i = 0; i < use_mechs->count; i++) { + desired_mech = &use_mechs->elements[i]; + /* this should really be folded into an extended + * gss_add_cred in gssapi that can accept a set of URIs +diff --git a/proxy/src/gp_rpc_debug.c b/proxy/src/gp_rpc_debug.c +index 2e2c050..a814448 100644 +--- a/proxy/src/gp_rpc_debug.c ++++ b/proxy/src/gp_rpc_debug.c +@@ -19,7 +19,7 @@ void gpdbg_octet_string(octet_string *x) + } + fprintf(stderr, "... ] "); + } else { +- for (int i = 0; i < x->octet_string_len; i++) { ++ for (unsigned i = 0; i < x->octet_string_len; i++) { + fprintf(stderr, "%x", x->octet_string_val[i]); + } + fprintf(stderr, " ] "); +@@ -55,7 +55,7 @@ void gpdbg_gssx_OID(gssx_OID *x) + void gpdbg_gssx_OID_set(gssx_OID_set *x) + { + gp_debug_printf("{ "); +- for (int i = 0; i < x->gssx_OID_set_len; i++) { ++ for (unsigned i = 0; i < x->gssx_OID_set_len; i++) { + gpdbg_gssx_OID(&x->gssx_OID_set_val[i]); + } + gp_debug_printf("} "); +@@ -90,7 +90,7 @@ void gpdbg_gssx_option(gssx_option *x) + #define gpdbg_extensions(x) do { \ + if ((x)->extensions.extensions_len > 0) { \ + gp_debug_printf("[ "); \ +- for (int i = 0; i < (x)->extensions.extensions_len; i++) { \ ++ for (unsigned i = 0; i < (x)->extensions.extensions_len; i++) { \ + gpdbg_gssx_option(&(x)->extensions.extensions_val[i]); \ + } \ + gp_debug_printf("] "); \ +@@ -100,7 +100,7 @@ void gpdbg_gssx_option(gssx_option *x) + #define gpdbg_options(x) do { \ + if ((x)->options.options_len > 0) { \ + gp_debug_printf("[ "); \ +- for (int i = 0; i < (x)->options.options_len; i++) { \ ++ for (unsigned i = 0; i < (x)->options.options_len; i++) { \ + gpdbg_gssx_option(&(x)->options.options_val[i]); \ + } \ + gp_debug_printf("] "); \ +@@ -168,7 +168,7 @@ void gpdbg_gssx_call_ctx(gssx_call_ctx *x) + #define gpdbg_name_attributes(X) do { \ + gp_debug_printf("[ "); \ + if (x->name_attributes.name_attributes_len > 0) { \ +- for (int i = 0; i < x->name_attributes.name_attributes_len; i++) { \ ++ for (unsigned i = 0; i < x->name_attributes.name_attributes_len; i++) { \ + gpdbg_gssx_name_attr( \ + &x->name_attributes.name_attributes_val[i]); \ + } \ +@@ -209,7 +209,7 @@ void gpdbg_gssx_cred(gssx_cred *x) + gp_debug_printf("{ "); + gpdbg_gssx_name(&x->desired_name); + gp_debug_printf("[ "); +- for (int i = 0; i < x->elements.elements_len; i++) { ++ for (unsigned i = 0; i < x->elements.elements_len; i++) { + gpdbg_gssx_cred_element(&x->elements.elements_val[i]); + } + gp_debug_printf("] "); +@@ -289,17 +289,17 @@ void gpdbg_gssx_res_indicate_mechs(gssx_res_indicate_mechs *x) + gp_debug_printf(" GSSX_RES_INDICATE_MECHS( status: "); + gpdbg_gssx_status(&x->status); + gp_debug_printf("mechs: [ "); +- for (int i = 0; i < x->mechs.mechs_len; i++) { ++ for (unsigned i = 0; i < x->mechs.mechs_len; i++) { + gpdbg_gssx_mech_info(&x->mechs.mechs_val[i]); + } + gp_debug_printf("] "); + gp_debug_printf("mech_attr_descs: [ "); +- for (int i = 0; i < x->mech_attr_descs.mech_attr_descs_len; i++) { ++ for (unsigned i = 0; i < x->mech_attr_descs.mech_attr_descs_len; i++) { + gpdbg_gssx_mech_attr(&x->mech_attr_descs.mech_attr_descs_val[i]); + } + gp_debug_printf("] "); + gp_debug_printf("supported_extensions: [ "); +- for (int i = 0; ++ for (unsigned i = 0; + i < x->supported_extensions.supported_extensions_len; i++) { + gpdbg_gssx_buffer( + &x->supported_extensions.supported_extensions_val[i]); +@@ -602,7 +602,7 @@ void gpdbg_gssx_arg_wrap(gssx_arg_wrap *x) + gp_debug_printf("conf_req: "); + gp_debug_printf("%d ", (int)x->conf_req); + gp_debug_printf("message_buffer: [ "); +- for (int i = 0; i < x->message_buffer.message_buffer_len; i++) { ++ for (unsigned i = 0; i < x->message_buffer.message_buffer_len; i++) { + gpdbg_octet_string(&x->message_buffer.message_buffer_val[i]); + } + gp_debug_printf("] "); +@@ -618,7 +618,7 @@ void gpdbg_gssx_res_wrap(gssx_res_wrap *x) + gp_debug_printf("context_handle: "); + GPRPCDEBUG(gssx_ctx, x->context_handle); + gp_debug_printf("token_buffer: [ "); +- for (int i = 0; i < x->token_buffer.token_buffer_len; i++) { ++ for (unsigned i = 0; i < x->token_buffer.token_buffer_len; i++) { + gpdbg_octet_string(&x->token_buffer.token_buffer_val[i]); + } + gp_debug_printf("] "); +@@ -640,7 +640,7 @@ void gpdbg_gssx_arg_unwrap(gssx_arg_unwrap *x) + gp_debug_printf("context_handle: "); + gpdbg_gssx_ctx(&x->context_handle); + gp_debug_printf("token_buffer: [ "); +- for (int i = 0; i < x->token_buffer.token_buffer_len; i++) { ++ for (unsigned i = 0; i < x->token_buffer.token_buffer_len; i++) { + gpdbg_octet_string(&x->token_buffer.token_buffer_val[i]); + } + gp_debug_printf("] "); +@@ -656,7 +656,7 @@ void gpdbg_gssx_res_unwrap(gssx_res_unwrap *x) + gp_debug_printf("context_handle: "); + GPRPCDEBUG(gssx_ctx, x->context_handle); + gp_debug_printf("message_buffer: [ "); +- for (int i = 0; i < x->message_buffer.message_buffer_len; i++) { ++ for (unsigned i = 0; i < x->message_buffer.message_buffer_len; i++) { + gpdbg_octet_string(&x->message_buffer.message_buffer_val[i]); + } + gp_debug_printf("] "); +diff --git a/proxy/src/gp_rpc_indicate_mechs.c b/proxy/src/gp_rpc_indicate_mechs.c +index 8abbc7f..6ae6756 100644 +--- a/proxy/src/gp_rpc_indicate_mechs.c ++++ b/proxy/src/gp_rpc_indicate_mechs.c +@@ -25,8 +25,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + uint32_t ret_maj; + uint32_t ret_min; + int present; +- int h, i, j; +- int ret; ++ int ret; + + ima = &arg->indicate_mechs; + imr = &res->indicate_mechs; +@@ -53,8 +52,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + } + imr->mechs.mechs_len = mech_set->count; + +- for (i = 0, h = 0; i < mech_set->count; i++, h++) { +- ++ for (unsigned i = 0, h = 0; i < mech_set->count; i++, h++) { + mi = &imr->mechs.mechs_val[h]; + + ret = gp_conv_oid_to_gssx(&mech_set->elements[i], &mi->mech); +@@ -104,8 +102,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + ret_min = ret; + goto done; + } +- for (j = 0; j < mech_attrs->count; j++) { +- ++ for (unsigned j = 0; j < mech_attrs->count; j++) { + ret_maj = gss_test_oid_set_member(&ret_min, + &mech_attrs->elements[j], + attr_set, +@@ -136,8 +133,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + goto done; + } + +- for (j = 0; j < known_mech_attrs->count; j++) { +- ++ for (unsigned j = 0; j < known_mech_attrs->count; j++) { + ret_maj = gss_test_oid_set_member(&ret_min, + &known_mech_attrs->elements[j], + attr_set, +@@ -205,8 +201,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + } + imr->mech_attr_descs.mech_attr_descs_len = attr_set->count; + +- for (i = 0; i < attr_set->count; i++) { +- ++ for (unsigned i = 0; i < attr_set->count; i++) { + ma = &imr->mech_attr_descs.mech_attr_descs_val[i]; + + ret = gp_conv_oid_to_gssx(&attr_set->elements[i], &ma->attr); +diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c +index 829ff21..17ecf7c 100644 +--- a/proxy/src/gp_socket.c ++++ b/proxy/src/gp_socket.c +@@ -303,7 +303,7 @@ static void gp_socket_read(verto_ctx *vctx, verto_ev *ev) + struct gp_buffer *rbuf; + uint32_t size; + bool header = false; +- size_t rn; ++ ssize_t rn; + int ret; + int fd; + +@@ -487,7 +487,7 @@ static void gp_socket_write(verto_ctx *vctx, verto_ev *ev) + return; + } + if (vecs == 2) { +- if (wn < sizeof(size)) { ++ if (wn < (ssize_t) sizeof(size)) { + /* don't bother trying to handle sockets that can't + * buffer even 4 bytes */ + gp_conn_free(wbuf->conn); +diff --git a/proxy/src/gp_util.c b/proxy/src/gp_util.c +index ca83eb3..f158b84 100644 +--- a/proxy/src/gp_util.c ++++ b/proxy/src/gp_util.c +@@ -109,7 +109,7 @@ char *gp_strerror(int errnum) + ssize_t gp_safe_read(int fd, void *buf, size_t count) + { + char *b = (char *)buf; +- ssize_t len = 0; ++ size_t len = 0; + ssize_t ret; + + do { +@@ -128,7 +128,7 @@ ssize_t gp_safe_read(int fd, void *buf, size_t count) + ssize_t gp_safe_write(int fd, const void *buf, size_t count) + { + const char *b = (const char *)buf; +- ssize_t len = 0; ++ size_t len = 0; + ssize_t ret; + + do { +diff --git a/proxy/tests/t_utils.c b/proxy/tests/t_utils.c +index 6af9a16..36f7bd1 100644 +--- a/proxy/tests/t_utils.c ++++ b/proxy/tests/t_utils.c +@@ -8,7 +8,7 @@ + int t_send_buffer(int fd, char *buf, uint32_t len) + { + uint32_t size; +- size_t wn; ++ ssize_t wn; + size_t pos; + + size = htonl(len); +@@ -36,7 +36,7 @@ int t_send_buffer(int fd, char *buf, uint32_t len) + int t_recv_buffer(int fd, char *buf, uint32_t *len) + { + uint32_t size; +- size_t rn; ++ ssize_t rn; + size_t pos; + + rn = read(fd, &size, sizeof(uint32_t)); diff --git a/SOURCES/Fix-most-memory-leaks.patch b/SOURCES/Fix-most-memory-leaks.patch new file mode 100644 index 0000000..c4a8d75 --- /dev/null +++ b/SOURCES/Fix-most-memory-leaks.patch @@ -0,0 +1,250 @@ +From 9f9ab1e13c72b7c1fd06b6ba085ba2853bb9c3ca Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 29 Jun 2017 10:59:46 -0400 +Subject: [PATCH] Fix most memory leaks + +Signed-off-by: Alexander Scheel +[rharwood@redhat.com: commit message, whitespace] +Reviewed-by: Robbie Harwood +Merges: #203 +Related: #176 +(cherry picked from commit 470cf4d745d57f0597124a35b2faf86ba1107bb5) +[rharwood@redhat.com: backport around missing program support] +--- + proxy/src/gp_config.c | 1 + + proxy/src/gp_creds.c | 2 ++ + proxy/src/gp_export.c | 3 ++- + proxy/src/gp_rpc_acquire_cred.c | 17 ++++++++++------ + proxy/src/gssproxy.c | 42 ++++++++++++++++++++++++++++------------ + proxy/src/mechglue/gpp_context.c | 2 ++ + proxy/tests/t_acquire.c | 3 +++ + 7 files changed, 51 insertions(+), 19 deletions(-) + +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index a671333..b4ab90c 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -75,6 +75,7 @@ static void gp_service_free(struct gp_service *svc) + free_cred_store_elements(&svc->krb5.store); + gp_free_creds_handle(&svc->krb5.creds_handle); + } ++ free(svc->socket); + SELINUX_context_free(svc->selinux_ctx); + memset(svc, 0, sizeof(struct gp_service)); + } +diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c +index fdc6bdf..2cb4ce7 100644 +--- a/proxy/src/gp_creds.c ++++ b/proxy/src/gp_creds.c +@@ -1049,6 +1049,8 @@ uint32_t gp_count_tickets(uint32_t *min, gss_cred_id_t cred, uint32_t *ccsum) + goto done; + } + ++ krb5_free_cred_contents(context, &creds); ++ + /* TODO: Should we do a real checksum over all creds->ticket data and + * flags in future ? */ + (*ccsum)++; +diff --git a/proxy/src/gp_export.c b/proxy/src/gp_export.c +index 4e081df..ab08bb7 100644 +--- a/proxy/src/gp_export.c ++++ b/proxy/src/gp_export.c +@@ -47,7 +47,7 @@ uint32_t gp_init_creds_with_keytab(uint32_t *min, const char *svc_name, + krb5_keytab ktid = NULL; + krb5_kt_cursor cursor; + krb5_keytab_entry entry; +- krb5_enctype *permitted; ++ krb5_enctype *permitted = NULL; + uint32_t ret_maj = 0; + uint32_t ret_min = 0; + int ret; +@@ -127,6 +127,7 @@ uint32_t gp_init_creds_with_keytab(uint32_t *min, const char *svc_name, + ret_maj = GSS_S_COMPLETE; + + done: ++ krb5_free_enctypes(handle->context, permitted); + if (ktid) { + (void)krb5_kt_close(handle->context, ktid); + } +diff --git a/proxy/src/gp_rpc_acquire_cred.c b/proxy/src/gp_rpc_acquire_cred.c +index fcb4fbe..7ddb427 100644 +--- a/proxy/src/gp_rpc_acquire_cred.c ++++ b/proxy/src/gp_rpc_acquire_cred.c +@@ -130,17 +130,18 @@ int gp_acquire_cred(struct gp_call_ctx *gpcall, + } + } + +- acr->output_cred_handle = calloc(1, sizeof(gssx_cred)); +- if (!acr->output_cred_handle) { +- ret_maj = GSS_S_FAILURE; +- ret_min = ENOMEM; +- goto done; +- } + + if (out_cred == in_cred) { + acr->output_cred_handle = aca->input_cred_handle; + aca->input_cred_handle = NULL; + } else { ++ acr->output_cred_handle = calloc(1, sizeof(gssx_cred)); ++ if (!acr->output_cred_handle) { ++ ret_maj = GSS_S_FAILURE; ++ ret_min = ENOMEM; ++ goto done; ++ } ++ + ret_maj = gp_export_gssx_cred(&ret_min, gpcall, + &out_cred, acr->output_cred_handle); + if (ret_maj) { +@@ -154,6 +155,10 @@ done: + + GPRPCDEBUG(gssx_res_acquire_cred, acr); + ++ if (add_out_cred != &in_cred && add_out_cred != &out_cred) ++ gss_release_cred(&ret_min, add_out_cred); ++ if (in_cred != out_cred) ++ gss_release_cred(&ret_min, &in_cred); + gss_release_cred(&ret_min, &out_cred); + gss_release_oid_set(&ret_min, &use_mechs); + gss_release_oid_set(&ret_min, &desired_mechs); +diff --git a/proxy/src/gssproxy.c b/proxy/src/gssproxy.c +index a020218..5c5937d 100644 +--- a/proxy/src/gssproxy.c ++++ b/proxy/src/gssproxy.c +@@ -157,7 +157,7 @@ int main(int argc, const char *argv[]) + verto_ctx *vctx; + verto_ev *ev; + int wait_fd; +- int ret; ++ int ret = -1; + + struct poptOption long_options[] = { + POPT_AUTOHELP +@@ -187,13 +187,17 @@ int main(int argc, const char *argv[]) + fprintf(stderr, "\nInvalid option %s: %s\n\n", + poptBadOption(pc, 0), poptStrerror(opt)); + poptPrintUsage(pc, stderr, 0); +- return 1; ++ ++ ret = 1; ++ goto cleanup; + } + } + + if (opt_version) { + puts(VERSION""DISTRO_VERSION""PRERELEASE_VERSION); +- return 0; ++ poptFreeContext(pc); ++ ret = 0; ++ goto cleanup; + } + + if (opt_debug || opt_debug_level > 0) { +@@ -204,7 +208,8 @@ int main(int argc, const char *argv[]) + if (opt_daemon && opt_interactive) { + fprintf(stderr, "Option -i|--interactive is not allowed together with -D|--daemon\n"); + poptPrintUsage(pc, stderr, 0); +- return 1; ++ ret = 0; ++ goto cleanup; + } + + if (opt_interactive) { +@@ -218,7 +223,8 @@ int main(int argc, const char *argv[]) + opt_config_socket, + opt_daemon); + if (!gpctx->config) { +- exit(EXIT_FAILURE); ++ ret = EXIT_FAILURE; ++ goto cleanup; + } + + init_server(gpctx->config->daemonize, &wait_fd); +@@ -229,7 +235,8 @@ int main(int argc, const char *argv[]) + if (!vctx) { + fprintf(stderr, "Failed to initialize event loop. " + "Is there at least one libverto backend installed?\n"); +- return 1; ++ ret = 1; ++ goto cleanup; + } + gpctx->vctx = vctx; + +@@ -237,12 +244,13 @@ int main(int argc, const char *argv[]) + ev = verto_add_signal(vctx, VERTO_EV_FLAG_PERSIST, hup_handler, SIGHUP); + if (!ev) { + fprintf(stderr, "Failed to register SIGHUP handler with verto!\n"); +- return 1; ++ ret = 1; ++ goto cleanup; + } + + ret = init_sockets(vctx, NULL); + if (ret != 0) { +- return ret; ++ goto cleanup; + } + + /* We need to tell nfsd that GSS-Proxy is available before it starts, +@@ -256,12 +264,14 @@ int main(int argc, const char *argv[]) + + ret = drop_privs(gpctx->config); + if (ret) { +- exit(EXIT_FAILURE); ++ ret = EXIT_FAILURE; ++ goto cleanup; + } + + ret = gp_workers_init(gpctx); + if (ret) { +- exit(EXIT_FAILURE); ++ ret = EXIT_FAILURE; ++ goto cleanup; + } + + verto_run(vctx); +@@ -271,9 +281,17 @@ int main(int argc, const char *argv[]) + + fini_server(); + +- poptFreeContext(pc); + + free_config(&gpctx->config); ++ free(gpctx); + +- return 0; ++ ret = 0; ++ ++cleanup: ++ poptFreeContext(pc); ++ free(opt_config_file); ++ free(opt_config_dir); ++ free(opt_config_socket); ++ ++ return ret; + } +diff --git a/proxy/src/mechglue/gpp_context.c b/proxy/src/mechglue/gpp_context.c +index 2f41e4f..69e69e0 100644 +--- a/proxy/src/mechglue/gpp_context.c ++++ b/proxy/src/mechglue/gpp_context.c +@@ -362,6 +362,8 @@ OM_uint32 gssi_delete_sec_context(OM_uint32 *minor_status, + } + } + ++ free(ctx); ++ + return rmaj; + } + +diff --git a/proxy/tests/t_acquire.c b/proxy/tests/t_acquire.c +index 2bb7706..5334565 100644 +--- a/proxy/tests/t_acquire.c ++++ b/proxy/tests/t_acquire.c +@@ -132,5 +132,8 @@ done: + gss_release_buffer(&ret_min, &in_token); + gss_release_buffer(&ret_min, &out_token); + gss_release_cred(&ret_min, &cred_handle); ++ gss_release_name(&ret_min, &target_name); ++ gss_delete_sec_context(&ret_min, &init_ctx, GSS_C_NO_BUFFER); ++ gss_delete_sec_context(&ret_min, &accept_ctx, GSS_C_NO_BUFFER); + return ret; + } diff --git a/SOURCES/Fix-potential-free-of-non-heap-address.patch b/SOURCES/Fix-potential-free-of-non-heap-address.patch new file mode 100644 index 0000000..11a572d --- /dev/null +++ b/SOURCES/Fix-potential-free-of-non-heap-address.patch @@ -0,0 +1,28 @@ +From 9c35e3e40f63e3bc98c258810abd422e5149d0ee Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Mon, 11 Sep 2017 10:52:03 -0400 +Subject: [PATCH] Fix potential free of non-heap address + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #211 +(cherry picked from commit 068f4174001c3ea4ae7913fb37210fec84abf1df) +--- + proxy/src/client/gpm_common.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index dba23a6..c65c69d 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -553,8 +553,8 @@ static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer, + ret = gpm_retry_socket(gpmctx); + + /* Free buffer and set it to NULL to prevent free(xdr_reply_ctx) */ +- free(recv_buffer); +- recv_buffer = NULL; ++ free(*recv_buffer); ++ *recv_buffer = NULL; + + if (ret != 0) + return ret; diff --git a/SOURCES/Fix-segfault-when-no-config-files-are-present.patch b/SOURCES/Fix-segfault-when-no-config-files-are-present.patch new file mode 100644 index 0000000..f41fbea --- /dev/null +++ b/SOURCES/Fix-segfault-when-no-config-files-are-present.patch @@ -0,0 +1,39 @@ +From 76954aa028a897021a9bdcb0a1b5249e2652f7b6 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 16 May 2017 14:16:23 -0400 +Subject: [PATCH] Fix segfault when no config files are present + +Resolves: rhbz#1451255 +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #185 +(cherry picked from commit df434333de34a13440857b511a4c60fbc6a71a5c) +--- + proxy/src/gp_config.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index b4ab90c..8fd60a3 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -844,16 +844,17 @@ int gp_config_init(const char *config_file, const char *config_dir, + + if (config_file) { + ret = gp_config_from_file(config_file, ini_config, collision_flags); +- if (ret == ENOENT) { +- GPDEBUG("Expected config file %s but did not find it.\n", ++ if (ret) { ++ GPDEBUG("Error when trying to read config file %s.\n", + config_file); +- } else if (ret) { + return ret; + } + } + if (config_dir) { + ret = gp_config_from_dir(config_dir, &ini_config, collision_flags); + if (ret) { ++ GPDEBUG("Error when trying to read config directory %s.\n", ++ config_dir); + return ret; + } + } diff --git a/SOURCES/Fix-silent-crash-with-duplicate-config-sections.patch b/SOURCES/Fix-silent-crash-with-duplicate-config-sections.patch new file mode 100644 index 0000000..ce4f3fb --- /dev/null +++ b/SOURCES/Fix-silent-crash-with-duplicate-config-sections.patch @@ -0,0 +1,220 @@ +From caec174b203206185b6075c0e822c6f45070dd87 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Wed, 9 Aug 2017 15:00:26 -0400 +Subject: [PATCH] Fix silent crash with duplicate config sections + +Signed-off-by: Alexander Scheel +Reviewed-by: Simo Sorce +Resolves: #194 +Merges: #202 +(cherry picked from commit c0d85387fc38f9554d601ec2ddb111031a694387) +--- + proxy/configure.ac | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++ + proxy/src/gp_config.c | 27 +++++------ + 2 files changed, 137 insertions(+), 15 deletions(-) + +diff --git a/proxy/configure.ac b/proxy/configure.ac +index c52dbb6..9e01f7d 100644 +--- a/proxy/configure.ac ++++ b/proxy/configure.ac +@@ -107,6 +107,131 @@ fi + AC_SUBST(INI_LIBS) + AC_SUBST(INI_CFLAGS) + ++AC_CHECK_LIB(ref_array, ref_array_destroy, [], ++ [AC_MSG_WARN([ref_array library must support ref_array_destroy])], ++ [$INI_CONFIG_LIBS]) ++ ++AC_RUN_IFELSE([AC_LANG_SOURCE([[ ++/* See: https://pagure.io/SSSD/ding-libs/pull-request/3172 */ ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++ ++static int write_to_file(char *path, char *text) ++{ ++ FILE *f = fopen(path, "w"); ++ int bytes = 0; ++ if (f == NULL) ++ return 1; ++ ++ bytes = fprintf(f, "%s", text); ++ if (bytes != strlen(text)) ++ return 1; ++ ++ return fclose(f); ++} ++ ++int main(void) ++{ ++ char base_path[PATH_MAX]; ++ char augment_path[PATH_MAX]; ++ ++ char config_base[] = ++ "[section]\n" ++ "key1 = first\n" ++ "key2 = exists\n"; ++ ++ char config_augment[] = ++ "[section]\n" ++ "key1 = augment\n" ++ "key3 = exists\n"; ++ ++ char *builddir; ++ ++ struct ini_cfgobj *in_cfg, *result_cfg; ++ struct ini_cfgfile *file_ctx; ++ ++ uint32_t merge_flags = INI_MS_DETECT | INI_MS_PRESERVE; ++ ++ int ret; ++ ++ builddir = getenv("builddir"); ++ if (builddir == NULL) { ++ builddir = strdup("."); ++ } ++ ++ snprintf(base_path, PATH_MAX, "%s/tmp_augment_base.conf", builddir); ++ snprintf(augment_path, PATH_MAX, "%s/tmp_augment_augment.conf", builddir); ++ ++ ret = write_to_file(base_path, config_base); ++ if (ret != 0) { ++ ret = 1; ++ goto cleanup; ++ } ++ ++ ret = write_to_file(augment_path, config_augment); ++ if (ret != 0) { ++ goto cleanup; ++ } ++ ++ /* Match only augment.conf */ ++ const char *m_patterns[] = { "^tmp_augment_augment.conf$", NULL }; ++ ++ /* Match all sections */ ++ const char *m_sections[] = { ".*", NULL }; ++ ++ /* Create config collection */ ++ ret = ini_config_create(&in_cfg); ++ if (ret != EOK) ++ goto cleanup; ++ ++ /* Open base.conf */ ++ ret = ini_config_file_open(base_path, 0, &file_ctx); ++ if (ret != EOK) ++ goto cleanup; ++ ++ /* Seed in_cfg with base.conf */ ++ ret = ini_config_parse(file_ctx, 1, 0, 0, in_cfg); ++ if (ret != EOK) ++ goto cleanup; ++ ++ /* Update base.conf with augment.conf */ ++ ret = ini_config_augment(in_cfg, ++ builddir, ++ m_patterns, ++ m_sections, ++ NULL, ++ INI_STOP_ON_NONE, ++ 0, ++ INI_PARSE_NOSPACE|INI_PARSE_NOTAB, ++ merge_flags, ++ &result_cfg, ++ NULL, ++ NULL); ++ /* We always expect EEXIST due to DETECT being set. */ ++ if (ret != EEXIST) ++ goto cleanup; ++ ++ ret = 0; ++ ++cleanup: ++ remove(base_path); ++ remove(augment_path); ++ ++ /* Per autoconf guidelines */ ++ if (ret != 0) ++ ret = 1; ++ ++ return ret; ++} ++]])] ++,, [AC_MSG_ERROR(["ini_config library must support extended INI_MS_DETECT. See: https://pagure.io/SSSD/ding-libs/pull-request/3172"])]) ++ + AX_PTHREAD(,[AC_MSG_ERROR([Could not find Pthreads support])]) + + LIBS="$PTHREAD_LIBS $LIBS" +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index 07f7c8d..cd057a0 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -728,7 +728,7 @@ static int gp_config_from_file(const char *config_file, + 0, /* metadata_flags, FIXME */ + &file_ctx); + if (ret) { +- GPDEBUG("Failed to open config file: %d (%s)\n", ++ GPERROR("Failed to open config file: %d (%s)\n", + ret, gp_strerror(ret)); + ini_config_destroy(ini_config); + return ret; +@@ -742,7 +742,7 @@ static int gp_config_from_file(const char *config_file, + if (ret) { + char **errors = NULL; + /* we had a parsing failure */ +- GPDEBUG("Failed to parse config file: %d (%s)\n", ++ GPERROR("Failed to parse config file: %d (%s)\n", + ret, gp_strerror(ret)); + if (ini_config_error_count(ini_config)) { + ini_config_get_errors(ini_config, &errors); +@@ -791,26 +791,25 @@ static int gp_config_from_dir(const char *config_dir, + INI_STOP_ON_ANY, /* error_level */ + collision_flags, + INI_PARSE_NOWRAP, +- /* do not allow colliding sections with the same +- * name in different files */ +- INI_MS_ERROR, ++ /* allow sections with the same name in ++ * different files, but log warnings */ ++ INI_MS_DETECT | INI_MS_PRESERVE, + &result_cfg, + &error_list, + NULL); +- if (ret) { ++ if (error_list) { + uint32_t len; +- +- if (!error_list) { +- GPAUDIT("Error when reading config directory number: %d\n", ret); +- return ret; +- } +- + len = ref_array_len(error_list); + for (uint32_t i = 0; i < len; i++) { + /* libini has an unfixable bug where error strings are (char **) */ + GPAUDIT("Error when reading config directory: %s\n", + *(char **)ref_array_get(error_list, i, NULL)); + } ++ ref_array_destroy(error_list); ++ } ++ ++ if (ret && ret != EEXIST) { ++ GPERROR("Error when reading config directory number: %d\n", ret); + + ref_array_destroy(error_list); + return ret; +@@ -821,9 +820,7 @@ static int gp_config_from_dir(const char *config_dir, + ini_config_destroy(*ini_config); + *ini_config = result_cfg; + } +- if (error_list) { +- ref_array_destroy(error_list); +- } ++ + return 0; + } + diff --git a/SOURCES/Fix-unused-variables.patch b/SOURCES/Fix-unused-variables.patch new file mode 100644 index 0000000..069b942 --- /dev/null +++ b/SOURCES/Fix-unused-variables.patch @@ -0,0 +1,503 @@ +From a3c9d87924455448cf3bcb20d34f1bd4e6b915d8 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 15 Mar 2017 13:52:36 -0400 +Subject: [PATCH] Fix unused variables + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #173 +(cherry picked from commit e72d1fa53df8af55b47639ed01f9f0bafa7a2ca8) +--- + proxy/src/client/gpm_common.c | 1 + + proxy/src/client/gpm_display_status.c | 2 +- + proxy/src/client/gpm_release_handle.c | 2 +- + proxy/src/gp_common.h | 1 + + proxy/src/gp_config.c | 8 ++------ + proxy/src/gp_conv.c | 4 ++-- + proxy/src/gp_conv.h | 3 +-- + proxy/src/gp_creds.c | 7 +++---- + proxy/src/gp_init.c | 2 +- + proxy/src/gp_rpc_accept_sec_context.c | 3 +-- + proxy/src/gp_rpc_acquire_cred.c | 3 +-- + proxy/src/gp_rpc_get_mic.c | 4 ++-- + proxy/src/gp_rpc_import_and_canon_name.c | 5 ++--- + proxy/src/gp_rpc_indicate_mechs.c | 5 ++--- + proxy/src/gp_rpc_init_sec_context.c | 3 +-- + proxy/src/gp_rpc_process.c | 21 ++++----------------- + proxy/src/gp_rpc_process.h | 6 ++++++ + proxy/src/gp_rpc_release_handle.c | 5 ++--- + proxy/src/gp_rpc_unwrap.c | 5 ++--- + proxy/src/gp_rpc_verify_mic.c | 5 ++--- + proxy/src/gp_rpc_wrap.c | 4 ++-- + proxy/src/gp_rpc_wrap_size_limit.c | 5 ++--- + proxy/src/gp_socket.c | 2 +- + proxy/src/gssproxy.c | 2 +- + 24 files changed, 44 insertions(+), 64 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 0a54dbc..030765a 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -320,6 +320,7 @@ static void gpm_release_ctx(struct gpm_ctx *gpmctx) + OM_uint32 gpm_release_buffer(OM_uint32 *minor_status, + gss_buffer_t buffer) + { ++ *minor_status = 0; + if (buffer != GSS_C_NO_BUFFER) { + if (buffer->value) { + free(buffer->value); +diff --git a/proxy/src/client/gpm_display_status.c b/proxy/src/client/gpm_display_status.c +index 1f8d755..bbb546f 100644 +--- a/proxy/src/client/gpm_display_status.c ++++ b/proxy/src/client/gpm_display_status.c +@@ -43,7 +43,7 @@ void gpm_save_internal_status(uint32_t err, char *err_str) + OM_uint32 gpm_display_status(OM_uint32 *minor_status, + OM_uint32 status_value, + int status_type, +- const gss_OID mech_type, ++ const gss_OID mech_type UNUSED, + OM_uint32 *message_context, + gss_buffer_t status_string) + { +diff --git a/proxy/src/client/gpm_release_handle.c b/proxy/src/client/gpm_release_handle.c +index 7a6aaed..8f49ee9 100644 +--- a/proxy/src/client/gpm_release_handle.c ++++ b/proxy/src/client/gpm_release_handle.c +@@ -58,7 +58,7 @@ done: + + OM_uint32 gpm_delete_sec_context(OM_uint32 *minor_status, + gssx_ctx **context_handle, +- gss_buffer_t output_token) ++ gss_buffer_t output_token UNUSED) + { + union gp_rpc_arg uarg; + union gp_rpc_res ures; +diff --git a/proxy/src/gp_common.h b/proxy/src/gp_common.h +index 36fd843..edc23b4 100644 +--- a/proxy/src/gp_common.h ++++ b/proxy/src/gp_common.h +@@ -8,6 +8,7 @@ + #include "gp_log.h" + + #define no_const(ptr) ((void *)((uintptr_t)(ptr))) ++#define UNUSED __attribute__((unused)) + + /* add element to list head */ + #define LIST_ADD(list, elem) do { \ +diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c +index 1b833fd..5c1ca02 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -720,7 +720,6 @@ void free_config(struct gp_config **cfg) + } + + static int gp_config_from_file(const char *config_file, +- struct gp_ini_context *ctx, + struct ini_cfgobj *ini_config, + const uint32_t collision_flags) + { +@@ -764,7 +763,6 @@ static int gp_config_from_file(const char *config_file, + } + + static int gp_config_from_dir(const char *config_dir, +- struct gp_ini_context *ctx, + struct ini_cfgobj **ini_config, + const uint32_t collision_flags) + { +@@ -847,8 +845,7 @@ int gp_config_init(const char *config_file, const char *config_dir, + } + + if (config_file) { +- ret = gp_config_from_file(config_file, ctx, ini_config, +- collision_flags); ++ ret = gp_config_from_file(config_file, ini_config, collision_flags); + if (ret == ENOENT) { + GPDEBUG("Expected config file %s but did not find it.\n", + config_file); +@@ -857,8 +854,7 @@ int gp_config_init(const char *config_file, const char *config_dir, + } + } + if (config_dir) { +- ret = gp_config_from_dir(config_dir, ctx, &ini_config, +- collision_flags); ++ ret = gp_config_from_dir(config_dir, &ini_config, collision_flags); + if (ret) { + return ret; + } +diff --git a/proxy/src/gp_conv.c b/proxy/src/gp_conv.c +index 6aa66a8..71d6d9d 100644 +--- a/proxy/src/gp_conv.c ++++ b/proxy/src/gp_conv.c +@@ -6,6 +6,7 @@ + #include + #include + #include "gp_conv.h" ++#include "src/gp_common.h" + + void *gp_memdup(void *in, size_t len) + { +@@ -488,8 +489,7 @@ done: + return ret_maj; + } + +-int gp_conv_status_to_gssx(struct gssx_call_ctx *call_ctx, +- uint32_t ret_maj, uint32_t ret_min, ++int gp_conv_status_to_gssx(uint32_t ret_maj, uint32_t ret_min, + gss_OID mech, struct gssx_status *status) + { + int ret; +diff --git a/proxy/src/gp_conv.h b/proxy/src/gp_conv.h +index e247dbd..699b301 100644 +--- a/proxy/src/gp_conv.h ++++ b/proxy/src/gp_conv.h +@@ -39,8 +39,7 @@ uint32_t gp_conv_name_to_gssx_alloc(uint32_t *min, + gss_name_t in, gssx_name **out); + uint32_t gp_conv_gssx_to_name(uint32_t *min, gssx_name *in, gss_name_t *out); + +-int gp_conv_status_to_gssx(struct gssx_call_ctx *call_ctx, +- uint32_t ret_maj, uint32_t ret_min, ++int gp_conv_status_to_gssx(uint32_t ret_maj, uint32_t ret_min, + gss_OID mech, struct gssx_status *status); + + int gp_copy_utf8string(utf8string *in, utf8string *out); +diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c +index 7d89b06..6570b06 100644 +--- a/proxy/src/gp_creds.c ++++ b/proxy/src/gp_creds.c +@@ -252,7 +252,6 @@ done: + + static int ensure_segregated_ccache(struct gp_call_ctx *gpcall, + int cc_num, +- struct gp_service *svc, + gss_key_value_set_desc *cs) + { + int ret; +@@ -482,7 +481,7 @@ static int gp_get_cred_environment(struct gp_call_ctx *gpcall, + } + } + +- ret = ensure_segregated_ccache(gpcall, cc_num, svc, cs); ++ ret = ensure_segregated_ccache(gpcall, cc_num, cs); + if (ret != 0) { + goto done; + } +@@ -587,8 +586,8 @@ uint32_t gp_add_krb5_creds(uint32_t *min, + gss_cred_id_t in_cred, + gssx_name *desired_name, + gss_cred_usage_t cred_usage, +- uint32_t initiator_time_req, +- uint32_t acceptor_time_req, ++ uint32_t initiator_time_req UNUSED, ++ uint32_t acceptor_time_req UNUSED, + gss_cred_id_t *output_cred_handle, + gss_OID_set *actual_mechs, + uint32_t *initiator_time_rec, +diff --git a/proxy/src/gp_init.c b/proxy/src/gp_init.c +index d367f92..e69934d 100644 +--- a/proxy/src/gp_init.c ++++ b/proxy/src/gp_init.c +@@ -96,7 +96,7 @@ void fini_server(void) + closelog(); + } + +-static void break_loop(verto_ctx *vctx, verto_ev *ev) ++static void break_loop(verto_ctx *vctx, verto_ev *ev UNUSED) + { + GPDEBUG("Exiting after receiving a signal\n"); + verto_break(vctx); +diff --git a/proxy/src/gp_rpc_accept_sec_context.c b/proxy/src/gp_rpc_accept_sec_context.c +index 22a4cf7..ae4de55 100644 +--- a/proxy/src/gp_rpc_accept_sec_context.c ++++ b/proxy/src/gp_rpc_accept_sec_context.c +@@ -152,8 +152,7 @@ done: + ret_maj = acpt_maj; + ret_min = acpt_min; + } +- ret = gp_conv_status_to_gssx(&asca->call_ctx, +- ret_maj, ret_min, oid, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, oid, + &ascr->status); + GPRPCDEBUG(gssx_res_accept_sec_context, ascr); + +diff --git a/proxy/src/gp_rpc_acquire_cred.c b/proxy/src/gp_rpc_acquire_cred.c +index 9a55937..e9c7d56 100644 +--- a/proxy/src/gp_rpc_acquire_cred.c ++++ b/proxy/src/gp_rpc_acquire_cred.c +@@ -150,8 +150,7 @@ int gp_acquire_cred(struct gp_call_ctx *gpcall, + } + + done: +- ret = gp_conv_status_to_gssx(&aca->call_ctx, +- ret_maj, ret_min, desired_mech, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, desired_mech, + &acr->status); + + GPRPCDEBUG(gssx_res_acquire_cred, acr); +diff --git a/proxy/src/gp_rpc_get_mic.c b/proxy/src/gp_rpc_get_mic.c +index 1d9a1fe..dfba77e 100644 +--- a/proxy/src/gp_rpc_get_mic.c ++++ b/proxy/src/gp_rpc_get_mic.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include + +-int gp_get_mic(struct gp_call_ctx *gpcall, ++int gp_get_mic(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -79,7 +79,7 @@ int gp_get_mic(struct gp_call_ctx *gpcall, + ret_min = 0; + + done: +- ret = gp_conv_status_to_gssx(&gma->call_ctx, ret_maj, ret_min, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, + GSS_C_NO_OID, &gmr->status); + GPRPCDEBUG(gssx_res_get_mic, gmr); + gss_release_buffer(&ret_min, &message_token); +diff --git a/proxy/src/gp_rpc_import_and_canon_name.c b/proxy/src/gp_rpc_import_and_canon_name.c +index 3d67f40..e7b8e63 100644 +--- a/proxy/src/gp_rpc_import_and_canon_name.c ++++ b/proxy/src/gp_rpc_import_and_canon_name.c +@@ -8,7 +8,7 @@ + * I am not kidding, if you hav not read it, go back and do it now, or do not + * touch this function */ + +-int gp_import_and_canon_name(struct gp_call_ctx *gpcall, ++int gp_import_and_canon_name(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -64,8 +64,7 @@ int gp_import_and_canon_name(struct gp_call_ctx *gpcall, + /* TODO: icna->name_attributes */ + + done: +- ret = gp_conv_status_to_gssx(&icna->call_ctx, +- ret_maj, ret_min, mech, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, mech, + &icnr->status); + GPRPCDEBUG(gssx_res_import_and_canon_name, icnr); + +diff --git a/proxy/src/gp_rpc_indicate_mechs.c b/proxy/src/gp_rpc_indicate_mechs.c +index c24b926..8abbc7f 100644 +--- a/proxy/src/gp_rpc_indicate_mechs.c ++++ b/proxy/src/gp_rpc_indicate_mechs.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include "gp_debug.h" + +-int gp_indicate_mechs(struct gp_call_ctx *gpcall, ++int gp_indicate_mechs(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -251,8 +251,7 @@ int gp_indicate_mechs(struct gp_call_ctx *gpcall, + } + + done: +- ret = gp_conv_status_to_gssx(&ima->call_ctx, +- ret_maj, ret_min, GSS_C_NO_OID, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, GSS_C_NO_OID, + &imr->status); + GPRPCDEBUG(gssx_res_indicate_mechs, imr); + +diff --git a/proxy/src/gp_rpc_init_sec_context.c b/proxy/src/gp_rpc_init_sec_context.c +index 413e2ec..e4af495 100644 +--- a/proxy/src/gp_rpc_init_sec_context.c ++++ b/proxy/src/gp_rpc_init_sec_context.c +@@ -187,8 +187,7 @@ done: + ret_maj = init_maj; + ret_min = init_min; + } +- ret = gp_conv_status_to_gssx(&isca->call_ctx, +- ret_maj, ret_min, mech_type, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, mech_type, + &iscr->status); + + GPRPCDEBUG(gssx_res_init_sec_context, iscr); +diff --git a/proxy/src/gp_rpc_process.c b/proxy/src/gp_rpc_process.c +index d1a0232..0ea17f0 100644 +--- a/proxy/src/gp_rpc_process.c ++++ b/proxy/src/gp_rpc_process.c +@@ -396,20 +396,7 @@ int gp_rpc_process_call(struct gp_call_ctx *gpcall, + return ret; + } + +-int gp_get_call_context(gp_exec_std_args) +-{ +- return 0; +-} +-int gp_export_cred(gp_exec_std_args) +-{ +- return 0; +-} +-int gp_import_cred(gp_exec_std_args) +-{ +- return 0; +-} +- +-int gp_store_cred(gp_exec_std_args) +-{ +- return 0; +-} ++GP_EXEC_UNUSED_FUNC(gp_get_call_context); ++GP_EXEC_UNUSED_FUNC(gp_export_cred); ++GP_EXEC_UNUSED_FUNC(gp_import_cred); ++GP_EXEC_UNUSED_FUNC(gp_store_cred); +diff --git a/proxy/src/gp_rpc_process.h b/proxy/src/gp_rpc_process.h +index eb02c95..da27795 100644 +--- a/proxy/src/gp_rpc_process.h ++++ b/proxy/src/gp_rpc_process.h +@@ -24,6 +24,12 @@ struct gp_service; + union gp_rpc_arg *arg, \ + union gp_rpc_res *res + ++#define GP_EXEC_UNUSED_FUNC(name) \ ++ int name(struct gp_call_ctx *gpcall UNUSED, \ ++ union gp_rpc_arg *arg UNUSED, \ ++ union gp_rpc_res *res UNUSED) \ ++ { return 0; } ++ + int gp_indicate_mechs(gp_exec_std_args); + int gp_get_call_context(gp_exec_std_args); + int gp_import_and_canon_name(gp_exec_std_args); +diff --git a/proxy/src/gp_rpc_release_handle.c b/proxy/src/gp_rpc_release_handle.c +index 4ffdfb9..c8ba8f2 100644 +--- a/proxy/src/gp_rpc_release_handle.c ++++ b/proxy/src/gp_rpc_release_handle.c +@@ -2,7 +2,7 @@ + + #include "gp_rpc_process.h" + +-int gp_release_handle(struct gp_call_ctx *gpcall, ++int gp_release_handle(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -35,8 +35,7 @@ int gp_release_handle(struct gp_call_ctx *gpcall, + break; + } + +- ret = gp_conv_status_to_gssx(&rha->call_ctx, +- ret_maj, ret_min, GSS_C_NO_OID, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, GSS_C_NO_OID, + &rhr->status); + GPRPCDEBUG(gssx_res_release_handle, rhr); + +diff --git a/proxy/src/gp_rpc_unwrap.c b/proxy/src/gp_rpc_unwrap.c +index bc052cb..fad8cfe 100644 +--- a/proxy/src/gp_rpc_unwrap.c ++++ b/proxy/src/gp_rpc_unwrap.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include + +-int gp_unwrap(struct gp_call_ctx *gpcall, ++int gp_unwrap(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -106,8 +106,7 @@ int gp_unwrap(struct gp_call_ctx *gpcall, + ret_min = 0; + + done: +- ret = gp_conv_status_to_gssx(&uwa->call_ctx, +- ret_maj, ret_min, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, + GSS_C_NO_OID, + &uwr->status); + GPRPCDEBUG(gssx_res_unwrap, uwr); +diff --git a/proxy/src/gp_rpc_verify_mic.c b/proxy/src/gp_rpc_verify_mic.c +index d2920d2..6da6dac 100644 +--- a/proxy/src/gp_rpc_verify_mic.c ++++ b/proxy/src/gp_rpc_verify_mic.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include + +-int gp_verify_mic(struct gp_call_ctx *gpcall, ++int gp_verify_mic(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -74,8 +74,7 @@ int gp_verify_mic(struct gp_call_ctx *gpcall, + ret_min = 0; + + done: +- ret = gp_conv_status_to_gssx(&vma->call_ctx, +- ret_maj, ret_min, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, + GSS_C_NO_OID, + &vmr->status); + GPRPCDEBUG(gssx_res_verify_mic, vmr); +diff --git a/proxy/src/gp_rpc_wrap.c b/proxy/src/gp_rpc_wrap.c +index d5c950e..ae20bdb 100644 +--- a/proxy/src/gp_rpc_wrap.c ++++ b/proxy/src/gp_rpc_wrap.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include + +-int gp_wrap(struct gp_call_ctx *gpcall, ++int gp_wrap(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -105,7 +105,7 @@ int gp_wrap(struct gp_call_ctx *gpcall, + ret_min = 0; + + done: +- ret = gp_conv_status_to_gssx(&wa->call_ctx, ret_maj, ret_min, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, + GSS_C_NO_OID, &wr->status); + GPRPCDEBUG(gssx_res_wrap, wr); + gss_release_buffer(&ret_min, &output_message_buffer); +diff --git a/proxy/src/gp_rpc_wrap_size_limit.c b/proxy/src/gp_rpc_wrap_size_limit.c +index 355113c..cab6826 100644 +--- a/proxy/src/gp_rpc_wrap_size_limit.c ++++ b/proxy/src/gp_rpc_wrap_size_limit.c +@@ -3,7 +3,7 @@ + #include "gp_rpc_process.h" + #include + +-int gp_wrap_size_limit(struct gp_call_ctx *gpcall, ++int gp_wrap_size_limit(struct gp_call_ctx *gpcall UNUSED, + union gp_rpc_arg *arg, + union gp_rpc_res *res) + { +@@ -51,8 +51,7 @@ int gp_wrap_size_limit(struct gp_call_ctx *gpcall, + ret_min = 0; + + done: +- ret = gp_conv_status_to_gssx(&wsla->call_ctx, +- ret_maj, ret_min, ++ ret = gp_conv_status_to_gssx(ret_maj, ret_min, + GSS_C_NO_OID, + &wslr->status); + GPRPCDEBUG(gssx_res_wrap_size_limit, wslr); +diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c +index 62d7dbc..829ff21 100644 +--- a/proxy/src/gp_socket.c ++++ b/proxy/src/gp_socket.c +@@ -146,7 +146,7 @@ static int set_fd_flags(int fd, int flags) + return 0; + } + +-void free_unix_socket(verto_ctx *ctx, verto_ev *ev) ++void free_unix_socket(verto_ctx *ctx UNUSED, verto_ev *ev) + { + struct gp_sock_ctx *sock_ctx = NULL; + sock_ctx = verto_get_private(ev); +diff --git a/proxy/src/gssproxy.c b/proxy/src/gssproxy.c +index 561188e..a020218 100644 +--- a/proxy/src/gssproxy.c ++++ b/proxy/src/gssproxy.c +@@ -119,7 +119,7 @@ static int init_sockets(verto_ctx *vctx, struct gp_config *old_config) + return 0; + } + +-static void hup_handler(verto_ctx *vctx, verto_ev *ev) ++static void hup_handler(verto_ctx *vctx, verto_ev *ev UNUSED) + { + int ret; + struct gp_config *new_config, *old_config; diff --git a/SOURCES/Handle-outdated-encrypted-ccaches.patch b/SOURCES/Handle-outdated-encrypted-ccaches.patch new file mode 100644 index 0000000..e73ba8d --- /dev/null +++ b/SOURCES/Handle-outdated-encrypted-ccaches.patch @@ -0,0 +1,121 @@ +From 044f2224ca5c86b51b2f2ce2878ede9e236e41d3 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Fri, 15 Sep 2017 18:07:28 -0400 +Subject: [PATCH] Handle outdated encrypted ccaches + +When the encrypting keytab changes, all credentials that it was used +to encrypt must be re-created. Otherwise, we log obtuse messages and +fail to do what the user wants. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #214 +(cherry picked from commit 657d3c8339309dd8e2bfa4ee10f005e0f0c055e8) +--- + proxy/src/gp_export.c | 11 ++++++----- + proxy/src/gp_rpc_accept_sec_context.c | 28 ++++++++++++++++++++++++++++ + proxy/src/gp_rpc_init_sec_context.c | 2 +- + 3 files changed, 35 insertions(+), 6 deletions(-) + +diff --git a/proxy/src/gp_export.c b/proxy/src/gp_export.c +index ab08bb7..0c39045 100644 +--- a/proxy/src/gp_export.c ++++ b/proxy/src/gp_export.c +@@ -268,7 +268,7 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key, + &enc_handle, + &data_out); + if (ret) { +- return EINVAL; ++ return ret; + } + + *len = data_out.length; +@@ -446,8 +446,8 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + { + gss_buffer_desc token = GSS_C_EMPTY_BUFFER; + struct gp_creds_handle *handle = NULL; +- uint32_t ret_maj; +- uint32_t ret_min; ++ uint32_t ret_maj = GSS_S_COMPLETE; ++ uint32_t ret_min = 0; + int ret; + + handle = gp_service_get_creds_handle(gpcall->service); +@@ -469,8 +469,9 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + &cred->cred_handle_reference, + &token.length, token.value); + if (ret) { +- ret_maj = GSS_S_FAILURE; +- ret_min = ENOENT; ++ /* Allow for re-issuance of the keytab. */ ++ GPDEBUG("Stored ccache failed to decrypt; treating as empty\n"); ++ *out = GSS_C_NO_CREDENTIAL; + goto done; + } + +diff --git a/proxy/src/gp_rpc_accept_sec_context.c b/proxy/src/gp_rpc_accept_sec_context.c +index ae4de55..2cdc94b 100644 +--- a/proxy/src/gp_rpc_accept_sec_context.c ++++ b/proxy/src/gp_rpc_accept_sec_context.c +@@ -25,6 +25,13 @@ int gp_accept_sec_context(struct gp_call_ctx *gpcall, + int exp_creds_type; + uint32_t acpt_maj; + uint32_t acpt_min; ++ struct gp_cred_check_handle gcch = { ++ .ctx = gpcall, ++ .options.options_len = arg->accept_sec_context.options.options_len, ++ .options.options_val = arg->accept_sec_context.options.options_val, ++ }; ++ uint32_t gccn_before = 0; ++ uint32_t gccn_after = 0; + int ret; + + asca = &arg->accept_sec_context; +@@ -52,6 +59,8 @@ int gp_accept_sec_context(struct gp_call_ctx *gpcall, + if (ret_maj) { + goto done; + } ++ ++ gccn_before = gp_check_sync_creds(&gcch, ach); + } + + if (ach == GSS_C_NO_CREDENTIAL) { +@@ -146,6 +155,25 @@ int gp_accept_sec_context(struct gp_call_ctx *gpcall, + src_name, oid, + &ascr->options.options_len, + &ascr->options.options_val); ++ if (ret_maj) { ++ goto done; ++ } ++ ++ gccn_after = gp_check_sync_creds(&gcch, ach); ++ ++ if (gccn_before != gccn_after) { ++ /* export creds back to client for sync up */ ++ ret_maj = gp_export_sync_creds(&ret_min, gpcall, &ach, ++ &ascr->options.options_val, ++ &ascr->options.options_len); ++ if (ret_maj) { ++ /* not fatal, log and continue */ ++ GPDEBUG("Failed to export sync creds (%d: %d)", ++ (int)ret_maj, (int)ret_min); ++ } ++ } ++ ++ ret_maj = GSS_S_COMPLETE; + + done: + if (ret_maj == GSS_S_COMPLETE) { +diff --git a/proxy/src/gp_rpc_init_sec_context.c b/proxy/src/gp_rpc_init_sec_context.c +index e4af495..f362dbc 100644 +--- a/proxy/src/gp_rpc_init_sec_context.c ++++ b/proxy/src/gp_rpc_init_sec_context.c +@@ -91,7 +91,7 @@ int gp_init_sec_context(struct gp_call_ctx *gpcall, + gp_conv_gssx_to_buffer(isca->input_token, &ibuf); + } + +- if (!isca->cred_handle) { ++ if (!ich) { + if (gss_oid_equal(mech_type, gss_mech_krb5)) { + ret_maj = gp_add_krb5_creds(&ret_min, gpcall, + ACQ_NORMAL, NULL, NULL, diff --git a/SOURCES/Include-header-for-writev.patch b/SOURCES/Include-header-for-writev.patch new file mode 100644 index 0000000..111e0e8 --- /dev/null +++ b/SOURCES/Include-header-for-writev.patch @@ -0,0 +1,49 @@ +From 3b912a01fa9b483fbbf3ef91df061bc5bc0c0db0 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 17 May 2017 12:21:37 -0400 +Subject: [PATCH] Include header for writev() + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #186 +(cherry picked from commit c8c5e8d2b2154d1006633634478a24bfa0b04b4d) +--- + proxy/src/gp_socket.c | 21 ++++++++++++--------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c +index 17ecf7c..29b6a44 100644 +--- a/proxy/src/gp_socket.c ++++ b/proxy/src/gp_socket.c +@@ -1,19 +1,22 @@ + /* Copyright (C) 2011,2015 the GSS-PROXY contributors, see COPYING for license */ + + #include "config.h" +-#include +-#include +-#include +-#include +-#include +-#include +-#include +-#include +-#include ++ + #include "gp_proxy.h" + #include "gp_creds.h" + #include "gp_selinux.h" + ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++ + #define FRAGMENT_BIT (1 << 31) + + struct unix_sock_conn { diff --git a/SOURCES/Make-proc-file-failure-loud-but-nonfatal.patch b/SOURCES/Make-proc-file-failure-loud-but-nonfatal.patch index 84d9a83..18a58df 100644 --- a/SOURCES/Make-proc-file-failure-loud-but-nonfatal.patch +++ b/SOURCES/Make-proc-file-failure-loud-but-nonfatal.patch @@ -1,4 +1,4 @@ -From b52f4f84531ee065f9553b9ecc5be53c89103800 Mon Sep 17 00:00:00 2001 +From 938bd1adc15342e8ebed3d4e135d862e362a619e Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Thu, 25 May 2017 13:06:17 -0400 Subject: [PATCH] Make proc file failure loud but nonfatal diff --git a/SOURCES/Only-empty-FILE-ccaches-when-storing-remote-creds.patch b/SOURCES/Only-empty-FILE-ccaches-when-storing-remote-creds.patch new file mode 100644 index 0000000..2118807 --- /dev/null +++ b/SOURCES/Only-empty-FILE-ccaches-when-storing-remote-creds.patch @@ -0,0 +1,55 @@ +From f2d1472f1557ceee70f2eaacf790c0222a36c4a1 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 10 Oct 2017 18:00:45 -0400 +Subject: [PATCH] Only empty FILE ccaches when storing remote creds + +This mitigates issues when services share a ccache between two +processes. We cannot fix this for FILE ccaches without introducing +other issues. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #216 +(cherry picked from commit d09e87f47a21dd250bfd7a9c59a5932b5c995057) +--- + proxy/src/mechglue/gpp_creds.c | 18 +++++++++++++----- + 1 file changed, 13 insertions(+), 5 deletions(-) + +diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c +index 9fe9bd1..6bdff45 100644 +--- a/proxy/src/mechglue/gpp_creds.c ++++ b/proxy/src/mechglue/gpp_creds.c +@@ -147,6 +147,7 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds, + char cred_name[creds->desired_name.display_name.octet_string_len + 1]; + XDR xdrctx; + bool xdrok; ++ const char *cc_type; + + *min = 0; + +@@ -193,13 +194,20 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool default_creds, + } + cred.ticket.length = xdr_getpos(&xdrctx); + +- /* Always initialize and destroy any existing contents to avoid pileup of +- * entries */ +- ret = krb5_cc_initialize(ctx, ccache, cred.client); +- if (ret == 0) { +- ret = krb5_cc_store_cred(ctx, ccache, &cred); ++ 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 ++ * 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) { ++ goto done; ++ } + } + ++ ret = krb5_cc_store_cred(ctx, ccache, &cred); ++ + done: + if (ctx) { + krb5_free_cred_contents(ctx, &cred); diff --git a/SOURCES/Prevent-uninitialized-read-in-error-path-of-XDR-cont.patch b/SOURCES/Prevent-uninitialized-read-in-error-path-of-XDR-cont.patch new file mode 100644 index 0000000..853ee9f --- /dev/null +++ b/SOURCES/Prevent-uninitialized-read-in-error-path-of-XDR-cont.patch @@ -0,0 +1,28 @@ +From 7b7de309a059ab1f770f9a1192be9299ab0e38f2 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 12 Sep 2017 12:40:27 -0400 +Subject: [PATCH] Prevent uninitialized read in error path of XDR contexts + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #211 +(cherry picked from commit 8ba0f42f06bc7d0ed68cb2eb3ef2794fc860ac2d) +--- + proxy/src/client/gpm_common.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index c65c69d..d0f99d6 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -656,8 +656,8 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) + { + struct gpm_ctx *gpmctx; + gp_rpc_msg msg; +- XDR xdr_call_ctx; +- XDR xdr_reply_ctx; ++ XDR xdr_call_ctx = {0}; ++ XDR xdr_reply_ctx = {0}; + char *send_buffer = NULL; + char *recv_buffer = NULL; + uint32_t send_length; diff --git a/SOURCES/Properly-initialize-ccaches-before-storing-into-them.patch b/SOURCES/Properly-initialize-ccaches-before-storing-into-them.patch new file mode 100644 index 0000000..6e76cea --- /dev/null +++ b/SOURCES/Properly-initialize-ccaches-before-storing-into-them.patch @@ -0,0 +1,38 @@ +From 7a343088a7b716532b1b5c32965fa9ef02c1987a Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 5 Dec 2017 13:14:29 -0500 +Subject: [PATCH] Properly initialize ccaches before storing into them + +krb5_cc_new_unique() doesn't initialize ccaches, which results in the +krb5 libraries being aware of their presence within the collection but +being unable to manipulate them. + +This is transparent to most gssproxy consumers because we just +re-fetch the ccache on error. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #223 +(cherry picked from commit be7df45b6a56631033de387d28a2c06b7658c36a) +--- + proxy/src/mechglue/gpp_creds.c | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/proxy/src/mechglue/gpp_creds.c b/proxy/src/mechglue/gpp_creds.c +index 187ada7..f8ab320 100644 +--- a/proxy/src/mechglue/gpp_creds.c ++++ b/proxy/src/mechglue/gpp_creds.c +@@ -247,6 +247,13 @@ uint32_t gpp_store_remote_creds(uint32_t *min, bool store_as_default_cred, + + ret = krb5_cc_new_unique(ctx, cc_type, NULL, &ccache); + free(cc_type); ++ if (ret) ++ goto done; ++ ++ /* krb5_cc_new_unique() doesn't initialize, and we need to initialize ++ * before storing into the ccache. Note that this will only clobber ++ * the ccache handle, not the whole collection. */ ++ ret = krb5_cc_initialize(ctx, ccache, cred.client); + } + if (ret) + goto done; diff --git a/SOURCES/Properly-locate-credentials-in-collection-caches-in-.patch b/SOURCES/Properly-locate-credentials-in-collection-caches-in-.patch new file mode 100644 index 0000000..53bad4d --- /dev/null +++ b/SOURCES/Properly-locate-credentials-in-collection-caches-in-.patch @@ -0,0 +1,147 @@ +From 51721282ae021e57888b38720a4acd69e88a8f4f Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +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 +Reviewed-by: Simo Sorce +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); + diff --git a/SOURCES/Remove-gpm_release_ctx-to-fix-double-unlock.patch b/SOURCES/Remove-gpm_release_ctx-to-fix-double-unlock.patch new file mode 100644 index 0000000..cce478c --- /dev/null +++ b/SOURCES/Remove-gpm_release_ctx-to-fix-double-unlock.patch @@ -0,0 +1,37 @@ +From 9e2bdfeee30331254d21eaf9e9c000fb9e642fe9 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Thu, 23 Mar 2017 13:42:55 -0400 +Subject: [PATCH] Remove gpm_release_ctx() to fix double unlock + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #173 +(cherry picked from commit b50a863b20649b80cc44c88aa325c6c3220af61b) +--- + proxy/src/client/gpm_common.c | 6 ------ + 1 file changed, 6 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 8c96986..69f4741 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -312,11 +312,6 @@ static struct gpm_ctx *gpm_get_ctx(void) + return &gpm_global_ctx; + } + +-static void gpm_release_ctx(struct gpm_ctx *gpmctx) +-{ +- gpm_release_sock(gpmctx); +-} +- + OM_uint32 gpm_release_buffer(OM_uint32 *minor_status, + gss_buffer_t buffer) + { +@@ -503,7 +498,6 @@ done: + xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg); + xdr_destroy(&xdr_call_ctx); + xdr_destroy(&xdr_reply_ctx); +- gpm_release_ctx(gpmctx); + return ret; + } + diff --git a/SOURCES/Separate-cred-and-ccache-manipulation-in-gpp_store_r.patch b/SOURCES/Separate-cred-and-ccache-manipulation-in-gpp_store_r.patch new file mode 100644 index 0000000..d9afad8 --- /dev/null +++ b/SOURCES/Separate-cred-and-ccache-manipulation-in-gpp_store_r.patch @@ -0,0 +1,107 @@ +From dfddf297c5876d9a5764a83aa7d436b8df020af9 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +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 +Reviewed-by: Simo Sorce +(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 diff --git a/SOURCES/Simplify-setting-NONBLOCK-on-socket.patch b/SOURCES/Simplify-setting-NONBLOCK-on-socket.patch new file mode 100644 index 0000000..ba874f4 --- /dev/null +++ b/SOURCES/Simplify-setting-NONBLOCK-on-socket.patch @@ -0,0 +1,53 @@ +From 4a857676879caa636ccbb3fb5c4601fff8afdee0 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Thu, 14 Sep 2017 10:57:12 -0500 +Subject: [PATCH] Simplify setting NONBLOCK on socket + +Signed-off-by: Alexander Scheel +Reviewed-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #213 +[rharwood@redhat.com: fixup commit message] +(cherry picked from commit ec808ee6a5e6746ed35acc865f253425701be352) +--- + proxy/src/client/gpm_common.c | 15 +-------------- + 1 file changed, 1 insertion(+), 14 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index d0f99d6..7d1158e 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -80,7 +80,6 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + struct sockaddr_un addr = {0}; + char name[PATH_MAX]; + int ret; +- unsigned flags; + int fd = -1; + + ret = get_pipe_name(name); +@@ -92,24 +91,12 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + strncpy(addr.sun_path, name, sizeof(addr.sun_path)-1); + addr.sun_path[sizeof(addr.sun_path)-1] = '\0'; + +- fd = socket(AF_UNIX, SOCK_STREAM, 0); ++ fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); + if (fd == -1) { + ret = errno; + goto done; + } + +- ret = fcntl(fd, F_GETFD, &flags); +- if (ret != 0) { +- ret = errno; +- goto done; +- } +- +- ret = fcntl(fd, F_SETFD, flags | O_NONBLOCK); +- if (ret != 0) { +- ret = errno; +- goto done; +- } +- + ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (ret == -1) { + ret = errno; diff --git a/SOURCES/Tolerate-NULL-pointers-in-gp_same.patch b/SOURCES/Tolerate-NULL-pointers-in-gp_same.patch new file mode 100644 index 0000000..15e2f97 --- /dev/null +++ b/SOURCES/Tolerate-NULL-pointers-in-gp_same.patch @@ -0,0 +1,31 @@ +From ea57f8351e8f1ec2ed4a628b5c235498e65fba0f Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 13 Jun 2017 14:22:44 -0400 +Subject: [PATCH] Tolerate NULL pointers in gp_same + +Fixes potential NULL derefs of program names + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #195 +(cherry picked from commit afe4c2fe6f7f939df914959dda11131bd80ccec6) +--- + proxy/src/gp_util.c | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +diff --git a/proxy/src/gp_util.c b/proxy/src/gp_util.c +index f158b84..5442992 100644 +--- a/proxy/src/gp_util.c ++++ b/proxy/src/gp_util.c +@@ -12,10 +12,9 @@ + + bool gp_same(const char *a, const char *b) + { +- if ((a == b) || strcmp(a, b) == 0) { ++ if (a == b || (a && b && strcmp(a, b) == 0)) { + return true; + } +- + return false; + } + diff --git a/SOURCES/Turn-on-Wextra.patch b/SOURCES/Turn-on-Wextra.patch new file mode 100644 index 0000000..bfec8e4 --- /dev/null +++ b/SOURCES/Turn-on-Wextra.patch @@ -0,0 +1,26 @@ +From a50ea0aa3dfd39ab4a3c39dde35c12fc51fe40d5 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 15 Mar 2017 13:28:26 -0400 +Subject: [PATCH] Turn on -Wextra + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #173 +(cherry picked from commit 85bc3d794efa52aba4c32f6109e7e7741521ec5f) +--- + proxy/Makefile.am | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/proxy/Makefile.am b/proxy/Makefile.am +index e1fbac1..5cd2255 100644 +--- a/proxy/Makefile.am ++++ b/proxy/Makefile.am +@@ -44,7 +44,7 @@ AM_CFLAGS += -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith \ + -Wcast-qual -Wcast-align -Wwrite-strings \ + -fstrict-aliasing -Wstrict-aliasing -Werror=strict-aliasing \ + -Werror-implicit-function-declaration \ +- -Werror=format-security ++ -Werror=format-security -Wextra + if BUILD_HARDENING + AM_CPPFLAGS += -D_FORTIFY_SOURCE=2 -Wdate-time + AM_CFLAGS += -fPIE -fstack-protector-strong diff --git a/SOURCES/Update-systemd-file.patch b/SOURCES/Update-systemd-file.patch new file mode 100644 index 0000000..6b2345b --- /dev/null +++ b/SOURCES/Update-systemd-file.patch @@ -0,0 +1,35 @@ +From 90d7a614b3eb451f0067dfacf0f0b6f41eb00180 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 26 Apr 2017 21:02:47 -0400 +Subject: [PATCH] Update systemd file + +Add `reload` capability, and remove dependency on nfs-utils. + +Closes: #127 +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +(cherry picked from commit c7e8b4066575508a91a38bb6a44694c8a171f0c5) +--- + proxy/systemd/gssproxy.service.in | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/proxy/systemd/gssproxy.service.in b/proxy/systemd/gssproxy.service.in +index dae39ee..f50f526 100644 +--- a/proxy/systemd/gssproxy.service.in ++++ b/proxy/systemd/gssproxy.service.in +@@ -3,7 +3,6 @@ Description=GSSAPI Proxy Daemon + # GSSPROXY will not be started until syslog is + After=syslog.target + Before=nfs-secure.service nfs-secure-server.service +-Requires=proc-fs-nfsd.mount + + [Service] + Environment=KRB5RCACHEDIR=/var/lib/gssproxy/rcache +@@ -12,6 +11,7 @@ ExecStart=@sbindir@/gssproxy -D + # consult systemd.service(5) for more details + Type=forking + PIDFile=@localstatedir@/run/gssproxy.pid ++ExecReload=/bin/kill -HUP $MAINPID + + [Install] + WantedBy=multi-user.target diff --git a/SOURCES/client-Switch-to-non-blocking-sockets.patch b/SOURCES/client-Switch-to-non-blocking-sockets.patch new file mode 100644 index 0000000..015d555 --- /dev/null +++ b/SOURCES/client-Switch-to-non-blocking-sockets.patch @@ -0,0 +1,485 @@ +From 3d08f71f576a381955f07a91198f5dcb320026ba Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Wed, 2 Aug 2017 15:11:49 -0400 +Subject: [PATCH] [client] Switch to non-blocking sockets + +Switch the gssproxy client library to non-blocking sockets, allowing +for timeout and retry operations. The client will automatically retry +both send() and recv() operations three times on ETIMEDOUT. If the +combined send() and recv() hit the three time limit, ETIMEDOUT will be +exposed to the caller in the minor status. + +Signed-off-by: Alexander Scheel +Reviewed-by: Simo Sorce +[rharwood@redhat.com: commit message cleanups, rebased] +Reviewed-by: Robbie Harwood +(cherry picked from commit d035646c8feb0b78f0c157580ca02c46cd00dd7e) +--- + proxy/src/client/gpm_common.c | 317 +++++++++++++++++++++++++++++++++++++++--- + 1 file changed, 295 insertions(+), 22 deletions(-) + +diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c +index 2133618..dba23a6 100644 +--- a/proxy/src/client/gpm_common.c ++++ b/proxy/src/client/gpm_common.c +@@ -7,9 +7,15 @@ + #include + #include + #include ++#include ++#include ++#include + + #define FRAGMENT_BIT (1 << 31) + ++#define RESPONSE_TIMEOUT 15 ++#define MAX_TIMEOUT_RETRY 3 ++ + struct gpm_ctx { + pthread_mutex_t lock; + int fd; +@@ -20,6 +26,9 @@ struct gpm_ctx { + gid_t gid; + + int next_xid; ++ ++ int epollfd; ++ int timerfd; + }; + + /* a single global struct is not particularly efficient, +@@ -39,6 +48,8 @@ static void gpm_init_once(void) + pthread_mutex_init(&gpm_global_ctx.lock, &attr); + + gpm_global_ctx.fd = -1; ++ gpm_global_ctx.epollfd = -1; ++ gpm_global_ctx.timerfd = -1; + + seedp = time(NULL) + getpid() + pthread_self(); + gpm_global_ctx.next_xid = rand_r(&seedp); +@@ -69,6 +80,7 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + struct sockaddr_un addr = {0}; + char name[PATH_MAX]; + int ret; ++ unsigned flags; + int fd = -1; + + ret = get_pipe_name(name); +@@ -86,6 +98,18 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx) + goto done; + } + ++ ret = fcntl(fd, F_GETFD, &flags); ++ if (ret != 0) { ++ ret = errno; ++ goto done; ++ } ++ ++ ret = fcntl(fd, F_SETFD, flags | O_NONBLOCK); ++ if (ret != 0) { ++ ret = errno; ++ goto done; ++ } ++ + ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (ret == -1) { + ret = errno; +@@ -163,6 +187,158 @@ static int gpm_release_sock(struct gpm_ctx *gpmctx) + return pthread_mutex_unlock(&gpmctx->lock); + } + ++static void gpm_timer_close(struct gpm_ctx *gpmctx) { ++ if (gpmctx->timerfd < 0) { ++ return; ++ } ++ ++ close(gpmctx->timerfd); ++ gpmctx->timerfd = -1; ++} ++ ++static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds) { ++ int ret; ++ struct itimerspec its; ++ ++ if (gpmctx->timerfd >= 0) { ++ gpm_timer_close(gpmctx); ++ } ++ ++ gpmctx->timerfd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); ++ if (gpmctx->timerfd < 0) { ++ return errno; ++ } ++ ++ its.it_interval.tv_sec = timeout_seconds; ++ its.it_interval.tv_nsec = 0; ++ its.it_value.tv_sec = timeout_seconds; ++ its.it_value.tv_nsec = 0; ++ ++ ret = timerfd_settime(gpmctx->timerfd, 0, &its, NULL); ++ if (ret) { ++ ret = errno; ++ gpm_timer_close(gpmctx); ++ return ret; ++ } ++ ++ return 0; ++} ++ ++static void gpm_epoll_close(struct gpm_ctx *gpmctx) { ++ if (gpmctx->epollfd < 0) { ++ return; ++ } ++ ++ close(gpmctx->epollfd); ++ gpmctx->epollfd = -1; ++} ++ ++static int gpm_epoll_setup(struct gpm_ctx *gpmctx) { ++ struct epoll_event ev; ++ int ret; ++ ++ if (gpmctx->epollfd >= 0) { ++ gpm_epoll_close(gpmctx); ++ } ++ ++ gpmctx->epollfd = epoll_create1(EPOLL_CLOEXEC); ++ if (gpmctx->epollfd == -1) { ++ return errno; ++ } ++ ++ /* Add timer */ ++ ev.events = EPOLLIN; ++ ev.data.fd = gpmctx->timerfd; ++ ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_ADD, gpmctx->timerfd, &ev); ++ if (ret == -1) { ++ ret = errno; ++ gpm_epoll_close(gpmctx); ++ return ret; ++ } ++ ++ return ret; ++} ++ ++static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) { ++ int ret; ++ int epoll_ret; ++ struct epoll_event ev; ++ struct epoll_event events[2]; ++ uint64_t timer_read; ++ ++ if (gpmctx->epollfd < 0) { ++ ret = gpm_epoll_setup(gpmctx); ++ if (ret) ++ return ret; ++ } ++ ++ ev.events = event_flags; ++ ev.data.fd = gpmctx->fd; ++ epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_ADD, gpmctx->fd, &ev); ++ if (epoll_ret == -1) { ++ ret = errno; ++ gpm_epoll_close(gpmctx); ++ return ret; ++ } ++ ++ do { ++ epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, -1); ++ } while (epoll_ret < 0 && errno == EINTR); ++ ++ if (epoll_ret < 0) { ++ /* Error while waiting that isn't EINTR */ ++ ret = errno; ++ gpm_epoll_close(gpmctx); ++ } else if (epoll_ret == 0) { ++ /* Shouldn't happen as timeout == -1; treat it like a timeout ++ * occurred. */ ++ ret = ETIMEDOUT; ++ gpm_epoll_close(gpmctx); ++ } else if (epoll_ret == 1 && events[0].data.fd == gpmctx->timerfd) { ++ /* Got an event which is only our timer */ ++ ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t)); ++ if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) { ++ /* In the case when reading from the timer failed, don't hide the ++ * timer error behind ETIMEDOUT such that it isn't retried */ ++ ret = errno; ++ } else { ++ /* If ret == 0, then we definitely timed out. Else, if ret == -1 ++ * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird ++ * edge case where epoll thinks the timer can be read, but it ++ * is blocking more; treat it like a TIMEOUT and retry, as ++ * nothing around us would handle EAGAIN from timer and retry ++ * it. */ ++ ret = ETIMEDOUT; ++ } ++ gpm_epoll_close(gpmctx); ++ } else { ++ /* If ret == 2, then we ignore the timerfd; that way if the next ++ * operation cannot be performed immediately, we timeout and retry. ++ * If ret == 1 and data.fd == gpmctx->fd, return 0. */ ++ ret = 0; ++ } ++ ++ epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_DEL, gpmctx->fd, NULL); ++ if (epoll_ret == -1) { ++ /* If we previously had an error, expose that error instead of ++ * clobbering it with errno; else if no error, then assume it is ++ * better to notify of the error deleting the event than it is ++ * to continue. */ ++ if (ret == 0) ++ ret = errno; ++ gpm_epoll_close(gpmctx); ++ } ++ ++ return ret; ++} ++ ++static int gpm_retry_socket(struct gpm_ctx *gpmctx) ++{ ++ gpm_epoll_close(gpmctx); ++ gpm_close_socket(gpmctx); ++ return gpm_open_socket(gpmctx); ++} ++ + /* must be called after the lock has been grabbed */ + static int gpm_send_buffer(struct gpm_ctx *gpmctx, + char *buffer, uint32_t length) +@@ -183,8 +359,13 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + retry = false; + do { + do { ++ ret = gpm_epoll_wait(gpmctx, EPOLLOUT); ++ if (ret != 0) { ++ goto done; ++ } ++ + ret = 0; +- wn = send(gpmctx->fd, &size, sizeof(uint32_t), MSG_NOSIGNAL); ++ wn = write(gpmctx->fd, &size, sizeof(uint32_t)); + if (wn == -1) { + ret = errno; + } +@@ -192,8 +373,7 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + if (wn != 4) { + /* reopen and retry once */ + if (retry == false) { +- gpm_close_socket(gpmctx); +- ret = gpm_open_socket(gpmctx); ++ ret = gpm_retry_socket(gpmctx); + if (ret == 0) { + retry = true; + continue; +@@ -208,9 +388,14 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx, + + pos = 0; + while (length > pos) { +- wn = send(gpmctx->fd, buffer + pos, length - pos, MSG_NOSIGNAL); ++ ret = gpm_epoll_wait(gpmctx, EPOLLOUT); ++ if (ret) { ++ goto done; ++ } ++ ++ wn = write(gpmctx->fd, buffer + pos, length - pos); + if (wn == -1) { +- if (errno == EINTR) { ++ if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { + continue; + } + ret = errno; +@@ -231,7 +416,7 @@ done: + + /* must be called after the lock has been grabbed */ + static int gpm_recv_buffer(struct gpm_ctx *gpmctx, +- char *buffer, uint32_t *length) ++ char **buffer, uint32_t *length) + { + uint32_t size; + ssize_t rn; +@@ -239,6 +424,11 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx, + int ret; + + do { ++ ret = gpm_epoll_wait(gpmctx, EPOLLIN); ++ if (ret) { ++ goto done; ++ } ++ + ret = 0; + rn = read(gpmctx->fd, &size, sizeof(uint32_t)); + if (rn == -1) { +@@ -258,11 +448,22 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx, + goto done; + } + ++ *buffer = malloc(*length); ++ if (*buffer == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ + pos = 0; + while (*length > pos) { +- rn = read(gpmctx->fd, buffer + pos, *length - pos); ++ ret = gpm_epoll_wait(gpmctx, EPOLLIN); ++ if (ret) { ++ goto done; ++ } ++ ++ rn = read(gpmctx->fd, *buffer + pos, *length - pos); + if (rn == -1) { +- if (errno == EINTR) { ++ if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { + continue; + } + ret = errno; +@@ -281,6 +482,7 @@ done: + if (ret) { + /* on errors we can only close the fd and return */ + gpm_close_socket(gpmctx); ++ gpm_epoll_close(gpmctx); + } + return ret; + } +@@ -309,6 +511,63 @@ static struct gpm_ctx *gpm_get_ctx(void) + return &gpm_global_ctx; + } + ++static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer, ++ uint32_t send_length, char** recv_buffer, ++ uint32_t *recv_length) ++{ ++ int ret; ++ int retry_count; ++ ++ /* setup timer */ ++ ret = gpm_timer_setup(gpmctx, RESPONSE_TIMEOUT); ++ if (ret) ++ return ret; ++ ++ for (retry_count = 0; retry_count < MAX_TIMEOUT_RETRY; retry_count++) { ++ /* send to proxy */ ++ ret = gpm_send_buffer(gpmctx, send_buffer, send_length); ++ ++ if (ret == 0) { ++ /* No error, continue to recv */ ++ } else if (ret == ETIMEDOUT) { ++ /* Close and reopen socket before trying again */ ++ ret = gpm_retry_socket(gpmctx); ++ if (ret != 0) ++ return ret; ++ ret = ETIMEDOUT; ++ ++ /* RETRY entire send */ ++ continue; ++ } else { ++ /* Other error */ ++ return ret; ++ } ++ ++ /* receive answer */ ++ ret = gpm_recv_buffer(gpmctx, recv_buffer, recv_length); ++ if (ret == 0) { ++ /* No error */ ++ break; ++ } else if (ret == ETIMEDOUT) { ++ /* Close and reopen socket before trying again */ ++ ret = gpm_retry_socket(gpmctx); ++ ++ /* Free buffer and set it to NULL to prevent free(xdr_reply_ctx) */ ++ free(recv_buffer); ++ recv_buffer = NULL; ++ ++ if (ret != 0) ++ return ret; ++ ret = ETIMEDOUT; ++ } else { ++ /* Other error */ ++ return ret; ++ } ++ } ++ ++ return ret; ++} ++ + OM_uint32 gpm_release_buffer(OM_uint32 *minor_status, + gss_buffer_t buffer) + { +@@ -399,15 +658,20 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) + gp_rpc_msg msg; + XDR xdr_call_ctx; + XDR xdr_reply_ctx; +- char buffer[MAX_RPC_SIZE]; +- uint32_t length; ++ char *send_buffer = NULL; ++ char *recv_buffer = NULL; ++ uint32_t send_length; ++ uint32_t recv_length; + uint32_t xid; + bool xdrok; + bool sockgrab = false; + int ret; + +- xdrmem_create(&xdr_call_ctx, buffer, MAX_RPC_SIZE, XDR_ENCODE); +- xdrmem_create(&xdr_reply_ctx, buffer, MAX_RPC_SIZE, XDR_DECODE); ++ send_buffer = malloc(MAX_RPC_SIZE); ++ if (send_buffer == NULL) ++ return ENOMEM; ++ ++ xdrmem_create(&xdr_call_ctx, send_buffer, MAX_RPC_SIZE, XDR_ENCODE); + + memset(&msg, 0, sizeof(gp_rpc_msg)); + msg.header.type = GP_RPC_CALL; +@@ -450,22 +714,22 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) + goto done; + } + +- /* send to proxy */ +- ret = gpm_send_buffer(gpmctx, buffer, xdr_getpos(&xdr_call_ctx)); +- if (ret) { +- goto done; +- } ++ /* set send_length */ ++ send_length = xdr_getpos(&xdr_call_ctx); + +- /* receive answer */ +- ret = gpm_recv_buffer(gpmctx, buffer, &length); +- if (ret) { ++ /* Send request, receive response with timeout */ ++ ret = gpm_send_recv_loop(gpmctx, send_buffer, send_length, &recv_buffer, ++ &recv_length); ++ if (ret) + goto done; +- } + + /* release the lock */ + gpm_release_sock(gpmctx); + sockgrab = false; + ++ /* Create the reply context */ ++ xdrmem_create(&xdr_reply_ctx, recv_buffer, recv_length, XDR_DECODE); ++ + /* decode header */ + memset(&msg, 0, sizeof(gp_rpc_msg)); + xdrok = xdr_gp_rpc_msg(&xdr_reply_ctx, &msg); +@@ -489,12 +753,21 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) + } + + done: ++ gpm_timer_close(gpmctx); ++ gpm_epoll_close(gpmctx); ++ + if (sockgrab) { + gpm_release_sock(gpmctx); + } + xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg); + xdr_destroy(&xdr_call_ctx); +- xdr_destroy(&xdr_reply_ctx); ++ ++ if (recv_buffer != NULL) ++ xdr_destroy(&xdr_reply_ctx); ++ ++ free(send_buffer); ++ free(recv_buffer); ++ + return ret; + } + diff --git a/SOURCES/server-Add-detailed-request-logging.patch b/SOURCES/server-Add-detailed-request-logging.patch new file mode 100644 index 0000000..6c86058 --- /dev/null +++ b/SOURCES/server-Add-detailed-request-logging.patch @@ -0,0 +1,123 @@ +From a45cb6a67530fe2224e2aaeb73808f9e1e99bfa9 Mon Sep 17 00:00:00 2001 +From: Alexander Scheel +Date: Fri, 4 Aug 2017 16:09:20 -0400 +Subject: [PATCH] [server] Add detailed request logging + +Add request logging to track requests through gssproxy. Requests are +logged as they are read, processed, handled, and replies sent. These +are identified by buffer memory address and size. + +Signed-off-by: Alexander Scheel +Reviewed-by: Simo Sorce +[rharwood@redhat.com: commit message cleanups, rebase] +Reviewed-by: Robbie Harwood +Merges: #205 +(cherry picked from commit 4097dafad3f276c3cf7b1255fe0540e16d59ae03) +--- + proxy/src/gp_rpc_process.c | 6 ++++++ + proxy/src/gp_socket.c | 12 ++++++++++++ + proxy/src/gp_workers.c | 5 +++++ + 3 files changed, 23 insertions(+) + +diff --git a/proxy/src/gp_rpc_process.c b/proxy/src/gp_rpc_process.c +index 0ea17f0..eaffc55 100644 +--- a/proxy/src/gp_rpc_process.c ++++ b/proxy/src/gp_rpc_process.c +@@ -372,9 +372,12 @@ int gp_rpc_process_call(struct gp_call_ctx *gpcall, + xdrmem_create(&xdr_reply_ctx, reply_buffer, MAX_RPC_SIZE, XDR_ENCODE); + + /* decode request */ ++ GPDEBUGN(3, "[status] Processing request [%p (%zu)]\n", inbuf, inlen); + ret = gp_rpc_decode_call(&xdr_call_ctx, &xid, &proc, &arg, &acc, &rej); + if (!ret) { + /* execute request */ ++ GPDEBUGN(3, "[status] Executing request %d (%s) from [%p (%zu)]\n", ++ proc, gp_rpc_procname(proc), inbuf, inlen); + ret = gp_rpc_execute(gpcall, proc, &arg, &res); + if (ret) { + acc = GP_RPC_SYSTEM_ERR; +@@ -388,6 +391,9 @@ int gp_rpc_process_call(struct gp_call_ctx *gpcall, + /* return encoded buffer */ + ret = gp_rpc_return_buffer(&xdr_reply_ctx, + reply_buffer, outbuf, outlen); ++ GPDEBUGN(3, "[status] Returned buffer %d (%s) from [%p (%zu)]: " ++ "[%p (%zu)]\n", proc, gp_rpc_procname(proc), inbuf, inlen, ++ *outbuf, *outlen); + } + /* free resources */ + gp_rpc_free_xdrs(proc, &arg, &res); +diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c +index 5064e51..8675a0e 100644 +--- a/proxy/src/gp_socket.c ++++ b/proxy/src/gp_socket.c +@@ -441,6 +441,8 @@ void gp_socket_send_data(verto_ctx *vctx, struct gp_conn *conn, + + wbuf = calloc(1, sizeof(struct gp_buffer)); + if (!wbuf) { ++ GPDEBUGN(3, "[status] OOM in gp_socket_send_data: %p (%zu)\n", ++ buffer, buflen); + /* too bad, must kill the client connection now */ + gp_conn_free(conn); + return; +@@ -467,6 +469,8 @@ static void gp_socket_write(verto_ctx *vctx, verto_ev *ev) + + vecs = 0; + ++ GPDEBUGN(3, "[status] Sending data: %p (%zu)\n", wbuf->data, wbuf->size); ++ + if (wbuf->pos == 0) { + /* first write, send the buffer size as packet header */ + size = wbuf->size | FRAGMENT_BIT; +@@ -489,6 +493,9 @@ static void gp_socket_write(verto_ctx *vctx, verto_ev *ev) + gp_socket_schedule_write(vctx, wbuf); + } else { + /* error on socket, close and release it */ ++ GPDEBUGN(3, "[status] Error %d in gp_socket_write on writing for " ++ "[%p (%zu:%zu)]\n", errno, wbuf->data, wbuf->pos, ++ wbuf->size); + gp_conn_free(wbuf->conn); + gp_buffer_free(wbuf); + } +@@ -498,6 +505,8 @@ static void gp_socket_write(verto_ctx *vctx, verto_ev *ev) + if (wn < (ssize_t) sizeof(size)) { + /* don't bother trying to handle sockets that can't + * buffer even 4 bytes */ ++ GPDEBUGN(3, "[status] Sending data [%p (%zu)]: failed with short " ++ "write of %d\n", wbuf->data, wbuf->size, wn); + gp_conn_free(wbuf->conn); + gp_buffer_free(wbuf); + return; +@@ -505,6 +514,9 @@ static void gp_socket_write(verto_ctx *vctx, verto_ev *ev) + wn -= sizeof(size); + } + ++ GPDEBUGN(3, "[status] Sending data [%p (%zu)]: successful write of %d\n", ++ wbuf->data, wbuf->size, wn); ++ + wbuf->pos += wn; + if (wbuf->size > wbuf->pos) { + /* short write, reschedule */ +diff --git a/proxy/src/gp_workers.c b/proxy/src/gp_workers.c +index d37e57c..2a33c21 100644 +--- a/proxy/src/gp_workers.c ++++ b/proxy/src/gp_workers.c +@@ -319,6 +319,7 @@ static void gp_handle_reply(verto_ctx *vctx, verto_ev *ev) + break; + + case GP_QUERY_OUT: ++ GPDEBUGN(3, "[status] Handling query reply: %p (%zu)\n", q->buffer, q->buflen); + gp_socket_send_data(vctx, q->conn, q->buffer, q->buflen); + gp_query_free(q, false); + break; +@@ -381,7 +382,11 @@ static void *gp_worker_main(void *pvt) + gp_debug_set_conn_id(gp_conn_get_cid(q->conn)); + + /* handle the client request */ ++ GPDEBUGN(3, "[status] Handling query input: %p (%zu)\n", q->buffer, ++ q->buflen); + gp_handle_query(t->pool, q); ++ GPDEBUGN(3 ,"[status] Handling query output: %p (%zu)\n", q->buffer, ++ q->buflen); + + /* now get lock on main queue, to play with the reply list */ + /* ======> POOL LOCK */ diff --git a/SPECS/gssproxy.spec b/SPECS/gssproxy.spec index 74513bd..8d37e7c 100644 --- a/SPECS/gssproxy.spec +++ b/SPECS/gssproxy.spec @@ -1,6 +1,6 @@ Name: gssproxy Version: 0.7.0 -Release: 4%{?dist} +Release: 17%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -18,9 +18,43 @@ Patch0: Properly-renew-expired-credentials.patch Patch1: Change-impersonator-check-code.patch Patch2: Allow-connection-to-self-when-impersonator-set.patch Patch3: Make-proc-file-failure-loud-but-nonfatal.patch +Patch4: Turn-on-Wextra.patch +Patch5: Fix-unused-variables.patch +Patch6: Fix-mismatched-sign-comparisons.patch +Patch7: Fix-error-checking-on-get_impersonator_fallback.patch +Patch8: Remove-gpm_release_ctx-to-fix-double-unlock.patch +Patch9: Appease-gcc-7-s-fallthrough-detection.patch +Patch10: Fix-memory-leak.patch +Patch11: Fix-most-memory-leaks.patch +Patch12: Fix-segfault-when-no-config-files-are-present.patch +Patch13: Update-systemd-file.patch +Patch14: Fix-error-handling-in-gp_config_from_dir.patch +Patch15: Fix-silent-crash-with-duplicate-config-sections.patch +Patch16: Do-not-call-gpm_grab_sock-twice.patch +Patch17: Fix-error-message-handling-in-gp_config_from_dir.patch +Patch18: Only-empty-FILE-ccaches-when-storing-remote-creds.patch +Patch19: Handle-outdated-encrypted-ccaches.patch +Patch20: Separate-cred-and-ccache-manipulation-in-gpp_store_r.patch +Patch21: Properly-locate-credentials-in-collection-caches-in-.patch +Patch22: Properly-initialize-ccaches-before-storing-into-them.patch +Patch23: Include-header-for-writev.patch +Patch24: Tolerate-NULL-pointers-in-gp_same.patch +Patch25: Add-Client-ID-to-debug-messages.patch +Patch26: client-Switch-to-non-blocking-sockets.patch +Patch27: server-Add-detailed-request-logging.patch +Patch28: Fix-potential-free-of-non-heap-address.patch +Patch29: Prevent-uninitialized-read-in-error-path-of-XDR-cont.patch +Patch30: Simplify-setting-NONBLOCK-on-socket.patch +Patch31: Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch +Patch32: Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch +Patch33: Emit-debug-on-queue-errors.patch +Patch34: Conditionally-reload-kernel-interface-on-SIGHUP.patch ### Dependencies ### +# From rhbz#1458913 and friends +Requires: libini_config >= 1.3.1-28 + Requires: krb5-libs >= 1.15 Requires: keyutils-libs Requires: libverto-module-base @@ -28,6 +62,9 @@ Requires(post): systemd-units Requires(preun): systemd-units Requires(postun): systemd-units +# Currently from rhbz#1458850 and friends +Conflicts: selinux-policy < 3.13.1-166.el7.noarch + ### Build Dependencies ### BuildRequires: autoconf @@ -39,7 +76,7 @@ BuildRequires: findutils BuildRequires: gettext-devel BuildRequires: keyutils-libs-devel BuildRequires: krb5-devel >= 1.15 -BuildRequires: libini_config-devel >= 1.0.0.1 +BuildRequires: libini_config-devel >= 1.3.1-28 BuildRequires: libselinux-devel BuildRequires: libtool BuildRequires: libverto-devel @@ -62,6 +99,37 @@ A proxy for GSSAPI credential handling %patch1 -p2 -b .Change-impersonator-check-code %patch2 -p2 -b .Allow-connection-to-self-when-impersonator-set %patch3 -p2 -b .Make-proc-file-failure-loud-but-nonfatal +%patch4 -p2 -b .Turn-on-Wextra +%patch5 -p2 -b .Fix-unused-variables +%patch6 -p2 -b .Fix-mismatched-sign-comparisons +%patch7 -p2 -b .Fix-error-checking-on-get_impersonator_fallback +%patch8 -p2 -b .Remove-gpm_release_ctx-to-fix-double-unlock +%patch9 -p2 -b .Appease-gcc-7-s-fallthrough-detection +%patch10 -p2 -b .Fix-memory-leak +%patch11 -p2 -b .Fix-most-memory-leaks +%patch12 -p2 -b .Fix-segfault-when-no-config-files-are-present +%patch13 -p2 -b .Update-systemd-file +%patch14 -p2 -b .Fix-error-handling-in-gp_config_from_dir +%patch15 -p2 -b .Fix-silent-crash-with-duplicate-config-sections +%patch16 -p2 -b .Do-not-call-gpm_grab_sock-twice +%patch17 -p2 -b .Fix-error-message-handling-in-gp_config_from_dir +%patch18 -p2 -b .Only-empty-FILE-ccaches-when-storing-remote-creds +%patch19 -p2 -b .Handle-outdated-encrypted-ccaches +%patch20 -p2 -b .Separate-cred-and-ccache-manipulation-in-gpp_store_r +%patch21 -p2 -b .Properly-locate-credentials-in-collection-caches-in- +%patch22 -p2 -b .Properly-initialize-ccaches-before-storing-into-them +%patch23 -p2 -b .Include-header-for-writev +%patch24 -p2 -b .Tolerate-NULL-pointers-in-gp_same +%patch25 -p2 -b .Add-Client-ID-to-debug-messages +%patch26 -p2 -b .client-Switch-to-non-blocking-sockets +%patch27 -p2 -b .server-Add-detailed-request-logging +%patch28 -p2 -b .Fix-potential-free-of-non-heap-address +%patch29 -p2 -b .Prevent-uninitialized-read-in-error-path-of-XDR-cont +%patch30 -p2 -b .Simplify-setting-NONBLOCK-on-socket +%patch31 -p2 -b .Fix-handling-of-non-EPOLLIN-EPOLLOUT-events +%patch32 -p2 -b .Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe +%patch33 -p2 -b .Emit-debug-on-queue-errors +%patch34 -p2 -b .Conditionally-reload-kernel-interface-on-SIGHUP %build autoreconf -f -i @@ -122,6 +190,58 @@ rm -rf -- "%{buildroot}" %changelog +* Wed Dec 13 2017 Robbie Harwood 0.7.0-17 +- Conditionally reload kernel interface on SIGHUP +- Resolves: #1507817 + +* Tue Dec 12 2017 Robbie Harwood 0.7.0-16 +- Backport epoll() logic +- Resolves: #1507817 + +* Wed Dec 06 2017 Robbie Harwood 0.7.0-15 +- Properly initialize ccaches before storing into them +- Resolves: #1488629 + +* Fri Dec 01 2017 Robbie Harwood 0.7.0-14 +- Properly locate credentials in collection caches in mechglue +- Resolves: #1488629 + +* Tue Oct 31 2017 Robbie Harwood 0.7.0-13 +- Handle outdated encrypted ccaches +- Resolves: #1488629 + +* Tue Oct 31 2017 Robbie Harwood 0.7.0-12 +- Handle outdated encrypted ccaches +- Resolves: #1488629 + +* Mon Oct 30 2017 Robbie Harwood 0.7.0-11 +- Fix error message handling in gp_config_from_dir() +- Resolves: #1458913 + +* Fri Oct 27 2017 Robbie Harwood 0.7.0-10 +- Fix concurrency issue around server socket handling +- Resolves: #1462974 + +* Tue Oct 17 2017 Robbie Harwood 0.7.0-9 +- Log useful warning and merge when config file has duplicate sections +- Resolves: #1458913 + +* Mon Oct 02 2017 Robbie Harwood 0.7.0-8 +- Add Conflicts: line for old selinux-policy +- Resolves: #1458850 + +* Thu Sep 21 2017 Robbie Harwood 0.7.0-7 +- Backport NFS-related gssproxy.service changes +- Resolves: #1326440 + +* Mon Sep 11 2017 Robbie Harwood 0.7.0-6 +- Fix segfault when no config files are present +- Resolves: #1451255 + +* Thu Aug 17 2017 Robbie Harwood 0.7.0-5 +- Backport hardening improvements +- Resolves: #1462974 + * Wed May 31 2017 Robbie Harwood 0.7.0-4 - Make proc file failure loud but nonfatal - Resolves: #1449238