People,
Recently I've faced some issues when testing the socket-activation
working running as sssd-user, which will force me to take a different
path for a few things and I really would like to know your opinion on
those things.
So, currently, this is what the nss.service looks like:
[Unit]
Description=SSSD NSS Service responder
Documentation=man:sssd.conf(5)
After=sssd.service
BindsTo=sssd.service
[Install]
Also=sssd-nss.socket
[Service]
ExecStartPre=-/bin/chown @SSSD_USER@:@SSSD_USER@ @logpath(a)/sssd_nss.log
ExecStart=@libexecdir@/sssd/sssd_nss --debug-to-files --unprivileged-start
Restart=on-failure
User=@SSSD_USER@
Group=@SSSD_USER@
PermissionsStartOnly=true
As you probably noticed, I've been using systemd's machinery to change
the debug files' owner and to start the responder by the proper user
(sssd or root). Well, it doesn't work that well as expected as systemd
ends up calling initgroups(sssd, ...) in order to start any service
using "sssd" user and this call is done _before_ starting the NSS
responder, which will hang for the "default client timeout" (300s).
Okay, we have to change it and here is where I need your help!
- Adding a new "sssd-user" option:
By adding this option we could just start the responders calling:
libexecdir@/sssd/sssd_nss --debug-to-files --sssd-user=@SSSD_USER@.
The unit file would be changed a little as the ExectStartPre, User and
Group options could just go away.
The server code would be changed a little in order to invoke
setenv("_SSS_LOOPS", "NO", 0) earlier, as done here:
index 52e6ed7..013e572 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -461,8 +461,6 @@ int server_setup(const char *name, int flags,
char *locale;
int watchdog_interval;
- setenv("_SSS_LOOPS", "NO", 0);
-
ret = chown_debug_file(NULL, uid, gid);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
@@ -481,6 +479,8 @@ int server_setup(const char *name, int flags,
return ENOMEM;
}
+ setenv("_SSS_LOOPS", "NO", 0);
+
/* To make sure the domain cannot be set from the environment, unset the
* variable explicitly when setting up any server. Backends later set the
* value after reading domain from the configuration */
Also, the server_setup() would receive the user passed to --sssd-user
and perform a getpwnam() in order to get the uid/gid and then be able
to change debug files' owner and become the user, as done here:
diff --git a/src/util/server.c b/src/util/server.c
index c9a2726..d9da803 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -445,6 +445,7 @@ static const char *get_pid_path(void)
int server_setup(const char *name, int flags,
uid_t uid, gid_t gid,
+ char *sssd_user,
const char *conf_entry,
struct main_context **main_ctx)
{
@@ -463,6 +464,22 @@ int server_setup(const char *name, int flags,
setenv("_SSS_LOOPS", "NO", 0);
+ if (sssd_user != NULL) {
+ struct passwd *pw;
+
+ pw = getpwnam(sssd_user);
+ if (pw == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cannot get the information about the user %s. "
+ "The process will be run as root\n", sssd_user);
+ uid = 0;
+ gid = 0;
+ } else {
+ uid = pw->pw_uid;
+ gid = pw->pw_gid;
+ }
+ }
+
ret = chown_debug_file(NULL, uid, gid);
Okay, okay. It would work. Would it be okay for the rest of the developers?
Considering that it will be okay ... doesn't it look a little bit
weird/inconsistent passing the sssd-user sometimes and passing the
uid/gid in the most part of the times? Wouldn't be better just pass
the sssd-user all the time and do the getpwnam() call here instead of
doing it in the monitor when starting it?
Now, considering we will user "--sssd-user" when calling the
responders ... I can take advantage of this in case we _only_ use this
option when socket-activating the responders, as the responder will
know from the beginning whether it was socket activated or explicitly
started (and it's useful for shutting down the responder when it's
inactive). Another option (that I personally prefer) is having, again,
a new option "--socket-activated" that would make things clear for
whoever ends up taking care of this code later on (probably the future
me). Would that be okay?
So, summing up the questions ...
- Is there any problem on calling 'setenv("_SSS_LOOPS", "NO",
0)'
earlier in the process?
- Would be okay adding "--sssd-user" option for starting the responders?
- Shall I get rid of the "--uid and --gid" options in order to only
use the new added "--sssd-user" one?
- Shall I add a new "--socket-activated" option for starting the
responders or just abuse the "--sssd-user" one?
Looking forward to hearing your opinions!
Best Regards,
--
Fabiano FidĂȘncio