- djm@cvs.openbsd.org 2008/05/08 12:02:23

     [auth-options.c auth1.c channels.c channels.h clientloop.c gss-serv.c]
     [monitor.c monitor_wrap.c nchan.c servconf.c serverloop.c session.c]
     [ssh.c sshd.c]
     Implement a channel success/failure status confirmation callback
     mechanism. Each channel maintains a queue of callbacks, which will
     be drained in order (RFC4253 guarantees confirm messages are not
     reordered within an channel).
     Also includes a abandonment callback to clean up if a channel is
     closed without sending confirmation messages. This probably
     shouldn't happen in compliant implementations, but it could be
     abused to leak memory.
     ok markus@ (as part of a larger diff)

diff --git a/ChangeLog b/ChangeLog
index 56a1d55b9b173053c92609412a5bf85dfb833a28..2ca100c89f755103aed246eba686d33957dc0c89 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -62,6 +62,19 @@
      [bufaux.c buffer.h channels.c packet.c packet.h]
      avoid extra malloc/copy/free when receiving data over the net;
      ~10% speedup for localhost-scp; ok djm@
+   - djm@cvs.openbsd.org 2008/05/08 12:02:23
+     [auth-options.c auth1.c channels.c channels.h clientloop.c gss-serv.c]
+     [monitor.c monitor_wrap.c nchan.c servconf.c serverloop.c session.c]
+     [ssh.c sshd.c]
+     Implement a channel success/failure status confirmation callback
+     mechanism. Each channel maintains a queue of callbacks, which will
+     be drained in order (RFC4253 guarantees confirm messages are not
+     reordered within an channel).
+     Also includes a abandonment callback to clean up if a channel is
+     closed without sending confirmation messages. This probably
+     shouldn't happen in compliant implementations, but it could be
+     abused to leak memory.
+     ok markus@ (as part of a larger diff)
 
 20080403
  - (djm) [openbsd-compat/bsd-poll.c] Include stdlib.h to avoid compile-

