From 7a40e0262f2883bbf30d88cf3078f1911b432305 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 30 Jan 2018 11:14:08 +1030
Subject: [PATCH] closingd: handle unexpected messages better.

In particular, handle pings.  The rest is modelled on the channeld one,
but annoyingly different enough that it's hard to share code without
significant work.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 closingd/Makefile  |   1 +
 closingd/closing.c | 115 ++++++++++++++++++++++++++++++++++++---------
 2 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/closingd/Makefile b/closingd/Makefile
index 5e64e9f14..4b7f1e982 100644
--- a/closingd/Makefile
+++ b/closingd/Makefile
@@ -54,6 +54,7 @@ CLOSINGD_COMMON_OBJS :=				\
 	common/msg_queue.o			\
 	common/peer_failed.o			\
 	common/permute_tx.o			\
+	common/ping.o				\
 	common/status.o				\
 	common/subdaemon.o			\
 	common/type_to_string.o			\
diff --git a/closingd/closing.c b/closingd/closing.c
index 0fbee2bcc..f370aca62 100644
--- a/closingd/closing.c
+++ b/closingd/closing.c
@@ -1,15 +1,18 @@
 #include <bitcoin/script.h>
+#include <ccan/structeq/structeq.h>
 #include <closingd/gen_closing_wire.h>
 #include <common/close_tx.h>
 #include <common/crypto_sync.h>
 #include <common/derive_basepoints.h>
 #include <common/htlc.h>
 #include <common/peer_failed.h>
+#include <common/ping.h>
 #include <common/status.h>
 #include <common/subdaemon.h>
 #include <common/type_to_string.h>
 #include <common/utils.h>
 #include <common/version.h>
+#include <common/wire_error.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <stdio.h>
@@ -80,6 +83,92 @@ static u64 one_towards(u64 target, u64 value)
 	return value;
 }
 
+static void handle_ping(const u8 *msg,
+			struct crypto_state *cs,
+			const struct channel_id *our_channel_id)
+{
+	u8 *pong;
+
+	if (!check_ping_make_pong(msg, msg, &pong))
+		peer_failed(PEER_FD, cs, our_channel_id, "Bad ping");
+
+	status_trace("Got ping, sending %s", pong ?
+		     wire_type_name(fromwire_peektype(pong))
+		     : "nothing");
+
+	if (pong && !sync_crypto_write(cs, PEER_FD, take(pong)))
+		status_failed(STATUS_FAIL_PEER_IO,
+			      "Failed writing pong: %s", strerror(errno));
+}
+
+/* Handle random messages we might get, returning NULL if we handled it. */
+static u8 *read_peer_msg(const tal_t *ctx,
+			 struct crypto_state *cs,
+			 const struct channel_id *our_channel_id)
+{
+	u8 *msg;
+	struct channel_id channel_id;
+
+	msg = sync_crypto_read(ctx, cs, PEER_FD);
+	if (!msg)
+		status_failed(STATUS_FAIL_PEER_IO,
+			      "Failed reading from peer: %s", strerror(errno));
+
+	if (is_gossip_msg(msg)) {
+		/* Forward to gossip daemon */
+		wire_sync_write(GOSSIP_FD, take(msg));
+		return NULL;
+	}
+
+	if (fromwire_peektype(msg) == WIRE_PING) {
+		handle_ping(msg, cs, our_channel_id);
+		return tal_free(msg);
+	}
+
+	if (fromwire_peektype(msg) == WIRE_ERROR) {
+		struct channel_id chanid;
+		char *err = sanitize_error(msg, msg, &chanid);
+
+		/* BOLT #1:
+		 *
+		 * The channel is referred to by `channel_id`, unless
+		 * `channel_id` is 0 (i.e. all bytes are 0), in which
+		 * case it refers to all channels.
+		 * ...
+
+		 * The receiving node:
+		 *   - upon receiving `error`:
+		 *    - MUST fail the channel referred to by the error
+		 *       message.
+		 *  - if no existing channel is referred to by the
+		 *    message:
+		 *    - MUST ignore the message.
+		 */
+		if (channel_id_is_all(&chanid)
+		    || structeq(&chanid, our_channel_id)) {
+			status_failed(STATUS_FAIL_PEER_BAD,
+				      "Received ERROR %s", err);
+		}
+		return tal_free(msg);
+	}
+
+	/* They're talking about a different channel? */
+	if (extract_channel_id(msg, &channel_id)
+	    && !structeq(&channel_id, our_channel_id)) {
+		status_trace("Rejecting %s for unknown channel_id %s",
+			     wire_type_name(fromwire_peektype(msg)),
+			     type_to_string(msg, struct channel_id,
+					    &channel_id));
+		sync_crypto_write(cs, PEER_FD,
+				  take(towire_errorfmt(msg, &channel_id,
+						       "Multiple channels"
+						       " unsupported")));
+		return tal_free(msg);
+	}
+
+	return msg;
+}
+
 static void do_reconnect(struct crypto_state *cs,
 			 const struct channel_id *channel_id,
 			 const u64 next_index[NUM_SIDES],
@@ -108,17 +197,8 @@ static void do_reconnect(struct crypto_state *cs,
 		status_failed(STATUS_FAIL_PEER_IO,
 			      "Failed writing reestablish: %s", strerror(errno));
 
-again:
-	msg = sync_crypto_read(tmpctx, cs, PEER_FD);
-	if (!msg)
-		status_failed(STATUS_FAIL_PEER_IO,
-			      "Failed reading reestablish: %s", strerror(errno));
-
-	if (is_gossip_msg(msg)) {
-		if (!wire_sync_write(GOSSIP_FD, take(msg)))
-			status_failed(STATUS_FAIL_GOSSIP_IO, "Writing gossip");
-		goto again;
-	}
+	/* Wait for them to say something interesting */
+	while ((msg = read_peer_msg(tmpctx, cs, channel_id)) == NULL);
 
 	if (!fromwire_channel_reestablish(msg, NULL, &their_channel_id,
 					  &next_local_commitment_number,
@@ -263,17 +343,8 @@ int main(int argc, char *argv[])
 			break;
 
 	again:
-		msg = sync_crypto_read(tmpctx, &cs, PEER_FD);
-		if (!msg)
-			status_failed(STATUS_FAIL_PEER_IO, "Reading input");
-
-		/* We don't send gossip at this stage, but we can recv it */
-		if (is_gossip_msg(msg)) {
-			if (!wire_sync_write(GOSSIP_FD, take(msg)))
-				status_failed(STATUS_FAIL_GOSSIP_IO,
-					      "Writing gossip");
-			goto again;
-		}
+		/* Wait for them to say something interesting */
+		while ((msg = read_peer_msg(tmpctx, &cs, &channel_id)) == NULL);
 
 		/* BOLT #2:
 		 *