On (24/06/16 13:51), Lukas Slebodnik wrote: >On (24/06/16 13:49), Lukas Slebodnik wrote: >>On (24/06/16 10:29), Michal Židek wrote: >>>On 06/24/2016 09:57 AM, Jakub Hrozek wrote: >>>> On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote: >>>> > On 06/23/2016 09:40 PM, Jakub Hrozek wrote: >>>> > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote: >>>> > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote: >>>> > > > > On (23/06/16 18:59), Michal Židek wrote: >>>> > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote: >>>> > > > > > > On (23/06/16 18:24), Michal Židek wrote: >>>> > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote: >>>> > > > > > > > > Lukas, >>>> > > > > > > > > >>>> > > > > > > > > did you have some local patches related to >>>> > > > > > > > > this patch, that you did not send here? >>>> > > > > > > > > >>>> > > > > > > > > Because I can not apply the patch on current >>>> > > > > > > > > master and the changes in the context of >>>> > > > > > > > > your patch seem like you did something with >>>> > > > > > > > > default config file path? >>>> > > > > > > > > >>>> > > > > > > > > Slightly modified version that applies on >>>> > > > > > > > > current master is attached (the changes >>>> > > > > > > > > are the same, but the context differs). >>>> > > > > > > > > >>>> > > > > > > > > Michal >>>> > > > > > > > > >>>> > > > > > > > >>>> > > > > > > > I added the missing access check for >>>> > > > > > > > snippets. If the snippets does not >>>> > > > > > > > pass the access check it is skipped >>>> > > > > > > > and a loud debug message is logged. >>>> > > > > > > > >>>> > > > > > > > I attach it as separate patch for >>>> > > > > > > > easier review, but it should be squashed >>>> > > > > > > > before pushing. I also attach the first >>>> > > > > > > > patch for convenience. >>>> > > > > > > > >>>> > > > > > > > Michal >>>> > > > > > > >>>> > > > > > > > From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 >>>> > > > > > > > 00:00:00 2001 >>>> > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >>>> > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100 >>>> > > > > > > > Subject: [PATCH] confdb: Make it possible to use config >>>> > > > > > > > snippets >>>> > > > > > > > >>>> > > > > > > > Resolves: >>>> > > > > > > > https://fedorahosted.org/sssd/ticket/2247 >>>> > > > > > > > >>>> > > > > > > > Signed-off-by: Lukas Slebodnik <lsleb...@redhat.com> >>>> > > > > > > > --- >>>> > > > > LGTM. >>>> > > > > >>>> > > > > It isn't ACK because I was involved in development >>>> > > > > and I didn;t test last version but code is fine fore me. >>>> > > > >>>> > > > I just ran a quick test and the patch works for me and the code >>>> > > > reads OK >>>> > > > as well. I will push after CI finishes. >>>> > > > >>>> > > > I also opened: >>>> > > > https://fedorahosted.org/sssd/ticket/3062 >>>> > > > because the patch is missing documentation and >>>> > > > https://fedorahosted.org/sssd/ticket/3063 >>>> > > > to add at least a simple integration test. >>>> > > >>>> > > Actually, can we get rid of this warning? >>>> > > >>>> > > Error: COMPILER_WARNING: >>>> > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable >>>> > > 'patterns' [-Wunused-variable] >>>> > > # const char *patterns[] = { "^[^\\.].*\\.conf", NULL }; >>>> > > # ^ >>>> > > # 214| int ret; >>>> > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 >>>> > > # 216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL }; >>>> > > # 217| const char *sections[] = { ".*", NULL }; >>>> > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 >>>> > > >>>> > > Error: COMPILER_WARNING: >>>> > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function >>>> > > 'sss_ini_get_config' >>>> > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable >>>> > > 'sections' [-Wunused-variable] >>>> > > # const char *sections[] = { ".*", NULL }; >>>> > > # ^ >>>> > > # 215| #ifdef HAVE_LIBINI_CONFIG_V1 >>>> > > # 216| const char *patterns[] = { "^[^\\.].*\\.conf", NULL }; >>>> > > # 217|-> const char *sections[] = { ".*", NULL }; >>>> > > # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3 >>>> > > # 219| uint32_t i = 0; >>>> > > _______________________________________________ >>>> > >>>> > New patch attached. >>>> > >>>> > Michal >>>> > >>>> >>>> The patch is fine, (so ack also from me), but can someone check if the >>>> attached rebase on top of Pavel's sssctl is OK? >>>> >>>> >>> >>>I did not check the differences between the rebased and >>>non-rebased version, but I just tried branch with sssctl >>>patches + this rebased patch + startup validation patches >>>and it worked for me. >>> >>>But the CI failed for me with the snippets in place. >>>Will take a look at what is happening there. >>> >>It failed beccause it bas broken with libini_config 1.0 .. 1.2 >>and current CI machines still have libini_config 1.2 >>or 1.1 (el6) >> >>Updated patch is attached. >> >Ups, > >ENOPATCH > >fixed in this version :-) > Previous version had an issue with SELinux and blocking access to conf.d.
Hopefully last version is attached. LS
>From c514db5699c8327c69f90b83885551e52df7b34b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> Date: Tue, 22 Mar 2016 14:09:34 +0100 Subject: [PATCH 1/3] confdb: Make it possible to use config snippets Resolves: https://fedorahosted.org/sssd/ticket/2247 Signed-off-by: Lukas Slebodnik <lsleb...@redhat.com> --- Makefile.am | 4 ++- contrib/sssd.spec.in | 1 + src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 29 +++++++------------- src/confdb/confdb_setup.h | 1 + src/external/libini_config.m4 | 12 +++++++++ src/monitor/monitor.c | 6 +++-- src/tools/common/sss_tools.c | 4 ++- src/util/sss_ini.c | 62 ++++++++++++++++++++++++++++++++++++++++++- src/util/sss_ini.h | 3 ++- 10 files changed, 97 insertions(+), 26 deletions(-) diff --git a/Makefile.am b/Makefile.am index 152fdbfc435179dfaf2dec1c248bd83a037e2420..d87896df403147acb69b7b50db779af6ad6e6162 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3849,6 +3849,7 @@ SSSD_USER_DIRS = \ $(DESTDIR)$(pubconfpath)/krb5.include.d \ $(DESTDIR)$(gpocachepath) \ $(DESTDIR)$(sssdconfdir) \ + $(DESTDIR)$(sssdconfdir)/conf.d \ $(DESTDIR)$(sssddefaultconfdir) \ $(DESTDIR)$(logpath) \ $(NULL) @@ -3883,7 +3884,8 @@ endif $(INSTALL) -d -m 0755 $(DESTDIR)$(mcpath) $(DESTDIR)$(pipepath) \ $(DESTDIR)$(pubconfpath) \ $(DESTDIR)$(pubconfpath)/krb5.include.d $(DESTDIR)$(gpocachepath) - $(INSTALL) -d -m 0711 $(DESTDIR)$(sssdconfdir) + $(INSTALL) -d -m 0711 $(DESTDIR)$(sssdconfdir) \ + $(DESTDIR)$(sssdconfdir)/conf.d if HAVE_DOXYGEN docs: diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 769a074848257e9e407d6e544496f6f14a6ea602..1c2c593dae9ae7ad7c73f7821d635c060e3954b9 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -782,6 +782,7 @@ done %attr(755,sssd,sssd) %dir %{gpocachepath} %attr(750,sssd,sssd) %dir %{_var}/log/%{name} %attr(711,sssd,sssd) %dir %{_sysconfdir}/sssd +%attr(711,sssd,sssd) %dir %{_sysconfdir}/sssd/conf.d %ghost %attr(0600,sssd,sssd) %config(noreplace) %{_sysconfdir}/sssd/sssd.conf %if (0%{?use_systemd} == 1) %attr(755,root,root) %dir %{_sysconfdir}/systemd/system/sssd.service.d diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 0a3d6a035ca53e7e5563138b5d8daf5a0770a693..2cd75b9e8b7d81261774303ad48fcec4112e3ae4 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -42,6 +42,7 @@ #define CONFDB_FILE "config.ldb" #define SSSD_DEFAULT_CONFIG_FILE SSSD_DEFAULT_CONF_DIR"/sssd.conf" #define SSSD_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" +#define CONFDB_DEFAULT_CONFIG_DIR SSSD_CONF_DIR"/conf.d" #define SSSD_MIN_ID 1 #define SSSD_LOCAL_MINID 1000 #define CONFDB_DEFAULT_SHELL_FALLBACK "/bin/sh" diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index c1db9965ffe3b13fce88e09b01777f5d686bce06..45e406c903bcab254793fca2e8e902adeb4652f6 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -126,14 +126,14 @@ static int confdb_create_base(struct confdb_ctx *cdb) return EOK; } -static int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) +static int confdb_init_db(const char *config_file, const char *config_dir, + struct confdb_ctx *cdb) { TALLOC_CTX *tmp_ctx; int ret; int sret = EOK; int version; char timestr[21]; - char *lasttimestr; bool in_transaction = false; const char *config_ldif; const char *vals[2] = { timestr, NULL }; @@ -205,9 +205,6 @@ static int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) goto done; } - /* Determine if the conf file has changed since we last updated - * the confdb - */ ret = sss_ini_get_stat(init_data); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, @@ -224,22 +221,13 @@ static int confdb_init_db(const char *config_file, struct confdb_ctx *cdb) "Failed to convert time_t to string ??\n"); ret = errno ? errno : EFAULT; } - ret = confdb_get_string(cdb, tmp_ctx, "config", "lastUpdate", - NULL, &lasttimestr); - if (ret == EOK) { - /* check if we lastUpdate and last file modification change differ*/ - if ((lasttimestr != NULL) && (strcmp(lasttimestr, timestr) == 0)) { - /* not changed, get out, nothing more to do */ - ret = EOK; - goto done; - } - } else { - DEBUG(SSSDBG_FATAL_FAILURE, "Failed to get lastUpdate attribute.\n"); - goto done; - } + /* FIXME: Determine if the conf file or any snippet has changed + * since we last updated the confdb or if some snippet was + * added or removed. + */ - ret = sss_ini_get_config(init_data, config_file); + ret = sss_ini_get_config(init_data, config_file, config_dir); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to load configuration\n"); goto done; @@ -357,6 +345,7 @@ done: errno_t confdb_setup(TALLOC_CTX *mem_ctx, const char *cdb_file, const char *config_file, + const char *config_dir, struct confdb_ctx **_cdb) { TALLOC_CTX *tmp_ctx; @@ -412,7 +401,7 @@ errno_t confdb_setup(TALLOC_CTX *mem_ctx, goto done; } - ret = confdb_init_db(config_file, cdb); + ret = confdb_init_db(config_file, config_dir, cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "ConfDB initialization has failed " "[%d]: %s\n", ret, sss_strerror(ret)); diff --git a/src/confdb/confdb_setup.h b/src/confdb/confdb_setup.h index d995fc98cf3cf109fcbe08b44e25908b81ac5bd4..a348b29739cab4a2f0008fdbe0b84ed68351bb32 100644 --- a/src/confdb/confdb_setup.h +++ b/src/confdb/confdb_setup.h @@ -48,6 +48,7 @@ errno_t confdb_setup(TALLOC_CTX *mem_ctx, const char *cdb_file, const char *config_file, + const char *config_dir, struct confdb_ctx **_cdb); diff --git a/src/external/libini_config.m4 b/src/external/libini_config.m4 index 9e5c69fbde6f64ddfdffd276fb9c5a21d96f6a01..a2bba4243b86996bdfc81e894ca8bdc489a9c4e6 100644 --- a/src/external/libini_config.m4 +++ b/src/external/libini_config.m4 @@ -19,6 +19,18 @@ PKG_CHECK_MODULES(INI_CONFIG_V0, [ INI_CONFIG_LIBS="$INI_CONFIG_V1_1_LIBS" HAVE_LIBINI_CONFIG_V1_1=1 AC_DEFINE_UNQUOTED(HAVE_LIBINI_CONFIG_V1_1, 1, [libini_config version 1.1.0 or greater]) + PKG_CHECK_MODULES(INI_CONFIG_V1_3, [ + ini_config >= 1.3.0], [ + + INI_CONFIG_CFLAGS="$INI_CONFIG_V1_3_CFLAGS" + INI_CONFIG_LIBS="$INI_CONFIG_V1_3_LIBS" + HAVE_LIBINI_CONFIG_V1_3=1 + AC_DEFINE_UNQUOTED(HAVE_LIBINI_CONFIG_V1_3, 1, + [libini_config version 1.3.0 or greater]) + ], [ + AC_MSG_WARN([libini_config-devel >= 1.3.0 not available, using older version]) + ] + ) ], [ AC_MSG_WARN([libini_config-devel >= 1.1.0 not available, using older version]) ] diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 8aec39656046bb4a2e6be0bf01f3c66d82414c6d..7ea0ce6b548329915aa8000eaa16ab16c7dd50cd 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -1872,6 +1872,7 @@ static int monitor_ctx_destructor(void *mem) */ errno_t load_configuration(TALLOC_CTX *mem_ctx, const char *config_file, + const char *config_dir, struct mt_ctx **monitor) { errno_t ret; @@ -1892,7 +1893,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx, goto done; } - ret = confdb_setup(ctx, cdb_file, config_file, &ctx->cdb); + ret = confdb_setup(ctx, cdb_file, config_file, config_dir, &ctx->cdb); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Unable to setup ConfDB [%d]: %s\n", ret, sss_strerror(ret)); @@ -3147,7 +3148,8 @@ int main(int argc, const char *argv[]) } /* Parse config file, fail if cannot be done */ - ret = load_configuration(tmp_ctx, config_file, &monitor); + ret = load_configuration(tmp_ctx, config_file, CONFDB_DEFAULT_CONFIG_DIR, + &monitor); if (ret != EOK) { switch (ret) { case ERR_MISSING_CONF: diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c index 6080bb0ac3c6dff5cc6159139d5a8fd01d6ce133..182b66e1e57ec8c9b007094dc793fbc8078d1839 100644 --- a/src/tools/common/sss_tools.c +++ b/src/tools/common/sss_tools.c @@ -103,7 +103,9 @@ static errno_t sss_tool_confdb_init(TALLOC_CTX *mem_ctx, return ENOMEM; } - ret = confdb_setup(mem_ctx, path, SSSD_CONFIG_FILE, &confdb); + ret = confdb_setup(mem_ctx, path, + SSSD_CONFIG_FILE, CONFDB_DEFAULT_CONFIG_DIR, + &confdb); talloc_zfree(path); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Unable to setup ConfDB [%d]: %s\n", diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c index 766a75ea6730b3a8f5fbec5f2086075fbcb2d81b..667447e17dae1bf20f50ce5e9cd25d56c1da3414 100644 --- a/src/util/sss_ini.c +++ b/src/util/sss_ini.c @@ -46,6 +46,8 @@ struct sss_ini_initdata { char **error_list; + struct ref_array *ra_success_list; + struct ref_array *ra_error_list; struct ini_cfgobj *sssd_config; struct value_obj *obj; const struct stat *cstat; @@ -205,10 +207,19 @@ void sss_ini_config_print_errors(char **error_list) /* Load configuration */ int sss_ini_get_config(struct sss_ini_initdata *init_data, - const char *config_file) + const char *config_file, + const char *config_dir) { int ret; #ifdef HAVE_LIBINI_CONFIG_V1 +#ifdef HAVE_LIBINI_CONFIG_V1_3 + const char *patterns[] = { "^[^\\.].*\\.conf", NULL }; + const char *sections[] = { ".*", NULL }; + uint32_t i = 0; + char *msg = NULL; + struct access_check snip_check; + struct ini_cfgobj *modified_sssd_config = NULL; +#endif /* HAVE_LIBINI_CONFIG_V1_3 */ /* Create config object */ ret = ini_config_create(&(init_data->sssd_config)); @@ -244,6 +255,55 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data, return ret; } +#ifdef HAVE_LIBINI_CONFIG_V1_3 + snip_check.flags = INI_ACCESS_CHECK_MODE | INI_ACCESS_CHECK_UID + | INI_ACCESS_CHECK_GID; + snip_check.uid = 0; /* owned by root */ + snip_check.gid = 0; /* owned by root */ + snip_check.mode = S_IRUSR; /* r**------ */ + snip_check.mask = ALLPERMS & ~(S_IWUSR | S_IXUSR); + + ret = ini_config_augment(init_data->sssd_config, + config_dir, + patterns, + sections, + &snip_check, + INI_STOP_ON_ANY, + INI_MV1S_OVERWRITE, + INI_PARSE_NOWRAP, + INI_MV2S_OVERWRITE, + &modified_sssd_config, + &init_data->ra_error_list, + &init_data->ra_success_list); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to augment configuration [%d]: %s", + ret, sss_strerror(ret)); + } + + while (ref_array_get(init_data->ra_success_list, i, &msg) != NULL) { + DEBUG(SSSDBG_TRACE_FUNC, + "Config merge success: %s\n", msg); + i++; + } + + i = 0; + while (ref_array_get(init_data->ra_error_list, i, &msg) != NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Config merge error: %s\n", msg); + i++; + } + + /* switch config objects if there are no errors */ + if (modified_sssd_config != NULL) { + ini_config_destroy(init_data->sssd_config); + init_data->sssd_config = modified_sssd_config; + } else { + DEBUG(SSSDBG_TRACE_FUNC, + "Using only main configuration file due to errors in merging\n"); + } +#endif + return ret; #else diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h index 3beaca15ba87ad12a016a3295371a56239165683..f5b36deb9cacfecbd68dd2a4d37a4398ce280c3c 100644 --- a/src/util/sss_ini.h +++ b/src/util/sss_ini.h @@ -58,7 +58,8 @@ int sss_ini_get_mtime(struct sss_ini_initdata *init_data, /* Load configuration */ int sss_ini_get_config(struct sss_ini_initdata *init_data, - const char *config_file); + const char *config_file, + const char *config_dir); /* Get configuration object */ int sss_ini_get_cfgobj(struct sss_ini_initdata *init_data, const char *section, const char *name); -- 2.7.4
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
