connectd: don't try to set TCP_CORK on websocket pipe.

Most of this is piping the flag through so we know it's a websocket!

Reported-by: @ShahanaFarooqui
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

connectd/connectd.c

@@ -31,7 +31,6 @@
 #include <connectd/connectd.h>
 #include <connectd/connectd_gossipd_wiregen.h>
 #include <connectd/connectd_wiregen.h>
-#include <connectd/handshake.h>
 #include <connectd/multiplex.h>
 #include <connectd/netaddress.h>
 #include <connectd/onion_message.h>
@@ -232,6 +231,7 @@ static struct peer *new_peer(struct daemon *daemon,
 			     const struct node_id *id,
 			     const struct crypto_state *cs,
 			     const u8 *their_features,
+			     enum is_websocket is_websocket,
 			     struct io_conn *conn STEALS,
 			     int *fd_for_subd)
 {
@@ -248,6 +248,7 @@ static struct peer *new_peer(struct daemon *daemon,
 	peer->draining = false;
 	peer->peer_outq = msg_queue_new(peer, false);
 	peer->last_recv_time = time_now();
+	peer->is_websocket = is_websocket;
 
 #if DEVELOPER
 	peer->dev_writes_enabled = NULL;
@@ -273,6 +274,7 @@ struct io_plan *peer_connected(struct io_conn *conn,
 			       const struct wireaddr *remote_addr,
 			       struct crypto_state *cs,
 			       const u8 *their_features TAKES,
+			       enum is_websocket is_websocket,
 			       bool incoming)
 {
 	u8 *msg;
@@ -333,7 +335,7 @@ struct io_plan *peer_connected(struct io_conn *conn,
 			      conn, find_connecting(daemon, id)->conn);
 
 	/* This contains the per-peer state info; gossipd fills in pps->gs */
-	peer = new_peer(daemon, id, cs, their_features, conn, &subd_fd);
+	peer = new_peer(daemon, id, cs, their_features, is_websocket, conn, &subd_fd);
 	/* Only takes over conn if it succeeds. */
 	if (!peer)
 		return io_close(conn);
@@ -371,13 +373,14 @@ static struct io_plan *handshake_in_success(struct io_conn *conn,
 					    const struct wireaddr_internal *addr,
 					    struct crypto_state *cs,
 					    struct oneshot *timeout,
+					    enum is_websocket is_websocket,
 					    struct daemon *daemon)
 {
 	struct node_id id;
 	node_id_from_pubkey(&id, id_key);
 	status_peer_debug(&id, "Connect IN");
 	return peer_exchange_initmsg(conn, daemon, daemon->our_features,
-				     cs, &id, addr, timeout, true);
+				     cs, &id, addr, timeout, is_websocket, true);
 }
 
 /*~ If the timer goes off, we simply free everything, which hangs up. */
@@ -429,6 +432,7 @@ static bool get_remote_address(struct io_conn *conn,
 struct conn_in {
 	struct wireaddr_internal addr;
 	struct daemon *daemon;
+	enum is_websocket is_websocket;
 };
 
 /*~ Once we've got a connection in, we set it up here (whether it's via the
@@ -451,6 +455,7 @@ static struct io_plan *conn_in(struct io_conn *conn,
 	 * leaked */
 	return responder_handshake(notleak(conn), &daemon->mykey,
 				   &conn_in_arg->addr, timeout,
+				   conn_in_arg->is_websocket,
 				   handshake_in_success, daemon);
 }
 
@@ -465,6 +470,7 @@ static struct io_plan *connection_in(struct io_conn *conn,
 		return io_close(conn);
 
 	conn_in_arg.daemon = daemon;
+	conn_in_arg.is_websocket = false;
 	return conn_in(conn, &conn_in_arg);
 }
 
@@ -545,6 +551,7 @@ static struct io_plan *websocket_connection_in(struct io_conn *conn,
 
 	/* New connection actually talks to proxy process. */
 	conn_in_arg.daemon = daemon;
+	conn_in_arg.is_websocket = true;
 	io_new_conn(tal_parent(conn), childmsg[0], conn_in, &conn_in_arg);
 
 	/* Abandon original (doesn't close since child has dup'd fd) */
@@ -568,6 +575,7 @@ static struct io_plan *handshake_out_success(struct io_conn *conn,
 					     const struct wireaddr_internal *addr,
 					     struct crypto_state *cs,
 					     struct oneshot *timeout,
+					     enum is_websocket is_websocket,
 					     struct connecting *connect)
 {
 	struct node_id id;
@@ -577,7 +585,7 @@ static struct io_plan *handshake_out_success(struct io_conn *conn,
 	status_peer_debug(&id, "Connect OUT");
 	return peer_exchange_initmsg(conn, connect->daemon,
 				     connect->daemon->our_features,
-				     cs, &id, addr, timeout, false);
+				     cs, &id, addr, timeout, is_websocket, false);
 }
 
 struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect)
@@ -602,7 +610,7 @@ struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect)
 	connect->connstate = "Cryptographic handshake";
 	return initiator_handshake(conn, &connect->daemon->mykey, &outkey,
 				   &connect->addrs[connect->addrnum],
-				   timeout, handshake_out_success, connect);
+				   timeout, NORMAL_SOCKET, handshake_out_success, connect);
 }
 
 /*~ When we've exhausted all addresses without success, we come here.
@@ -931,15 +939,6 @@ next:
 	try_connect_one_addr(connect);
 }
 
-/*~ Sometimes it's nice to have an explicit enum instead of a bool to make
- * arguments clearer: it kind of hacks around C's lack of naming formal
- * arguments in callers (e.g. in Python we'd simply call func(websocket=False)).
- */
-enum is_websocket {
-	NORMAL_SOCKET,
-	WEBSOCKET,
-};
-
 /*~ connectd is responsible for incoming connections, but it's the process of
  * setting up the listening ports which gives us information we need for startup
  * (such as our own address).  So we perform setup in two phases: first we bind

connectd/connectd.h

@@ -10,6 +10,7 @@
 #include <common/node_id.h>
 #include <common/pseudorand.h>
 #include <common/wireaddr.h>
+#include <connectd/handshake.h>
 
 struct io_conn;
 struct connecting;
@@ -45,6 +46,9 @@ struct peer {
 	/* Main daemon */
 	struct daemon *daemon;
 
