These patches mainly fix the ways that things are executed. The previous
implementation was not compatible with FreeBSD in a few ways...
First, the comment in the code about needing to parse the arguments to
'entrance' rather than executing 'sh -c' was handled by prepending 'exec ' to
the string so that /bin/sh can parse the arguments, and then get out of the
way. Apparantly, ash is one of the sh-shells that doesn't pass SIGUSR1.
I also prepended 'exec ' to the string that execs entrance_login, because
otherwise waitpid fails because (at least on my system) the forked child is no
longer technically a child of entrance_login, but, rather, a child of its
parent (sh).
Login should be setup by prepending '-' to argv[0] rather than passing '-l' as
an argument, which not all shell support. I changed that and also made use of
pwent->pw_shell.
Also, in entrance_session.c, I replaced snprintf with a macro that makes sure
that the result is null terminated. This isn't really the right way to do it,
but it's burned me once already.
Other than that, I made a few symantic changes. Some were necessary, others
were for readability -- In particular, in entrance_auth.c I replaced '#ifdef
HAVE_PAM' with 'if (HAVE_PAM &&' for improved readability. For consistency,
that format should either be continued to other such macros, or should be
reverted. The 'if (...' method optimizes out to the same code as the ifdef,
and offers several advantages, but I won't complain of there are objections.
One diff is relative to src/client and the other src/daemon
--
Kris Maglione
? changes.diff
Index: entrance_auth.c
===================================================================
RCS file: /root/e17/apps/entrance/src/client/entrance_auth.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 entrance_auth.c
--- entrance_auth.c 1 Nov 2005 12:05:01 -0000 1.1.1.1
+++ entrance_auth.c 2 Feb 2006 02:36:59 -0000
@@ -5,6 +5,16 @@
#include "entrance_auth.h"
#include "util.h"
+/* XXX:
+ * This should probably be done another way, but this way
+ * is better than leaving the chance of a non-terminated
+ * string (which has afflicted me once already) */
+#define snprintf(str, size, format, ...) do { \
+ snprintf(str, size, format, ##__VA_ARGS__); \
+ (str)[size-1] = 0; \
+ } \
+ while (0)
+
static char *
_entrance_auth_get_running_username(void)
{
@@ -244,7 +254,8 @@
else
result = ERROR_BAD_PASS;
- syslog(LOG_CRIT, "PAM: %s.", pam_strerror(e->pam.handle, pamerr));
+ if (result != AUTH_SUCCESS)
+ syslog(LOG_CRIT, "PAM: %s.", pam_strerror(e->pam.handle, pamerr));
return result;
}
@@ -391,7 +402,9 @@
#if HAVE_CLEARENV
clearenv();
#else
- environ = NULL;
+ /* environ=0 causes a null pointer exception in BSD setenv
+ * *envoron=0 works as expected */
+ *environ = NULL;
#endif
e->env = environ;
Index: entrance_session.c
===================================================================
RCS file: /root/e17/apps/entrance/src/client/entrance_session.c,v
retrieving revision 1.1.1.5
diff -u -r1.1.1.5 entrance_session.c
--- entrance_session.c 10 Dec 2005 06:06:44 -0000 1.1.1.5
+++ entrance_session.c 2 Feb 2006 02:36:59 -0000
@@ -9,6 +9,14 @@
#include "entrance_user.h"
#include "entrance_x_session.h"
#include "entrance_ipc.h"
+/* To make some if-them-else constructs less ugly, give
+ * HAVE_PAM a definate value and use 'if (HAVE_PAM && ...'
+ * and let (g)cc optimize it out */
+#ifdef HAVE_PAM
+# define HAVE_PAM 1
+#else
+# define HAVE_PAM 0
+#endif
/**
@file entrance_session.c
@@ -174,13 +182,13 @@
{
case ENTRANCE_AUTOLOGIN_NONE:
ecore_evas_show(e->ee);
- if (e->config->presel.mode==ENTRANCE_PRESEL_PREV &&
strlen(e->config->presel.prevuser)) {
+ if (e->config->presel.mode==ENTRANCE_PRESEL_PREV &&
strlen(e->config->presel.prevuser)) {
Evas_Object *oo =
evas_object_name_find(evas_object_evas_get(e->edje), "entrance.entry.user");
- if (oo) {
+ if (oo) {
esmart_text_entry_text_set(oo, e->config->presel.prevuser);
entrance_session_user_set(e, e->config->presel.prevuser);
- }
- }
+ }
+ }
break;
case ENTRANCE_AUTOLOGIN_DEFAULT:
if ((eu =
@@ -417,8 +425,7 @@
syslog(LOG_NOTICE, "Starting session for user \"%s\".", e->auth->user);
-#ifdef HAVE_PAM
- if (e->config->auth == ENTRANCE_USE_PAM)
+ if (HAVE_PAM && e->config->auth == ENTRANCE_USE_PAM)
{
/* Tell PAM that session has begun */
if (pam_open_session(e->auth->pam.handle, 0) != PAM_SUCCESS)
@@ -433,7 +440,6 @@
syslog(LOG_INFO, "Opened PAM session. %s : %s.", e->auth->pw->pw_name,
e->display);
}
-#endif
_entrance_session_user_list_fix(e);
@@ -454,6 +460,7 @@
switch ((pid = fork()))
{
case 0:
+ /* Child fork: setuid/gid and execute a login shell with session
choice */
if (initgroups(pwent->pw_name, pwent->pw_gid))
syslog(LOG_CRIT,
"Unable to initialize group (is entrance running as
root?).");
@@ -461,27 +468,41 @@
syslog(LOG_CRIT, "Unable to set group id.");
if (setuid(pwent->pw_uid))
syslog(LOG_CRIT, "Unable to set user id.");
- shell = strdup(pwent->pw_shell);
+ /* For a shell to be a login shell, argv[0] must start with a '-'
+ * Some shells can act as a login shell when given the '-l' argument,
+ * but I think that the only /bin/sh that takes that argument is bash
*/
+ asprintf(&shell, "-%s", pwent->pw_shell);
+
+ struct_passwd_free(pwent);
+ entrance_session_free(e);
+
+ execl(&shell[1], shell, "-c", buf, NULL);
+
+ syslog(LOG_CRIT, "Unable to execute user's login shell.");
+ exit(1);
break;
case -1:
syslog(LOG_INFO, "FORK FAILED, UH OH");
exit(0);
default:
syslog(LOG_NOTICE, "Replacing Entrance with simple login program to
wait for session end.");
-#ifdef HAVE_PAM
- if (e->config->auth == ENTRANCE_USE_PAM)
+
+ /* The command passed to "sh -c" must be exec'd, otherwise, the forked
+ * child will technically not be a child of entrance_login and so will
+ * not be able to waitpid (at least not on FreeBSD, maybe on Linux,
etc.)
+ */
+ if (HAVE_PAM && e->config->auth == ENTRANCE_USE_PAM)
{
- snprintf(buf, sizeof(buf), "%s/entrance_login %i %s %s",
+ snprintf(buf, sizeof(buf), "exec %s/entrance_login %i %s %s",
PACKAGE_BIN_DIR, (int) pid, pwent->pw_name, e->display);
}
else
-#endif
{
- snprintf(buf, sizeof(buf), "%s/entrance_login %i", PACKAGE_BIN_DIR,
+ snprintf(buf, sizeof(buf), "exec %s/entrance_login %i",
PACKAGE_BIN_DIR,
(int) pid);
}
- shell = strdup("/bin/sh");
- /* this bypasses a race condition where entrance loses its x
+
+ /* this helps prevent a race condition where entrance loses its x
connection before the wm gets it and x goes and resets itself */
sleep(10);
/*
@@ -489,13 +510,14 @@
ecore_x_shutdown();
ecore_shutdown();
*/
+
+ struct_passwd_free(pwent);
+ entrance_session_free(e);
+ /* replace this process with a clean small one that just waits for its
*/
+ /* child to exit.. passed on the cmd-line */
+ execl("/bin/sh", "sh", "-c", buf, NULL);
break;
}
- struct_passwd_free(pwent);
- entrance_session_free(e);
- /* replace this process with a clean small one that just waits for its */
- /* child to exit.. passed on the cmd-line */
- execl("/bin/sh", "/bin/sh", "-l", "-c", buf, NULL);
}
? changes.diff
Index: spawner.c
===================================================================
RCS file: /root/e17/apps/entrance/src/daemon/spawner.c,v
retrieving revision 1.1.1.5
diff -u -r1.1.1.5 spawner.c
--- spawner.c 15 Jan 2006 11:36:18 -0000 1.1.1.5
+++ spawner.c 2 Feb 2006 00:51:20 -0000
@@ -141,10 +141,14 @@
return -1;
}
- snprintf(x_cmd, PATH_MAX, "%s -auth %s %s", d->xprog, d->authfile,
d->name);
+ /* We need to exec these commands to avoid the chance of the shell not
passing
+ * SIGUSR1 to us from the process */
+ snprintf(x_cmd, PATH_MAX, "exec %s -auth %s %s", d->xprog, d->authfile,
d->name);
}
else
{
+ /* We need to exec these commands to avoid the chance of the shell not
passing
+ * SIGUSR1 to us from the process */
snprintf(x_cmd, PATH_MAX, "%s %s", d->xprog, d->name);
}
entranced_debug("Entranced_Start_Server_Once: Executing %s\n", x_cmd);
@@ -160,9 +164,7 @@
_entrance_x_sa.sa_flags = 0;
sigemptyset(&_entrance_x_sa.sa_mask);
sigaction(SIGUSR1, &_entrance_x_sa, NULL);
- /* FIXME: need to parse command and NOT go thru /bin/sh!!!! */
- /* why? some /bin/sh's wont pass on this SIGUSR1 thing... */
- execl("/bin/sh", "/bin/sh", "-c", x_cmd, NULL);
+ execl("/bin/sh", "sh", "-c", x_cmd, NULL);
syslog(LOG_WARNING, "Could not execute X server.");
exit(1);
default:
