wire: use 26-bit lengths for inter-daemon messaging.

Fixes: ElementsProject#289
Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

lightningd/peer_control.c

@@ -2525,11 +2525,6 @@ void peer_fundee_open(struct peer *peer, const u8 *from_peer,
 	msg = towire_opening_fundee(peer, peer->minimum_depth,
 				    7500, 150000, from_peer);
 
-	/* Careful here!  Their message could push us overlength! */
-	if (tal_len(msg) >= 65536) {
-		peer_fail_permanent_str(peer, "Unacceptably long open_channel");
-		return;
-	}
 	subd_req(peer, peer->owner, take(msg), -1, 2,
 		 opening_fundee_finished, peer);
 }

wire/wire_io.c

@@ -1,11 +1,11 @@
-#include <ccan/endian/endian.h>
 /* FIXME: io_plan needs size_t */
  #include <unistd.h>
 #include <ccan/io/io_plan.h>
 #include <ccan/mem/mem.h>
 #include <ccan/short_types/short_types.h>
 #include <ccan/take/take.h>
 #include <ccan/tal/tal.h>
+#include <errno.h>
 #include <wire/wire_io.h>
 
 /*
@@ -14,11 +14,11 @@
  * scratch data, and it's almost enough for our purposes.
  */
 
-/* 2 bytes for the length header. */
-#define HEADER_LEN (sizeof(le16))
+/* 4 bytes for the length header. */
+#define HEADER_LEN (sizeof(wire_len_t))
 
-/* Since length can only be 64k, this is an impossible value. */
-#define INSIDE_HEADER_BIT 0x80000000
+/* We carefully never allow sizes > 64M, so this is an impossible value. */
+#define INSIDE_HEADER_BIT WIRE_LEN_LIMIT
 
 /* arg->u2.s contains length we've read, arg->u1.vp contains u8 **data. */
 static int do_read_wire_header(int fd, struct io_plan_arg *arg)
@@ -32,9 +32,13 @@ static int do_read_wire_header(int fd, struct io_plan_arg *arg)
 		return -1;
 	arg->u2.s += ret;
 
-	/* Both bytes read?  Set up for normal read of data. */
+	/* Length bytes read?  Set up for normal read of data. */
 	if (arg->u2.s == INSIDE_HEADER_BIT + HEADER_LEN) {
-		arg->u2.s = be16_to_cpu(*(be16 *)p);
+		arg->u2.s = *(wire_len_t *)p;
+		if (arg->u2.s >= INSIDE_HEADER_BIT) {
+			errno = E2BIG;
+			return -1;
+		}
 		/* A type-only message is not unheard of, so optimize a little */
 		if (arg->u2.s != HEADER_LEN)
 			tal_resize((u8 **)arg->u1.vp, arg->u2.s);
@@ -84,7 +88,7 @@ static int do_write_wire_header(int fd, struct io_plan_arg *arg)
 {
 	ssize_t ret;
 	size_t len = arg->u2.s & ~INSIDE_HEADER_BIT;
-	be16 hdr = cpu_to_be16(tal_count(arg->u1.const_vp));
+	wire_len_t hdr = tal_count(arg->u1.const_vp);
 
 	ret = write(fd, (char *)&hdr + len, HEADER_LEN - len);
 	if (ret <= 0)
@@ -128,6 +132,11 @@ struct io_plan *io_write_wire_(struct io_conn *conn,
 {
 	struct io_plan_arg *arg = io_plan_arg(conn, IO_OUT);
 
+	if (tal_len(data) >= INSIDE_HEADER_BIT) {
+		errno = E2BIG;
+		return io_close(conn);
+	}
+
 	arg->u1.const_vp = tal_dup_arr(conn, u8, memcheck(data, tal_len(data)),
 				       tal_len(data), 0);
 

wire/wire_io.h

@@ -4,6 +4,11 @@
 #include <ccan/io/io.h>
 #include <ccan/short_types/short_types.h>
 
+/* We don't allow > 64M msgs: enough for 483 64k failure msgs. */
+#define WIRE_LEN_LIMIT (1 << 26)
+
+typedef u32 wire_len_t;
+
 /* Read message into *data, allocating off ctx. */
 struct io_plan *io_read_wire_(struct io_conn *conn,
 			      const tal_t *ctx,

wire/wire_sync.c

@@ -1,16 +1,18 @@
-#include "wire/wire_sync.h"
 #include <assert.h>
 #include <ccan/endian/endian.h>
 #include <ccan/read_write_all/read_write_all.h>
+#include <errno.h>
+#include <wire/wire_io.h>
+#include <wire/wire_sync.h>
 
 bool wire_sync_write(int fd, const void *msg TAKES)
 {
-	be16 be_len = cpu_to_be16(tal_count(msg));
+	wire_len_t len = tal_len(msg);
 	bool ret;
 
-	assert(be16_to_cpu(be_len) == tal_count(msg));
-	ret = write_all(fd, &be_len, sizeof(be_len))
-		&& write_all(fd, msg, tal_count(msg));
+	assert(tal_len(msg) < WIRE_LEN_LIMIT);
+	ret = write_all(fd, &len, sizeof(len))
+		&& write_all(fd, msg, len);
 
 	if (taken(msg))
 		tal_free(msg);
@@ -19,13 +21,17 @@ bool wire_sync_write(int fd, const void *msg TAKES)
 
 u8 *wire_sync_read(const tal_t *ctx, int fd)
 {
-	be16 be_len;
+	wire_len_t len;
 	u8 *msg;
 
-	if (!read_all(fd, &be_len, sizeof(be_len)))
+	if (!read_all(fd, &len, sizeof(len)))
 		return NULL;
-	msg = tal_arr(ctx, u8, be16_to_cpu(be_len));
-	if (!read_all(fd, msg, be16_to_cpu(be_len)))
+	if (len >= WIRE_LEN_LIMIT) {
+		errno = E2BIG;
+		return NULL;
+	}
+	msg = tal_arr(ctx, u8, len);
+	if (!read_all(fd, msg, len))
 		return tal_free(msg);
 	return msg;
 }