[SSSD] [PATCH] SSSD: Add the options to specify a UID and GID to run as

Jakub Hrozek jhrozek at redhat.com
Mon Oct 20 15:40:49 UTC 2014


On Mon, Oct 20, 2014 at 04:49:10PM +0200, Pavel Reichl wrote:
> 
> On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> >On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> >>I found a bug in the IFP initialization. The rest of the patches is
> >>unchanged, only ifpsrv.c changed.
> >Sorry, one more respin. I added a patch to chown the debug logs.
> >Initially, I wanted to open the debug logs as root and then just pass
> >the fd, but then I realized this wouldn't work for logfile rotation. So
> >I'm afraid we need to chown the log files..
> >
> >The other patches are unchanged, just a new one was added.
> >
> >
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> Sorry test linking fails for me:
> 
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getpwnam_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:104:
> undefined reference to `sysdb_search_user_override_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:126:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getpwuid_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:209:
> undefined reference to `sysdb_search_user_override_by_uid'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:231:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_enumpwent_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:317:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getgrnam_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:396:
> undefined reference to `sysdb_search_group_override_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:428:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:435:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getgrgid_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:548:
> undefined reference to `sysdb_search_group_override_by_gid'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:580:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:587:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_enumgrent_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:737:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:745:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_initgroups_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:959:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_get_user_attr_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1083:
> undefined reference to `sysdb_search_user_override_attrs_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1105:
> undefined reference to `sysdb_add_overrides_to_object'
> 
> 
> Do I have to apply some views patches first?
> Sorry, I'm getting lost in patch-sets dependencies.

Yes, I didn't rebase the patches on top of views. Here goes a rebased
version..

> You should push it all already. :-)

Working on it :-)
-------------- next part --------------
>From 2e78ffa1f9578a8012548e2d1209aaa04137fff6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 6 Oct 2014 16:28:13 +0200
Subject: [PATCH 1/6] SSSD: Add the options to specify a UID and GID to run as

Adds new command line options --uid and --gid to all SSSD servers,
making it possible to switch to another user ID if needed.

So far all code still runs as root.
---
 Makefile.am                       | 7 +++++--
 src/monitor/monitor.c             | 3 ++-
 src/providers/data_provider_be.c  | 5 ++++-
 src/providers/proxy/proxy_child.c | 5 ++++-
 src/responder/autofs/autofssrv.c  | 6 +++++-
 src/responder/ifp/ifpsrv.c        | 6 +++++-
 src/responder/nss/nsssrv.c        | 5 ++++-
 src/responder/pac/pacsrv.c        | 5 ++++-
 src/responder/pam/pamsrv.c        | 5 ++++-
 src/responder/ssh/sshsrv.c        | 5 ++++-
 src/responder/sudo/sudosrv.c      | 6 +++++-
 src/util/server.c                 | 8 ++++++++
 src/util/util.h                   | 7 +++++++
 13 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 6a8124b5ad30f9e54a0325dc574a96c91ec4e805..405d2aabbac56047921a28e1a37a651b8234e013 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -710,14 +710,17 @@ libsss_util_la_SOURCES = \
     src/util/util_sss_idmap.c \
     src/util/well_known_sids.c \
     src/util/string_utils.c \
+    src/util/become_user.c \
     $(NULL)
 libsss_util_la_CFLAGS = \
     $(AM_CFLAGS) \
-    $(SYSTEMD_LOGIN_CFLAGS)
+    $(SYSTEMD_LOGIN_CFLAGS) \
+    $(NULL)
 libsss_util_la_LIBADD = \
     $(SSSD_LIBS) \
     $(SYSTEMD_LOGIN_LIBS) \
-    $(UNICODE_LIBS)
+    $(UNICODE_LIBS) \
+    $(NULL)
 if BUILD_SUDO
     libsss_util_la_SOURCES += src/db/sysdb_sudo.c
 endif
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 624e45026b01842ab81d8c37d50751977185d20c..edd1c2dfc674d8a7ca9d069d6499c0dcc959f210 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2855,7 +2855,8 @@ int main(int argc, const char *argv[])
     ret = close(STDIN_FILENO);
     if (ret != EOK) return 6;
 
