ssh: clean up some channels plumbing

Damien Miller <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
Hi,

This makes a struct that is only used internally by the ssh channels
code private, and plumbs the main ssh context down a bit further.

I want this for some further cleanups and support for TCP keepalives
on sockets opened by TCP forwarding.

ok?

diff --git a/channels.c b/channels.c
index bdfad84..b08ec3c 100644
--- a/channels.c
+++ b/channels.c
@@ -109,6 +109,13 @@ struct permission {
 	Channel *downstream;		/* Downstream mux*/
 };
 
+/* Context for non-blocking connects */
+struct channel_connect {
+	char *host;
+	int port;
+	struct addrinfo *ai, *aitop;
+};
+
 /*
  * Stores the forwarding permission state for a single direction (local or
  * remote).
@@ -212,8 +219,7 @@ static void port_open_helper(struct ssh *ssh, Channel *c, char *rtype);
 static const char *channel_rfwd_bind_host(const char *listen_host);
 
 /* non-blocking connect helpers */
-static int connect_next(struct channel_connect *);
-static void channel_connect_ctx_free(struct channel_connect *);
+static int connect_next(struct ssh *, struct channel_connect *);
 static Channel *rdynamic_connect_prepare(struct ssh *, char *, char *);
 static int rdynamic_connect_finish(struct ssh *, Channel *);
 
@@ -299,6 +305,29 @@ channel_lookup(struct ssh *ssh, int id)
 	return NULL;
 }
 
+static void
+free_connect_ctx(struct channel_connect *cctx)
+{
+	free(cctx->host);
+	if (cctx->aitop) {
+		if (cctx->aitop->ai_family == AF_UNIX)
+			free(cctx->aitop);
+		else
+			freeaddrinfo(cctx->aitop);
+	}
+	memset(cctx, 0, sizeof(*cctx));
+}
+
+static void
+channel_free_connect_ctx(Channel *c)
+{
+	if (c == NULL || c->connect_ctx)
+		return
+	free_connect_ctx(c->connect_ctx);
+	free(c->connect_ctx);
+	c->connect_ctx = NULL;
+}
+
 /*
  * Add a timeout for open channels whose c->ctype (or c->xctype if it is set)
  * match type_pattern.
@@ -809,6 +838,7 @@ channel_free(struct ssh *ssh, Channel *c)
 	c->listening_addr = NULL;
 	free(c->xctype);
 	c->xctype = NULL;
+	channel_free_connect_ctx(c);
 	while ((cc = TAILQ_FIRST(&c->status_confirms)) != NULL) {
 		if (cc->abandon_cb != NULL)
 			cc->abandon_cb(ssh, c, cc->ctx);
@@ -2116,6 +2146,9 @@ channel_post_connecting(struct ssh *ssh, Channel *c)
 		return;
 	if (!c->have_remote_id)
 		fatal_f("channel %d: no remote id", c->self);
+	if (c->connect_ctx == NULL)
+		fatal_f("channel %d: context missing", c->self);
+
 	/* for rdynamic the OPEN_CONFIRMATION has been sent already */
 	isopen = (c->type == SSH_CHANNEL_RDYNAMIC_FINISH);
 
