- A nice cleaner structure for tcp (acceptor) forwarding.

- still a checkpoint-ish commit
- sorted out listening on localhost only

--HG--
extra : convert_revision : c030ac0a3950dba81f2324e2ba9d4b77fc8f8149

cli-tcpfwd.c

@@ -12,22 +12,26 @@ static const struct ChanType cli_chan_tcplocal = {
 	NULL
 };
 
+void setup_localtcp() {
 
+	qv
 
+}
 
-static int cli_localtcp(char* port) {
+static int cli_localtcp(unsigned int listenport, const char* remoteaddr,
+		unsigned int remoteport) {
 
 	struct TCPListener* tcpinfo = NULL;
 
 	tcpinfo = (struct TCPListener*)m_malloc(sizeof(struct TCPListener*));
-	tcpinfo->addr = NULL;
-	tcpinfo->port = port;
+	tcpinfo->sendaddr = remoteaddr;
+	tcpinfo->sendport = remoteport;
+	tcpinfo->listenport = listenport;
 	tcpinfo->chantype = &cli_chan_tcplocal;
 
 	ret = listen_tcpfwd(tcpinfo);
 
 	if (ret == DROPBEAR_FAILURE) {
-		DROPBEAR_LOG(LOG_WARNING, "Failed to listen on port %s", port);
 		m_free(tcpinfo);
 	}
 	return ret;

dbutil.c

@@ -114,10 +114,8 @@ void dropbear_trace(const char* format, ...) {
 #endif /* DEBUG_TRACE */
 
 /* Listen on address:port. Unless address is NULL, in which case listen on
- * everything (ie 0.0.0.0, or ::1 - note that this is IPv? agnostic. Linux is
- * broken with respect to listening to v6 or v4, so the addresses you get when
- * people connect will be wrong. It doesn't break things, just looks quite
- * ugly. Returns the number of sockets bound on success, or -1 on failure. On
+ * everything. If called with address == "", we'll listen on localhost/loopback.
+ * Returns the number of sockets bound on success, or -1 on failure. On
  * failure, if errstring wasn't NULL, it'll be a newly malloced error
  * string.*/
 int dropbear_listen(const char* address, const char* port,
@@ -135,7 +133,14 @@ int dropbear_listen(const char* address, const char* port,
 	memset(&hints, 0, sizeof(hints));
 	hints.ai_family = AF_UNSPEC; /* TODO: let them flag v4 only etc */
 	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_flags = AI_PASSIVE;
+
+	if (address && address[0] == '\0') {
+		TRACE(("dropbear_listen: local loopback"));
+		address = NULL;
+	} else {
+		TRACE(("dropbear_listen: not local loopback"));
+		hints.ai_flags = AI_PASSIVE;
+	}
 	err = getaddrinfo(address, port, &hints, &res0);
 
 	if (err) {

listener.c

@@ -42,7 +42,7 @@ void handle_listeners(fd_set * readfds) {
 			for (j = 0; j < listener->nsocks; j++) {
 				sock = listener->socks[j];
 				if (FD_ISSET(sock, readfds)) {
-					listener->accepter(listener, sock);
+					listener->acceptor(listener, sock);
 				}
 			}
 		}
@@ -50,11 +50,11 @@ void handle_listeners(fd_set * readfds) {
 }
 
 
-/* accepter(int fd, void* typedata) is a function to accept connections, 
+/* acceptor(int fd, void* typedata) is a function to accept connections, 
  * cleanup(void* typedata) happens when cleaning up */
 struct Listener* new_listener(int socks[], unsigned int nsocks,
 		int type, void* typedata, 
-		void (*accepter)(struct Listener*, int sock), 
+		void (*acceptor)(struct Listener* listener, int sock), 
 		void (*cleanup)(struct Listener*)) {
 
 	unsigned int i, j;
@@ -99,7 +99,7 @@ struct Listener* new_listener(int socks[], unsigned int nsocks,
 	newlisten->typedata = typedata;
 	newlisten->nsocks = nsocks;
 	memcpy(newlisten->socks, socks, nsocks * sizeof(int));
-	newlisten->accepter = accepter;
+	newlisten->acceptor = acceptor;
 	newlisten->cleanup = cleanup;
 
 	ses.listeners[i] = newlisten;

listener.h

@@ -11,7 +11,7 @@ struct Listener {
 
 	int index; /* index in the array of listeners */
 
-	void (*accepter)(struct Listener*, int sock);
+	void (*acceptor)(struct Listener*, int sock);
 	void (*cleanup)(struct Listener*);
 
 	int type; /* CHANNEL_ID_X11, CHANNEL_ID_AGENT, 
@@ -28,7 +28,7 @@ void set_listener_fds(fd_set * readfds);
 
 struct Listener* new_listener(int socks[], unsigned int nsocks, 
 		int type, void* typedata, 
-		void (*accepter)(struct Listener*, int sock), 
+		void (*acceptor)(struct Listener* listener, int sock), 
 		void (*cleanup)(struct Listener*));
 
 struct Listener * get_listener(int type, void* typedata,

svr-main.c

@@ -188,7 +188,8 @@ int main(int argc, char ** argv)
 
 			/* child connection XXX - ip6 stuff here */
 			remoteaddrlen = sizeof(remoteaddr);
-			childsock = accept(listensocks[i], &remoteaddr, &remoteaddrlen);
+			childsock = accept(listensocks[i], 
+					(struct sockaddr*)&remoteaddr, &remoteaddrlen);
 
 			if (childsock < 0) {
 				/* accept failed */
@@ -295,7 +296,7 @@ static void sigintterm_handler(int fish) {
 static int listensockets(int *sock, int sockcount, int *maxfd) {
 	
 	unsigned int i;
-	char portstring[6];
+	char portstring[NI_MAXSERV];
 	char* errstring = NULL;
 	unsigned int sockpos = 0;
 	int nsock;

svr-tcpfwd.c

@@ -104,9 +104,9 @@ static int matchtcp(void* typedata1, void* typedata2) {
 	const struct TCPListener *info1 = (struct TCPListener*)typedata1;
 	const struct TCPListener *info2 = (struct TCPListener*)typedata2;
 
-	return (info1->port == info2->port)
+	return (info1->sendport == info2->sendport)
 			&& (info1->chantype == info2->chantype)
-			&& (strcmp(info1->addr, info2->addr) == 0);
+			&& (strcmp(info1->sendaddr, info2->sendaddr) == 0);
 }
 
 static int svr_cancelremotetcp() {
@@ -128,8 +128,8 @@ static int svr_cancelremotetcp() {
 
 	port = buf_getint(ses.payload);
 
-	tcpinfo.addr = bindaddr;
-	tcpinfo.port = port;
+	tcpinfo.sendaddr = bindaddr;
+	tcpinfo.sendport = port;
 	listener = get_listener(CHANNEL_ID_TCPFORWARDED, &tcpinfo, matchtcp);
 	if (listener) {
 		remove_listener( listener );
@@ -152,6 +152,7 @@ static int svr_remotetcpreq() {
 
 	TRACE(("enter remotetcpreq"));
 
+	/* NOTE: at this stage, we ignore bindaddr. see below and listen_tcpfwd */
 	bindaddr = buf_getstring(ses.payload, &addrlen);
 	if (addrlen > MAX_IP_LEN) {
 		TRACE(("addr len too long: %d", addrlen));
@@ -176,18 +177,20 @@ static int svr_remotetcpreq() {
 	}
 
 	tcpinfo = (struct TCPListener*)m_malloc(sizeof(struct TCPListener));
-	tcpinfo->addr = bindaddr;
-	tcpinfo->port = port;
-	tcpinfo->localport = -1;
+	tcpinfo->sendaddr = bindaddr;
+	TRACE(("sendport = %d", port));
+	tcpinfo->sendport = port;
 	tcpinfo->chantype = &svr_chan_tcpremote;
 
+	/* Note: bindaddr is actually ignored by listen_tcpfwd, since
+	 * we only want to bind to localhost */
 	ret = listen_tcpfwd(tcpinfo);
 
 out:
 	if (ret == DROPBEAR_FAILURE) {
 		/* we only free it if a listener wasn't created, since the listener
 		 * has to remember it if it's to be cancelled */
-		m_free(tcpinfo->addr);
+		m_free(tcpinfo->sendaddr);
 		m_free(tcpinfo);
 	}
 	TRACE(("leave remotetcpreq"));

tcp-accept.c

@@ -10,7 +10,16 @@
 
 #ifndef DISABLE_TCP_ACCEPT
 
-static void accept_tcp(struct Listener *listener, int sock) {
+
+static void cleanup_tcp(struct Listener *listener) {
+
+	struct TCPListener *tcpinfo = (struct TCPListener*)(listener->typedata);
+
+	m_free(tcpinfo->sendaddr);
+	m_free(tcpinfo);
+}
+
+static void tcp_acceptor(struct Listener *listener, int sock) {
 
 	int fd;
 	struct sockaddr_storage addr;
@@ -33,10 +42,12 @@ static void accept_tcp(struct Listener *listener, int sock) {
 
 	if (send_msg_channel_open_init(fd, tcpinfo->chantype) == DROPBEAR_SUCCESS) {
 
-		buf_putstring(ses.writepayload, tcpinfo->addr, strlen(tcpinfo->addr));
-		buf_putint(ses.writepayload, tcpinfo->port);
+		buf_putstring(ses.writepayload, tcpinfo->sendaddr, 
+				strlen(tcpinfo->sendaddr));
+		buf_putint(ses.writepayload, tcpinfo->sendport);
 		buf_putstring(ses.writepayload, ipstring, strlen(ipstring));
 		buf_putint(ses.writepayload, atol(portstring));
+
 		encrypt_packet();
 
 	} else {
@@ -45,35 +56,33 @@ static void accept_tcp(struct Listener *listener, int sock) {
 	}
 }
 
-static void cleanup_tcp(struct Listener *listener) {
-
-	struct TCPListener *tcpinfo = (struct TCPListener*)(listener->typedata);
-
-	m_free(tcpinfo->addr);
-	m_free(tcpinfo);
-}
-
-
 int listen_tcpfwd(struct TCPListener* tcpinfo) {
 
-	char portstring[6]; /* "65535\0" */
+	char portstring[NI_MAXSERV];
 	int socks[DROPBEAR_MAX_SOCKS];
 	struct Listener *listener = NULL;
 	int nsocks;
+	char* errstring = NULL;
 
 	TRACE(("enter listen_tcpfwd"));
 
 	/* first we try to bind, so don't need to do so much cleanup on failure */
-	snprintf(portstring, sizeof(portstring), "%d", tcpinfo->port);
-	nsocks = dropbear_listen(tcpinfo->addr, portstring, socks, 
-			DROPBEAR_MAX_SOCKS, NULL, &ses.maxfd);
+	snprintf(portstring, sizeof(portstring), "%d", tcpinfo->sendport);
+
+	/* XXX Note: we're just listening on localhost, no matter what they tell
+	 * us. If someone wants to make it listen otherways, then change
+	 * the "" argument. but that requires UI changes too */
+	nsocks = dropbear_listen("", portstring, socks, 
+			DROPBEAR_MAX_SOCKS, &errstring, &ses.maxfd);
 	if (nsocks < 0) {
+		dropbear_log(LOG_INFO, "TCP forward failed: %s", errstring);
+		m_free(errstring);
 		TRACE(("leave listen_tcpfwd: dropbear_listen failed"));
 		return DROPBEAR_FAILURE;
 	}
 
 	listener = new_listener(socks, nsocks, CHANNEL_ID_TCPFORWARDED, tcpinfo, 
-			accept_tcp, cleanup_tcp);
+			tcp_acceptor, cleanup_tcp);
 
 	if (listener == NULL) {
 		m_free(tcpinfo);

tcp-accept.h

@@ -3,12 +3,18 @@
 
 struct TCPListener {
 
-	/* Local ones */
-	unsigned char *localaddr; /* Can be NULL */
-	unsigned int localport;
-	/* Remote ones: */
-	unsigned char *remoteaddr;
-	unsigned int remoteport;
+	/* sendaddr/sendport are what we send in the channel init request. For a 
+	 * forwarded-tcpip request, it's the addr/port we were binding to.
+	 * For a direct-tcpip request, it's the addr/port we want the other
+	 * end to connect to */
+	
+	unsigned char *sendaddr;
+	unsigned int sendport;
+
+	/* This is for direct-tcpip (ie the client listening), and specifies the
+	 * port to listen on. Is unspecified for the server */
+	unsigned int listenport;
+
 	const struct ChanType *chantype;
 
 };

tcp-connect.c

@@ -15,7 +15,7 @@ int newtcpdirect(struct Channel * channel) {
 	unsigned int destport;
 	unsigned char* orighost = NULL;
 	unsigned int origport;
-	char portstring[6];
+	char portstring[NI_MAXSERV];
 	int sock;
 	int len;
 	int ret = DROPBEAR_FAILURE;