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?= <[email protected]>
>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 <[email protected]>
>---
> Makefile.am                   |  4 ++-
> src/confdb/confdb.h           |  1 +
> src/confdb/confdb_setup.c     | 41 ++++--------------------
> src/confdb/confdb_setup.h     |  3 +-
> src/external/libini_config.m4 | 12 +++++++
> src/monitor/monitor.c         |  6 ++--
> src/util/sss_ini.c            | 73 ++++++++++++++++++++++++++++++++++++++-----
> src/util/sss_ini.h            |  3 +-
> 8 files changed, 96 insertions(+), 47 deletions(-)
>
>diff --git a/Makefile.am b/Makefile.am
>index 7161bef..67fa639 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -3566,6 +3566,7 @@ SSSD_USER_DIRS = \
>     $(DESTDIR)$(pubconfpath) \
>     $(DESTDIR)$(pubconfpath)/krb5.include.d \
>     $(DESTDIR)$(gpocachepath) \
>+    $(DESTDIR)$(sssdconfdir)/conf.d \
>     $(DESTDIR)$(sssdconfdir) \
>     $(DESTDIR)$(logpath) \
>     $(NULL)
>@@ -3600,7 +3601,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/src/confdb/confdb.h b/src/confdb/confdb.h
>index c478ef0..9d9a887 100644
>--- a/src/confdb/confdb.h
>+++ b/src/confdb/confdb.h
>@@ -41,6 +41,7 @@
> #define CONFDB_DEFAULT_CFG_FILE_VER 2
> #define CONFDB_FILE "config.ldb"
> #define CONFDB_DEFAULT_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 694a7f0..3cfc8b0 100644
>--- a/src/confdb/confdb_setup.c
>+++ b/src/confdb/confdb_setup.c
>@@ -125,14 +125,14 @@ int confdb_create_base(struct confdb_ctx *cdb)
>     return EOK;
> }
> 
>-int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
>+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 };
>@@ -174,41 +174,12 @@ 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
>+    /* 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_stat(init_data);
>-    if (ret != EOK) {
>-        DEBUG(SSSDBG_FATAL_FAILURE,
>-              "Status check on config file failed.\n");
>-        ret = errno;
>-        goto done;
>-    }
>-
>-    errno = 0;
>-
>-    ret = sss_ini_get_mtime(init_data, sizeof(timestr), timestr);
>-    if (ret <= 0 || ret >= sizeof(timestr)) {
>-        DEBUG(SSSDBG_FATAL_FAILURE,
>-              "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;
>-    }
> 
>-    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;
>diff --git a/src/confdb/confdb_setup.h b/src/confdb/confdb_setup.h
>index 2b8802f..f4f03b4 100644
>--- a/src/confdb/confdb_setup.h
>+++ b/src/confdb/confdb_setup.h
>@@ -47,6 +47,7 @@
> 
> int confdb_create_base(struct confdb_ctx *cdb);
> int confdb_test(struct confdb_ctx *cdb);
>-int confdb_init_db(const char *config_file, struct confdb_ctx *cdb);
>+int confdb_init_db(const char *config_file, const char *config_dir,
>+                   struct confdb_ctx *cdb);
> 
> #endif /* CONFDB_SETUP_H_ */
>diff --git a/src/external/libini_config.m4 b/src/external/libini_config.m4
>index 9e5c69f..a2bba42 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 ac3af28..e6bb624 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;
>@@ -1934,7 +1935,7 @@ errno_t load_configuration(TALLOC_CTX *mem_ctx,
>         goto done;
>     }
> 
>-    ret = confdb_init_db(config_file, ctx->cdb);
>+    ret = confdb_init_db(config_file, config_dir, ctx->cdb);
>     if (ret != EOK) {
>         DEBUG(SSSDBG_FATAL_FAILURE, "ConfDB initialization has failed [%s]\n",
>               sss_strerror(ret));
>@@ -3189,7 +3190,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/util/sss_ini.c b/src/util/sss_ini.c
>index 766a75e..3dad18b 100644
>--- a/src/util/sss_ini.c
>+++ b/src/util/sss_ini.c
>@@ -46,7 +46,10 @@
> 
> 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 ini_cfgobj *orig_sssd_config;
>     struct value_obj *obj;
>     const struct stat *cstat;
>     struct ini_cfgfile *file;
>@@ -205,13 +208,23 @@ 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
>+    const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>+    const char *sections[] = { ".*", NULL };
>+#ifdef HAVE_LIBINI_CONFIG_V1_3
>+    uint32_t i = 0;
>+    char *msg = NULL;
>+#endif /* HAVE_LIBINI_CONFIG_V1_3 */
>+
>+    init_data->sssd_config = NULL;
>+    init_data->ra_success_list = NULL;
> 
>     /* Create config object */
>-    ret = ini_config_create(&(init_data->sssd_config));
>+    ret = ini_config_create(&(init_data->orig_sssd_config));
>     if (ret != EOK) {
>         DEBUG(SSSDBG_FATAL_FAILURE,
>                 "Failed to create config object. Error %d.\n", ret);
>@@ -223,27 +236,73 @@ int sss_ini_get_config(struct sss_ini_initdata 
>*init_data,
>                            INI_STOP_ON_ANY,
>                            INI_MV1S_OVERWRITE,
>                            INI_PARSE_NOWRAP,
>-                           init_data->sssd_config);
>+                           init_data->orig_sssd_config);
> 
>     if (ret != EOK) {
>         DEBUG(SSSDBG_FATAL_FAILURE,
>                 "Failed to parse configuration. Error %d.\n", ret);
> 
>-        if (ini_config_error_count(init_data->sssd_config)) {
>+        if (ini_config_error_count(init_data->orig_sssd_config)) {
>             DEBUG(SSSDBG_FATAL_FAILURE,
>                     "Errors detected while parsing: %s\n",
>                      ini_config_get_filename(init_data->file));
> 
>-            ini_config_get_errors(init_data->sssd_config,
>+            ini_config_get_errors(init_data->orig_sssd_config,
>                                   &init_data->error_list);
>             sss_ini_config_print_errors(init_data->error_list);
>             ini_config_free_errors(init_data->error_list);
>         }
>-        ini_config_destroy(init_data->sssd_config);
>-        init_data->sssd_config = NULL;
>+        ini_config_destroy(init_data->orig_sssd_config);
>+        init_data->orig_sssd_config = NULL;
>+        return ret;
>+    }
>+
>+    /* Create config object */
>+    ret = ini_config_create(&(init_data->sssd_config));
>+    if (ret != EOK) {
>+        DEBUG(SSSDBG_FATAL_FAILURE,
>+              "Failed to create config object. Error %d.\n", ret);
>         return ret;
>     }
> 
>+#ifdef HAVE_LIBINI_CONFIG_V1_3
>+    ret = ini_config_augment(init_data->orig_sssd_config,
>+                             config_dir,
>+                             patterns,
>+                             sections,
>+                             NULL,
>+                             INI_STOP_ON_ANY,
>+                             INI_MV1S_OVERWRITE,
>+                             INI_PARSE_NOWRAP,
>+                             INI_MV2S_OVERWRITE,
>+                             &init_data->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_error_list, i, &msg) != NULL) {
>+        DEBUG(SSSDBG_CRIT_FAILURE,
>+              "Config merge error: %s\n", msg);
>+        i++;
>+    }
>+
>+    i = 0;
>+    while (ref_array_get(init_data->ra_success_list, i, &msg) != NULL) {
>+        DEBUG(SSSDBG_TRACE_FUNC,
>+              "Config merge success: %s\n", msg);
>+        i++;
>+    }
>+#endif
>+
>+    if (init_data->sssd_config == NULL) {
>+        /* Fall back to original configuration */
>+        init_data->sssd_config = init_data->orig_sssd_config;
>+    }
>+
>     return ret;
> 
> #else
>diff --git a/src/util/sss_ini.h b/src/util/sss_ini.h
>index 3beaca1..f5b36de 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.5.0
>

>From 97b8542bd8ad0603d97cf0b2201807d5a3073e51 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Michal=20=C5=BDidek?= <[email protected]>
>Date: Thu, 23 Jun 2016 18:13:50 +0200
>Subject: [PATCH] Squash me before pushing
>
>Just add missing access check
>for snippets.
>---
> src/util/sss_ini.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/src/util/sss_ini.c b/src/util/sss_ini.c
>index 3dad18b..3c014b3 100644
>--- a/src/util/sss_ini.c
>+++ b/src/util/sss_ini.c
>@@ -218,6 +218,14 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data,
> #ifdef HAVE_LIBINI_CONFIG_V1_3
>     uint32_t i = 0;
>     char *msg = NULL;
>+    struct access_check snip_check;
>+
>+    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);
> #endif /* HAVE_LIBINI_CONFIG_V1_3 */
> 
>     init_data->sssd_config = NULL;
>@@ -270,7 +278,7 @@ int sss_ini_get_config(struct sss_ini_initdata *init_data,
>                              config_dir,
>                              patterns,
>                              sections,
>-                             NULL,
>+                             &snip_check,
LGTM
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to