OpenSSH tmp cleanup

Markus Friedl Markus.Friedl at informatik.uni-erlangen.de
Wed Jun 6 00:54:04 EST 2001


On Tue, Jun 05, 2001 at 09:21:46AM +0300, Jarno Huuskonen wrote:
> I noticed that Markus has fixed the temporary file cleanup problems in
> OpenSSH cvs. What files need patching for this ? I only noticed
> changes in: session.c, channels.h and channels.c.

yes.

i tried to port this back to 2.9, but i don't have time for
testing etc.

simple fix is
	s/cookies/x11_forwarding_with_openssh_is_fun/
in session.c

correct fix looks like:

Index: channels.c
===================================================================
RCS file: /home/markus/cvs/ssh/channels.c,v
retrieving revision 1.109
diff -u -r1.109 channels.c
--- channels.c	2001/04/17 12:55:03	1.109
+++ channels.c	2001/06/05 14:38:42
@@ -2524,10 +2524,17 @@
 /* removes the agent forwarding socket */
 
 void
-cleanup_socket(void)
+auth_sock_cleanup_proc(void *_pw)
 {
-	unlink(channel_forwarded_auth_socket_name);
-	rmdir(channel_forwarded_auth_socket_dir);
+	struct passwd *pw = _pw;
+
+	if (channel_forwarded_auth_socket_name) {
+		temporarily_use_uid(pw);
+		unlink(channel_forwarded_auth_socket_name);
+		rmdir(channel_forwarded_auth_socket_dir);
+		channel_forwarded_auth_socket_name = NULL;
+		restore_uid();
+	}
 }
 
 /*
@@ -2566,11 +2573,9 @@
 	snprintf(channel_forwarded_auth_socket_name, MAX_SOCKET_NAME, "%s/agent.%d",
 		 channel_forwarded_auth_socket_dir, (int) getpid());
 
-	if (atexit(cleanup_socket) < 0) {
-		int saved = errno;
-		cleanup_socket();
-		packet_disconnect("socket: %.100s", strerror(saved));
-	}
+	/* delete agent socket on fatal() */
+	fatal_add_cleanup(auth_sock_cleanup_proc, pw);
+
 	/* Create the socket. */
 	sock = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (sock < 0)
Index: channels.h
===================================================================
RCS file: /home/markus/cvs/ssh/channels.h,v
retrieving revision 1.31
diff -u -r1.31 channels.h
--- channels.h	2001/04/13 22:46:53	1.31
+++ channels.h	2001/06/05 14:37:23
@@ -293,6 +293,8 @@
  */
 char   *auth_get_socket_name(void);
 
+void	auth_sock_cleanup_proc(void *_pw);
+
 /*
  * This is called to process SSH_CMSG_AGENT_REQUEST_FORWARDING on the server.
  * This starts forwarding authentication requests.
Index: session.c
===================================================================
RCS file: /home/markus/cvs/ssh/session.c,v
retrieving revision 1.74
diff -u -r1.74 session.c
--- session.c	2001/04/17 19:34:25	1.74
+++ session.c	2001/06/05 14:39:54
@@ -89,12 +89,15 @@
 void	session_set_fds(Session *s, int fdin, int fdout, int fderr);
 void	session_pty_cleanup(Session *s);
 void	session_proctitle(Session *s);
+int	session_setup_x11fwd(Session *s);
+void	session_close(Session *s);
 void	do_exec_pty(Session *s, const char *command);
 void	do_exec_no_pty(Session *s, const char *command);
 void	do_login(Session *s, const char *command);
 void	do_child(Session *s, const char *command);
 void	do_motd(void);
 int	check_quietlogin(Session *s, const char *command);
+void	xauthfile_cleanup_proc(void *pw);
 
 void	do_authenticated1(Authctxt *authctxt);
 void	do_authenticated2(Authctxt *authctxt);
@@ -154,18 +157,26 @@
 		do_authenticated2(authctxt);
 	else
 		do_authenticated1(authctxt);
+
+	/* remote user's local Xauthority file and agent socket */
+	if (xauthfile)
+		xauthfile_cleanup_proc(authctxt->pw);
+	if (auth_get_socket_name())
+		auth_sock_cleanup_proc(authctxt->pw);
 }
 
 /*
  * Remove local Xauthority file.
  */
 void