+	/* Are we connected via a websocket? */
+	enum is_websocket is_websocket;
+
 	/* The pubkey of the node */
 	struct node_id id;
 	/* Counters and keys for symmetric crypto */
@@ -214,6 +218,7 @@ struct io_plan *peer_connected(struct io_conn *conn,
 			       const struct wireaddr *remote_addr,
 			       struct crypto_state *cs,
 			       const u8 *their_features TAKES,
+			       enum is_websocket is_websocket,
 			       bool incoming);
 
 /* Removes peer from hash table, tells gossipd and lightningd. */

connectd/handshake.c

@@ -176,12 +176,16 @@ struct handshake {
 	/* Timeout timer if we take too long. */
 	struct oneshot *timeout;
 
+	/* Are we connected via a websocket? */
+	enum is_websocket is_websocket;
+
 	/* Function to call once handshake complete. */
 	struct io_plan *(*cb)(struct io_conn *conn,
 			      const struct pubkey *their_id,
 			      const struct wireaddr_internal *wireaddr,
 			      struct crypto_state *cs,
 			      struct oneshot *timeout,
+			      enum is_websocket is_websocket,
 			      void *cbarg);
 	void *cbarg;
 };
@@ -353,11 +357,13 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn,
 			      const struct wireaddr_internal *addr,
 			      struct crypto_state *cs,
 			      struct oneshot *timeout,
+			      enum is_websocket is_websocket,
 			      void *cbarg);
 	void *cbarg;
 	struct pubkey their_id;
 	struct wireaddr_internal addr;
 	struct oneshot *timeout;