diff --git a/auth-options.c b/auth-options.c
index 6e2256961005562349addbf3b165ec2cd0fe02f9..3a6c3c0f35e48d8283654c26d2256b7173791888 100644 (file)
--- a/auth-options.c
+++ b/auth-options.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth-options.c,v 1.41 2008/03/26 21:28:14 djm Exp $ */
+/* $OpenBSD: auth-options.c,v 1.42 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -20,6 +20,7 @@
 #include <stdio.h>
 #include <stdarg.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "match.h"
 #include "log.h"

diff --git a/auth1.c b/auth1.c
index c17cc91335fb7baed4870f17270a15e011b03a45..b5798f634801ab625ce0e60add40d9c0e430312d 100644 (file)
--- a/auth1.c
+++ b/auth1.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth1.c,v 1.71 2007/09/21 08:15:29 djm Exp $ */
+/* $OpenBSD: auth1.c,v 1.72 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -20,6 +20,7 @@
 #include <unistd.h>
 #include <pwd.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "rsa.h"
 #include "ssh1.h"

diff --git a/channels.c b/channels.c
index 05c23e59c2b14551621b38de6ebc28c163ae11c6..b5e28dabf16491697486f84080acc44caae4245e 100644 (file)
--- a/channels.c
+++ b/channels.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.274 2008/05/08 06:59:01 markus Exp $ */
+/* $OpenBSD: channels.c,v 1.275 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -61,6 +61,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "ssh1.h"
@@ -319,10 +320,11 @@ channel_new(char *ctype, int type, int rfd, int wfd, int efd,
        c->single_connection = 0;
        c->detach_user = NULL;
        c->detach_close = 0;
-       c->confirm = NULL;
-       c->confirm_ctx = NULL;
+       c->open_confirm = NULL;
+       c->open_confirm_ctx = NULL;
        c->input_filter = NULL;
        c->output_filter = NULL;
+       TAILQ_INIT(&c->status_confirms);
        debug("channel %d: new [%s]", found, remote_name);
        return c;
 }
@@ -379,6 +381,7 @@ channel_free(Channel *c)
 {
        char *s;
        u_int i, n;
+       struct channel_confirm *cc;
 
        for (n = 0, i = 0; i < channels_alloc; i++)
                if (channels[i])
@@ -402,6 +405,13 @@ channel_free(Channel *c)
                xfree(c->remote_name);
                c->remote_name = NULL;
        }
+       while ((cc = TAILQ_FIRST(&c->status_confirms)) != NULL) {
+               if (cc->abandon_cb != NULL)
+                       cc->abandon_cb(c, cc->ctx);
+               TAILQ_REMOVE(&c->status_confirms, cc, entry);
+               bzero(cc, sizeof(*cc));
+               xfree(cc);
+       }
        channels[c->self] = NULL;
        xfree(c);
 }
@@ -660,16 +670,33 @@ channel_request_start(int id, char *service, int wantconfirm)
 }
 
 void
-channel_register_confirm(int id, channel_callback_fn *fn, void *ctx)
+channel_register_status_confirm(int id, channel_confirm_cb *cb,
+    channel_confirm_abandon_cb *abandon_cb, void *ctx)
+{
+       struct channel_confirm *cc;
+       Channel *c;
+
+       if ((c = channel_lookup(id)) == NULL)
+               fatal("channel_register_expect: %d: bad id", id);
+
+       cc = xmalloc(sizeof(*cc));
+       cc->cb = cb;
+       cc->abandon_cb = abandon_cb;
+       cc->ctx = ctx;
+       TAILQ_INSERT_TAIL(&c->status_confirms, cc, entry);
+}
+
+void
+channel_register_open_confirm(int id, channel_callback_fn *fn, void *ctx)
 {
        Channel *c = channel_lookup(id);
 
        if (c == NULL) {
-               logit("channel_register_comfirm: %d: bad id", id);
+               logit("channel_register_open_comfirm: %d: bad id", id);
                return;
        }
-       c->confirm = fn;
-       c->confirm_ctx = ctx;
+       c->open_confirm = fn;
+       c->open_confirm_ctx = ctx;
 }
 
 void
@@ -2209,9 +2236,9 @@ channel_input_open_confirmation(int type, u_int32_t seq, void *ctxt)
        if (compat20) {
                c->remote_window = packet_get_int();
                c->remote_maxpacket = packet_get_int();
-               if (c->confirm) {
+               if (c->open_confirm) {
                        debug2("callback start");
-                       c->confirm(c->self, c->confirm_ctx);
+                       c->open_confirm(c->self, c->open_confirm_ctx);
                        debug2("callback done");
                }
                debug2("channel %d: open confirm rwindow %u rmax %u", c->self,
@@ -2328,6 +2355,34 @@ channel_input_port_open(int type, u_int32_t seq, void *ctxt)
        xfree(host);
 }
 
+/* ARGSUSED */
+void
+channel_input_status_confirm(int type, u_int32_t seq, void *ctxt)
+{
+       Channel *c;
+       struct channel_confirm *cc;
+       int remote_id;
+
+       /* Reset keepalive timeout */
+       keep_alive_timeouts = 0;
+
+       remote_id = packet_get_int();
+       packet_check_eom();
+
+       debug2("channel_input_confirm: type %d id %d", type, remote_id);
+
+       if ((c = channel_lookup(remote_id)) == NULL) {
+               logit("channel_input_success_failure: %d: unknown", remote_id);
+               return;
+       }       
+       ;
+       if ((cc = TAILQ_FIRST(&c->status_confirms)) == NULL)
+               return;
+       cc->cb(type, c, cc->ctx);
+       TAILQ_REMOVE(&c->status_confirms, cc, entry);
+       bzero(cc, sizeof(*cc));
+       xfree(cc);
+}
 
 /* -- tcp forwarding */
 

