URL: https://github.com/SSSD/sssd/pull/183 Author: fidencio Title: #183: More socket-activation fixes Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/183/head:pr183 git checkout pr183
From d0adb344eeaa3196b055ddd08b7a97981be8e5fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 13 Mar 2017 17:06:04 +0100 Subject: [PATCH 1/3] NSS: Don't set SocketUser/SocketGroup as "sssd" in sssd-nss.socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NSS service is always run as root, so there's no need to change its socket ownership to the sssd user. More than that, by setting up the SocketUser and SocketGroup to "sssd" a loop would be caused as the "sssd" would trigger an initgroups call during the NSS socket setup. The problem was found when starting up a machine with SSSD built with "--with-sssd-user=sssd" and having "sss" before "files" in the name-service switch. Related: https://pagure.io/SSSD/sssd/issue/3322 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/sysv/systemd/sssd-nss.socket.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sysv/systemd/sssd-nss.socket.in b/src/sysv/systemd/sssd-nss.socket.in index 39d30e8..83c1285 100644 --- a/src/sysv/systemd/sssd-nss.socket.in +++ b/src/sysv/systemd/sssd-nss.socket.in @@ -9,8 +9,6 @@ Conflicts=shutdown.target [Socket] ExecStartPre=@libexecdir@/sssd/sssd_check_socket_activated_responders -r nss ListenStream=@pipepath@/nss -SocketUser=@SSSD_USER@ -SocketGroup=@SSSD_USER@ [Install] WantedBy=sssd.service From e5f8fb9ba9b50e346c11dcceedb72495c6384498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 13 Mar 2017 17:35:03 +0100 Subject: [PATCH 2/3] NSS: Ensure the NSS socket is started before any other services' sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although I didn't have any problem with this before I'd like to ensure that the NSS socket is always up _before_ any other (SSSD) services' sockets as they may trigger initgroups calls as some of them have SocketUser and SocketGroup set to the "sssd" user. Related: https://pagure.io/SSSD/sssd/issue/3322 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/sysv/systemd/sssd-nss.socket.in | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sysv/systemd/sssd-nss.socket.in b/src/sysv/systemd/sssd-nss.socket.in index 83c1285..e5d6eda 100644 --- a/src/sysv/systemd/sssd-nss.socket.in +++ b/src/sysv/systemd/sssd-nss.socket.in @@ -3,6 +3,7 @@ Description=SSSD NSS Service responder socket Documentation=man:sssd.conf(5) After=sssd.service BindsTo=sssd.service +Before=sssd-autofs.socket sssd-pac.socket sssd-pam.socket sssd-ssh.socket sssd-sudo.socket DefaultDependencies=no Conflicts=shutdown.target From 8316029e28e2be5403812f1404654a5df50e9e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 7 Mar 2017 23:15:30 +0100 Subject: [PATCH 3/3] NSS: Don't call chown on NSS service's ExecStartPre MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sssd-nss.service attempts to chown its log file to ensure it has the correct owner. Unfortunately, when this happens, it enters a loop trying to call into the name-service switch and hangs forever. The approach taken to solve this issue is by using fchown() in the NSS service startup, which won't make any NSS calls itself. Resolves: https://pagure.io/SSSD/sssd/issue/3322 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/nss/nsssrv.c | 8 ++++ src/sysv/systemd/sssd-nss.service.in | 3 +- src/util/debug.c | 78 +++++++++++++++++++++++++++++++----- src/util/server.c | 14 +++---- src/util/util.h | 2 + 5 files changed, 86 insertions(+), 19 deletions(-) diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index 807b5e8..fa2ac13 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -540,6 +540,14 @@ int main(int argc, const char *argv[]) /* set up things like debug, signals, daemonization, etc... */ debug_log_file = "sssd_nss"; + if (is_socket_activated()) { + ret = open_and_fchown_debug_file(uid, gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Cannot chown the debug files, debugging might not work!\n"); + } + } + ret = server_setup("sssd[nss]", 0, uid, gid, CONFDB_NSS_CONF_ENTRY, &main_ctx); if (ret != EOK) return 2; diff --git a/src/sysv/systemd/sssd-nss.service.in b/src/sysv/systemd/sssd-nss.service.in index e2f68bc..5a5837d 100644 --- a/src/sysv/systemd/sssd-nss.service.in +++ b/src/sysv/systemd/sssd-nss.service.in @@ -9,6 +9,5 @@ RefuseManualStart=true Also=sssd-nss.socket [Service] -ExecStartPre=-/bin/chown root:root @logpath@/sssd_nss.log -ExecStart=@libexecdir@/sssd/sssd_nss --debug-to-files --socket-activated +ExecStart=@libexecdir@/sssd/sssd_nss --uid 0 --gid 0 --debug-to-files --socket-activated Restart=on-failure diff --git a/src/util/debug.c b/src/util/debug.c index ca4fa4c..7d81aaa 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -406,6 +406,45 @@ int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec) return EOK; } +int open_and_fchown_debug_file(uid_t uid, gid_t gid) +{ + char *logpath; + int fd = -1; + int ret; + + ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, debug_log_file); + if (ret == -1) { + ret = ENOMEM; + goto done; + } + + errno = 0; + fd = open(logpath, FD_CLOEXEC, O_WRONLY); + if (fd == -1) { + ret = errno; + goto done; + } + + ret = fchown(fd, uid, gid); + if (ret == -1) { + ret = errno; + close(fd); + goto done; + } + + ret = set_debug_file_from_fd(fd); + if (ret != EOK) { + close(fd); + goto done; + } + + ret = EOK; + +done: + free(logpath); + return ret; +} + int open_debug_file(void) { return open_debug_file_ex(NULL, NULL, true); @@ -414,10 +453,32 @@ int open_debug_file(void) int rotate_debug_files(void) { int ret; - errno_t error; if (!debug_to_file) return EOK; + ret = close_debug_files(); + if (ret != 0) { + sss_log(SSS_LOG_ALERT, "Attempting to open new file anyway. " + "Be aware that this is a resource leak\n"); + } + + return open_debug_file(); +} + +void talloc_log_fn(const char *message) +{ + DEBUG(SSSDBG_FATAL_FAILURE, "%s\n", message); +} + +int close_debug_files(void) +{ + int ret; + errno_t error; + + if (debug_file == NULL) { + return EOK; + } + do { error = 0; ret = fclose(debug_file); @@ -441,16 +502,13 @@ int rotate_debug_files(void) */ sss_log(SSS_LOG_ALERT, "Could not close debug file [%s]. [%d][%s]\n", debug_log_file, error, strerror(error)); - sss_log(SSS_LOG_ALERT, "Attempting to open new file anyway. " - "Be aware that this is a resource leak\n"); + ret = error; + goto done; } - debug_file = NULL; - - return open_debug_file(); -} + ret = EOK; -void talloc_log_fn(const char *message) -{ - DEBUG(SSSDBG_FATAL_FAILURE, "%s\n", message); +done: + debug_file = NULL; + return ret; } diff --git a/src/util/server.c b/src/util/server.c index 9c94418..826c711 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -473,7 +473,7 @@ int server_setup(const char *name, int flags, } if (!is_socket_activated()) { - ret = chown_debug_file(NULL, uid, gid); + ret = open_and_fchown_debug_file(uid, gid); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Cannot chown the debug files, debugging might not work!\n"); @@ -660,13 +660,13 @@ int server_setup(const char *name, int flags, return EIO; } - /* open log file if told so */ - if (debug_to_file) { - ret = open_debug_file(); + if (debug_to_file == 0) { + ret = close_debug_files(); if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, "Error setting up logging (%d) " - "[%s]\n", ret, strerror(ret)); - return ret; + DEBUG(SSSDBG_MINOR_FAILURE, + "Cannot close the debug file, even if it's explicitly set " + "to not debug to file: [%d] [%s]\n", + ret, sss_strerror(ret)); } } diff --git a/src/util/util.h b/src/util/util.h index a2dc89b..f44a076 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -158,6 +158,8 @@ 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); +int open_and_fchown_debug_file(uid_t uid, gid_t gid); +int close_debug_files(void); void talloc_log_fn(const char *msg); /* From sss_log.c */
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