+	enum is_websocket is_websocket;
 
 	/* BOLT #8:
 	 *
@@ -384,9 +390,10 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn,
 	their_id = h->their_id;
 	addr = h->addr;
 	timeout = h->timeout;
+	is_websocket = h->is_websocket;
 
 	tal_free(h);
-	return cb(conn, &their_id, &addr, &cs, timeout, cbarg);
+	return cb(conn, &their_id, &addr, &cs, timeout, is_websocket, cbarg);
 }
 
 static struct handshake *new_handshake(const tal_t *ctx,
@@ -964,11 +971,13 @@ struct io_plan *responder_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct wireaddr_internal *addr,
 				     struct oneshot *timeout,
+				     enum is_websocket is_websocket,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
 							   struct oneshot *,
+							   enum is_websocket,
 							   void *cbarg),
 				     void *cbarg)
 {
@@ -980,6 +989,7 @@ struct io_plan *responder_handshake_(struct io_conn *conn,
 	h->cbarg = cbarg;
 	h->cb = cb;
 	h->timeout = timeout;
+	h->is_websocket = is_websocket;
 
 	return act_one_responder(conn, h);
 }
@@ -989,11 +999,13 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 				     const struct pubkey *their_id,
 				     const struct wireaddr_internal *addr,
 				     struct oneshot *timeout,
+				     enum is_websocket is_websocket,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
 							   struct oneshot *timeout,
+							   enum is_websocket is_websocket,
 							   void *cbarg),
 				     void *cbarg)
 {
@@ -1005,6 +1017,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 	h->addr = *addr;
 	h->cbarg = cbarg;
 	h->cb = cb;
+	h->is_websocket = is_websocket;
 	h->timeout = timeout;
 
 	return act_one_initiator(conn, h);

connectd/handshake.h

@@ -8,15 +8,25 @@ struct wireaddr_internal;
 struct pubkey;
 struct oneshot;
 
-#define initiator_handshake(conn, my_id, their_id, addr, timeout, cb, cbarg) \
+/*~ Sometimes it's nice to have an explicit enum instead of a bool to make
-	initiator_handshake_((conn), (my_id), (their_id), (addr), (timeout), \
+ * arguments clearer: it kind of hacks around C's lack of naming formal
+ * arguments in callers (e.g. in Python we'd simply call func(websocket=False)).
+ */
+enum is_websocket {
+	NORMAL_SOCKET,
+	WEBSOCKET,
+};
+
+#define initiator_handshake(conn, my_id, their_id, addr, timeout, is_ws, cb, cbarg) \
+	initiator_handshake_((conn), (my_id), (their_id), (addr), (timeout), (is_ws), \
 			     typesafe_cb_preargs(struct io_plan *, void *, \
 						 (cb), (cbarg),		\
 						 struct io_conn *,	\
 						 const struct pubkey *,	\
 						 const struct wireaddr_internal *, \
 						 struct crypto_state *, \
-						 struct oneshot *),	\
+						 struct oneshot *,	\
+						 enum is_websocket),	\
 			     (cbarg))
 
 
@@ -25,35 +35,40 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 				     const struct pubkey *their_id,
 				     const struct wireaddr_internal *addr,
 				     struct oneshot *timeout,
+				     enum is_websocket is_websocket,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
 							   struct oneshot *timeout,
+							   enum is_websocket,
 							   void *cbarg),
 				     void *cbarg);
 
 
-#define responder_handshake(conn, my_id, addr, timeout, cb, cbarg)	\
+#define responder_handshake(conn, my_id, addr, timeout, is_ws, cb, cbarg)	\
-	responder_handshake_((conn), (my_id), (addr), (timeout),	\
+	responder_handshake_((conn), (my_id), (addr), (timeout), (is_ws), \
 			     typesafe_cb_preargs(struct io_plan *, void *, \
 						 (cb), (cbarg),		\
 						 struct io_conn *,	\
 						 const struct pubkey *,	\
 						 const struct wireaddr_internal *, \
 						 struct crypto_state *, \
-						 struct oneshot *),	\
+						 struct oneshot *,	\
+						 enum is_websocket),	\
 			     (cbarg))
 
 struct io_plan *responder_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct wireaddr_internal *addr,
 				     struct oneshot *timeout,
+				     enum is_websocket is_websocket,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
 							   struct oneshot *,
+							   enum is_websocket,
 							   void *cbarg),
 				     void *cbarg);
 #endif /* LIGHTNING_CONNECTD_HANDSHAKE_H */

connectd/multiplex.c

@@ -320,6 +320,11 @@ static void set_urgent_flag(struct peer *peer, bool urgent)
 	if (urgent == peer->urgent)
 		return;
 