diff --git a/channels.h b/channels.h
index b632a86af3682abfb9b9848f390df4a00e95739e..46cde0309a60223c173fe09e75f3c57b5a9e9b90 100644 (file)
--- a/channels.h
+++ b/channels.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.h,v 1.89 2007/06/11 09:14:00 markus Exp $ */
+/* $OpenBSD: channels.h,v 1.90 2008/05/08 12:02:23 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -64,6 +64,17 @@ typedef void channel_callback_fn(int, void *);
 typedef int channel_infilter_fn(struct Channel *, char *, int);
 typedef u_char *channel_outfilter_fn(struct Channel *, u_char **, u_int *);
 
+/* Channel success/failure callbacks */
+typedef void channel_confirm_cb(int, struct Channel *, void *);
+typedef void channel_confirm_abandon_cb(struct Channel *, void *);
+struct channel_confirm {
+       TAILQ_ENTRY(channel_confirm) entry;
+       channel_confirm_cb *cb;
+       channel_confirm_abandon_cb *abandon_cb;
+       void *ctx;
+};
+TAILQ_HEAD(channel_confirms, channel_confirm);
+
 struct Channel {
        int     type;           /* channel type/state */
        int     self;           /* my own channel identifier */
@@ -104,10 +115,11 @@ struct Channel {
        char   *ctype;          /* type */
 
        /* callback */
-       channel_callback_fn     *confirm;
-       void                    *confirm_ctx;
+       channel_callback_fn     *open_confirm;
+       void                    *open_confirm_ctx;
        channel_callback_fn     *detach_user;
        int                     detach_close;
+       struct channel_confirms status_confirms;
 
        /* filter */
        channel_infilter_fn     *input_filter;
@@ -170,8 +182,11 @@ void        channel_stop_listening(void);
 void    channel_send_open(int);
 void    channel_request_start(int, char *, int);
 void    channel_register_cleanup(int, channel_callback_fn *, int);
-void    channel_register_confirm(int, channel_callback_fn *, void *);
-void    channel_register_filter(int, channel_infilter_fn *, channel_outfilter_fn *);
+void    channel_register_open_confirm(int, channel_callback_fn *, void *);
+void    channel_register_filter(int, channel_infilter_fn *,
+    channel_outfilter_fn *);
+void    channel_register_status_confirm(int, channel_confirm_cb *,
+    channel_confirm_abandon_cb *, void *);
 void    channel_cancel_cleanup(int);
 int     channel_close_fd(int *);
 void    channel_send_window_changes(void);
@@ -188,6 +203,7 @@ void         channel_input_open_confirmation(int, u_int32_t, void *);
 void    channel_input_open_failure(int, u_int32_t, void *);
 void    channel_input_port_open(int, u_int32_t, void *);
 void    channel_input_window_adjust(int, u_int32_t, void *);
+void    channel_input_status_confirm(int, u_int32_t, void *);
 
 /* file descriptor handling (read/write) */
 

diff --git a/clientloop.c b/clientloop.c
index 8a40bc71e0ed9828429d322e4c15c3f0297e559e..edd80144007c989a99b0dccdc9d64bc0ab21ddc3 100644 (file)
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.188 2008/02/22 20:44:02 dtucker Exp $ */
+/* $OpenBSD: clientloop.c,v 1.189 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -86,6 +86,7 @@
 #include <pwd.h>
 #include <unistd.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "ssh1.h"
@@ -700,7 +701,7 @@ client_extra_session2_setup(int id, void *arg)
            cctx->term, &cctx->tio, c->rfd, &cctx->cmd, cctx->env,
            client_subsystem_reply);
 
-       c->confirm_ctx = NULL;
+       c->open_confirm_ctx = NULL;
        buffer_free(&cctx->cmd);
        xfree(cctx->term);
        if (cctx->env != NULL) {
@@ -940,7 +941,8 @@ client_process_control(fd_set *readset)
        debug3("%s: channel_new: %d", __func__, c->self);
 
        channel_send_open(c->self);
-       channel_register_confirm(c->self, client_extra_session2_setup, cctx);
+       channel_register_open_confirm(c->self,
+           client_extra_session2_setup, cctx);
 }
 
 static void
@@ -2068,6 +2070,8 @@ client_init_dispatch_20(void)
        dispatch_set(SSH2_MSG_CHANNEL_OPEN_FAILURE, &channel_input_open_failure);
        dispatch_set(SSH2_MSG_CHANNEL_REQUEST, &client_input_channel_req);
        dispatch_set(SSH2_MSG_CHANNEL_WINDOW_ADJUST, &channel_input_window_adjust);
+       dispatch_set(SSH2_MSG_CHANNEL_SUCCESS, &channel_input_status_confirm);
+       dispatch_set(SSH2_MSG_CHANNEL_FAILURE, &channel_input_status_confirm);
        dispatch_set(SSH2_MSG_GLOBAL_REQUEST, &client_input_global_request);
 
        /* rekeying */

diff --git a/gss-serv.c b/gss-serv.c
index bc498fd47eb2144d7c3cb17dc87ae4ed863d3c3f..2ec7ea19c2da3fb41f1ed55ba02d94ef56ab5117 100644 (file)
--- a/gss-serv.c
+++ b/gss-serv.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: gss-serv.c,v 1.21 2007/06/12 08:20:00 djm Exp $ */
+/* $OpenBSD: gss-serv.c,v 1.22 2008/05/08 12:02:23 djm Exp $ */
 
 /*
  * Copyright (c) 2001-2003 Simon Wilkinson. All rights reserved.
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <unistd.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "buffer.h"
 #include "key.h"

diff --git a/monitor.c b/monitor.c
index cc0e0fcac03c9de8beb188f4bbf63c3a632f4e92..04f6924b6bf177ea2788ef162dfb5077ce24c841 100644 (file)
--- a/monitor.c
+++ b/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.94 2007/10/29 04:08:08 dtucker Exp $ */
+/* $OpenBSD: monitor.c,v 1.95 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -51,6 +51,7 @@
 
 #include <openssl/dh.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "key.h"

diff --git a/monitor_wrap.c b/monitor_wrap.c
index e895f19240ae6971bcee8346d5d35d1f5ef2a986..72fd5c83ca75ccbe129f5cb9a13182352da48035 100644 (file)
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.60 2007/10/29 04:08:08 dtucker Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.61 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -41,6 +41,7 @@
 #include <openssl/bn.h>
 #include <openssl/dh.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "dh.h"

diff --git a/nchan.c b/nchan.c
index ad461f4af6a36c57362a773601e2f4ea8645653e..0d0faddb3d889dea9aa3e53cfebc0e7cf2ac0abb 100644 (file)
--- a/nchan.c
+++ b/nchan.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: nchan.c,v 1.57 2006/08/03 03:34:42 deraadt Exp $ */
+/* $OpenBSD: nchan.c,v 1.58 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -32,6 +32,7 @@
 #include <string.h>
 #include <stdarg.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "ssh1.h"
 #include "ssh2.h"
 #include "buffer.h"

diff --git a/servconf.c b/servconf.c
index e6d49099b05a7d9b934ac67588c5880080df2ca1..b8a968aa3afccb4d57373457c4e886b3931b0011 100644 (file)
--- a/servconf.c
+++ b/servconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.c,v 1.178 2008/05/07 05:49:37 pyr Exp $ */
+/* $OpenBSD: servconf.c,v 1.179 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -24,6 +24,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "log.h"

diff --git a/serverloop.c b/serverloop.c
index bf3f9c9f0af67b53c45b394dc52bc68d02025204..20991c3ceacf641a40a4feb5f8f595b63d37eda1 100644 (file)
--- a/serverloop.c
+++ b/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.148 2008/02/22 20:44:02 dtucker Exp $ */
+/* $OpenBSD: serverloop.c,v 1.149 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -56,6 +56,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "packet.h"
 #include "buffer.h"
@@ -1188,8 +1189,9 @@ server_init_dispatch_20(void)
        dispatch_set(SSH2_MSG_CHANNEL_REQUEST, &server_input_channel_req);
        dispatch_set(SSH2_MSG_CHANNEL_WINDOW_ADJUST, &channel_input_window_adjust);
        dispatch_set(SSH2_MSG_GLOBAL_REQUEST, &server_input_global_request);
+       dispatch_set(SSH2_MSG_CHANNEL_SUCCESS, &channel_input_status_confirm);
+       dispatch_set(SSH2_MSG_CHANNEL_FAILURE, &channel_input_status_confirm);
        /* client_alive */
-       dispatch_set(SSH2_MSG_CHANNEL_FAILURE, &server_input_keep_alive);
        dispatch_set(SSH2_MSG_REQUEST_SUCCESS, &server_input_keep_alive);
        dispatch_set(SSH2_MSG_REQUEST_FAILURE, &server_input_keep_alive);
        /* rekeying */

diff --git a/session.c b/session.c
index 16e455588fe5f7c975d6ccf1315327692ff3555f..ca04a453217da8f56bb743289f9473e7a4253104 100644 (file)
--- a/session.c
+++ b/session.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: session.c,v 1.235 2008/05/07 05:49:37 pyr Exp $ */
+/* $OpenBSD: session.c,v 1.236 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -59,6 +59,7 @@
 #include <string.h>
 #include <unistd.h>
 
+#include "openbsd-compat/sys-queue.h"
 #include "xmalloc.h"
 #include "ssh.h"
 #include "ssh1.h"

diff --git a/ssh.c b/ssh.c
index 2ed76c9a122ff38c93c781b1335080450670dd81..b144a7130bdf30be01251f868655860a08876986 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.309 2008/01/19 20:51:26 djm Exp $ */
+/* $OpenBSD: ssh.c,v 1.310 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -73,6 +73,7 @@
 #include <openssl/evp.h>
 #include <openssl/err.h>
 #include "openbsd-compat/openssl-compat.h"
+#include "openbsd-compat/sys-queue.h"
 
 #include "xmalloc.h"
 #include "ssh.h"
@@ -1195,7 +1196,8 @@ ssh_session2_open(void)
 
        channel_send_open(c->self);
        if (!no_shell_flag)
-               channel_register_confirm(c->self, ssh_session2_setup, NULL);
+               channel_register_open_confirm(c->self,
+                   ssh_session2_setup, NULL);
 
        return c->self;
 }

diff --git a/sshd.c b/sshd.c
index 796310b03d0c0646f0f0f0e00f73236da05e05aa..aefbaaa421ce837fd68e54eff24d877f09915757 100644 (file)
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.356 2008/04/13 00:22:17 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.357 2008/05/08 12:02:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -54,6 +54,7 @@
 # include <sys/time.h>
 #endif
 #include "openbsd-compat/sys-tree.h"
+#include "openbsd-compat/sys-queue.h"
 #include <sys/wait.h>
 
 #include <errno.h>