@@ -2127,8 +2160,8 @@ channel_post_connecting(struct ssh *ssh, Channel *c)
 	if (err == 0) {
 		/* Non-blocking connection completed */
 		debug("channel %d: connected to %s port %d",
-		    c->self, c->connect_ctx.host, c->connect_ctx.port);
-		channel_connect_ctx_free(&c->connect_ctx);
+		    c->self, c->connect_ctx->host, c->connect_ctx->port);
+		channel_free_connect_ctx(c);
 		c->type = SSH_CHANNEL_OPEN;
 		channel_set_used_time(ssh, c);
 		if (isopen) {
@@ -2152,11 +2185,11 @@ channel_post_connecting(struct ssh *ssh, Channel *c)
 	debug("channel %d: connection failed: %s", c->self, strerror(err));
 
 	/* Try next address, if any */
-	if ((sock = connect_next(&c->connect_ctx)) == -1) {
+	if ((sock = connect_next(ssh, c->connect_ctx)) == -1) {
 		/* Exhausted all addresses for this destination */
 		error("connect_to %.100s port %d: failed.",
-		    c->connect_ctx.host, c->connect_ctx.port);
-		channel_connect_ctx_free(&c->connect_ctx);
+		    c->connect_ctx->host, c->connect_ctx->port);
+		channel_free_connect_ctx(c);
 		if (isopen) {
 			rdynamic_close(ssh, c);
 		} else {
@@ -4625,7 +4658,7 @@ channel_update_permission(struct ssh *ssh, int idx, int newport)
 
 /* Try to start non-blocking connect to next host in cctx list */
 static int
-connect_next(struct channel_connect *cctx)
+connect_next(struct ssh *ssh, struct channel_connect *cctx)
 {
 	int sock, saved_errno;
 	struct sockaddr_un *sunaddr;
@@ -4683,19 +4716,6 @@ connect_next(struct channel_connect *cctx)
 	return -1;
 }
 
-static void
-channel_connect_ctx_free(struct channel_connect *cctx)
-{
-	free(cctx->host);
-	if (cctx->aitop) {
-		if (cctx->aitop->ai_family == AF_UNIX)
-			free(cctx->aitop);
-		else
-			freeaddrinfo(cctx->aitop);
-	}
-	memset(cctx, 0, sizeof(*cctx));
-}
-
 /*
  * Return connecting socket to remote host:port or local socket path,
  * passing back the failure reason if appropriate.
@@ -4721,7 +4741,7 @@ connect_to_helper(struct ssh *ssh, const char *name, int port, int socktype,
 
 		/*
 		 * Fake up a struct addrinfo for AF_UNIX connections.
-		 * channel_connect_ctx_free() must check ai_family
+		 * free_connect_ctx() must check ai_family
 		 * and use free() not freeaddrinfo() for AF_UNIX.
 		 */
 		ai = xcalloc(1, sizeof(*ai) + sizeof(*sunaddr));
@@ -4755,7 +4775,7 @@ connect_to_helper(struct ssh *ssh, const char *name, int port, int socktype,
 	cctx->port = port;
 	cctx->ai = cctx->aitop;
 
-	if ((sock = connect_next(cctx)) == -1) {
+	if ((sock = connect_next(ssh, cctx)) == -1) {
 		error("connect to %.100s port %d failed: %s",
 		    name, port, strerror(errno));
 		return -1;
@@ -4777,14 +4797,15 @@ connect_to(struct ssh *ssh, const char *host, int port,
 	sock = connect_to_helper(ssh, host, port, SOCK_STREAM, ctype, rname,
 	    &cctx, NULL, NULL);
 	if (sock == -1) {
-		channel_connect_ctx_free(&cctx);
+		free_connect_ctx(&cctx);
 		return NULL;
 	}
 	c = channel_new(ssh, ctype, SSH_CHANNEL_CONNECTING, sock, sock, -1,
 	    CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, rname, 1);
 	c->host_port = port;
 	c->path = xstrdup(host);
-	c->connect_ctx = cctx;
+	c->connect_ctx = xmalloc(sizeof(cctx));
+	memcpy(c->connect_ctx, &cctx, sizeof(cctx));
 
 	return c;
 }
@@ -4891,7 +4912,7 @@ channel_connect_to_port(struct ssh *ssh, const char *host, u_short port,
 	sock = connect_to_helper(ssh, host, port, SOCK_STREAM, ctype, rname,
 	    &cctx, reason, errmsg);
 	if (sock == -1) {
-		channel_connect_ctx_free(&cctx);
+		free_connect_ctx(&cctx);
 		return NULL;
 	}
 
@@ -4899,7 +4920,8 @@ channel_connect_to_port(struct ssh *ssh, const char *host, u_short port,
 	    CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, rname, 1);
 	c->host_port = port;
 	c->path = xstrdup(host);
-	c->connect_ctx = cctx;
+	c->connect_ctx = xmalloc(sizeof(cctx));
+	memcpy(c->connect_ctx, &cctx, sizeof(cctx));
 
 	return c;
 }
@@ -5023,11 +5045,12 @@ rdynamic_connect_finish(struct ssh *ssh, Channel *c)
 	sock = connect_to_helper(ssh, c->path, c->host_port, SOCK_STREAM, NULL,
 	    NULL, &cctx, NULL, NULL);
 	if (sock == -1)
-		channel_connect_ctx_free(&cctx);
+		free_connect_ctx(&cctx);
 	else {
 		/* similar to SSH_CHANNEL_CONNECTING but we've already sent the open */
 		c->type = SSH_CHANNEL_RDYNAMIC_FINISH;
-		c->connect_ctx = cctx;
+		c->connect_ctx = xmalloc(sizeof(cctx));
+		memcpy(c->connect_ctx, &cctx, sizeof(cctx));
 		channel_register_fds(ssh, c, sock, sock, -1, 0, 1, 0);
 	}
 	return sock;
diff --git a/channels.h b/channels.h
index d187836..103f725 100644
--- a/channels.h
+++ b/channels.h
@@ -90,6 +90,7 @@
 struct ssh;
 struct Channel;
 typedef struct Channel Channel;
+struct channel_connect;
 
 typedef void channel_open_fn(struct ssh *, int, int, void *);
 typedef void channel_callback_fn(struct ssh *, int, int, void *);
@@ -109,13 +110,6 @@ struct channel_confirm {
 };
 TAILQ_HEAD(channel_confirms, channel_confirm);
 
-/* Context for non-blocking connects */
-struct channel_connect {
-	char *host;
-	int port;
-	struct addrinfo *ai, *aitop;
-};
-
 /* Callbacks for mux channels back into client-specific code */
 typedef int mux_callback_fn(struct ssh *, struct Channel *);
 
@@ -203,8 +197,7 @@ struct Channel {
 	int			datagram;
 
 	/* non-blocking connect */
-	/* XXX make this a pointer so the structure can be opaque */
-	struct channel_connect	connect_ctx;
+	struct channel_connect	*connect_ctx;
 
 	/* multiplexing protocol hook, called for each packet received */
 	mux_callback_fn		*mux_rcb;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.