+	/* FIXME: We can't do this on websockets, but we could signal our
+	 * websocket proxy via some magic message to do so! */
+	if (peer->is_websocket != NORMAL_SOCKET)
+		return;
+
 #ifdef TCP_CORK
 	opt = TCP_CORK;
 	optname = "TCP_CORK";

connectd/peer_exchange_initmsg.c

@@ -27,6 +27,9 @@ struct early_peer {
 	/* Buffer for reading/writing message. */
 	u8 *msg;
 
+	/* Are we connected via a websocket? */
+	enum is_websocket is_websocket;
+
 	bool incoming;
 };
 
@@ -137,6 +140,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn,
 			      remote_addr,
 			      &peer->cs,
 			      take(features),
+			      peer->is_websocket,
 			      peer->incoming);
 }
 
@@ -192,6 +196,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 				      const struct node_id *id,
 				      const struct wireaddr_internal *addr,
 				      struct oneshot *timeout,
+				      enum is_websocket is_websocket,
 				      bool incoming)
 {
 	/* If conn is closed, forget peer */
@@ -204,6 +209,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 	peer->addr = *addr;
 	peer->cs = *cs;
 	peer->incoming = incoming;
+	peer->is_websocket = is_websocket;
 
 	/* Attach timer to early peer, so it gets freed with it. */
 	notleak(tal_steal(peer, timeout));

connectd/peer_exchange_initmsg.h

@@ -18,6 +18,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 				      const struct node_id *id,
 				      const struct wireaddr_internal *addr,
 				      struct oneshot *timeout,
+				      enum is_websocket is_websocket,
 				      bool incoming);
 
 #endif /* LIGHTNING_CONNECTD_PEER_EXCHANGE_INITMSG_H */

connectd/test/run-initiator-success.c

@@ -285,6 +285,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED,
 			       const struct wireaddr_internal *addr UNUSED,
 			       struct crypto_state *cs,
 			       struct oneshot *timeout UNUSED,
+			       enum is_websocket is_websocket UNUSED,
 			       void *unused UNUSED)
 {
 	assert(pubkey_eq(them, &rs_pub));
@@ -327,7 +328,7 @@ int main(int argc, char *argv[])
 
 	dummy.itype = ADDR_INTERNAL_WIREADDR;
 	dummy.u.wireaddr.addrlen = 0;
-	initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, NULL, success, NULL);
+	initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, NULL, NORMAL_SOCKET, success, NULL);
 	/* Should not exit! */
 	abort();
 }

connectd/test/run-responder-success.c

@@ -284,6 +284,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED,
 			       const struct wireaddr_internal *addr UNUSED,
 			       struct crypto_state *cs,
 			       struct oneshot *timeout UNUSED,
+			       enum is_websocket is_websocket UNUSED,
 			       void *unused UNUSED)
 {
 	assert(secret_eq_str(&cs->sk, expect_sk));
@@ -321,7 +322,7 @@ int main(int argc, char *argv[])
 
 	dummy.itype = ADDR_INTERNAL_WIREADDR;
 	dummy.u.wireaddr.addrlen = 0;
-	responder_handshake((void *)tmpctx, &ls_pub, &dummy, NULL, success, NULL);
+	responder_handshake((void *)tmpctx, &ls_pub, &dummy, NULL, NORMAL_SOCKET, success, NULL);
 	/* Should not exit! */
 	abort();
 }

devtools/gossipwith.c

@@ -168,6 +168,7 @@ static struct io_plan *handshake_success(struct io_conn *conn,
 					 const struct wireaddr_internal *addr,
 					 struct crypto_state *cs,
 					 struct oneshot *timer,
+					 enum is_websocket is_websocket,
 					 char **args)
 {
 	int peer_fd = io_conn_fd(conn);
@@ -375,7 +376,7 @@ int main(int argc, char *argv[])
 	if (connect(conn->fd, ai->ai_addr, ai->ai_addrlen) != 0)
 		err(1, "Connecting to %s", at+1);
 
-	initiator_handshake(conn, &us, &them, &addr, NULL,
+	initiator_handshake(conn, &us, &them, &addr, NULL, NORMAL_SOCKET,
 			    handshake_success, argv+2);
 	exit(0);
 }