-xauthfile_cleanup_proc(void *ignore)
+xauthfile_cleanup_proc(void *_pw)
 {
-	debug("xauthfile_cleanup_proc called");
+	struct passwd *pw = _pw;
+	char *p;
 
+	debug("xauthfile_cleanup_proc called");
 	if (xauthfile != NULL) {
-		char *p;
+		temporarily_use_uid(pw);
 		unlink(xauthfile);
 		p = strrchr(xauthfile, '/');
 		if (p != NULL) {
@@ -174,6 +185,7 @@
 		}
 		xfree(xauthfile);
 		xauthfile = NULL;
+		restore_uid();
 	}
 }
 
@@ -209,7 +221,7 @@
 {
 	Session *s;
 	char *command;
-	int success, type, fd, n_bytes, plen, screen_flag, have_pty = 0;
+	int success, type, n_bytes, plen, screen_flag, have_pty = 0;
 	int compression_level = 0, enable_compression_after_reply = 0;
 	u_int proto_len, data_len, dlen;
 
@@ -290,22 +302,6 @@
 			break;
 
 		case SSH_CMSG_X11_REQUEST_FORWARDING:
-			if (!options.x11_forwarding) {
-				packet_send_debug("X11 forwarding disabled in server configuration file.");
-				break;
-			}
-			if (!options.xauth_location) {
-				packet_send_debug("No xauth program; cannot forward with spoofing.");
-				break;
-			}
-			if (no_x11_forwarding_flag) {
-				packet_send_debug("X11 forwarding not permitted for this authentication.");
-				break;
-			}
-			debug("Received request for X11 forwarding with auth spoofing.");
-			if (s->display != NULL)
-				packet_disconnect("Protocol error: X11 display already set.");
-
 			s->auth_proto = packet_get_string(&proto_len);
 			s->auth_data = packet_get_string(&data_len);
 
@@ -325,31 +321,11 @@
 				    4 + proto_len + 4 + data_len, type);
 				s->screen = 0;
 			}
-			s->display = x11_create_display_inet(s->screen, options.x11_display_offset);
-
-			if (s->display == NULL)
-				break;
-
-			/* Setup to always have a local .Xauthority. */
-			xauthfile = xmalloc(MAXPATHLEN);
-			strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN);
-			temporarily_use_uid(s->pw);
-			if (mkdtemp(xauthfile) == NULL) {
-				restore_uid();
-				error("private X11 dir: mkdtemp %s failed: %s",
-				    xauthfile, strerror(errno));
-				xfree(xauthfile);
-				xauthfile = NULL;
-				/* XXXX remove listening channels */
-				break;
+			success = session_setup_x11fwd(s);
+			if (!success) {
+				xfree(s->auth_proto);
+				xfree(s->auth_data);
 			}
-			strlcat(xauthfile, "/cookies", MAXPATHLEN);
-			fd = open(xauthfile, O_RDWR|O_CREAT|O_EXCL, 0600);
-			if (fd >= 0)
-				close(fd);
-			restore_uid();
-			fatal_add_cleanup(xauthfile_cleanup_proc, NULL);
-			success = 1;
 			break;
 
 		case SSH_CMSG_AGENT_REQUEST_FORWARDING:
@@ -402,9 +378,7 @@
 
 			if (command != NULL)
 				xfree(command);
-			/* Cleanup user's local Xauthority file. */
-			if (xauthfile)
-				xauthfile_cleanup_proc(NULL);
+			session_close(s);
 			return;
 
 		default:
@@ -1372,23 +1346,7 @@
 int
 session_x11_req(Session *s)
 {
-	int fd;
-	if (no_x11_forwarding_flag) {
-		debug("X11 forwarding disabled in user configuration file.");
-		return 0;
-	}
-	if (!options.x11_forwarding) {
-		debug("X11 forwarding disabled in server configuration file.");
-		return 0;
-	}
-	if (xauthfile != NULL) {
-		debug("X11 fwd already started.");
-		return 0;
-	}
-
-	debug("Received request for X11 forwarding with auth spoofing.");
-	if (s->display != NULL)
-		packet_disconnect("Protocol error: X11 display already set.");
+	int success;
 
 	s->single_connection = packet_get_char();
 	s->auth_proto = packet_get_string(NULL);
@@ -1396,33 +1354,12 @@
 	s->screen = packet_get_int();
 	packet_done();
 
-	s->display = x11_create_display_inet(s->screen, options.x11_display_offset);
-	if (s->display == NULL) {
+	success = session_setup_x11fwd(s);
+	if (!success) {
 		xfree(s->auth_proto);
 		xfree(s->auth_data);
-		return 0;
 	}
-	xauthfile = xmalloc(MAXPATHLEN);
-	strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN);
-	temporarily_use_uid(s->pw);
-	if (mkdtemp(xauthfile) == NULL) {
-		restore_uid();
-		error("private X11 dir: mkdtemp %s failed: %s",
-		    xauthfile, strerror(errno));
-		xfree(xauthfile);
-		xauthfile = NULL;
-		xfree(s->auth_proto);
-		xfree(s->auth_data);
-		/* XXXX remove listening channels */
-		return 0;
-	}
-	strlcat(xauthfile, "/cookies", MAXPATHLEN);
-	fd = open(xauthfile, O_RDWR|O_CREAT|O_EXCL, 0600);
-	if (fd >= 0)
-		close(fd);
-	restore_uid();
-	fatal_add_cleanup(xauthfile_cleanup_proc, s);
-	return 1;
+	return success;
 }
 
 int
@@ -1636,6 +1573,10 @@
 void
 session_close(Session *s)
 {
+	if (s->display) {
+		xauthfile_cleanup_proc(s->pw);
+		fatal_remove_cleanup(xauthfile_cleanup_proc, s->pw);
+	}
 	session_pty_cleanup(s);
 	session_free(s);
 	session_proctitle(s);
@@ -1710,11 +1651,57 @@
 		setproctitle("%s@%s", s->pw->pw_name, session_tty_list());
 }
 
+int
+session_setup_x11fwd(Session *s)
+{
+	int fd;
+	struct stat st;
+
+	if (no_x11_forwarding_flag) {
+		packet_send_debug("X11 forwarding disabled in user configuration file.");
+		return 0;
+	}
+	if (!options.x11_forwarding) {
+		debug("X11 forwarding disabled in server configuration file.");
+		return 0;
+	}
+	if (!options.xauth_location ||
+	    (stat(options.xauth_location, &st) == -1)) {
+		packet_send_debug("No xauth program; cannot forward with spoofing.");
+		return 0;
+	}
+	if (s->display != NULL) {
+		debug("X11 display already set.");
+		return 0;
+	}
+	xauthfile = xmalloc(MAXPATHLEN);
+	strlcpy(xauthfile, "/tmp/ssh-XXXXXXXX", MAXPATHLEN);
+	temporarily_use_uid(s->pw);
+	if (mkdtemp(xauthfile) == NULL) {
+		restore_uid();
+		error("private X11 dir: mkdtemp %s failed: %s",
+		    xauthfile, strerror(errno));
+		xfree(xauthfile);
+		xauthfile = NULL;
+		return 0;
+	}
+	strlcat(xauthfile, "/cookies", MAXPATHLEN);
+	fd = open(xauthfile, O_RDWR|O_CREAT|O_EXCL, 0600);
+	if (fd >= 0)
+		close(fd);
+	restore_uid();
+	s->display = x11_create_display_inet(s->screen, options.x11_display_offset);
+	if (s->display == NULL) {
+		xauthfile_cleanup_proc(s->pw);
+		return 0;
+	}
+	fatal_add_cleanup(xauthfile_cleanup_proc, s->pw);
+	return 1;
+}
+
 void
 do_authenticated2(Authctxt *authctxt)
 {
 
 	server_loop2();
-	if (xauthfile)
-		xauthfile_cleanup_proc(NULL);
 }



More information about the openssh-unix-dev mailing list