From 2160205aee71702b0f73adcd7ef449ca1bae3c8e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Mon, 27 Jan 2020 19:34:15 +0000 Subject: [PATCH] Merge the two low-level portfwd setup systems. In commit 09954a87c I introduced the portfwdmgr_connect_socket() system, which opened a port forwarding given a callback to create the Socket itself, with the aim of using it to make forwardings to Unix- domain sockets and Windows named pipes (both initially for agent forwarding). But I forgot that a year and a bit ago, in commit 834396170, I already introduced a similar low-level system for creating a PortForwarding around an unusual kind of socket: the portfwd_raw_new() system, which in place of a callback uses a two-phase setup protocol (you create the socket in between the two setup calls, and can roll it back if the socket can't be created). There's really no need to have _both_ these systems! So now I'm merging them, which is to say, I'm enhancing portfwd_raw_new to have the one new feature it needs, and throwing away the newer system completely. The new feature is to be able to control the initial state of the 'ready' flag: portfwd_raw_new was always used for initiating port forwardings in response to an incoming local connection, which means you need to start off with ready=false and set it true when the other end of the SSH connection sends back OPEN_CONFIRMATION. Now it's being used for initiating port forwardings in response to a CHANNEL_OPEN, we need to be able to start with ready=true. This commit reverts 09954a87c24e84dac133a9c29ffaef45f145eeca and its followup fix 12aa06ccc98cf8a912eb2ea54f02d234f2f8c173, and simplifies the agent_connect system down to a single trivial function that makes a Socket given a Plug. --- portfwd.c | 65 +++++++++++++---------------------------- putty.h | 11 ++----- sesschan.c | 4 +-- ssh.h | 5 +--- ssh1connection-client.c | 19 ++++++------ ssh2connection-client.c | 33 +++++++++------------ unix/uxagentc.c | 24 ++------------- windows/winpgntc.c | 25 ++++------------ 8 files changed, 56 insertions(+), 130 deletions(-) diff --git a/portfwd.c b/portfwd.c index 2f3ee6e5..c910e539 100644 --- a/portfwd.c +++ b/portfwd.c @@ -468,7 +468,7 @@ static const struct ChannelVtable PortForwarding_channelvt = { chan_no_request_response, }; -Channel *portfwd_raw_new(ConnectionLayer *cl, Plug **plug) +Channel *portfwd_raw_new(ConnectionLayer *cl, Plug **plug, bool start_ready) { struct PortForwarding *pf; @@ -482,7 +482,7 @@ Channel *portfwd_raw_new(ConnectionLayer *cl, Plug **plug) pf->cl = cl; pf->input_wanted = true; - pf->ready = false; + pf->ready = start_ready; pf->socks_state = SOCKS_NONE; pf->hostname = NULL; @@ -523,7 +523,7 @@ static int pfl_accepting(Plug *p, accept_fn_t constructor, accept_ctx_t ctx) Socket *s; const char *err; - chan = portfwd_raw_new(pl->cl, &plug); + chan = portfwd_raw_new(pl->cl, &plug, false); s = constructor(ctx, plug); if ((err = sk_socket_error(s)) != NULL) { portfwd_raw_free(chan); @@ -1120,20 +1120,6 @@ bool portfwdmgr_unlisten(PortFwdManager *mgr, const char *host, int port) return true; } -struct portfwdmgr_connect_ctx { - SockAddr *addr; - int port; - char *canonical_hostname; - Conf *conf; -}; -static Socket *portfwdmgr_connect_helper(void *vctx, Plug *plug) -{ - struct portfwdmgr_connect_ctx *ctx = (struct portfwdmgr_connect_ctx *)vctx; - return new_connection(sk_addr_dup(ctx->addr), ctx->canonical_hostname, - ctx->port, false, true, false, false, plug, - ctx->conf); -} - /* * Called when receiving a PORT OPEN from the server to make a * connection to a destination host. @@ -1145,39 +1131,26 @@ char *portfwdmgr_connect(PortFwdManager *mgr, Channel **chan_ret, char *hostname, int port, SshChannel *c, int addressfamily) { - struct portfwdmgr_connect_ctx ctx[1]; - const char *err_retd; - char *err_toret; + SockAddr *addr; + const char *err; + char *dummy_realhost = NULL; + struct PortForwarding *pf; /* * Try to find host. */ - ctx->addr = name_lookup(hostname, port, &ctx->canonical_hostname, - mgr->conf, addressfamily, NULL, NULL); - if ((err_retd = sk_addr_error(ctx->addr)) != NULL) { - err_toret = dupstr(err_retd); - goto out; + addr = name_lookup(hostname, port, &dummy_realhost, mgr->conf, + addressfamily, NULL, NULL); + if ((err = sk_addr_error(addr)) != NULL) { + char *err_ret = dupstr(err); + sk_addr_free(addr); + sfree(dummy_realhost); + return err_ret; } - ctx->conf = mgr->conf; - ctx->port = port; - - err_toret = portfwdmgr_connect_socket( - mgr, chan_ret, portfwdmgr_connect_helper, ctx, c); - - out: - sk_addr_free(ctx->addr); - sfree(ctx->canonical_hostname); - return err_toret; -} - -char *portfwdmgr_connect_socket(PortFwdManager *mgr, Channel **chan_ret, - Socket *(*connect)(void *, Plug *), void *ctx, - SshChannel *c) -{ - struct PortForwarding *pf; - const char *err; - + /* + * Open socket. + */ pf = new_portfwd_state(); *chan_ret = &pf->chan; pf->plug.vt = &PortForwarding_plugvt; @@ -1189,7 +1162,9 @@ char *portfwdmgr_connect_socket(PortFwdManager *mgr, Channel **chan_ret, pf->cl = mgr->cl; pf->socks_state = SOCKS_NONE; - pf->s = connect(ctx, &pf->plug); + pf->s = new_connection(addr, dummy_realhost, port, + false, true, false, false, &pf->plug, mgr->conf); + sfree(dummy_realhost); if ((err = sk_socket_error(pf->s)) != NULL) { char *err_ret = dupstr(err); sk_close(pf->s); diff --git a/putty.h b/putty.h index f1769d15..0c4816cb 100644 --- a/putty.h +++ b/putty.h @@ -1889,15 +1889,8 @@ void agent_cancel_query(agent_pending_query *); void agent_query_synchronous(strbuf *in, void **out, int *outlen); bool agent_exists(void); -/* For stream-oriented agent connections, if available: agent_connect - * is a callback for use with portfwdmgr_connect_socket, and the - * context structure it requires is created and freed by the next two - * functions. agent_get_connect_ctx may return NULL if no streaming - * agent connection is available at all on this platform. */ -typedef struct agent_connect_ctx agent_connect_ctx; -Socket *agent_connect(void *vctx, Plug *plug); -agent_connect_ctx *agent_get_connect_ctx(void); -void agent_free_connect_ctx(agent_connect_ctx *ctx); +/* For stream-oriented agent connections, if available. */ +Socket *agent_connect(Plug *plug); /* * Exports from wildcard.c diff --git a/sesschan.c b/sesschan.c index f9ea1418..5006423c 100644 --- a/sesschan.c +++ b/sesschan.c @@ -365,7 +365,7 @@ static int xfwd_accepting(Plug *p, accept_fn_t constructor, accept_ctx_t ctx) SocketPeerInfo *pi; const char *err; - chan = portfwd_raw_new(sess->c->cl, &plug); + chan = portfwd_raw_new(sess->c->cl, &plug, false); s = constructor(ctx, plug); if ((err = sk_socket_error(s)) != NULL) { portfwd_raw_free(chan); @@ -441,7 +441,7 @@ static int agentfwd_accepting( Socket *s; const char *err; - chan = portfwd_raw_new(sess->c->cl, &plug); + chan = portfwd_raw_new(sess->c->cl, &plug, false); s = constructor(ctx, plug); if ((err = sk_socket_error(s)) != NULL) { portfwd_raw_free(chan); diff --git a/ssh.h b/ssh.h index db189892..193116cb 100644 --- a/ssh.h +++ b/ssh.h @@ -380,13 +380,10 @@ void portfwdmgr_close_all(PortFwdManager *mgr); char *portfwdmgr_connect(PortFwdManager *mgr, Channel **chan_ret, char *hostname, int port, SshChannel *c, int addressfamily); -char *portfwdmgr_connect_socket(PortFwdManager *mgr, Channel **chan_ret, - Socket *(*connect)(void *, Plug *), void *ctx, - SshChannel *c); bool portfwdmgr_listen(PortFwdManager *mgr, const char *host, int port, const char *keyhost, int keyport, Conf *conf); bool portfwdmgr_unlisten(PortFwdManager *mgr, const char *host, int port); -Channel *portfwd_raw_new(ConnectionLayer *cl, Plug **plug); +Channel *portfwd_raw_new(ConnectionLayer *cl, Plug **plug, bool start_ready); void portfwd_raw_free(Channel *pfchan); void portfwd_raw_setup(Channel *pfchan, Socket *s, SshChannel *sc); diff --git a/ssh1connection-client.c b/ssh1connection-client.c index f209056e..59fe641f 100644 --- a/ssh1connection-client.c +++ b/ssh1connection-client.c @@ -168,16 +168,15 @@ bool ssh1_handle_direction_specific_packet( * agent and set up an ordinary port-forwarding type * channel over it. */ - agent_connect_ctx *ctx = agent_get_connect_ctx(); - bool got_stream_connection = false; - if (ctx) { - char *err = portfwdmgr_connect_socket( - s->portfwdmgr, &c->chan, agent_connect, ctx, &c->sc); - agent_free_connect_ctx(ctx); - if (err == NULL) - got_stream_connection = true; - } - if (!got_stream_connection) { + Plug *plug; + Channel *ch = portfwd_raw_new(&s->cl, &plug, true); + Socket *skt = agent_connect(plug); + if (!sk_socket_error(skt)) { + portfwd_raw_setup(ch, skt, &c->sc); + c->chan = ch; + } else { + portfwd_raw_free(ch); + /* * Otherwise, fall back to the old-fashioned system of * parsing the forwarded data stream ourselves for diff --git a/ssh2connection-client.c b/ssh2connection-client.c index 96302bf6..c355e475 100644 --- a/ssh2connection-client.c +++ b/ssh2connection-client.c @@ -99,27 +99,22 @@ static ChanopenResult chan_open_auth_agent( * If possible, make a stream-oriented connection to the agent and * set up an ordinary port-forwarding type channel over it. */ - agent_connect_ctx *ctx = agent_get_connect_ctx(); - if (ctx) { - Channel *ch; - char *err = portfwdmgr_connect_socket( - s->portfwdmgr, &ch, agent_connect, ctx, sc); - agent_free_connect_ctx(ctx); + Plug *plug; + Channel *ch = portfwd_raw_new(&s->cl, &plug, true); + Socket *skt = agent_connect(plug); - if (err == NULL) { - CHANOPEN_RETURN_SUCCESS(ch); - } else { - sfree(err); - /* now continue to the fallback case below */ - } + if (!sk_socket_error(skt)) { + portfwd_raw_setup(ch, skt, sc); + CHANOPEN_RETURN_SUCCESS(ch); + } else { + portfwd_raw_free(ch); + /* + * Otherwise, fall back to the old-fashioned system of parsing the + * forwarded data stream ourselves for message boundaries, and + * passing each individual message to the one-off agent_query(). + */ + CHANOPEN_RETURN_SUCCESS(agentf_new(sc)); } - - /* - * Otherwise, fall back to the old-fashioned system of parsing the - * forwarded data stream ourselves for message boundaries, and - * passing each individual message to the one-off agent_query(). - */ - CHANOPEN_RETURN_SUCCESS(agentf_new(sc)); } ChanopenResult ssh2_connection_parse_channel_open( diff --git a/unix/uxagentc.c b/unix/uxagentc.c index cc6063fd..e4d671d2 100644 --- a/unix/uxagentc.c +++ b/unix/uxagentc.c @@ -130,30 +130,12 @@ static const char *agent_socket_path(void) return getenv("SSH_AUTH_SOCK"); } -struct agent_connect_ctx { - SockAddr *addr; -}; - -Socket *agent_connect(void *vctx, Plug *plug) -{ - agent_connect_ctx *ctx = (agent_connect_ctx *)vctx; - return sk_new(sk_addr_dup(ctx->addr), 0, false, false, false, false, plug); -} - -agent_connect_ctx *agent_get_connect_ctx(void) +Socket *agent_connect(Plug *plug) { const char *path = agent_socket_path(); if (!path) - return NULL; - agent_connect_ctx *ctx = snew(agent_connect_ctx); - ctx->addr = unix_sock_addr(path); - return ctx; -} - -void agent_free_connect_ctx(agent_connect_ctx *ctx) -{ - sk_addr_free(ctx->addr); - sfree(ctx); + return new_error_socket_fmt(plug, "SSH_AUTH_SOCK not set"); + return sk_new(unix_sock_addr(path), 0, false, false, false, false, plug); } agent_pending_query *agent_query( diff --git a/windows/winpgntc.c b/windows/winpgntc.c index 442d3730..2bd2f154 100644 --- a/windows/winpgntc.c +++ b/windows/winpgntc.c @@ -142,27 +142,12 @@ char *agent_named_pipe_name(void) return pipename; } -struct agent_connect_ctx { - char *pipename; -}; - -Socket *agent_connect(void *vctx, Plug *plug) +Socket *agent_connect(Plug *plug) { - agent_connect_ctx *ctx = (agent_connect_ctx *)vctx; - return new_named_pipe_client(ctx->pipename, plug); -} - -agent_connect_ctx *agent_get_connect_ctx(void) -{ - agent_connect_ctx *ctx = snew(agent_connect_ctx); - ctx->pipename = agent_named_pipe_name(); - return ctx; -} - -void agent_free_connect_ctx(agent_connect_ctx *ctx) -{ - sfree(ctx->pipename); - sfree(ctx); + char *pipename = agent_named_pipe_name(); + Socket *s = new_named_pipe_client(pipename, plug); + sfree(pipename); + return s; } static bool named_pipe_agent_exists(void)