set euid/egid to 0 before verify check

Thu May 31 03:17:32 2001  George Lebl <jirka@5z.com>

	* daemon/gdm.c: set euid/egid to 0 before verify check

	* daemon/slave.c: set euid/egid to 0 at a bunch of places just to
	  make sure that's how they're set.  When children want to exit they
	  shouldn't use the slave_exit function as that's dangerous to the
	  slave's health.  Instead use a new function for this which
	  doesn't do slave specific cleanup.  Also fix segfaults on startup.

	* daemon/verify-pam.c: the credential setting is supposed to be done
	  after openning a session.  Closing of a session is done silently as
	  well, jsut for good meassure as wel don't have anything to talk to
	  anymore anyway

	* daemon/slave.c, daemon/server.c: when reinitting ignore X errors
	  and do not reopen the display.  When openning the display the first
	  time, don't try so hard for a local display and instead wipe slave
	  and try again.

5 files changed, 177 insertions, 97 deletions

diff --git a/ChangeLog b/ChangeLog
index 67a3b029..9aed0dfa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+Thu May 31 03:17:32 2001  George Lebl <jirka@5z.com>
+
+	* daemon/gdm.c: set euid/egid to 0 before verify check
+
+	* daemon/slave.c: set euid/egid to 0 at a bunch of places just to
+	  make sure that's how they're set.  When children want to exit they
+	  shouldn't use the slave_exit function as that's dangerous to the
+	  slave's health.  Instead use a new function for this which
+	  doesn't do slave specific cleanup.  Also fix segfaults on startup.
+
+	* daemon/verify-pam.c: the credential setting is supposed to be done
+	  after openning a session.  Closing of a session is done silently as
+	  well, jsut for good meassure as wel don't have anything to talk to
+	  anymore anyway
+
+	* daemon/slave.c, daemon/server.c: when reinitting ignore X errors
+	  and do not reopen the display.  When openning the display the first
+	  time, don't try so hard for a local display and instead wipe slave
+	  and try again.
+
 Wed May 30 21:43:21 2001  George Lebl <jirka@5z.com>
 
 	* gui/gdmlogin.c: make iconify button nicer by making it smaller and
diff --git a/daemon/gdm.c b/daemon/gdm.c
index 3fbfcc48..a253fa3b 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -374,12 +374,11 @@ gdm_config_parse (void)
 	gdm_fail (_("gdm_config_parse: Authdir %s has wrong permissions. Should be 750. Aborting."), 
 		  GdmServAuthDir, statbuf.st_mode);
 
+    seteuid (0);
+    setegid (0);
 
     /* Check that user authentication is properly configured */
     gdm_verify_check ();
-
-    seteuid (0);
-    setegid (0);
 }
 
 
