diff --git a/SOURCES/Add-a-safety-timeout-to-epoll.patch b/SOURCES/Add-a-safety-timeout-to-epoll.patch new file mode 100644 index 0000000..d909965 --- /dev/null +++ b/SOURCES/Add-a-safety-timeout-to-epoll.patch @@ -0,0 +1,48 @@ +From b4b7e3fc0f2008967202f2453e9c33b378e7a000 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Wed, 6 Mar 2019 10:36:11 -0500 +Subject: [PATCH] Add a safety timeout to epoll + +Add a safety timeout just in case something goes wrong with the use of +timerfd. This way the process should't be stuck forever. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: remove outdated comment] +Reviewed-by: Robbie Harwood +Merges: #241 +(cherry picked from commit d55be9fa2455fe52b6eb904ad427f22141ab3f26) +(cherry picked from commit a494f23b6d8d43fe1a824cd69c3dd93a18fc75a1) +--- + src/client/gpm_common.c | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index 36df5cc..808f350 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -14,6 +14,7 @@ + #define FRAGMENT_BIT (1 << 31) + + #define RESPONSE_TIMEOUT 15 ++#define SAFETY_TIMEOUT RESPONSE_TIMEOUT * 10 * 1000 + #define MAX_TIMEOUT_RETRY 3 + + struct gpm_ctx { +@@ -291,7 +292,7 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) + } + + do { +- epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, -1); ++ epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, SAFETY_TIMEOUT); + } while (epoll_ret < 0 && errno == EINTR); + + if (epoll_ret < 0) { +@@ -299,8 +300,6 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) + 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) { diff --git a/SOURCES/Always-initialize-out-cred-in-gp_import_gssx_cred.patch b/SOURCES/Always-initialize-out-cred-in-gp_import_gssx_cred.patch new file mode 100644 index 0000000..b1387f3 --- /dev/null +++ b/SOURCES/Always-initialize-out-cred-in-gp_import_gssx_cred.patch @@ -0,0 +1,34 @@ +From 8f787b66bc23b8317d95c6cf64fe6e0e6409f869 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Mon, 15 Apr 2019 19:54:17 -0400 +Subject: [PATCH] Always initialize out cred in gp_import_gssx_cred() + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #244 +(cherry picked from commit 5697dfd94345c945f93070c40b9d4480f3d3d7ea) +--- + src/gp_export.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/gp_export.c b/src/gp_export.c +index 5e8e160..403e339 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -449,6 +449,8 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + uint32_t ret_min = 0; + int ret; + ++ *out = GSS_C_NO_CREDENTIAL; ++ + handle = gp_service_get_creds_handle(gpcall->service); + if (!handle) { + ret_maj = GSS_S_FAILURE; +@@ -470,7 +472,6 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + if (ret) { + /* 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/SOURCES/Avoid-uninitialized-free-when-allocating-buffers.patch b/SOURCES/Avoid-uninitialized-free-when-allocating-buffers.patch new file mode 100644 index 0000000..aaf5478 --- /dev/null +++ b/SOURCES/Avoid-uninitialized-free-when-allocating-buffers.patch @@ -0,0 +1,39 @@ +From 160f7a7c66e7e3d249de853cd5a1ebe0becd9fe1 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 1 May 2019 11:27:13 -0400 +Subject: [PATCH] Avoid uninitialized free when allocating buffers + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Resolves: #248 +(cherry picked from commit eafa3c9272c95646400123f8e4d6fb50cf36d36c) +--- + src/gp_export.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/gp_export.c b/src/gp_export.c +index dbfddeb..a5681c0 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -300,6 +300,7 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + &data_in, + &enc_handle); + if (ret) { ++ free(enc_handle.ciphertext.data); + ret = EINVAL; + goto done; + } +@@ -308,12 +309,12 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + enc_handle.ciphertext.data, + out); + if (ret) { ++ free(enc_handle.ciphertext.data); + goto done; + } + + done: + free(padded); +- free(enc_handle.ciphertext.data); + return ret; + } + diff --git a/SOURCES/Change-the-way-we-handle-encrypted-buffers.patch b/SOURCES/Change-the-way-we-handle-encrypted-buffers.patch new file mode 100644 index 0000000..bfce327 --- /dev/null +++ b/SOURCES/Change-the-way-we-handle-encrypted-buffers.patch @@ -0,0 +1,193 @@ +From 51bba6bf325716534c509e0528d2ccfd0050d28c Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Wed, 17 Apr 2019 18:00:59 -0400 +Subject: [PATCH] Change the way we handle encrypted buffers + +The previous change has backwards incompatible behavior that may also +lead to buffer overruns. + +Because we have no easy way to indicate a format change and to maintain +backwards compatibility for the ciphers that were working (those that +added padding were hopelessly borken anyway) introduce code to simply +add padding that we can recognize and remove when we read back the token. + +On ciphers that do not add padding this is basically a no op and the +tokens will be identical to the ones we previously emitted. + +On ciphers that add padding we pad the plaintext so that we hit a block +boundary and cause no extra padding to be added by krb5_c_encrypt +itself. On decryption we check if padding bytes are appended to the +buffer and remove them. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #246 +(cherry picked from commit 839be8aa7e54e93819e8291b570e4c7cfe7e98f1) +--- + src/gp_export.c | 110 +++++++++++++++++++++++++++++++++++++----------- + 1 file changed, 86 insertions(+), 24 deletions(-) + +diff --git a/src/gp_export.c b/src/gp_export.c +index aa0a8ec..dbfddeb 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -193,9 +193,15 @@ done: + return ret_maj; + } + +-/* We need to include a length in our payloads because krb5_c_decrypt() will +- * pad the contents for some enctypes, and gss_import_cred() doesn't like +- * having extra bytes on tokens. */ ++#define ENC_MIN_PAD_LEN 8 ++ ++/* We need to pad our payloads because krb5_c_decrypt() may pad the ++ * contents for some enctypes, and gss_import_cred() doesn't like ++ * having extra bytes on tokens. ++ * Explicit padding and depadding is used in order to maintain backwards ++ * compatibility over upgrades (and downgrades), it would have been ++ * better if we simply had a better formatting of the returned blob ++ * so we could simply change a "blob version" number */ + static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + size_t len, void *buf, octet_string *out) + { +@@ -203,8 +209,9 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + krb5_data data_in; + krb5_enc_data enc_handle; + size_t cipherlen; +- char *packed = NULL; +- uint32_t netlen; ++ size_t padcheck; ++ uint8_t pad = 0; ++ char *padded = NULL; + + if (len > (uint32_t)(-1)) { + /* Needs to fit in 4 bytes of payload, so... */ +@@ -212,28 +219,72 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + goto done; + } + +- packed = malloc(len); +- if (!packed) { +- ret = errno; ++ ret = krb5_c_encrypt_length(context, ++ key->enctype, ++ len, &cipherlen); ++ if (ret) { + goto done; + } + +- netlen = htonl(len); +- memcpy(packed, (uint8_t *)&netlen, 4); +- memcpy(packed + 4, buf, len); +- +- data_in.length = len + 4; +- data_in.data = packed; +- +- memset(&enc_handle, '\0', sizeof(krb5_enc_data)); +- ++ /* try again with len + 1 to see if padding is required */ + ret = krb5_c_encrypt_length(context, + key->enctype, +- data_in.length, +- &cipherlen); ++ len + 1, &padcheck); + if (ret) { + goto done; + } ++ if (padcheck == cipherlen) { ++ int i; ++ /* padding required */ ++ pad = ENC_MIN_PAD_LEN; ++ /* always add enough padding that it makes it extremely unlikley ++ * legitimate plaintext will be incorrectly depadded in the ++ * decrypt function */ ++ ret = krb5_c_encrypt_length(context, ++ key->enctype, ++ len + pad, &cipherlen); ++ if (ret) { ++ goto done; ++ } ++ /* we support only block sizes up to 16 bytes as this is the largest ++ * supported block size in krb ciphers for now */ ++ for (i = 0; i < 15; i++) { ++ /* find the point at which padcheck increases, that's when we ++ * cross a blocksize boundary internally and we can calculate ++ * the padding that will be used */ ++ ret = krb5_c_encrypt_length(context, ++ key->enctype, ++ len + pad + i + 1, &padcheck); ++ if (ret) { ++ goto done; ++ } ++ if (padcheck > cipherlen) { ++ pad += i; ++ break; ++ } ++ } ++ if (i > 15) { ++ ret = EINVAL; ++ goto done; ++ } ++ } ++ ++ if (pad != 0) { ++ padded = malloc(len + pad); ++ if (!padded) { ++ ret = errno; ++ goto done; ++ } ++ ++ memcpy(padded, buf, len); ++ memset(padded + len, pad, pad); ++ ++ data_in.length = len + pad; ++ data_in.data = padded; ++ } else { ++ data_in.length = len; ++ data_in.data = buf; ++ } + + enc_handle.ciphertext.length = cipherlen; + enc_handle.ciphertext.data = malloc(enc_handle.ciphertext.length); +@@ -261,7 +312,7 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + } + + done: +- free(packed); ++ free(padded); + free(enc_handle.ciphertext.data); + return ret; + } +@@ -273,7 +324,8 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key, + int ret; + krb5_data data_out; + krb5_enc_data enc_handle; +- uint32_t netlen; ++ uint8_t pad; ++ int i, j; + + memset(&enc_handle, '\0', sizeof(krb5_enc_data)); + +@@ -295,9 +347,19 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key, + } + + /* And handle the padding. */ +- memcpy(&netlen, buf, 4); +- *len = ntohl(netlen); +- memmove(buf, buf + 4, *len); ++ i = data_out.length - 1; ++ pad = data_out.data[i]; ++ if (pad >= ENC_MIN_PAD_LEN && pad < i) { ++ j = pad; ++ while (j > 0) { ++ j--; ++ if (pad != data_out.data[i - j]) break; ++ } ++ if (j == 0) { ++ data_out.length -= pad; ++ } ++ } ++ *len = data_out.length; + + return 0; + } diff --git a/SOURCES/Close-epoll-fd-within-the-lock.patch b/SOURCES/Close-epoll-fd-within-the-lock.patch new file mode 100644 index 0000000..129b316 --- /dev/null +++ b/SOURCES/Close-epoll-fd-within-the-lock.patch @@ -0,0 +1,159 @@ +From 01ff7b67bfaad9b4f6cebc7c46ac9b1d99671d4f Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Wed, 6 Mar 2019 10:31:13 -0500 +Subject: [PATCH] Close epoll fd within the lock + +A race condition may happen where we close the epoll socket, after +another thread grabbed the lock and is using epoll itself. +On some kernels this may cause epoll to not fire any event leaving the +thread stuck forever. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: cleanup commit message, adjusted function ordering] +Reviewed-by: Robbie Harwood +Merges: #241 +(cherry picked from commit 0ccfd32f8ef16caf65698c5319dfa251d43433af) + +Squashed with: + +Reorder functions + +Keep related functions closer together like before + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Resolves: #242 +(cherry picked from commit 6accc0afead574e11447447c949f2abcb1a34826) +(cherry picked from commit c33de0c213d570f370fd954869c2ad99901b2cf3) +--- + src/client/gpm_common.c | 96 ++++++++++++++++++++++------------------- + 1 file changed, 51 insertions(+), 45 deletions(-) + +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index c254280..36df5cc 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -139,43 +139,6 @@ static void gpm_close_socket(struct gpm_ctx *gpmctx) + gpmctx->fd = -1; + } + +-static int gpm_grab_sock(struct gpm_ctx *gpmctx) +-{ +- int ret; +- pid_t p; +- uid_t u; +- gid_t g; +- +- ret = pthread_mutex_lock(&gpmctx->lock); +- if (ret) { +- return ret; +- } +- +- /* Detect fork / setresuid and friends */ +- p = getpid(); +- u = geteuid(); +- g = getegid(); +- +- if (gpmctx->fd != -1 && +- (p != gpmctx->pid || u != gpmctx->uid || g != gpmctx->gid)) { +- gpm_close_socket(gpmctx); +- } +- +- if (gpmctx->fd == -1) { +- ret = gpm_open_socket(gpmctx); +- } +- +- if (ret) { +- pthread_mutex_unlock(&gpmctx->lock); +- } +- return ret; +-} +- +-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) { +@@ -253,6 +216,57 @@ static int gpm_epoll_setup(struct gpm_ctx *gpmctx) + return ret; + } + ++static int gpm_release_sock(struct gpm_ctx *gpmctx) ++{ ++ gpm_epoll_close(gpmctx); ++ gpm_timer_close(gpmctx); ++ return pthread_mutex_unlock(&gpmctx->lock); ++} ++ ++static int gpm_grab_sock(struct gpm_ctx *gpmctx) ++{ ++ int ret; ++ pid_t p; ++ uid_t u; ++ gid_t g; ++ ++ ret = pthread_mutex_lock(&gpmctx->lock); ++ if (ret) { ++ return ret; ++ } ++ ++ /* Detect fork / setresuid and friends */ ++ p = getpid(); ++ u = geteuid(); ++ g = getegid(); ++ ++ if (gpmctx->fd != -1 && ++ (p != gpmctx->pid || u != gpmctx->uid || g != gpmctx->gid)) { ++ gpm_close_socket(gpmctx); ++ } ++ ++ if (gpmctx->fd == -1) { ++ ret = gpm_open_socket(gpmctx); ++ if (ret) { ++ goto done; ++ } ++ } ++ ++ /* setup timer */ ++ ret = gpm_timer_setup(gpmctx, RESPONSE_TIMEOUT); ++ if (ret) { ++ goto done; ++ } ++ /* create epoll fd as well */ ++ ret = gpm_epoll_setup(gpmctx); ++ ++done: ++ if (ret) { ++ gpm_release_sock(gpmctx); ++ } ++ return ret; ++} ++ + static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) + { + int ret; +@@ -530,11 +544,6 @@ static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer, + 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); +@@ -761,9 +770,6 @@ 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); + } diff --git a/SOURCES/Handle-gss_import_cred-failure-when-importing-gssx-c.patch b/SOURCES/Handle-gss_import_cred-failure-when-importing-gssx-c.patch new file mode 100644 index 0000000..ddc7faa --- /dev/null +++ b/SOURCES/Handle-gss_import_cred-failure-when-importing-gssx-c.patch @@ -0,0 +1,31 @@ +From 0379411547792a58b3d36c9928354072b5f6cabf Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Mon, 15 Apr 2019 19:56:50 -0400 +Subject: [PATCH] Handle gss_import_cred() failure when importing gssx creds + +Otherwise, we might attempt to set options on a non-existent handle, +leading to a segfault. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #244 +(cherry picked from commit 84cf88f6c6cfeb8e3fd2c26ed0fe9fe5bf3810d2) +--- + src/gp_export.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/src/gp_export.c b/src/gp_export.c +index 403e339..7ad8037 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -476,6 +476,10 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall, + } + + ret_maj = gss_import_cred(&ret_min, &token, out); ++ if (ret_maj) { ++ GPDEBUG("gss_import_cred failed when importing gssx cred\n"); ++ goto done; ++ } + + /* check if there is any client option we need to set on credentials */ + gp_set_cred_options(cred, *out); diff --git a/SOURCES/Include-length-when-using-krb5_c_decrypt.patch b/SOURCES/Include-length-when-using-krb5_c_decrypt.patch new file mode 100644 index 0000000..dcff9c3 --- /dev/null +++ b/SOURCES/Include-length-when-using-krb5_c_decrypt.patch @@ -0,0 +1,98 @@ +From 5dec1aeb0a6080ea661061b52248e60afc969426 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Tue, 16 Apr 2019 16:08:32 -0400 +Subject: [PATCH] Include length when using krb5_c_decrypt() + +For some enctypes, krb5_c_decrypt() will add padding bytes which are +included in the returned length. However, functions which use the +objects we're storing aren't always prepared for that: in particular, +gss_import_cred() will declare a token invalid if there's trailing +garbage. + +Work around this by including 4 bytes of length on encrypted objects. + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #244 +(cherry picked from commit 87957caf541114f6f15a495dd7d30556dc5801d9) +--- + src/gp_export.c | 35 +++++++++++++++++++++++++++++++---- + 1 file changed, 31 insertions(+), 4 deletions(-) + +diff --git a/src/gp_export.c b/src/gp_export.c +index 7ad8037..aa0a8ec 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -193,6 +193,9 @@ done: + return ret_maj; + } + ++/* We need to include a length in our payloads because krb5_c_decrypt() will ++ * pad the contents for some enctypes, and gss_import_cred() doesn't like ++ * having extra bytes on tokens. */ + static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + size_t len, void *buf, octet_string *out) + { +@@ -200,9 +203,27 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + krb5_data data_in; + krb5_enc_data enc_handle; + size_t cipherlen; ++ char *packed = NULL; ++ uint32_t netlen; + +- data_in.length = len; +- data_in.data = buf; ++ if (len > (uint32_t)(-1)) { ++ /* Needs to fit in 4 bytes of payload, so... */ ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ packed = malloc(len); ++ if (!packed) { ++ ret = errno; ++ goto done; ++ } ++ ++ netlen = htonl(len); ++ memcpy(packed, (uint8_t *)&netlen, 4); ++ memcpy(packed + 4, buf, len); ++ ++ data_in.length = len + 4; ++ data_in.data = packed; + + memset(&enc_handle, '\0', sizeof(krb5_enc_data)); + +@@ -240,16 +261,19 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + } + + done: ++ free(packed); + free(enc_handle.ciphertext.data); + return ret; + } + ++/* See comment above on gp_encrypt_buffer(). */ + static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key, +- octet_string *in, size_t *len, void *buf) ++ octet_string *in, size_t *len, char *buf) + { + int ret; + krb5_data data_out; + krb5_enc_data enc_handle; ++ uint32_t netlen; + + memset(&enc_handle, '\0', sizeof(krb5_enc_data)); + +@@ -270,7 +294,10 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key, + return ret; + } + +- *len = data_out.length; ++ /* And handle the padding. */ ++ memcpy(&netlen, buf, 4); ++ *len = ntohl(netlen); ++ memmove(buf, buf + 4, *len); + + return 0; + } diff --git a/SOURCES/Update-NFS-service-name-in-systemd-unit.patch b/SOURCES/Update-NFS-service-name-in-systemd-unit.patch new file mode 100644 index 0000000..6ee71e5 --- /dev/null +++ b/SOURCES/Update-NFS-service-name-in-systemd-unit.patch @@ -0,0 +1,27 @@ +From 9860e73b5da0f0448594ecc700ccc7ba08177718 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 24 Apr 2019 12:07:47 -0400 +Subject: [PATCH] Update NFS service name in systemd unit + +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +Merges: #247 +(cherry picked from commit 1a789a645175d5aea109a3c0831806b94337b20e) +(cherry picked from commit aa4f43049d1037d1c23becd78ad2f7dd601132f4) +--- + systemd/gssproxy.service.in | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/systemd/gssproxy.service.in b/systemd/gssproxy.service.in +index f50f526..ac37df6 100644 +--- a/systemd/gssproxy.service.in ++++ b/systemd/gssproxy.service.in +@@ -2,7 +2,7 @@ + Description=GSSAPI Proxy Daemon + # GSSPROXY will not be started until syslog is + After=syslog.target +-Before=nfs-secure.service nfs-secure-server.service ++Before=rpc-gssd.service + + [Service] + Environment=KRB5RCACHEDIR=/var/lib/gssproxy/rcache diff --git a/SOURCES/Use-pthread-keys-for-thread-local-storage.patch b/SOURCES/Use-pthread-keys-for-thread-local-storage.patch new file mode 100644 index 0000000..e49ad31 --- /dev/null +++ b/SOURCES/Use-pthread-keys-for-thread-local-storage.patch @@ -0,0 +1,150 @@ +From e0b142320342ef16260b6072f1c83d6fcf4142e6 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 20 Sep 2018 17:37:53 -0400 +Subject: [PATCH] Use pthread keys for thread local storage + +This interface is slower but also more portable, and more importantly +it provides a way to specify destructor that is called when a thread +is canceled so we stop leaking memory. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #233 +(cherry picked from commit 0faccc1441bc7a6b3e8bd806f22c8a961e5f586e) +(cherry picked from commit 89dc0ee157caa4617d32fd72287849296d7fe26d) +--- + src/client/gpm_common.c | 2 ++ + src/client/gpm_display_status.c | 57 ++++++++++++++++++++++----------- + src/client/gssapi_gpm.h | 1 + + 3 files changed, 42 insertions(+), 18 deletions(-) + +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index dd29519..c254280 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -55,6 +55,8 @@ static void gpm_init_once(void) + gpm_global_ctx.next_xid = rand_r(&seedp); + + pthread_mutexattr_destroy(&attr); ++ ++ gpm_display_status_init_once(); + } + + static int get_pipe_name(char *name) +diff --git a/src/client/gpm_display_status.c b/src/client/gpm_display_status.c +index bbb546f..e3aa4ea 100644 +--- a/src/client/gpm_display_status.c ++++ b/src/client/gpm_display_status.c +@@ -1,27 +1,47 @@ + /* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */ + + #include "gssapi_gpm.h" ++#include + +-__thread gssx_status *tls_last_status = NULL; ++static pthread_key_t gpm_last_status; + +-/* Thread local storage for return status. +- * FIXME: it's not the most portable construct, so may need fixing in future */ ++static void gpm_destroy_last_status(void *arg) ++{ ++ gssx_status *status = (gssx_status *)arg; ++ xdr_free((xdrproc_t)xdr_gssx_status, (char *)status); ++ free(status); ++} ++ ++void gpm_display_status_init_once(void) ++{ ++ (void)pthread_key_create(&gpm_last_status, gpm_destroy_last_status); ++} ++ ++/* Portable thread local storage for return status. */ + void gpm_save_status(gssx_status *status) + { ++ gssx_status *last_status; + int ret; + +- if (tls_last_status) { +- xdr_free((xdrproc_t)xdr_gssx_status, (char *)tls_last_status); +- free(tls_last_status); ++ last_status = (gssx_status *)pthread_getspecific(gpm_last_status); ++ if (last_status != NULL) { ++ /* store NULL first so we do not risk a double free if we are ++ * racing on a pthread_cancel */ ++ pthread_setspecific(gpm_last_status, NULL); ++ gpm_destroy_last_status(last_status); + } + +- ret = gp_copy_gssx_status_alloc(status, &tls_last_status); +- if (ret) { +- /* make sure tls_last_status is zeored on error */ +- tls_last_status = NULL; ++ ret = gp_copy_gssx_status_alloc(status, &last_status); ++ if (ret == 0) { ++ pthread_setspecific(gpm_last_status, last_status); + } + } + ++gssx_status *gpm_get_saved_status(void) ++{ ++ return (gssx_status *)pthread_getspecific(gpm_last_status); ++} ++ + /* This funciton is used to record internal mech errors that are + * generated by the proxy client code */ + void gpm_save_internal_status(uint32_t err, char *err_str) +@@ -47,15 +67,16 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + OM_uint32 *message_context, + gss_buffer_t status_string) + { ++ gssx_status *last_status = gpm_get_saved_status(); + utf8string tmp; + int ret; + + switch(status_type) { + case GSS_C_GSS_CODE: +- if (tls_last_status && +- tls_last_status->major_status == status_value && +- tls_last_status->major_status_string.utf8string_len) { +- ret = gp_copy_utf8string(&tls_last_status->major_status_string, ++ if (last_status && ++ last_status->major_status == status_value && ++ last_status->major_status_string.utf8string_len) { ++ ret = gp_copy_utf8string(&last_status->major_status_string, + &tmp); + if (ret) { + *minor_status = ret; +@@ -70,9 +91,9 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + return GSS_S_UNAVAILABLE; + } + case GSS_C_MECH_CODE: +- if (tls_last_status && +- tls_last_status->minor_status == status_value && +- tls_last_status->minor_status_string.utf8string_len) { ++ if (last_status && ++ last_status->minor_status == status_value && ++ last_status->minor_status_string.utf8string_len) { + + if (*message_context) { + /* we do not support multiple messages for now */ +@@ -80,7 +101,7 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- ret = gp_copy_utf8string(&tls_last_status->minor_status_string, ++ ret = gp_copy_utf8string(&last_status->minor_status_string, + &tmp); + if (ret) { + *minor_status = ret; +diff --git a/src/client/gssapi_gpm.h b/src/client/gssapi_gpm.h +index 22beecf..61124e0 100644 +--- a/src/client/gssapi_gpm.h ++++ b/src/client/gssapi_gpm.h +@@ -23,6 +23,7 @@ OM_uint32 gpm_release_name(OM_uint32 *minor_status, + OM_uint32 gpm_release_buffer(OM_uint32 *minor_status, + gss_buffer_t buffer); + ++void gpm_display_status_init_once(void); + void gpm_save_status(gssx_status *status); + void gpm_save_internal_status(uint32_t err, char *err_str); + diff --git a/SPECS/gssproxy.spec b/SPECS/gssproxy.spec index 85b6897..0ad789f 100644 --- a/SPECS/gssproxy.spec +++ b/SPECS/gssproxy.spec @@ -1,7 +1,7 @@ Name: gssproxy Version: 0.8.0 -Release: 5%{?dist} +Release: 14%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -17,6 +17,15 @@ Source0: https://releases.pagure.org/%{name}/%{name}-%{version}.tar.gz Patch0: Always-use-the-encype-we-selected.patch Patch1: Clarify-debug-and-debug_level-in-man-pages.patch Patch2: Always-choose-highest-requested-debug-level.patch +Patch3: Use-pthread-keys-for-thread-local-storage.patch +Patch4: Close-epoll-fd-within-the-lock.patch +Patch5: Add-a-safety-timeout-to-epoll.patch +Patch7: Update-NFS-service-name-in-systemd-unit.patch +Patch8: Always-initialize-out-cred-in-gp_import_gssx_cred.patch +Patch9: Handle-gss_import_cred-failure-when-importing-gssx-c.patch +Patch10: Include-length-when-using-krb5_c_decrypt.patch +Patch11: Change-the-way-we-handle-encrypted-buffers.patch +Patch12: Avoid-uninitialized-free-when-allocating-buffers.patch ### Dependencies ### Requires: krb5-libs >= 1.12.0 @@ -35,22 +44,23 @@ Conflicts: selinux-policy < 3.13.1-283.5 ### Build Dependencies ### BuildRequires: autoconf BuildRequires: automake -BuildRequires: libtool -BuildRequires: m4 -BuildRequires: libxslt -BuildRequires: libxml2 BuildRequires: docbook-style-xsl BuildRequires: doxygen +BuildRequires: findutils BuildRequires: gettext-devel -BuildRequires: pkgconfig -BuildRequires: krb5-devel >= 1.12.0 -BuildRequires: libselinux-devel BuildRequires: keyutils-libs-devel +BuildRequires: krb5-devel >= 1.12.0 BuildRequires: libini_config-devel >= 1.2.0 +BuildRequires: libselinux-devel +BuildRequires: libtool BuildRequires: libverto-devel +BuildRequires: libxml2 +BuildRequires: libxslt +BuildRequires: m4 +BuildRequires: pkgconfig BuildRequires: popt-devel -BuildRequires: findutils BuildRequires: systemd-units + BuildRequires: git %description @@ -110,6 +120,42 @@ mkdir -p %{buildroot}%{gpstatedir}/rcache %systemd_postun_with_restart gssproxy.service %changelog +* Mon May 13 2019 Robbie Harwood - 0.8.0-14 +- Fix explicit NULL deref around encrypted token processing +- Resolves: #1700539 + +* Fri May 03 2019 Robbie Harwood - 0.8.0-13 +- Update NFS service name in systemd unit +- Resolves: #1701820 + +* Wed May 01 2019 Robbie Harwood - 0.8.0-12 +- Avoid uninitialized free when allocating buffers +- Resolves: #1682281 + +* Fri Mar 22 2019 Robbie Harwood - 0.8.0-11 +- Fix race condition around epoll and socket release +- Resolves: #1690082 + +* Fri Mar 22 2019 Robbie Harwood - 0.8.0-10 +- Add a safety timeout to epoll +- Resolves: #1690082 + +* Wed Mar 20 2019 Robbie Harwood - 0.8.0-9 +- Bump to re-run gating +- Resolves: #1682281 + +* Tue Mar 19 2019 Robbie Harwood - 0.8.0-8 +- Bump to re-run gating +- Resolves: #1682281 + +* Mon Mar 18 2019 Robbie Harwood - 0.8.0-7 +- Use pthread keys for thread local storage +- Resolves: #1631564 + +* Wed Mar 13 2019 Robbie Harwood - 0.8.0-6 +- Add gating tests +- Resolves: #1682281 + * Fri Jul 13 2018 Fedora Release Engineering - 0.8.0-5 - Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild