OpenSSH tmp cleanup
Markus Friedl
markus.friedl at informatik.uni-erlangen.de
Fri Jun 8 06:52:33 EST 2001
did someone check this?
-m
On Tue, Jun 05, 2001 at 04:54:04PM +0200, Markus Friedl wrote:
> 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