diff --git a/daemon/server.c b/daemon/server.c
index d9aff5c3..14c82c85 100644
--- a/daemon/server.c
+++ b/daemon/server.c
@@ -71,25 +71,19 @@ static GdmDisplay *d = NULL;
 void
 gdm_server_reinit (GdmDisplay *disp)
 {
-	gboolean had_connection = FALSE;
-
 	if (disp == NULL ||
 	    disp->servpid <= 0)
 		return;
 
-	/* Kill our connection */
+	/* Kill our connection if one existed */
 	if (disp->dsp != NULL) {
 		XCloseDisplay (disp->dsp);
 		disp->dsp = NULL;
-		had_connection = TRUE;
 	}
 
 	gdm_debug ("gdm_server_reinit: Server for %s is about to be reinitialized!", disp->name);
 
 	kill (disp->servpid, SIGHUP);
-
-	if (had_connection)
-		disp->dsp = XOpenDisplay (disp->name);
 }
 
 /**
@@ -108,7 +102,7 @@ gdm_server_stop (GdmDisplay *disp)
 
     gdm_debug ("gdm_server_stop: Server for %s going down!", disp->name);
 
-    /* Kill our connection */
+    /* Kill our connection if one existed */
     if (disp->dsp != NULL) {
 	XCloseDisplay (disp->dsp);
 	disp->dsp = NULL;
diff --git a/daemon/slave.c b/daemon/slave.c
index 84652794..8a97d022 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -114,9 +114,28 @@ static void     gdm_slave_alrm_handler (int sig);
 static void     gdm_slave_term_handler (int sig);
 static void     gdm_slave_child_handler (int sig);
 static void     gdm_slave_exit (gint status, const gchar *format, ...);
+static void     gdm_child_exit (gint status, const gchar *format, ...);
 static gint     gdm_slave_exec_script (GdmDisplay*, gchar *dir);
 
 
+/* Yay thread unsafety */
+static gboolean x_error_occured = FALSE;
+
+/* ignore handlers */
+static int
+ignore_xerror_handler (Display *disp, XErrorEvent *evt)
+{
+	x_error_occured = TRUE;
+	return 0;
+}
+
+static int
+ignore_xioerror_handler (Display *disp)
+{
+	x_error_occured = TRUE;
+	return 0;
+}
+
 void 
 gdm_slave_start (GdmDisplay *display)
 {  
@@ -128,7 +147,7 @@ gdm_slave_start (GdmDisplay *display)
 		return;
 
 	gdm_debug ("gdm_slave_start: Starting slave process for %s", display->name);
-	if (d->type != TYPE_LOCAL &&
+	if (display->type != TYPE_LOCAL &&
 	    GdmPingInterval > 0) {
 		/* Handle a ALRM signals from our ping alarms */
 		alrm.sa_handler = gdm_slave_alrm_handler;
@@ -165,7 +184,7 @@ gdm_slave_start (GdmDisplay *display)
 	sigdelset (&mask, SIGINT);
 	sigdelset (&mask, SIGTERM);
 	sigdelset (&mask, SIGCHLD);
-	if (d->type != TYPE_LOCAL &&
+	if (display->type != TYPE_LOCAL &&
 	    GdmPingInterval > 0) {
 		sigdelset (&mask, SIGALRM);
 	}
@@ -180,7 +199,7 @@ gdm_slave_start (GdmDisplay *display)
 		gdm_debug ("gdm_slave_start: Loop Thingie");
 		gdm_slave_run (display);
 
-		if (d->type != TYPE_LOCAL)
+		if (display->type != TYPE_LOCAL)
 			break;
 
 		the_time = time (NULL);
@@ -217,26 +236,6 @@ setup_automatic_session (GdmDisplay *display, const char *name)
 	gdm_debug ("gdm_slave_start: DisplayInit script finished");
 }
 
-#ifdef HAVE_LIBXINERAMA
-/* Yay thread unsafety */
-static gboolean x_error_occured = FALSE;
-
-/* ignore handlers */
-static int
-ignore_xerror_handler (Display *disp, XErrorEvent *evt)
-{
-	x_error_occured = TRUE;
-	return 0;
-}
-
-static int
-ignore_xioerror_handler (Display *disp)
-{
-	x_error_occured = TRUE;
-	return 0;
-}
-#endif
-
 static void 
 gdm_screen_init (GdmDisplay *display) 
 {
@@ -288,6 +287,7 @@ static void
 gdm_slave_run (GdmDisplay *display)
 {  
     gint openretries = 0;
+    gint maxtries = 0;
     
     d = display;
 
@@ -315,55 +315,66 @@ gdm_slave_run (GdmDisplay *display)
 
     gdm_debug ("gdm_slave_start: Opening display %s", d->name);
     d->dsp = NULL;
+
+    /* if local then the the server should be ready for openning, so
+     * don't try so long before killing it and trying again */
+    if (d->type == TYPE_LOCAL)
+	    maxtries = 2;
+    else
+	    maxtries = 10;
     
-    while (openretries < 10 &&
+    while (openretries < maxtries &&
 	   d->dsp == NULL) {
 	d->dsp = XOpenDisplay (d->name);
 	
 	if (d->dsp == NULL) {
-	    gdm_debug ("gdm_slave_start: Sleeping %d on a retry", openretries*2);
-	    sleep (openretries*2);
+	    gdm_debug ("gdm_slave_start: Sleeping %d on a retry", 1+openretries*2);
+	    sleep (1+openretries*2);
 	    openretries++;
 	}
     }
 
-    if (d->dsp != NULL) {
+    /* something may have gone wrong, try remanaging, if local, the toplevel
+     * loop of death will handle us */ 
+    if (d->dsp == NULL) {
+	    gdm_server_stop (d);
+	    if (d->type == TYPE_LOCAL)
+		    _exit (DISPLAY_REMANAGE);
+	    else
+		    _exit (DISPLAY_ABORT);
+    }
 
-	/* If XDMCP setup pinging */
-	if (d->type != TYPE_LOCAL &&
-	    GdmPingInterval > 0) {
-		alarm (GdmPingInterval * 60);
-	}
+    /* If XDMCP setup pinging */
+    if (d->type != TYPE_LOCAL &&
+	GdmPingInterval > 0) {
+	    alarm (GdmPingInterval * 60);
+    }
 
-	/* checkout xinerama */
-        gdm_screen_init (d);
+    /* checkout xinerama */
+    gdm_screen_init (d);
 
-	if (d->type == TYPE_LOCAL &&
-	    gdm_first_login &&
-	    ! gdm_string_empty (GdmAutomaticLogin) &&
-	    strcmp (GdmAutomaticLogin, "root") != 0) {
-		gdm_first_login = FALSE;
+    if (d->type == TYPE_LOCAL &&
+	gdm_first_login &&
+	! gdm_string_empty (GdmAutomaticLogin) &&
+	strcmp (GdmAutomaticLogin, "root") != 0) {
+	    gdm_first_login = FALSE;
 
-		setup_automatic_session (d, GdmAutomaticLogin);
+	    setup_automatic_session (d, GdmAutomaticLogin);
 
-		gdm_slave_session_start();
+	    gdm_slave_session_start();
 
-		gdm_debug ("gdm_slave_start: Automatic login done");
-	} else {
-		if (gdm_first_login)
-			gdm_first_login = FALSE;
-		gdm_slave_greeter ();  /* Start the greeter */
-		gdm_slave_wait_for_login (); /* wait for a password */
-		if (do_timed_login) {
-			/* timed out into a timed login */
-			do_timed_login = FALSE;
-			setup_automatic_session (d, GdmTimedLogin);
-		}
-		gdm_slave_session_start ();
-	}
+	    gdm_debug ("gdm_slave_start: Automatic login done");
     } else {
-	gdm_server_stop (d);
-	_exit (DISPLAY_ABORT);
+	    if (gdm_first_login)
+		    gdm_first_login = FALSE;
+	    gdm_slave_greeter ();  /* Start the greeter */
+	    gdm_slave_wait_for_login (); /* wait for a password */
+	    if (do_timed_login) {
+		    /* timed out into a timed login */
+		    do_timed_login = FALSE;
+		    setup_automatic_session (d, GdmTimedLogin);
+	    }
+	    gdm_slave_session_start ();
     }
 }
 
@@ -624,6 +635,10 @@ gdm_slave_wait_for_login (void)
 
 	/* Chat with greeter */
 	while (login == NULL) {
+		/* just for paranoia's sake */
+		seteuid (0);
+		setegid (0);
+
 		gdm_debug ("gdm_slave_wait_for_login: In loop");
 		login = gdm_verify_user (NULL /* username*/,
 					 d->name,
@@ -886,13 +901,13 @@ gdm_slave_greeter (void)
 	    dup2 (pipe2[1], STDOUT_FILENO);
 	
 	if (setgid (GdmGroupId) < 0) 
-	    gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Couldn't set groupid to %d"), GdmGroupId);
+	    gdm_child_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Couldn't set groupid to %d"), GdmGroupId);
 
 	if (initgroups (GdmUser, GdmGroupId) < 0)
-            gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: initgroups() failed for %s"), GdmUser);
+            gdm_child_exit (DISPLAY_ABORT, _("gdm_slave_greeter: initgroups() failed for %s"), GdmUser);
 	
 	if (setuid (GdmUserId) < 0) 
-	    gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Couldn't set userid to %d"), GdmUserId);
+	    gdm_child_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Couldn't set userid to %d"), GdmUserId);
 	
 	gdm_clearenv_no_lang ();
 	gdm_setenv ("XAUTHORITY", d->authfile);
@@ -974,7 +989,7 @@ gdm_slave_greeter (void)
 			    "Try logging in by other means and\n"
 			    "editting the configuration file"));
 	
-	gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Error starting greeter on display %s"), d->name);
+	gdm_child_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Error starting greeter on display %s"), d->name);
 	
     case -1:
 	gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Can't fork gdmgreeter process"));
@@ -1502,16 +1517,16 @@ gdm_slave_session_start (void)
 	umask (022);
 	
 	if (setgid (pwent->pw_gid) < 0) 
-	    gdm_slave_exit (DISPLAY_REMANAGE,
-			    _("gdm_slave_session_start: Could not setgid %d. Aborting."), pwent->pw_gid);
-	
+		gdm_child_exit (DISPLAY_REMANAGE,
+				_("gdm_slave_session_start: Could not setgid %d. Aborting."), pwent->pw_gid);
+
 	if (initgroups (login, pwent->pw_gid) < 0)
-	    gdm_slave_exit (DISPLAY_REMANAGE,
-			    _("gdm_slave_session_start: initgroups() failed for %s. Aborting."), login);
-	
+		gdm_child_exit (DISPLAY_REMANAGE,
+				_("gdm_slave_session_start: initgroups() failed for %s. Aborting."), login);
+
 	if (setuid (pwent->pw_uid) < 0) 
-	    gdm_slave_exit (DISPLAY_REMANAGE,
-			    _("gdm_slave_session_start: Could not become %s. Aborting."), login);
+		gdm_child_exit (DISPLAY_REMANAGE,
+				_("gdm_slave_session_start: Could not become %s. Aborting."), login);
 	
 	chdir (pwent->pw_dir);
 
@@ -1712,11 +1727,14 @@ gdm_slave_session_stop (pid_t sesspid)
     local_login = login;
     login = NULL;
 
+    seteuid (0);
+    setegid (0);
+
     gdm_debug ("gdm_slave_session_stop: %s on %s", local_login, d->name);
 
     if (sesspid > 0)
 	    kill (- (sesspid), SIGTERM);
-    
+
     gdm_verify_cleanup();
     
     pwent = getpwnam (local_login);	/* PAM overwrites our pwent */
@@ -1733,15 +1751,15 @@ gdm_slave_session_stop (pid_t sesspid)
     gdm_auth_user_remove (d, pwent->pw_uid);
 
     seteuid (0);
-    setegid (GdmGroupId);
+    setegid (0);
 }
 
 static void
 gdm_slave_session_cleanup (void)
 {
-    gdm_debug ("gdm_slave_session_cleanup: %s on %s", login, d->name);
+    gdm_debug ("gdm_slave_session_cleanup: on %s", d->name);
 
-    /* kill login */
+    /* kill login if it still exists */
     g_free (login);
     login = NULL;
     
@@ -1749,6 +1767,10 @@ gdm_slave_session_cleanup (void)
     gdm_debug ("gdm_slave_session_cleanup: Running post session script");
     gdm_slave_exec_script (d, GdmPostSession);
 
+    /* things are going to be killed, so ignore errors */
+    XSetErrorHandler (ignore_xerror_handler);
+    XSetIOErrorHandler (ignore_xioerror_handler);
+
     /* Cleanup */
     gdm_debug ("gdm_slave_session_cleanup: Killing windows");
     gdm_server_reinit (d);
@@ -1759,6 +1781,10 @@ gdm_slave_term_handler (int sig)
 {
     gdm_debug ("gdm_slave_term_handler: %s got TERM signal", d->name);
 
+    /* just for paranoia's sake */
+    seteuid (0);
+    setegid (0);
+
     if (d->greetpid != 0) {
 	gdm_debug ("gdm_slave_term_handler: Whacking greeter");
 	if (kill (d->greetpid, sig) == 0)
@@ -1827,6 +1853,10 @@ gdm_slave_child_handler (int sig)
 		       (int)pid, (int)WTERMSIG (status));
 
 	if (pid == d->greetpid && greet) {
+		/* just for paranoia's sake */
+		seteuid (0);
+		setegid (0);
+
 		/* if greet is TRUE, then the greeter died outside of our
 		 * control really */
 		gdm_server_stop (d);
@@ -1862,11 +1892,16 @@ static gint
 gdm_slave_xioerror_handler (Display *disp)
 {
     gdm_debug ("gdm_slave_xioerror_handler: I/O error for display %s", d->name);
+
+    /* just for paranoia's sake */
+    seteuid (0);
+    setegid (0);
     
     if (login)
 	gdm_slave_session_stop (d->sesspid);
     
     gdm_error (_("gdm_slave_xioerror_handler: Fatal X error - Restarting %s"), d->name);
+    
 
     gdm_server_stop (d);
     gdm_verify_cleanup ();
@@ -1923,6 +1958,10 @@ gdm_slave_exit (gint status, const gchar *format, ...)
     
     g_free (s);
 
+    /* just for paranoia's sake */
+    seteuid (0);
+    setegid (0);
+
     gdm_server_stop (d);
     gdm_verify_cleanup ();
 
@@ -1946,6 +1985,22 @@ gdm_slave_exit (gint status, const gchar *format, ...)
     _exit (status);
 }
 
+static void 
+gdm_child_exit (gint status, const gchar *format, ...)
+{
+    va_list args;
+    gchar *s;
+
+    va_start (args, format);
+    s = g_strdup_vprintf (format, args);
+    va_end (args);
+    
+    syslog (LOG_ERR, s);
+    
+    g_free (s);
+
+    _exit (status);
+}
 
 static gint
 gdm_slave_exec_script (GdmDisplay *d, gchar *dir)
diff --git a/daemon/verify-pam.c b/daemon/verify-pam.c
index 61d363f6..46a89203 100644
--- a/daemon/verify-pam.c
+++ b/daemon/verify-pam.c
@@ -37,7 +37,7 @@ extern gboolean GdmAllowRoot;
 extern gboolean GdmAllowRemoteRoot;
 
 /* Local PAM handle */
-pam_handle_t *pamh = NULL;
+static pam_handle_t *pamh = NULL;
 
 
 /* Internal PAM conversation function. Interfaces between the PAM
@@ -149,6 +149,7 @@ gdm_verify_user (const char *username,
     gchar *login;
     struct passwd *pwent;
     gboolean error_msg_given = FALSE;
+    gboolean opened_session = FALSE;
 
     /* start the timer for timed logins */
     if (local)
@@ -239,6 +240,15 @@ gdm_verify_user (const char *username,
 	    goto pamerr;
     }
     
+    /* Register the session */
+    if ((pamerr = pam_open_session (pamh, 0)) != PAM_SUCCESS) {
+	    if (gdm_slave_should_complain ())
+		    gdm_error (_("Couldn't open session for %s"), login);
+	    goto pamerr;
+    }
+
+    opened_session = TRUE;
+
     /* Set credentials */
     if ((pamerr = pam_setcred (pamh, 0)) != PAM_SUCCESS) {
 	    if (gdm_slave_should_complain ())
@@ -246,12 +256,6 @@ gdm_verify_user (const char *username,
 	    goto pamerr;
     }
     
-    /* Register the session */
-    if ((pamerr = pam_open_session (pamh, 0)) != PAM_SUCCESS) {
-	    if (gdm_slave_should_complain ())
-		    gdm_error (_("Couldn't open session for %s"), login);
-	    goto pamerr;
-    }
     
     return login;
     
@@ -262,6 +266,8 @@ gdm_verify_user (const char *username,
     if ( ! error_msg_given &&
 	gdm_slave_should_complain ())
 	    gdm_slave_greeter_ctl_no_ret (GDM_MSGERR, _("Authentication failed"));
+    if (opened_session)
+	    pam_close_session (pamh, 0);
     
     pam_end (pamh, pamerr);
     pamh = NULL;
@@ -286,6 +292,7 @@ void
 gdm_verify_setup_user (const gchar *login, const gchar *display) 
 {
     gint pamerr;
+    gboolean opened_session = FALSE;
 
     if (!login)
 	return;
@@ -318,17 +325,19 @@ gdm_verify_setup_user (const gchar *login, const gchar *display)
     }
 #endif
     
-    /* Set credentials */
-    if ((pamerr = pam_setcred (pamh, PAM_SILENT)) != PAM_SUCCESS) {
+    /* Register the session */
+    if ((pamerr = pam_open_session (pamh, PAM_SILENT)) != PAM_SUCCESS) {
 	    if (gdm_slave_should_complain ())
-		    gdm_error (_("Couldn't set credentials for %s"), login);
+		    gdm_error (_("Couldn't open session for %s"), login);
 	    goto setup_pamerr;
     }
+
+    opened_session = TRUE;
     
-    /* Register the session */
-    if ((pamerr = pam_open_session (pamh, PAM_SILENT)) != PAM_SUCCESS) {
+    /* Set credentials */
+    if ((pamerr = pam_setcred (pamh, PAM_SILENT)) != PAM_SUCCESS) {
 	    if (gdm_slave_should_complain ())
-		    gdm_error (_("Couldn't open session for %s"), login);
+		    gdm_error (_("Couldn't set credentials for %s"), login);
 	    goto setup_pamerr;
     }
     
@@ -336,6 +345,9 @@ gdm_verify_setup_user (const gchar *login, const gchar *display)
     
  setup_pamerr:
     
+    if (opened_session)
+	    pam_close_session (pamh, PAM_SILENT);
+    
     pam_end (pamh, pamerr);
     pamh = NULL;
     
@@ -355,7 +367,7 @@ void
 gdm_verify_cleanup (void)
 {
     if (pamh) {
-	pam_close_session (pamh, 0);
+	pam_close_session (pamh, PAM_SILENT);
 	pam_end (pamh, PAM_SUCCESS);
 	pamh = NULL;