From 34bb43b0e00e686bce6a546b8cb2f4f0475dd19b Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Fri, 29 Sep 2017 11:33:49 -0400 Subject: [PATCH] Fix silent death if config file has duplicate sections --- ...error-handling-in-gp_config_from_dir.patch | 50 ++++ ...crash-with-duplicate-config-sections.patch | 220 ++++++++++++++++++ gssproxy.spec | 7 +- 3 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 Fix-error-handling-in-gp_config_from_dir.patch create mode 100644 Fix-silent-crash-with-duplicate-config-sections.patch diff --git a/Fix-error-handling-in-gp_config_from_dir.patch b/Fix-error-handling-in-gp_config_from_dir.patch new file mode 100644 index 0000000..5c881be --- /dev/null +++ b/Fix-error-handling-in-gp_config_from_dir.patch @@ -0,0 +1,50 @@ +From b4660370dabc3be1282459d0ff22cbfcbbc2fd39 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 409cd74..c507472 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -800,17 +800,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/Fix-silent-crash-with-duplicate-config-sections.patch b/Fix-silent-crash-with-duplicate-config-sections.patch new file mode 100644 index 0000000..facb2ae --- /dev/null +++ b/Fix-silent-crash-with-duplicate-config-sections.patch @@ -0,0 +1,220 @@ +From 2c44a2ded88990de44665ed297c135bec844d016 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 c507472..f3ff1fd 100644 +--- a/proxy/src/gp_config.c ++++ b/proxy/src/gp_config.c +@@ -730,7 +730,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; +@@ -744,7 +744,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); +@@ -793,26 +793,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; +@@ -823,9 +822,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/gssproxy.spec b/gssproxy.spec index 48f1afc..84ef84a 100644 --- a/gssproxy.spec +++ b/gssproxy.spec @@ -1,6 +1,6 @@ Name: gssproxy Version: 0.7.0 -Release: 17%{?dist} +Release: 18%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -33,6 +33,8 @@ Patch15: Simplify-setting-NONBLOCK-on-socket.patch Patch16: Fix-handling-of-non-EPOLLIN-EPOLLOUT-events.patch Patch17: Fix-error-handling-in-gpm_send_buffer-gpm_recv_buffe.patch Patch18: Handle-outdated-encrypted-ccaches.patch +Patch19: Fix-error-handling-in-gp_config_from_dir.patch +Patch20: Fix-silent-crash-with-duplicate-config-sections.patch ### Dependencies ### Requires: krb5-libs >= 1.12.0 @@ -125,6 +127,9 @@ rm -rf %{buildroot} %systemd_postun_with_restart gssproxy.service %changelog +* Fri Sep 29 2017 Robbie Harwood - 0.7.0-18 +- Fix silent death if config file has duplicate sections + * Thu Sep 21 2017 Robbie Harwood - 0.7.0-17 - Handle outdated encrypted ccaches