diff --git a/SOURCES/Always-free-ciphertext-data-in-gp_encrypt_buffer.patch b/SOURCES/Always-free-ciphertext-data-in-gp_encrypt_buffer.patch new file mode 100644 index 0000000..979e277 --- /dev/null +++ b/SOURCES/Always-free-ciphertext-data-in-gp_encrypt_buffer.patch @@ -0,0 +1,32 @@ +From ccac7b766cd871aa0baeaebd697b386a47c28812 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 15:35:40 -0400 +Subject: [PATCH] Always free ciphertext data in gp_encrypt_buffer + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: rewrote commit message] +Reviewed-by: Robbie Harwood +(cherry picked from commit fe9e3c29caab90daf19028fb31ff28622d8708a9) +(cherry picked from commit d9a37354c9a040b151fbd737b84b7cacb315ec9d) +--- + src/gp_export.c | 7 +++---- + 1 file changed, 3 insertions(+), 4 deletions(-) + +diff --git a/src/gp_export.c b/src/gp_export.c +index a5681c0..fb2f81b 100644 +--- a/src/gp_export.c ++++ b/src/gp_export.c +@@ -308,10 +308,9 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key, + ret = gp_conv_octet_string(enc_handle.ciphertext.length, + enc_handle.ciphertext.data, + out); +- if (ret) { +- free(enc_handle.ciphertext.data); +- goto done; +- } ++ /* the conversion function copies the data, so free our copy ++ * unconditionally, or we leak */ ++ free(enc_handle.ciphertext.data); + + done: + free(padded); diff --git a/SOURCES/Avoid-leak-of-special-mechs-in-gss_mech_interposer.patch b/SOURCES/Avoid-leak-of-special-mechs-in-gss_mech_interposer.patch new file mode 100644 index 0000000..5baa122 --- /dev/null +++ b/SOURCES/Avoid-leak-of-special-mechs-in-gss_mech_interposer.patch @@ -0,0 +1,34 @@ +From 87a1335a9618788f5d82de08ed0587feebe92c74 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Fri, 31 Jul 2020 13:23:30 -0400 +Subject: [PATCH] Avoid leak of special mechs in gss_mech_interposer() + +Signed-off-by: Robbie Harwood +(cherry picked from commit dc405df92173cceac2cafc09a70b1724bb2b97c8) +(cherry picked from commit 4b9e5f00d36d9b5c1f80835a989fa8865c045ff3) +--- + src/mechglue/gss_plugin.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/mechglue/gss_plugin.c b/src/mechglue/gss_plugin.c +index d735537..8b799cf 100644 +--- a/src/mechglue/gss_plugin.c ++++ b/src/mechglue/gss_plugin.c +@@ -76,6 +76,7 @@ gss_OID_set gss_mech_interposer(gss_OID mech_type) + gss_OID_set interposed_mechs; + OM_uint32 maj, min; + char *envval; ++ gss_OID_set special_mechs; + + /* avoid looping in the gssproxy daemon by avoiding to interpose + * any mechanism */ +@@ -118,7 +119,8 @@ gss_OID_set gss_mech_interposer(gss_OID mech_type) + } + + /* while there also initiaize special_mechs */ +- (void)gpp_special_available_mechs(interposed_mechs); ++ special_mechs = gpp_special_available_mechs(interposed_mechs); ++ (void)gss_release_oid_set(&min, &special_mechs); + + done: + if (maj != 0) { diff --git a/SOURCES/Avoid-unnecessary-allocation-in-gpm_inquire_mechs_fo.patch b/SOURCES/Avoid-unnecessary-allocation-in-gpm_inquire_mechs_fo.patch new file mode 100644 index 0000000..3f7d0f3 --- /dev/null +++ b/SOURCES/Avoid-unnecessary-allocation-in-gpm_inquire_mechs_fo.patch @@ -0,0 +1,57 @@ +From 167d9775dd88cc91f74393fa487f126d21c560c7 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 17:20:44 -0400 +Subject: [PATCH] Avoid unnecessary allocation in gpm_inquire_mechs_for_name() + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: clarified commit message] +Reviewed-by: Robbie Harwood +(cherry picked from commit c0561c078bc22b9523ac25f515ad85b735c26a92) +(cherry picked from commit ebd66fbf42887220a0ff38cfea03a7b20fa4da17) +--- + src/client/gpm_indicate_mechs.c | 12 +++--------- + 1 file changed, 3 insertions(+), 9 deletions(-) + +diff --git a/src/client/gpm_indicate_mechs.c b/src/client/gpm_indicate_mechs.c +index 4041dcd..73fadf0 100644 +--- a/src/client/gpm_indicate_mechs.c ++++ b/src/client/gpm_indicate_mechs.c +@@ -390,7 +390,7 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status, + uint32_t ret_min; + uint32_t ret_maj; + uint32_t discard; +- gss_OID name_type = GSS_C_NO_OID; ++ gss_OID_desc name_type; + int present; + + if (!minor_status) { +@@ -407,19 +407,14 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- ret_min = gp_conv_gssx_to_oid_alloc(&input_name->name_type, &name_type); +- if (ret_min) { +- ret_maj = GSS_S_FAILURE; +- goto done; +- } +- + ret_maj = gss_create_empty_oid_set(&ret_min, mech_types); + if (ret_maj) { + goto done; + } + ++ gp_conv_gssx_to_oid(&input_name->name_type, &name_type); + for (unsigned i = 0; i < global_mechs.info_len; i++) { +- ret_maj = gss_test_oid_set_member(&ret_min, name_type, ++ ret_maj = gss_test_oid_set_member(&ret_min, &name_type, + global_mechs.info[i].name_types, + &present); + if (ret_maj) { +@@ -437,7 +432,6 @@ OM_uint32 gpm_inquire_mechs_for_name(OM_uint32 *minor_status, + } + + done: +- gss_release_oid(&discard, &name_type); + if (ret_maj) { + gss_release_oid_set(&discard, mech_types); + *minor_status = ret_min; diff --git a/SOURCES/Correctly-size-loop-counter-in-gpp_special_available.patch b/SOURCES/Correctly-size-loop-counter-in-gpp_special_available.patch new file mode 100644 index 0000000..13e2b39 --- /dev/null +++ b/SOURCES/Correctly-size-loop-counter-in-gpp_special_available.patch @@ -0,0 +1,34 @@ +From 06cee2eb9ba3096cf5f1e532dae56132fd69c948 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Thu, 9 Apr 2020 12:00:04 -0400 +Subject: [PATCH] Correctly size loop counter in gpp_special_available_mechs() + +Fixes compiler warning for clang in CI. + +Signed-off-by: Robbie Harwood +(cherry picked from commit f9c0abb935125683972c9289db38dfe840f41b37) +--- + src/mechglue/gss_plugin.c | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +diff --git a/src/mechglue/gss_plugin.c b/src/mechglue/gss_plugin.c +index bf70d87..b9813dc 100644 +--- a/src/mechglue/gss_plugin.c ++++ b/src/mechglue/gss_plugin.c +@@ -306,7 +306,6 @@ gss_OID_set gpp_special_available_mechs(const gss_OID_set mechs) + struct gpp_special_oid_list *item; + gss_OID n; + uint32_t maj, min; +- int i; + + item = gpp_get_special_oids(); + +@@ -314,7 +313,7 @@ gss_OID_set gpp_special_available_mechs(const gss_OID_set mechs) + if (maj) { + return GSS_C_NO_OID_SET; + } +- for (i = 0; i < mechs->count; i++) { ++ for (size_t i = 0; i < mechs->count; i++) { + while (item) { + if (gpp_is_special_oid(&mechs->elements[i])) { + maj = gss_add_oid_set_member(&min, diff --git a/SOURCES/Document-config-file-non-merging.patch b/SOURCES/Document-config-file-non-merging.patch new file mode 100644 index 0000000..5fa05a4 --- /dev/null +++ b/SOURCES/Document-config-file-non-merging.patch @@ -0,0 +1,30 @@ +From ceeb1ff9226d21ff166d6737bab34b91fa6660fa Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 10 Jun 2020 15:50:36 -0400 +Subject: [PATCH] Document config file non-merging + +Merges: #4 +Signed-off-by: Robbie Harwood +Reviewed-by: Simo Sorce +(cherry picked from commit a05b876badd52ba99d95c981f5f8b0e50de28c63) +(cherry picked from commit 2592d32c5c6d39f30dc0bfdb78b5c292ed0af2ae) +--- + man/gssproxy.conf.5.xml | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/man/gssproxy.conf.5.xml b/man/gssproxy.conf.5.xml +index 53cae3d..c8dd504 100644 +--- a/man/gssproxy.conf.5.xml ++++ b/man/gssproxy.conf.5.xml +@@ -37,7 +37,10 @@ + of the form "##-foo.conf" (that is, start with two numbers + followed by a dash, and end in ".conf"). Files not conforming to + this will be ignored unless specifically requested through command +- line parameters. ++ line parameters. Within a single file, any duplicate values or ++ sections will be merged. Across multiple files, duplicates will ++ generate a warning, and the first value encountered will take ++ precedence (i.e., there is no merging). + + + diff --git a/SOURCES/Expand-use-of-global-static-mechs-to-conform-to-SPI.patch b/SOURCES/Expand-use-of-global-static-mechs-to-conform-to-SPI.patch new file mode 100644 index 0000000..dd3fcf0 --- /dev/null +++ b/SOURCES/Expand-use-of-global-static-mechs-to-conform-to-SPI.patch @@ -0,0 +1,218 @@ +From 2ca80c193ffa13c89b9b63fb9cb690a9789d5842 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 11:34:45 -0400 +Subject: [PATCH] Expand use of global static mechs to conform to SPI + +GSSAPI requires some specific APIs to return "static" OIDs that the user +does not have to free. The krb5 mechglue in fact requires mechanisms to +also honor this or the mech oid will be irretrievably leaked in some +cases. + +To accomodate this, expand use of global mechs structure we already +allocate for the gss_inidicate_mechs case so we can return "static" OIDs +from calls like ISC and ASC. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: commit message fixups] +Reviewed-by: Robbie Harwood +(cherry picked from commit a3f13b30ef3c90ff7344c3913f6e26e55b82451f) +(cherry picked from commit b7ccb627f4663ca949e3483486478add8f61cb27) +--- + src/client/gpm_accept_sec_context.c | 22 ++++++------------- + src/client/gpm_common.c | 1 - + src/client/gpm_indicate_mechs.c | 34 +++++++++++++++++++++++++++++ + src/client/gpm_init_sec_context.c | 19 +++++----------- + src/client/gssapi_gpm.h | 3 +++ + src/mechglue/gss_plugin.c | 5 +++++ + 6 files changed, 55 insertions(+), 29 deletions(-) + +diff --git a/src/client/gpm_accept_sec_context.c b/src/client/gpm_accept_sec_context.c +index ef5e79c..ab20b03 100644 +--- a/src/client/gpm_accept_sec_context.c ++++ b/src/client/gpm_accept_sec_context.c +@@ -21,7 +21,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, + gssx_res_accept_sec_context *res = &ures.accept_sec_context; + gssx_ctx *ctx = NULL; + gssx_name *name = NULL; +- gss_OID_desc *mech = NULL; + gss_buffer_t outbuf = NULL; + uint32_t ret_maj; + int ret; +@@ -70,15 +69,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, + goto done; + } + +- if (mech_type) { +- if (res->status.mech.octet_string_len) { +- ret = gp_conv_gssx_to_oid_alloc(&res->status.mech, &mech); +- if (ret) { +- goto done; +- } +- } +- } +- + ctx = res->context_handle; + /* we are stealing the delegated creds on success, so we do not want + * it to be freed by xdr_free */ +@@ -101,8 +91,14 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, + } + + if (mech_type) { +- *mech_type = mech; ++ gss_OID_desc mech; ++ gp_conv_gssx_to_oid(&res->status.mech, &mech); ++ ret = gpm_mech_to_static(&mech, mech_type); ++ if (ret) { ++ goto done; ++ } + } ++ + if (src_name) { + *src_name = name; + } +@@ -145,10 +141,6 @@ done: + xdr_free((xdrproc_t)xdr_gssx_name, (char *)name); + free(name); + } +- if (mech) { +- free(mech->elements); +- free(mech); +- } + if (outbuf) { + free(outbuf->value); + free(outbuf); +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index d932ba2..02325c4 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -795,4 +795,3 @@ void gpm_free_xdrs(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res) + xdr_free(gpm_xdr_set[proc].arg_fn, (char *)arg); + xdr_free(gpm_xdr_set[proc].res_fn, (char *)res); + } +- +diff --git a/src/client/gpm_indicate_mechs.c b/src/client/gpm_indicate_mechs.c +index b019a96..86c7de3 100644 +--- a/src/client/gpm_indicate_mechs.c ++++ b/src/client/gpm_indicate_mechs.c +@@ -300,6 +300,40 @@ static int gpmint_init_global_mechs(void) + return 0; + } + ++/* GSSAPI requires some APIs to return "static" mechs that callers do not need ++ * to free. So match a radom mech and return from our global "static" array */ ++int gpm_mech_to_static(gss_OID mech_type, gss_OID *mech_static) ++{ ++ int ret; ++ ++ ret = gpmint_init_global_mechs(); ++ if (ret) { ++ return ret; ++ } ++ ++ *mech_static = GSS_C_NO_OID; ++ for (size_t i = 0; i < global_mechs.mech_set->count; i++) { ++ if (gpm_equal_oids(&global_mechs.mech_set->elements[i], mech_type)) { ++ *mech_static = &global_mechs.mech_set->elements[i]; ++ return 0; ++ } ++ } ++ /* TODO: potentially in future add the mech to the list if missing */ ++ return ENOENT; ++} ++ ++bool gpm_mech_is_static(gss_OID mech_type) ++{ ++ if (global_mechs.mech_set) { ++ for (size_t i = 0; i < global_mechs.mech_set->count; i++) { ++ if (&global_mechs.mech_set->elements[i] == mech_type) { ++ return true; ++ } ++ } ++ } ++ return false; ++} ++ + OM_uint32 gpm_indicate_mechs(OM_uint32 *minor_status, gss_OID_set *mech_set) + { + uint32_t ret_min; +diff --git a/src/client/gpm_init_sec_context.c b/src/client/gpm_init_sec_context.c +index bea2010..b84ff94 100644 +--- a/src/client/gpm_init_sec_context.c ++++ b/src/client/gpm_init_sec_context.c +@@ -43,7 +43,6 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, + gssx_arg_init_sec_context *arg = &uarg.init_sec_context; + gssx_res_init_sec_context *res = &ures.init_sec_context; + gssx_ctx *ctx = NULL; +- gss_OID_desc *mech = NULL; + gss_buffer_t outbuf = NULL; + uint32_t ret_maj = GSS_S_COMPLETE; + uint32_t ret_min = 0; +@@ -100,11 +99,12 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, + + /* return values */ + if (actual_mech_type) { +- if (res->status.mech.octet_string_len) { +- ret = gp_conv_gssx_to_oid_alloc(&res->status.mech, &mech); +- if (ret) { +- goto done; +- } ++ gss_OID_desc mech; ++ gp_conv_gssx_to_oid(&res->status.mech, &mech); ++ ret = gpm_mech_to_static(&mech, actual_mech_type); ++ if (ret) { ++ gpm_save_internal_status(ret, gp_strerror(ret)); ++ goto done; + } + } + +@@ -151,9 +151,6 @@ done: + gpm_free_xdrs(GSSX_INIT_SEC_CONTEXT, &uarg, &ures); + + if (ret_maj == GSS_S_COMPLETE || ret_maj == GSS_S_CONTINUE_NEEDED) { +- if (actual_mech_type) { +- *actual_mech_type = mech; +- } + if (outbuf) { + *output_token = *outbuf; + free(outbuf); +@@ -170,10 +167,6 @@ done: + free(ctx); + ctx = NULL; + } +- if (mech) { +- free(mech->elements); +- free(mech); +- } + if (outbuf) { + free(outbuf->value); + free(outbuf); +diff --git a/src/client/gssapi_gpm.h b/src/client/gssapi_gpm.h +index 61124e0..b7ba04b 100644 +--- a/src/client/gssapi_gpm.h ++++ b/src/client/gssapi_gpm.h +@@ -27,6 +27,9 @@ 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); + ++int gpm_mech_to_static(gss_OID mech_type, gss_OID *mech_static); ++bool gpm_mech_is_static(gss_OID mech_type); ++ + OM_uint32 gpm_display_status(OM_uint32 *minor_status, + OM_uint32 status_value, + int status_type, +diff --git a/src/mechglue/gss_plugin.c b/src/mechglue/gss_plugin.c +index 8b799cf..bf70d87 100644 +--- a/src/mechglue/gss_plugin.c ++++ b/src/mechglue/gss_plugin.c +@@ -377,6 +377,11 @@ OM_uint32 gssi_internal_release_oid(OM_uint32 *minor_status, gss_OID *oid) + item = gpp_next_special_oids(item); + } + ++ if (gpm_mech_is_static(*oid)) { ++ *oid = GSS_C_NO_OID; ++ return GSS_S_COMPLETE; ++ } ++ + /* none matched, it's not ours */ + return GSS_S_CONTINUE_NEEDED; + } diff --git a/SOURCES/Fix-leak-of-mech-OID-in-gssi_inquire_context.patch b/SOURCES/Fix-leak-of-mech-OID-in-gssi_inquire_context.patch new file mode 100644 index 0000000..dbde259 --- /dev/null +++ b/SOURCES/Fix-leak-of-mech-OID-in-gssi_inquire_context.patch @@ -0,0 +1,27 @@ +From 7777d261923e0f0c3bd9cb2b7f0c2ac81b83f2c3 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 26 Aug 2020 13:36:50 -0400 +Subject: [PATCH] Fix leak of mech OID in gssi_inquire_context() + +The name it creates holds a copy of the OID, which we need to release. + +Signed-off-by: Robbie Harwood +(cherry picked from commit 482349fa6bd536471216a898713c83260c78c08d) +(cherry picked from commit ce271e38be223a9442efd406c9a8fa961930e35b) +--- + src/mechglue/gpp_import_and_canon_name.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/mechglue/gpp_import_and_canon_name.c b/src/mechglue/gpp_import_and_canon_name.c +index 745be20..7d6829f 100644 +--- a/src/mechglue/gpp_import_and_canon_name.c ++++ b/src/mechglue/gpp_import_and_canon_name.c +@@ -257,6 +257,8 @@ OM_uint32 gssi_release_name(OM_uint32 *minor_status, + return GSS_S_BAD_NAME; + } + ++ (void)gss_release_oid(&rmin, &name->mech_type); ++ + rmaj = gpm_release_name(&rmin, &name->remote); + + if (name->local) { diff --git a/SOURCES/Fix-leaks-in-our-test-suite-itself.patch b/SOURCES/Fix-leaks-in-our-test-suite-itself.patch new file mode 100644 index 0000000..70348af --- /dev/null +++ b/SOURCES/Fix-leaks-in-our-test-suite-itself.patch @@ -0,0 +1,157 @@ +From 5881a9dbc87f20cd149f53f444b95e8b579638c7 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 13:23:49 -0400 +Subject: [PATCH] Fix leaks in our test suite itself + +These are mostly laziness in freeing since the programs are short-lived. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: rewrote commit message] +Reviewed-by: Robbie Harwood +(cherry picked from commit dc56c86f1dcb1ae4dbc35facf5f50fb21c9d5049) +(cherry picked from commit 617d9ee9ce967cf20462e3cc7a575fda0f945075) +--- + tests/interposetest.c | 22 +++++++++++++++------- + tests/t_impersonate.c | 11 ++++++++--- + tests/t_init.c | 2 ++ + tests/t_setcredopt.c | 8 ++++++-- + 4 files changed, 31 insertions(+), 12 deletions(-) + +diff --git a/tests/interposetest.c b/tests/interposetest.c +index a00904f..0cdd473 100644 +--- a/tests/interposetest.c ++++ b/tests/interposetest.c +@@ -71,6 +71,8 @@ static int gptest_inq_context(gss_ctx_id_t ctx) + DEBUG("Context validity: %d sec.\n", time_rec); + + done: ++ (void)gss_release_name(&min, &src_name); ++ (void)gss_release_name(&min, &targ_name); + (void)gss_release_buffer(&min, &sname); + (void)gss_release_buffer(&min, &tname); + (void)gss_release_buffer(&min, &mechstr); +@@ -274,7 +276,7 @@ void run_client(struct aproc *data) + gp_log_failure(GSS_C_NO_OID, ret_maj, ret_min); + goto done; + } +- fprintf(stdout, "Client, RECV: [%s]\n", buffer); ++ fprintf(stdout, "Client, RECV: [%*s]\n", buflen, buffer); + + /* test gss_wrap_iov_length */ + +@@ -837,19 +839,22 @@ int main(int argc, const char *main_argv[]) + + if (opt_version) { + puts(VERSION""DISTRO_VERSION""PRERELEASE_VERSION); +- return 0; ++ ret = 0; ++ goto done; + } + + if (opt_target == NULL) { + fprintf(stderr, "Missing target!\n"); + poptPrintUsage(pc, stderr, 0); +- return 1; ++ ret = 1; ++ goto done; + } + + if (!opt_all) { +- return run_cli_srv_test(PROXY_LOCAL_ONLY, +- PROXY_LOCAL_ONLY, +- opt_target); ++ ret = run_cli_srv_test(PROXY_LOCAL_ONLY, ++ PROXY_LOCAL_ONLY, ++ opt_target); ++ goto done; + } + + for (i=0; i<4; i++) { +@@ -861,10 +866,13 @@ int main(int argc, const char *main_argv[]) + lookup_gssproxy_behavior(k), + ret ? "failed" : "succeeded"); + if (ret) { +- return ret; ++ goto done; + } + } + } + ++done: ++ poptFreeContext(pc); ++ free(opt_target); + return ret; + } +diff --git a/tests/t_impersonate.c b/tests/t_impersonate.c +index 8ca6e9c..e7b0bc2 100644 +--- a/tests/t_impersonate.c ++++ b/tests/t_impersonate.c +@@ -12,9 +12,9 @@ int main(int argc, const char *argv[]) + gss_ctx_id_t accept_ctx = GSS_C_NO_CONTEXT; + gss_buffer_desc in_token = GSS_C_EMPTY_BUFFER; + gss_buffer_desc out_token = GSS_C_EMPTY_BUFFER; +- gss_name_t user_name; +- gss_name_t proxy_name; +- gss_name_t target_name; ++ gss_name_t user_name = GSS_C_NO_NAME; ++ gss_name_t proxy_name = GSS_C_NO_NAME; ++ gss_name_t target_name = GSS_C_NO_NAME; + gss_OID_set_desc oid_set = { 1, discard_const(gss_mech_krb5) }; + uint32_t ret_maj; + uint32_t ret_min; +@@ -207,9 +207,14 @@ int main(int argc, const char *argv[]) + ret = 0; + + done: ++ gss_release_name(&ret_min, &user_name); ++ gss_release_name(&ret_min, &proxy_name); ++ gss_release_name(&ret_min, &target_name); + gss_release_buffer(&ret_min, &in_token); + gss_release_buffer(&ret_min, &out_token); + gss_release_cred(&ret_min, &impersonator_cred_handle); + gss_release_cred(&ret_min, &cred_handle); ++ gss_delete_sec_context(&ret_min, &accept_ctx, GSS_C_NO_BUFFER); ++ gss_delete_sec_context(&ret_min, &init_ctx, GSS_C_NO_BUFFER); + return ret; + } +diff --git a/tests/t_init.c b/tests/t_init.c +index 02407ce..76bd4c1 100644 +--- a/tests/t_init.c ++++ b/tests/t_init.c +@@ -82,6 +82,8 @@ int main(int argc, const char *argv[]) + goto done; + } + ++ gss_release_buffer(&ret_min, &out_token); ++ + ret = t_recv_buffer(STDIN_FD, buffer, &buflen); + if (ret != 0) { + DEBUG("Failed to read token from STDIN\n"); +diff --git a/tests/t_setcredopt.c b/tests/t_setcredopt.c +index 1399474..bc5e13f 100644 +--- a/tests/t_setcredopt.c ++++ b/tests/t_setcredopt.c +@@ -12,8 +12,8 @@ int main(int argc, const char *argv[]) + gss_ctx_id_t accept_ctx = GSS_C_NO_CONTEXT; + gss_buffer_desc in_token = GSS_C_EMPTY_BUFFER; + gss_buffer_desc out_token = GSS_C_EMPTY_BUFFER; +- gss_name_t user_name; +- gss_name_t target_name; ++ gss_name_t user_name = GSS_C_NO_NAME; ++ gss_name_t target_name = GSS_C_NO_NAME; + gss_OID_set_desc oid_set = { 1, discard_const(gss_mech_krb5) }; + uint32_t ret_maj; + uint32_t ret_min; +@@ -160,8 +160,12 @@ int main(int argc, const char *argv[]) + ret = 0; + + done: ++ gss_release_name(&ret_min, &user_name); ++ gss_release_name(&ret_min, &target_name); + gss_release_buffer(&ret_min, &in_token); + gss_release_buffer(&ret_min, &out_token); + gss_release_cred(&ret_min, &cred_handle); ++ 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/Initialize-interposed-mech-list-without-allocation.patch b/SOURCES/Initialize-interposed-mech-list-without-allocation.patch new file mode 100644 index 0000000..3b8bb4f --- /dev/null +++ b/SOURCES/Initialize-interposed-mech-list-without-allocation.patch @@ -0,0 +1,93 @@ +From 70f30d61e7f5da178e47dcfc8feb083a17be74ff Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 12:32:06 -0400 +Subject: [PATCH] Initialize interposed mech list without allocation + +While we had already fixed the leak here in main, the code performed +unnecessary extra work, so just replacethe whole lot with a function +that does not do any extra allocation or copy. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: commit message] +Reviewed-by: Robbie Harwood +(cherry picked from commit 447d5352c2a81e219ccf04348a87b2ff25b7de15) +(cherry picked from commit 4abda7e47551f39adfc074fc017f6006a4b91a19) +--- + src/mechglue/gss_plugin.c | 31 ++++++++++++++++++++++++++----- + 1 file changed, 26 insertions(+), 5 deletions(-) + +diff --git a/src/mechglue/gss_plugin.c b/src/mechglue/gss_plugin.c +index b9813dc..79e04d0 100644 +--- a/src/mechglue/gss_plugin.c ++++ b/src/mechglue/gss_plugin.c +@@ -65,6 +65,8 @@ enum gpp_behavior gpp_get_behavior(void) + return behavior; + } + ++static void gpp_init_special_available_mechs(const gss_OID_set mechs); ++ + /* 2.16.840.1.113730.3.8.15.1 */ + const gss_OID_desc gssproxy_mech_interposer = { + .length = 11, +@@ -76,7 +78,6 @@ gss_OID_set gss_mech_interposer(gss_OID mech_type) + gss_OID_set interposed_mechs; + OM_uint32 maj, min; + char *envval; +- gss_OID_set special_mechs; + + /* avoid looping in the gssproxy daemon by avoiding to interpose + * any mechanism */ +@@ -119,8 +120,7 @@ gss_OID_set gss_mech_interposer(gss_OID mech_type) + } + + /* while there also initiaize special_mechs */ +- special_mechs = gpp_special_available_mechs(interposed_mechs); +- (void)gss_release_oid_set(&min, &special_mechs); ++ gpp_init_special_available_mechs(interposed_mechs); + + done: + if (maj != 0) { +@@ -307,13 +307,13 @@ gss_OID_set gpp_special_available_mechs(const gss_OID_set mechs) + gss_OID n; + uint32_t maj, min; + +- item = gpp_get_special_oids(); +- + maj = gss_create_empty_oid_set(&min, &amechs); + if (maj) { + return GSS_C_NO_OID_SET; + } + for (size_t i = 0; i < mechs->count; i++) { ++ item = gpp_get_special_oids(); ++ + while (item) { + if (gpp_is_special_oid(&mechs->elements[i])) { + maj = gss_add_oid_set_member(&min, +@@ -354,6 +354,27 @@ done: + return amechs; + } + ++static void gpp_init_special_available_mechs(const gss_OID_set mechs) ++{ ++ struct gpp_special_oid_list *item; ++ ++ for (size_t i = 0; i < mechs->count; i++) { ++ item = gpp_get_special_oids(); ++ ++ while (item) { ++ if (gpp_is_special_oid(&mechs->elements[i]) || ++ gpp_special_equal(&item->special_oid, &mechs->elements[i])) { ++ break; ++ } ++ item = gpp_next_special_oids(item); ++ } ++ if (item == NULL) { ++ /* not found, add to static list */ ++ (void)gpp_new_special_mech(&mechs->elements[i]); ++ } ++ } ++} ++ + OM_uint32 gssi_internal_release_oid(OM_uint32 *minor_status, gss_OID *oid) + { + struct gpp_special_oid_list *item = NULL; diff --git a/SOURCES/Initialize-our-epoll_event-structures.patch b/SOURCES/Initialize-our-epoll_event-structures.patch new file mode 100644 index 0000000..44751d9 --- /dev/null +++ b/SOURCES/Initialize-our-epoll_event-structures.patch @@ -0,0 +1,38 @@ +From c824b8ef3b5ec630edb0f8be78b64b2431c4482f Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Thu, 30 Jul 2020 16:43:30 -0400 +Subject: [PATCH] Initialize our epoll_event structures + +Fixes a valgrind error for the other fields of epoll_event. + +Signed-off-by: Robbie Harwood +(cherry picked from commit 48bfadc538bca3b9ca478c711af75245163d0b67) +(cherry picked from commit 35579d9de1d3f295fb4548c73fc6a729d04128c6) +--- + src/client/gpm_common.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index 808f350..d932ba2 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -195,6 +195,8 @@ static int gpm_epoll_setup(struct gpm_ctx *gpmctx) + struct epoll_event ev; + int ret; + ++ memset(&ev, 0, sizeof(ev)); ++ + if (gpmctx->epollfd >= 0) { + gpm_epoll_close(gpmctx); + } +@@ -276,6 +278,10 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) + struct epoll_event events[2]; + uint64_t timer_read; + ++ memset(&ev, 0, sizeof(ev)); ++ memset(&events[0], 0, sizeof(events[0])); ++ memset(&events[1], 0, sizeof(events[1])); ++ + if (gpmctx->epollfd < 0) { + ret = gpm_epoll_setup(gpmctx); + if (ret) diff --git a/SOURCES/Make-sure-to-free-also-the-remote-ctx-struct.patch b/SOURCES/Make-sure-to-free-also-the-remote-ctx-struct.patch new file mode 100644 index 0000000..95f93f1 --- /dev/null +++ b/SOURCES/Make-sure-to-free-also-the-remote-ctx-struct.patch @@ -0,0 +1,28 @@ +From a02741d82ff44b3c93747615f560dae1bbe7c57b Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 12:44:45 -0400 +Subject: [PATCH] Make sure to free also the remote ctx struct + +The xdr_free() call only frees the contents and not the containing +structure itself. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +(cherry picked from commit e6811347c23b6c62d9f1869da089ab9900f97a84) +(cherry picked from commit 8d5457c290d513781b54be54ede9c81cc5d1fff8) +--- + src/client/gpm_release_handle.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/client/gpm_release_handle.c b/src/client/gpm_release_handle.c +index 8f49ee9..2f70781 100644 +--- a/src/client/gpm_release_handle.c ++++ b/src/client/gpm_release_handle.c +@@ -106,5 +106,7 @@ rel_done: + gpm_free_xdrs(GSSX_RELEASE_HANDLE, &uarg, &ures); + done: + xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)r); ++ free(r); ++ *context_handle = NULL; + return ret; + } diff --git a/SOURCES/Return-static-oids-for-naming-functions.patch b/SOURCES/Return-static-oids-for-naming-functions.patch new file mode 100644 index 0000000..3444050 --- /dev/null +++ b/SOURCES/Return-static-oids-for-naming-functions.patch @@ -0,0 +1,157 @@ +From 0987e0e137854285d4022f5a910e7923d4e663fd Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 17:01:39 -0400 +Subject: [PATCH] Return static oids for naming functions + +gss_display_name and gss_inquire_name reteurn "static" oids, that are +generally not freed by callers, so make sure to match and return actual +static OIDs exported by GSSAPI. + +Also remove gpm_equal_oids() and use the library provided gss_oid_equal +function instead. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +(cherry picked from commit 6ea8391257e687dfb3981b634c06cf7a55008eb0) +(cherry picked from commit 41cb9683627d6c3b136a4b48e1b1842619132f16) +--- + src/client/gpm_import_and_canon_name.c | 28 ++++++++++++++++++++++++-- + src/client/gpm_indicate_mechs.c | 24 +++++----------------- + src/client/gssapi_gpm.h | 1 + + 3 files changed, 32 insertions(+), 21 deletions(-) + +diff --git a/src/client/gpm_import_and_canon_name.c b/src/client/gpm_import_and_canon_name.c +index 70149a3..88b8d7c 100644 +--- a/src/client/gpm_import_and_canon_name.c ++++ b/src/client/gpm_import_and_canon_name.c +@@ -2,6 +2,26 @@ + + #include "gssapi_gpm.h" + ++static int gpm_name_oid_to_static(gss_OID name_type, gss_OID *name_static) ++{ ++#define ret_static(b) \ ++ if (gss_oid_equal(name_type, b)) { \ ++ *name_static = b; \ ++ return 0; \ ++ } ++ ret_static(GSS_C_NT_USER_NAME); ++ ret_static(GSS_C_NT_MACHINE_UID_NAME); ++ ret_static(GSS_C_NT_STRING_UID_NAME); ++ ret_static(GSS_C_NT_HOSTBASED_SERVICE_X); ++ ret_static(GSS_C_NT_HOSTBASED_SERVICE); ++ ret_static(GSS_C_NT_ANONYMOUS); ++ ret_static(GSS_C_NT_EXPORT_NAME); ++ ret_static(GSS_C_NT_COMPOSITE_EXPORT); ++ ret_static(GSS_KRB5_NT_PRINCIPAL_NAME); ++ ret_static(gss_nt_krb5_name); ++ return ENOENT; ++} ++ + OM_uint32 gpm_display_name(OM_uint32 *minor_status, + gssx_name *in_name, + gss_buffer_t output_name_buffer, +@@ -57,7 +77,9 @@ OM_uint32 gpm_display_name(OM_uint32 *minor_status, + } + + if (output_name_type) { +- ret = gp_conv_gssx_to_oid_alloc(&in_name->name_type, output_name_type); ++ gss_OID_desc oid; ++ gp_conv_gssx_to_oid(&in_name->name_type, &oid); ++ ret = gpm_name_oid_to_static(&oid, output_name_type); + if (ret) { + gss_release_buffer(&discard, output_name_buffer); + ret_min = ret; +@@ -285,7 +307,9 @@ OM_uint32 gpm_inquire_name(OM_uint32 *minor_status, + } + + if (MN_mech != NULL) { +- ret = gp_conv_gssx_to_oid_alloc(&name->name_type, MN_mech); ++ gss_OID_desc oid; ++ gp_conv_gssx_to_oid(&name->name_type, &oid); ++ ret = gpm_name_oid_to_static(&oid, MN_mech); + if (ret) { + *minor_status = ret; + return GSS_S_FAILURE; +diff --git a/src/client/gpm_indicate_mechs.c b/src/client/gpm_indicate_mechs.c +index 86c7de3..4041dcd 100644 +--- a/src/client/gpm_indicate_mechs.c ++++ b/src/client/gpm_indicate_mechs.c +@@ -95,20 +95,6 @@ static uint32_t gpm_copy_gss_buffer(uint32_t *minor_status, + return GSS_S_COMPLETE; + } + +-static bool gpm_equal_oids(gss_const_OID a, gss_const_OID b) +-{ +- int ret; +- +- if (a->length == b->length) { +- ret = memcmp(a->elements, b->elements, a->length); +- if (ret == 0) { +- return true; +- } +- } +- +- return false; +-} +- + static void gpmint_indicate_mechs(void) + { + union gp_rpc_arg uarg; +@@ -313,7 +299,7 @@ int gpm_mech_to_static(gss_OID mech_type, gss_OID *mech_static) + + *mech_static = GSS_C_NO_OID; + for (size_t i = 0; i < global_mechs.mech_set->count; i++) { +- if (gpm_equal_oids(&global_mechs.mech_set->elements[i], mech_type)) { ++ if (gss_oid_equal(&global_mechs.mech_set->elements[i], mech_type)) { + *mech_static = &global_mechs.mech_set->elements[i]; + return 0; + } +@@ -383,7 +369,7 @@ OM_uint32 gpm_inquire_names_for_mech(OM_uint32 *minor_status, + } + + for (unsigned i = 0; i < global_mechs.info_len; i++) { +- if (!gpm_equal_oids(global_mechs.info[i].mech, mech_type)) { ++ if (!gss_oid_equal(global_mechs.info[i].mech, mech_type)) { + continue; + } + ret_maj = gpm_copy_gss_OID_set(&ret_min, +@@ -481,7 +467,7 @@ OM_uint32 gpm_inquire_attrs_for_mech(OM_uint32 *minor_status, + } + + for (unsigned i = 0; i < global_mechs.info_len; i++) { +- if (!gpm_equal_oids(global_mechs.info[i].mech, mech)) { ++ if (!gss_oid_equal(global_mechs.info[i].mech, mech)) { + continue; + } + +@@ -540,7 +526,7 @@ OM_uint32 gpm_inquire_saslname_for_mech(OM_uint32 *minor_status, + } + + for (unsigned i = 0; i < global_mechs.info_len; i++) { +- if (!gpm_equal_oids(global_mechs.info[i].mech, desired_mech)) { ++ if (!gss_oid_equal(global_mechs.info[i].mech, desired_mech)) { + continue; + } + ret_maj = gpm_copy_gss_buffer(&ret_min, +@@ -598,7 +584,7 @@ OM_uint32 gpm_display_mech_attr(OM_uint32 *minor_status, + } + + for (unsigned i = 0; i < global_mechs.desc_len; i++) { +- if (!gpm_equal_oids(global_mechs.desc[i].attr, mech_attr)) { ++ if (!gss_oid_equal(global_mechs.desc[i].attr, mech_attr)) { + continue; + } + ret_maj = gpm_copy_gss_buffer(&ret_min, +diff --git a/src/client/gssapi_gpm.h b/src/client/gssapi_gpm.h +index b7ba04b..bdf12e1 100644 +--- a/src/client/gssapi_gpm.h ++++ b/src/client/gssapi_gpm.h +@@ -10,6 +10,7 @@ + #include + #include + #include ++#include + #include "rpcgen/gp_rpc.h" + #include "rpcgen/gss_proxy.h" + #include "src/gp_common.h" diff --git a/SOURCES/Use-static-OIDs-in-gss_inquire_context.patch b/SOURCES/Use-static-OIDs-in-gss_inquire_context.patch new file mode 100644 index 0000000..614d9e4 --- /dev/null +++ b/SOURCES/Use-static-OIDs-in-gss_inquire_context.patch @@ -0,0 +1,31 @@ +From 448501f1b3e0204353544ab245dd4ec77d46faae Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 17:21:03 -0400 +Subject: [PATCH] Use static OIDs in gss_inquire_context() + +As per other functions gssapi expect a static OID here. + +Signed-off-by: Simo Sorce +[rharwood@redhat.com: commit message fixup] +Reviewed-by: Robbie Harwood +(cherry picked from commit 502e448b3b126bf828ed871496dd7520d5075564) +(cherry picked from commit 9cc525b1f1184241483705dfc0a4162bc0c55632) +--- + src/client/gpm_inquire_context.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/client/gpm_inquire_context.c b/src/client/gpm_inquire_context.c +index 8c683fe..5800a8d 100644 +--- a/src/client/gpm_inquire_context.c ++++ b/src/client/gpm_inquire_context.c +@@ -51,7 +51,9 @@ OM_uint32 gpm_inquire_context(OM_uint32 *minor_status, + } + + if (mech_type) { +- ret = gp_conv_gssx_to_oid_alloc(&context_handle->mech, mech_type); ++ gss_OID_desc mech; ++ gp_conv_gssx_to_oid(&context_handle->mech, &mech); ++ ret = gpm_mech_to_static(&mech, mech_type); + if (ret) { + if (src_name) { + (void)gpm_release_name(&tmp_min, src_name); diff --git a/SOURCES/Use-the-correct-function-to-free-unused-creds.patch b/SOURCES/Use-the-correct-function-to-free-unused-creds.patch new file mode 100644 index 0000000..6e275c7 --- /dev/null +++ b/SOURCES/Use-the-correct-function-to-free-unused-creds.patch @@ -0,0 +1,40 @@ +From a23fd33ce8bdf4cdc4d2d00153d3bbf89f363475 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 27 Aug 2020 13:20:49 -0400 +Subject: [PATCH] Use the correct function to free unused creds + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +(cherry picked from commit a2ffd1230fd572d7fa9099af2365dfb7ac394d07) +(cherry picked from commit f77b75b7928a2c7813aebc8a1ec107d495627685) +--- + src/mechglue/gpp_creds.c | 2 +- + src/mechglue/gpp_init_sec_context.c | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/mechglue/gpp_creds.c b/src/mechglue/gpp_creds.c +index e87da82..338fadd 100644 +--- a/src/mechglue/gpp_creds.c ++++ b/src/mechglue/gpp_creds.c +@@ -895,7 +895,7 @@ done: + if (maj == GSS_S_COMPLETE) { + *cred_handle = (gss_cred_id_t)cred; + } else { +- free(cred); ++ (void)gpp_cred_handle_free(&min, cred); + } + (void)gss_release_buffer(&min, &wrap_token); + return maj; +diff --git a/src/mechglue/gpp_init_sec_context.c b/src/mechglue/gpp_init_sec_context.c +index 94d9b01..bb878df 100644 +--- a/src/mechglue/gpp_init_sec_context.c ++++ b/src/mechglue/gpp_init_sec_context.c +@@ -215,7 +215,7 @@ done: + *context_handle = (gss_ctx_id_t)ctx_handle; + + if (claimant_cred_handle == GSS_C_NO_CREDENTIAL) { +- free(cred_handle); ++ (void)gpp_cred_handle_free(&min, cred_handle); + } + return maj; + } diff --git a/SPECS/gssproxy.spec b/SPECS/gssproxy.spec index 577c2e5..61737db 100644 --- a/SPECS/gssproxy.spec +++ b/SPECS/gssproxy.spec @@ -1,7 +1,7 @@ Name: gssproxy Version: 0.8.0 -Release: 16%{?dist} +Release: 19%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -28,6 +28,20 @@ Patch11: Change-the-way-we-handle-encrypted-buffers.patch Patch12: Avoid-uninitialized-free-when-allocating-buffers.patch Patch13: Make-syslog-of-call-status-configurable.patch Patch14: Delay-gssproxy-start-until-after-network.target.patch +Patch15: Document-config-file-non-merging.patch +Patch16: Initialize-our-epoll_event-structures.patch +Patch17: Avoid-leak-of-special-mechs-in-gss_mech_interposer.patch +Patch18: Fix-leak-of-mech-OID-in-gssi_inquire_context.patch +Patch19: Expand-use-of-global-static-mechs-to-conform-to-SPI.patch +Patch20: Correctly-size-loop-counter-in-gpp_special_available.patch +Patch21: Initialize-interposed-mech-list-without-allocation.patch +Patch22: Make-sure-to-free-also-the-remote-ctx-struct.patch +Patch23: Use-the-correct-function-to-free-unused-creds.patch +Patch24: Fix-leaks-in-our-test-suite-itself.patch +Patch25: Always-free-ciphertext-data-in-gp_encrypt_buffer.patch +Patch26: Return-static-oids-for-naming-functions.patch +Patch27: Avoid-unnecessary-allocation-in-gpm_inquire_mechs_fo.patch +Patch28: Use-static-OIDs-in-gss_inquire_context.patch ### Dependencies ### Requires: krb5-libs >= 1.12.0 @@ -122,6 +136,18 @@ mkdir -p %{buildroot}%{gpstatedir}/rcache %systemd_postun_with_restart gssproxy.service %changelog +* Thu Oct 29 2020 Robbie Harwood - 0.8.0-19 +- More leak fixes +- Resolves: #1813200 + +* Wed Oct 14 2020 Robbie Harwood - 0.8.0-18 +- Fix leak of mech OID in gssi_inquire_context() +- Resolves: #1813200 + +* Tue Oct 13 2020 Robbie Harwood - 0.8.0-17 +- Document config file non-merging +- Resolves: #1838222 + * Mon Apr 06 2020 Robbie Harwood - 0.8.0-16 - Delay gssproxy start until after network.target - Resolves: #1780876