-    ret = server_setup(MONITOR_NAME, flags, monitor->conf_path, &main_ctx);
+    ret = server_setup(MONITOR_NAME, flags, 0, 0,
+                       monitor->conf_path, &main_ctx);
     if (ret != EOK) return 2;
 
     monitor->is_daemon = !opt_interactive;
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index e7f345f922361b60775e149c1b28f45cf16ed00e..18b50214b0795709d583d5891bf4f6fd220bcb11 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -2804,10 +2804,13 @@ int main(int argc, const char *argv[])
     struct main_context *main_ctx;
     char *confdb_path;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         {"domain", 0, POPT_ARG_STRING, &be_domain, 0,
          _("Domain of the information provider (mandatory)"), NULL },
         POPT_TABLEEND
@@ -2847,7 +2850,7 @@ int main(int argc, const char *argv[])
     confdb_path = talloc_asprintf(NULL, CONFDB_DOMAIN_PATH_TMPL, be_domain);
     if (!confdb_path) return 2;
 
-    ret = server_setup(srv_name, 0, confdb_path, &main_ctx);
+    ret = server_setup(srv_name, 0, 0, 0, confdb_path, &main_ctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Could not set up mainloop [%d]\n", ret);
         return 2;
diff --git a/src/providers/proxy/proxy_child.c b/src/providers/proxy/proxy_child.c
index 6bee1c7f63c4bb5b33f11601c2be32df05206756..e261b2f588a89a27ed6fe78f7fc5cfa053b49833 100644
--- a/src/providers/proxy/proxy_child.c
+++ b/src/providers/proxy/proxy_child.c
@@ -504,10 +504,13 @@ int main(int argc, const char *argv[])
     int ret;
     long id;
     char *pam_target = NULL;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         {"domain", 0, POPT_ARG_STRING, &domain, 0,
          _("Domain of the information provider (mandatory)"), NULL },
         {"id", 0, POPT_ARG_LONG, &id, 0,
@@ -557,7 +560,7 @@ int main(int argc, const char *argv[])
     conf_entry = talloc_asprintf(NULL, CONFDB_DOMAIN_PATH_TMPL, domain);
     if (!conf_entry) return 2;
 
-    ret = server_setup(srv_name, 0, conf_entry, &main_ctx);
+    ret = server_setup(srv_name, 0, 0, 0, conf_entry, &main_ctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Could not set up mainloop [%d]\n", ret);
         return 2;
diff --git a/src/responder/autofs/autofssrv.c b/src/responder/autofs/autofssrv.c
index bd5aa135a3fb64a020e74a9dd39fa1b67dc76533..931cf018bfe15b37bf8e5f93a21c7ab61d238c18 100644
--- a/src/responder/autofs/autofssrv.c
+++ b/src/responder/autofs/autofssrv.c
@@ -207,10 +207,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -235,7 +238,8 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_autofs";
 
-    ret = server_setup("sssd[autofs]", 0, CONFDB_AUTOFS_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[autofs]", 0, 0, 0,
+                       CONFDB_AUTOFS_CONF_ENTRY, &main_ctx);
     if (ret != EOK) {
         return 2;
     }
diff --git a/src/responder/ifp/ifpsrv.c b/src/responder/ifp/ifpsrv.c
index 4af836543d3945603d22099b7ab98a90588bce35..8d8fe885abb8ef53ee7f49e763ba78c4dda9a983 100644
--- a/src/responder/ifp/ifpsrv.c
+++ b/src/responder/ifp/ifpsrv.c
@@ -441,10 +441,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -469,7 +472,8 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_ifp";
 
-    ret = server_setup("sssd[ifp]", 0, CONFDB_IFP_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[ifp]", 0, 0, 0,
+                       CONFDB_IFP_CONF_ENTRY, &main_ctx);
     if (ret != EOK) return 2;
 
     ret = die_if_parent_died();
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index 84a6b7fedd096a7d4159b7ac6670820c1d8fd941..420fd3d316959a67737f23e9a8b3d1c797583ea3 100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -537,10 +537,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -565,7 +568,7 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_nss";
 
-    ret = server_setup("sssd[nss]", 0, CONFDB_NSS_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[nss]", 0, 0, 0, CONFDB_NSS_CONF_ENTRY, &main_ctx);
     if (ret != EOK) return 2;
 
     ret = die_if_parent_died();
diff --git a/src/responder/pac/pacsrv.c b/src/responder/pac/pacsrv.c
index 47a9d1a68fbf1615277af41cef297b4955e7f7c3..b76691de829b4f40937a07ea83825a606950aa1e 100644
--- a/src/responder/pac/pacsrv.c
+++ b/src/responder/pac/pacsrv.c
@@ -216,10 +216,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -244,7 +247,7 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_pac";
 
-    ret = server_setup("sssd[pac]", 0, CONFDB_PAC_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[pac]", 0, 0, 0, CONFDB_PAC_CONF_ENTRY, &main_ctx);
     if (ret != EOK) return 2;
 
     ret = die_if_parent_died();
diff --git a/src/responder/pam/pamsrv.c b/src/responder/pam/pamsrv.c
index 428b252acb56406291e3cac5d2a13e6f6b581c36..91b395080820b27f5d57341e59dd739e674be31a 100644
--- a/src/responder/pam/pamsrv.c
+++ b/src/responder/pam/pamsrv.c
@@ -316,10 +316,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -344,7 +347,7 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_pam";
 
-    ret = server_setup("sssd[pam]", 0, CONFDB_PAM_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[pam]", 0, 0, 0, CONFDB_PAM_CONF_ENTRY, &main_ctx);
     if (ret != EOK) return 2;
 
     ret = die_if_parent_died();
diff --git a/src/responder/ssh/sshsrv.c b/src/responder/ssh/sshsrv.c
index 8aa603d79dddb327606f02d9e7ddf2b18a98e700..1328d1746b9e2d6474d6c2f8ce2825be463ca3e7 100644
--- a/src/responder/ssh/sshsrv.c
+++ b/src/responder/ssh/sshsrv.c
@@ -184,10 +184,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -212,7 +215,7 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_ssh";
 
-    ret = server_setup("sssd[ssh]", 0, CONFDB_SSH_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[ssh]", 0, 0, 0, CONFDB_SSH_CONF_ENTRY, &main_ctx);
     if (ret != EOK) {
         return 2;
     }
diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c
index 8a197159b23abde45953b65121ff2e3fc3f2f67a..30752c9dacdc390b24fe837c0630333b5e171448 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -164,10 +164,13 @@ int main(int argc, const char *argv[])
     poptContext pc;
     struct main_context *main_ctx;
     int ret;
+    uid_t uid;
+    gid_t gid;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
         SSSD_MAIN_OPTS
+        SSSD_SERVER_OPTS(uid, gid)
         POPT_TABLEEND
     };
 
@@ -192,7 +195,8 @@ int main(int argc, const char *argv[])
     /* set up things like debug, signals, daemonization, etc... */
     debug_log_file = "sssd_sudo";
 
-    ret = server_setup("sssd[sudo]", 0, CONFDB_SUDO_CONF_ENTRY, &main_ctx);
+    ret = server_setup("sssd[sudo]", 0, 0, 0, CONFDB_SUDO_CONF_ENTRY,
+                       &main_ctx);
     if (ret != EOK) {
         return 2;
     }
diff --git a/src/util/server.c b/src/util/server.c
index 51934f8ba287d6f6e395231ed893753e10b4c9b0..3a84dee0cee06cb98c94a1d57209c2bcf7c4340a 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -412,6 +412,7 @@ errno_t server_common_rotate_logs(struct confdb_ctx *confdb,
 }
 
 int server_setup(const char *name, int flags,
+                 uid_t uid, gid_t gid,
                  const char *conf_entry,
                  struct main_context **main_ctx)
 {
@@ -426,6 +427,13 @@ int server_setup(const char *name, int flags,
     struct tevent_signal *tes;
     struct logrotate_ctx *lctx;
 
+    ret = become_user(uid, gid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FUNC_DATA,
+              "Cannot become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid);
+        return ret;
+    }
+
     debug_prg_name = strdup(name);
     if (!debug_prg_name) {
         return ENOMEM;
diff --git a/src/util/util.h b/src/util/util.h
index 0ac9b0104ca941b52bb37720dd16d2bbc9fbb9f8..544ca02aef5095ca89deb688f309462d22d02d9c 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -175,6 +175,12 @@ errno_t set_debug_file_from_fd(const int fd);
 
 #define SSSD_MAIN_OPTS SSSD_DEBUG_OPTS
 
+#define SSSD_SERVER_OPTS(uid, gid) \
+        {"uid", 0, POPT_ARG_INT, &uid, 0, \
+          _("The user ID to run the server as"), NULL}, \
+        {"gid", 0, POPT_ARG_INT, &gid, 0, \
+          _("The group ID to run the server as"), NULL},
+
 #define FLAGS_NONE 0x0000
 #define FLAGS_DAEMON 0x0001
 #define FLAGS_INTERACTIVE 0x0002
@@ -242,6 +248,7 @@ errno_t server_common_rotate_logs(struct confdb_ctx *confdb,
 int die_if_parent_died(void);
 int pidfile(const char *path, const char *name);
 int server_setup(const char *name, int flags,
+                 uid_t uid, gid_t gid,
                  const char *conf_entry,
                  struct main_context **main_ctx);
 void server_loop(struct main_context *main_ctx);
-- 
1.9.3

-------------- next part --------------
>From 130eff379725ddacc82bd843594ae284745735d1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 20 Oct 2014 13:59:49 +0200
Subject: [PATCH 2/6] SSSD: Chown the log files

We need to chown the log files before dropping root to make sure they
are usable by the SSSD user. Unfortunately, we can't just rely on
passing the fd opened by root, because we need to be also able to rotate
the log files.
---
 src/util/debug.c  | 33 +++++++++++++++++++++++++++++++++
 src/util/server.c |  6 ++++++
 src/util/util.h   |  1 +
 3 files changed, 40 insertions(+)

diff --git a/src/util/debug.c b/src/util/debug.c
index a99d5403a238f125010b9b309355b30f9f528c44..41375709170abe33b0c7fd90e3b1244299ed0241 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -297,6 +297,39 @@ void ldb_debug_messages(void *context, enum ldb_debug_level level,
     free(message);
 }
 
+/* In cases SSSD used to run as the root user, but runs as the SSSD user now,
+ * we need to chown the log files
+ */
+int chown_debug_file(const char *filename,
+                     uid_t uid, gid_t gid)
+{
+    char *logpath;
+    const char *log_file;
+    errno_t ret;
+
+    if (filename == NULL) {
+        log_file = debug_log_file;
+    } else {
+        log_file = filename;
+    }
+
+    ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);
+    if (ret == -1) {
+        return ENOMEM;
+    }
+
+    ret = chown(logpath, uid, gid);
+    free(logpath);
+    if (ret != 0) {
+        ret = errno;
+        DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",
+              log_file, ret);
+        return ret;
+    }
+
+    return EOK;
+}
+
 int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec)
 {
     FILE *f = NULL;
diff --git a/src/util/server.c b/src/util/server.c
index 3a84dee0cee06cb98c94a1d57209c2bcf7c4340a..a908470cdcf2cb85a6742e44905ae12d136c83d5 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -427,6 +427,12 @@ int server_setup(const char *name, int flags,
     struct tevent_signal *tes;
     struct logrotate_ctx *lctx;
 
+    ret = chown_debug_file(NULL, uid, gid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              "Cannot chown the debug files, debugging might not work!\n");
+    }
+
     ret = become_user(uid, gid);
     if (ret != EOK) {
         DEBUG(SSSDBG_FUNC_DATA,
diff --git a/src/util/util.h b/src/util/util.h
index 544ca02aef5095ca89deb688f309462d22d02d9c..2a7934234f083d49706ee983af770d6157328dec 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -218,6 +218,7 @@ errno_t set_debug_file_from_fd(const int fd);
 /* From debug.c */
 void ldb_debug_messages(void *context, enum ldb_debug_level level,
                         const char *fmt, va_list ap);
+int chown_debug_file(const char *filename, uid_t uid, gid_t gid);
 int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec);
 int open_debug_file(void);
 int rotate_debug_files(void);
-- 
1.9.3

-------------- next part --------------
>From 8ff2902e7e1268f78dcc4a1258ea66f6f7fba781 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 3 Oct 2014 16:09:38 +0200
Subject: [PATCH 3/6] UTIL: Use a custom PID_PATH and DB_PATH when unit testing
 server.c

server.c used hardcoded PID_PATH and DB_PATH from config.h. Normally,
this path resides in a system directory (like /var/) and should not be
written to by tests. In order to specify a different one for tests, we
need to conditionalize normal builds and unit test builds.
---
 src/util/server.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/util/server.c b/src/util/server.c
index a908470cdcf2cb85a6742e44905ae12d136c83d5..03f4b9588e6c5322d9b68b8cac90128bcb5cfcb6 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -411,6 +411,32 @@ errno_t server_common_rotate_logs(struct confdb_ctx *confdb,
     return EOK;
 }
 
+static const char *get_db_path(void)
+{
+#ifdef UNIT_TESTING
+#ifdef TEST_DB_PATH
+    return TEST_DB_PATH;
+#else
+    #error "TEST_DB_PATH must be defined when unit testing server.c!"
+#endif /* TEST_DB_PATH */
+#else
+    return DB_PATH;
+#endif /* UNIT_TESTING */
+}
+
+static const char *get_pid_path(void)
+{
+#ifdef UNIT_TESTING
+#ifdef TEST_PID_PATH
+    return TEST_PID_PATH;
+#else
+    #error "TEST_PID_PATH must be defined when unit testing server.c!"
+#endif /* TEST_PID_PATH */
+#else
+    return PID_PATH;
+#endif
+}
+
 int server_setup(const char *name, int flags,
                  uid_t uid, gid_t gid,
                  const char *conf_entry,
@@ -468,10 +494,10 @@ int server_setup(const char *name, int flags,
     }
 
     if (flags & FLAGS_PID_FILE) {
-        ret = pidfile(PID_PATH, name);
+        ret = pidfile(get_pid_path(), name);
         if (ret != EOK) {
-            DEBUG(SSSDBG_FATAL_FAILURE, "Error creating pidfile: %s/%s! "
-                  "(%d [%s])\n", PID_PATH, name, ret, strerror(ret));
+            DEBUG(SSSDBG_FATAL_FAILURE, "Error creating pidfile: %s/%s.pid! "
+                  "(%d [%s])\n", get_pid_path(), name, ret, strerror(ret));
             return ret;
         }
     }
@@ -513,7 +539,8 @@ int server_setup(const char *name, int flags,
     ctx->parent_pid = getppid();
     ctx->event_ctx = event_ctx;
 
-    conf_db = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
+    conf_db = talloc_asprintf(ctx, "%s/%s",
+                              get_db_path(), CONFDB_FILE);
     if (conf_db == NULL) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Out of memory, aborting!\n");
         return ENOMEM;
-- 
1.9.3

-------------- next part --------------
>From c9d980d0511a2a3be280978ffd18d892e89e6546 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 3 Oct 2014 16:11:08 +0200
Subject: [PATCH 4/6] TESTS: Unit tests can use confdb without using sysdb

Previously, if a test used the utility functions for setting up a test,
it had to use both sysdb and confdb. Some unit tests only need to use of
of them, for example the unit tests for the server module only need
confdb.
---
 src/tests/common_dom.c | 52 +++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/src/tests/common_dom.c b/src/tests/common_dom.c
index b96db8d9084fea9ec07be88d9c5f72c6885d8f09..bc69c5a093e17fc56e42ccedd998ab6434d85003 100644
--- a/src/tests/common_dom.c
+++ b/src/tests/common_dom.c
@@ -159,34 +159,38 @@ void test_dom_suite_cleanup(const char *tests_path,
         return;
     }
 
-    conf_db = talloc_asprintf(tmp_ctx, "%s/%s", tests_path, confdb_path);
-    if (!conf_db) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Could not construct conf_db path\n");
-        goto done;
-    }
+    if (confdb_path != NULL) {
+        conf_db = talloc_asprintf(tmp_ctx, "%s/%s", tests_path, confdb_path);
+        if (!conf_db) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                "Could not construct conf_db path\n");
+            goto done;
+        }
 
-    errno = 0;
-    ret = unlink(conf_db);
-    if (ret != 0 && errno != ENOENT) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Could not delete the test config ldb file (%d) (%s)\n",
-               errno, strerror(errno));
+        errno = 0;
+        ret = unlink(conf_db);
+        if (ret != 0 && errno != ENOENT) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                "Could not delete the test config ldb file (%d) (%s)\n",
+                errno, strerror(errno));
+        }
     }
 
-    sys_db = talloc_asprintf(tmp_ctx, "%s/%s", tests_path, sysdb_path);
-    if (!sys_db) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Could not construct sys_db path\n");
-        goto done;
-    }
+    if (sysdb_path != NULL) {
+        sys_db = talloc_asprintf(tmp_ctx, "%s/%s", tests_path, sysdb_path);
+        if (!sys_db) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                "Could not construct sys_db path\n");
+            goto done;
+        }
 
-    errno = 0;
-    ret = unlink(sys_db);
-    if (ret != 0 && errno != ENOENT) {
-        DEBUG(SSSDBG_CRIT_FAILURE,
-              "Could not delete the test ldb file (%d) (%s)\n",
-               errno, strerror(errno));
+        errno = 0;
+        ret = unlink(sys_db);
+        if (ret != 0 && errno != ENOENT) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                "Could not delete the test ldb file (%d) (%s)\n",
+                errno, strerror(errno));
+        }
     }
 
     errno = 0;
-- 
1.9.3

-------------- next part --------------
>From e6a43252e49c4949fd879b99b8f6d58823e2abda Mon Sep 17 00:00:00 2001
From: Pavel Reichl <preichl at redhat.com>
Date: Tue, 14 Oct 2014 18:02:47 +0100
Subject: [PATCH 5/6] TESTS: Add -std=gnu99 to cwrap tests CFLAGS
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In function ‘subdomain_enumerates’:
../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9: error: ‘for’ loop initial declarations are only allowed in C99 mode
         for (int i=0; parent->sd_enumerate[i]; i++) {
         ^
../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9: note: use option -std=c99 or -std=gnu99 to compile your code
make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1
---
 src/tests/cwrap/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am
index 34aec92c197a60c47067c72df9c8f3644e1a3c45..96b9a52435d9e80bea3ec132cc86c3e6abd0adf2 100644
--- a/src/tests/cwrap/Makefile.am
+++ b/src/tests/cwrap/Makefile.am
@@ -1,4 +1,5 @@
 AM_CPPFLAGS = \
+    -std=gnu99 \
     -Wall \
     -I$(top_srcdir)/src \
     -I. \
-- 
1.9.3

-------------- next part --------------
>From 78a285710666b3b00263be085fddf210cfd2b7ea Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 3 Oct 2014 16:12:01 +0200
Subject: [PATCH 6/6] TESTS: Unit tests for server_setup

We changed server_setup, so we must make sure the function continues to
work as expected.
---
 src/tests/cwrap/Makefile.am   |  63 ++++++++++++-
 src/tests/cwrap/test_server.c | 204 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 266 insertions(+), 1 deletion(-)
 create mode 100644 src/tests/cwrap/test_server.c

diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am
index 96b9a52435d9e80bea3ec132cc86c3e6abd0adf2..28d60e7f5bc346543f51740ec4b2781a99a48909 100644
--- a/src/tests/cwrap/Makefile.am
+++ b/src/tests/cwrap/Makefile.am
@@ -3,7 +3,10 @@ AM_CPPFLAGS = \
     -Wall \
     -I$(top_srcdir)/src \
     -I. \
+    -DLOCALEDIR=\"$(localedir)\" \
+    -DVARDIR=\"$(localstatedir)\" \
     $(DBUS_CFLAGS) \
+    $(GLIB2_CFLAGS) \
     $(NULL)
 
 TESTS_ENVIRONMENT = \
@@ -16,6 +19,20 @@ dist_noinst_SCRIPTS = \
     cwrap_test_setup.sh \
     $(NULL)
 
+SSSD_LIBS = \
+    $(TALLOC_LIBS) \
+    $(TEVENT_LIBS) \
+    $(POPT_LIBS) \
+    $(LDB_LIBS) \
+    $(DBUS_LIBS) \
+    $(PCRE_LIBS) \
+    $(INI_CONFIG_LIBS) \
+    $(COLLECTION_LIBS) \
+    $(DHASH_LIBS) \
+    $(SSS_CRYPT_LIBS) \
+    $(OPENLDAP_LIBS) \
+    $(TDB_LIBS)
+
 dist_noinst_DATA = \
     group \
     passwd \
@@ -25,7 +42,10 @@ check_PROGRAMS =
 if HAVE_CMOCKA
 if HAVE_NSS_WRAPPER
 if HAVE_UID_WRAPPER
-check_PROGRAMS += become_user-tests
+check_PROGRAMS = \
+	become_user-tests \
+	server-tests \
+	$(NULL)
 endif # HAVE_UID_WRAPPER
 endif # HAVE_NSS_WRAPPER
 endif # HAVE_CMOCKA
@@ -45,4 +65,45 @@ become_user_tests_LDADD = \
     $(abs_top_builddir)/libsss_test_common.la \
     $(NULL)
 
+server_tests_SOURCES = \
+    test_server.c \
+    ../../../src/util/server.c \
+    ../../../src/util/become_user.c \
+    ../../../src/util/backup_file.c \
+    ../../../src/util/domain_info_utils.c \
+    ../../../src/util/atomic_io.c \
+    ../../../src/util/signal.c \
+    ../../../src/util/util.c \
+    ../../../src/util/strtonum.c \
+    ../../../src/util/util_errors.c \
+    ../../../src/util/safe-format-string.c \
+    ../../../src/util/sss_tc_utf8.c \
+    ../../../src/util/sss_utf8.c \
+    ../../../src/util/usertools.c \
+    ../../../src/confdb/confdb.c \
+    ../../../src/db/sysdb.c \
+    ../../../src/db/sysdb_upgrade.c \
+    ../../../src/db/sysdb_ops.c \
+    ../../../src/db/sysdb_search.c \
+    ../../../src/db/sysdb_autofs.c \
+    ../../../src/db/sysdb_services.c \
+    ../../../src/db/sysdb_views.c \
+    $(NULL)
+server_tests_CFLAGS = \
+    $(AM_CFLAGS) \
+    $(LIBCAPNG_CFLAGS) \
+    -DTEST_DB_PATH=\"server_tests\" \
+    -DTEST_PID_PATH=\"server_tests\" \
+    -DUNIT_TESTING \
+    $(NULL)
+server_tests_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(LIBCAPNG_LIBS) \
+    $(UNICODE_LIBS) \
+    $(SSSD_LIBS) \
+    $(abs_top_builddir)/libsss_debug.la \
+    $(abs_top_builddir)/libsss_crypt.la \
+    $(abs_top_builddir)/libsss_test_common.la \
+    $(NULL)
+
 tests: $(check_PROGRAMS)
diff --git a/src/tests/cwrap/test_server.c b/src/tests/cwrap/test_server.c
new file mode 100644
index 0000000000000000000000000000000000000000..26ecfee1e189b6a474ae52fdbfff6b8922b3f0d7
--- /dev/null
+++ b/src/tests/cwrap/test_server.c
@@ -0,0 +1,204 @@
+/*
+    Authors:
+        Jakub Hrozek <jhrozek at redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    SSSD tests: Server instantiation
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <popt.h>
+#include "util/util.h"
+#include "tests/cmocka/common_mock.h"
+
+static void wait_for_fg_server(pid_t pid)
+{
+    pid_t wpid;
+    int status;
+
+    assert_int_not_equal(pid, -1);
+
+    wpid = waitpid(pid, &status, 0);
+    assert_int_equal(wpid, pid);
+    assert_true(WIFEXITED(status));
+    assert_int_equal(WEXITSTATUS(status), 0);
+}
+
+static void wait_for_bg_server(const char *pidfile)
+{
+    int fd;
+    long tmp;
+    char buf[16];
+    pid_t pid;
+    int ret;
+    int count;
+
+    count = 0;
+    do {
+        struct stat sb;
+
+        count++;
+        if (count > 100) {
+            break;
+        }
+
+        ret = stat(pidfile, &sb);
+        usleep(50);
+    } while (ret != 0);
+
+    /* read the pidfile */
+    fd = open(pidfile, O_RDONLY);
+    assert_false(fd < 0);
+
+    ret = read(fd, buf, sizeof(buf));
+    close(fd);
+    assert_false(ret <= 0);
+
+    buf[sizeof(buf) - 1] = '\0';
+
+    tmp = strtol(buf, NULL, 10);
+    assert_false(tmp == 0 || tmp > 0xFFFF || errno == ERANGE);
+
+    pid = (pid_t) (tmp & 0xFFFF);
+
+    /* Make sure the daemon goes away! */
+    ret = kill(pid, SIGTERM);
+    fprintf(stderr, "killing %u\n", pid);
+    assert_true(ret == 0);
+
+    unlink(pidfile);
+}
+
+void test_run_as_root_fg(void **state)
+{
+    int ret;
+    struct main_context *main_ctx;
+    pid_t pid;
+
+    /* Must root as root, real or fake */
+    assert_int_equal(geteuid(), 0);
+
+    pid = fork();
+    if (pid == 0) {
+        ret = server_setup(__FUNCTION__, 0, 0, 0,
+                           __FUNCTION__, &main_ctx);
+        assert_int_equal(ret, 0);
+        exit(0);
+    }
+    wait_for_fg_server(pid);
+}
+
+void test_run_as_sssd_fg(void **state)
+{
+    int ret;
+    struct main_context *main_ctx;
+    struct passwd *sssd;
+    pid_t pid;
+
+    /* Must root as root, real or fake */
+    assert_int_equal(geteuid(), 0);
+
+    sssd = getpwnam("sssd");
+    assert_non_null(sssd);
+
+    pid = fork();
+    if (pid == 0) {
+        ret = server_setup(__FUNCTION__, 0, sssd->pw_uid, sssd->pw_gid,
+                           __FUNCTION__, &main_ctx);
+        assert_int_equal(ret, 0);
+        exit(0);
+    }
+    wait_for_fg_server(pid);
+}
+
+void test_run_as_root_daemon(void **state)
+{
+    int ret;
+    struct main_context *main_ctx;
+    pid_t pid;
+    char *pidfile;
+
+    /* Must root as root, real or fake */
+    assert_int_equal(geteuid(), 0);
+
+    pidfile = talloc_asprintf(NULL, "%s/%s.pid", TEST_PID_PATH, __FUNCTION__);
+
+    /* Make sure there are no leftovers */
+    unlink(pidfile);
+
+    pid = fork();
+    if (pid == 0) {
+        ret = server_setup(__FUNCTION__, FLAGS_PID_FILE,
+                           0, 0, __FUNCTION__, &main_ctx);
+        assert_int_equal(ret, 0);
+
+        server_loop(main_ctx);
+        exit(0);
+    }
+
+    wait_for_bg_server(pidfile);
+    talloc_free(pidfile);
+}
+
+int main(int argc, const char *argv[])
+{
+    poptContext pc;
+    int opt;
+    int rv;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test(test_run_as_root_fg),
+        unit_test(test_run_as_sssd_fg),
+        unit_test(test_run_as_root_daemon),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_CLI_INIT(debug_level);
+
+    /* Even though normally the tests should clean up after themselves
+     * they might not after a failed run. Remove the old db to be sure */
+    tests_set_cwd();
+    test_dom_suite_cleanup(TEST_DB_PATH, CONFDB_FILE, NULL);
+    test_dom_suite_setup(TEST_DB_PATH);
+
+    rv = run_tests(tests);
+    test_dom_suite_cleanup(TEST_DB_PATH, CONFDB_FILE, NULL);
+
+    return rv;
+}
-- 
1.9.3



More information about the sssd-devel mailing list