Remove legacy onion support, simplify internal APIs

common/onion.c

@@ -111,76 +111,6 @@ u8 *onion_final_hop(const tal_t *ctx,
 	return make_tlv_hop(ctx, tlv);
 }
 
-/* Returns true if valid, and fills in type. */
-static bool pull_payload_length(const u8 **cursor,
-				size_t *max,
-				bool has_realm,
-				enum onion_payload_type *type,
-				size_t *len)
-{
-	/* *len will incorporate bytes we read from cursor */
-	const u8 *start = *cursor;
-
-	/* BOLT #4:
-	 *
-	 * The `length` field determines both the length and the format of the
-	 * `hop_payload` field; the following formats are defined:
-	 */
-	*len = fromwire_bigsize(cursor, max);
-	if (!cursor)
-		return false;
-
-	/* BOLT #4:
-	 * - Legacy `hop_data` format, identified by a single `0x00` byte for
-	 *   length. In this case the `hop_payload_length` is defined to be 32
-	 *   bytes.
-	 */
-	if (has_realm && *len == 0) {
-		if (type)
-			*type = ONION_V0_PAYLOAD;
-		assert(*cursor - start == 1);
-		*len = 1 + 32;
-		return true;
-	}
-
-	/* BOLT #4:
-	 * - `tlv_payload` format, identified by any length over `1`. In this
-	 *   case the `hop_payload_length` is equal to the numeric value of
-	 *   `length`.
-	 */
-	if (!has_realm || *len > 1) {
-		/* It's still invalid if it claims to be too long! */
-		if (has_realm) {
-			if (*len > ROUTING_INFO_SIZE - HMAC_SIZE)
-				return false;
-		} else {
-			if (*len > *max)
-				return false;
-		}
-
-		if (type)
-			*type = ONION_TLV_PAYLOAD;
-		*len += (*cursor - start);
-		return true;
-	}
-
-	return false;
-}
-
-size_t onion_payload_length(const u8 *raw_payload, size_t len, bool has_realm,
-			    bool *valid,
-			    enum onion_payload_type *type)
-{
-	size_t max = len, payload_len;
-	*valid = pull_payload_length(&raw_payload, &max, has_realm, type, &payload_len);
-
-	/* If it's not valid, copy the entire thing. */
-	if (!*valid)
-		return len;
-
-	return payload_len;
-}
-
 #if EXPERIMENTAL_FEATURES
 static struct tlv_tlv_payload *decrypt_tlv(const tal_t *ctx,
 					   const struct secret *blinding_ss,
@@ -229,32 +159,19 @@ struct onion_payload *onion_decode(const tal_t *ctx,
 	size_t max = tal_bytelen(cursor), len;
 	struct tlv_tlv_payload *tlv;
 
-	if (!pull_payload_length(&cursor, &max, true, &p->type, &len)) {
+	/* BOLT-remove-legacy-onion #4:
+	 * 1. type: `hop_payloads`
+	 * 2. data:
+	 *    * [`bigsize`:`length`]
+	 *    * [`length*byte`:`payload`]
+	 */
+	len = fromwire_bigsize(&cursor, &max);
+	if (!cursor || len > max) {
 		*failtlvtype = 0;
 		*failtlvpos = tal_bytelen(rs->raw_payload);
 		goto fail_no_tlv;
 	}
 
-	/* Very limited legacy handling: forward only. */
-	if (p->type == ONION_V0_PAYLOAD && rs->nextcase == ONION_FORWARD) {
-		p->forward_channel = tal(p, struct short_channel_id);
-		fromwire_short_channel_id(&cursor, &max, p->forward_channel);
-		p->total_msat = NULL;
-		p->amt_to_forward = fromwire_amount_msat(&cursor, &max);
-		p->outgoing_cltv = fromwire_u32(&cursor, &max);
-		p->payment_secret = NULL;
-		p->payment_metadata = NULL;
-		p->blinding = NULL;
-		/* We can't handle blinding with a legacy payload */
-		if (blinding)
-			return tal_free(p);
-		/* If they somehow got an invalid onion this far, fail. */
-		if (!cursor)
-			return tal_free(p);
-		p->tlv = NULL;
-		return p;
-	}
-
 	/* We do this manually so we can accept extra types, and get
 	 * error off and type. */
 	tlv = tlv_tlv_payload_new(p);

common/onion.h

@@ -6,14 +6,7 @@
 
 struct route_step;
 
-enum onion_payload_type {
-	ONION_V0_PAYLOAD = 0,
-	ONION_TLV_PAYLOAD = 1,
-};
-
 struct onion_payload {
-	enum onion_payload_type type;
-
 	struct amount_msat amt_to_forward;
 	u32 outgoing_cltv;
 	struct amount_msat *total_msat;
@@ -46,23 +39,6 @@ u8 *onion_final_hop(const tal_t *ctx,
 		    const struct secret *payment_secret,
 		    const u8 *payment_metadata);
 
-/**
- * onion_payload_length: measure payload length in decrypted onion.
- * @raw_payload: payload to look at.
- * @len: length of @raw_payload in bytes.
- * @has_realm: used for HTLCs, where first byte 0 is magical.
- * @valid: set to true if it is valid, false otherwise.
- * @type: if non-NULL, set to type of payload if *@valid is true.
- *
- * If @valid is set, there is room for the HMAC immediately following,
- * as the return value is <= ROUTING_INFO_SIZE - HMAC_SIZE.  Otherwise,
- * the return value is @len (i.e. the entire payload).
- */
-size_t onion_payload_length(const u8 *raw_payload, size_t len,
-			    bool has_realm,
-			    bool *valid,
-			    enum onion_payload_type *type);
-
 /**
  * onion_decode: decode payload from a decrypted onion.
  * @ctx: context to allocate onion_contents off.

common/sphinx.c

@@ -4,6 +4,7 @@
 #include <ccan/mem/mem.h>
 #include <common/onion.h>
 #include <common/onionreply.h>
+#include <common/overflows.h>
 #include <common/sphinx.h>
 
 
@@ -103,17 +104,29 @@ size_t sphinx_path_payloads_size(const struct sphinx_path *path)
 	return size;
 }
 
-void sphinx_add_hop(struct sphinx_path *path, const struct pubkey *pubkey,
-		    const u8 *payload TAKES)
+bool sphinx_add_hop_has_length(struct sphinx_path *path, const struct pubkey *pubkey,
+			       const u8 *payload TAKES)
 {
 	struct sphinx_hop sp;
+	bigsize_t lenlen, prepended_len;
+
+	/* You promised size was prepended! */
+	if (tal_bytelen(payload) == 0)
+		return false;
+	lenlen = bigsize_get(payload, tal_bytelen(payload), &prepended_len);
+	if (add_overflows_u64(lenlen, prepended_len))
+		return false;
+	if (lenlen + prepended_len != tal_bytelen(payload))
+		return false;
+
 	sp.raw_payload = tal_dup_talarr(path, u8, payload);
 	sp.pubkey = *pubkey;
 	tal_arr_expand(&path->hops, sp);
+	return true;
 }
 
-void sphinx_add_modern_hop(struct sphinx_path *path, const struct pubkey *pubkey,
-			   const u8 *payload TAKES)
+void sphinx_add_hop(struct sphinx_path *path, const struct pubkey *pubkey,
+		    const u8 *payload TAKES)
 {
 	u8 *with_len = tal_arr(NULL, u8, 0);
 	size_t len = tal_bytelen(payload);
@@ -122,7 +135,8 @@ void sphinx_add_modern_hop(struct sphinx_path *path, const struct pubkey *pubkey
 	if (taken(payload))
 		tal_free(payload);
 
-	sphinx_add_hop(path, pubkey, take(with_len));
+	if (!sphinx_add_hop_has_length(path, pubkey, take(with_len)))
+		abort();
 }
 
 /* Small helper to append data to a buffer and update the position
@@ -601,7 +615,8 @@ struct route_step *process_onionpacket(
 	u8 *paddedheader;
 	size_t payload_size;
 	bigsize_t shift_size;
-	bool valid;
+	const u8 *cursor;
+	size_t max;
 
 	step->next = talz(step, struct onionpacket);
 	step->next->version = msg->version;
@@ -624,23 +639,29 @@ struct route_step *process_onionpacket(
 	if (!blind_group_element(&step->next->ephemeralkey, &msg->ephemeralkey, blind))
 		return tal_free(step);
 
-	payload_size = onion_payload_length(paddedheader,
-					    tal_bytelen(msg->routinginfo),
-					    has_realm,
-					    &valid, NULL);
+	/* Now, try to pull data out. */
+	cursor = paddedheader;
+	max = tal_bytelen(msg->routinginfo);
+
+	/* Any of these could fail, falling thru with cursor == NULL */
+	payload_size = fromwire_bigsize(&cursor, &max);
+	/* FIXME: raw_payload *includes* the length, which is redundant and
+	 * means we can't just ust fromwire_tal_arrn. */
+	fromwire_pad(&cursor, &max, payload_size);
+	if (cursor != NULL)
+		step->raw_payload = tal_dup_arr(step, u8, paddedheader,
+						cursor - paddedheader, 0);
+	fromwire_hmac(&cursor, &max, &step->next->hmac);
+
+	/* BOLT-remove-legacy-onion #4:
+	 * Since no `payload` TLV value can ever be shorter than 2 bytes, `length` values of 0 and 1 are
+	 * reserved.  (`0` indicated a legacy format no longer supported, and `1` is reserved for future
+	 * use). */
+	if (payload_size < 2 || !cursor)
+		return tal_free(step);
 
-	/* Can't decode?  Treat it as terminal. */
-	if (!valid) {
-		shift_size = payload_size;
-		memset(step->next->hmac.bytes, 0, sizeof(step->next->hmac.bytes));
-	} else {
-		assert(payload_size <= tal_bytelen(msg->routinginfo) - HMAC_SIZE);
-		/* Copy hmac */
-		shift_size = payload_size + HMAC_SIZE;
-		memcpy(step->next->hmac.bytes,
-		       paddedheader + payload_size, HMAC_SIZE);
-	}
-	step->raw_payload = tal_dup_arr(step, u8, paddedheader, payload_size, 0);
+	/* This includes length field and hmac */
+	shift_size = cursor - paddedheader;
 
 	/* Left shift the current payload out and make the remainder the new onion */
 	step->next->routinginfo = tal_dup_arr(step->next,

common/sphinx.h

@@ -53,27 +53,6 @@ enum route_next_case {
  */
 struct sphinx_path;
 
-/* BOLT #4:
- *
- * ## Legacy `hop_data` payload format
- *
- * The `hop_data` format is identified by a single `0x00`-byte length,
- * for backward compatibility.  Its payload is defined as:
- *
- * 1. type: `hop_data` (for `realm` 0)
- * 2. data:
- *    * [`short_channel_id`:`short_channel_id`]
- *    * [`u64`:`amt_to_forward`]
- *    * [`u32`:`outgoing_cltv_value`]
- *    * [`12*byte`:`padding`]
- */
-struct hop_data_legacy {
-	u8 realm;
-	struct short_channel_id channel_id;
-	struct amount_msat amt_forward;
-	u32 outgoing_cltv;
-};
-
 /*
  * All the necessary information to generate a valid onion for this hop on a
  * sphinx path. The payload is preserialized in order since the onion
@@ -223,17 +202,19 @@ struct sphinx_path *sphinx_path_new_with_key(const tal_t *ctx,
 					     const struct secret *session_key);
 
 /**
- * Add a payload hop to the path.
+ * Add a payload hop to the path (already has length prepended).
+ *
+ * Fails if length actually isn't prepended!
  */
-void sphinx_add_hop(struct sphinx_path *path, const struct pubkey *pubkey,
-		    const u8 *payload TAKES);
+bool sphinx_add_hop_has_length(struct sphinx_path *path, const struct pubkey *pubkey,
+			       const u8 *payload TAKES);
 
 /**
  * Prepend length to payload and add: for onionmessage, any size is OK,
  * for HTLC onions tal_bytelen(payload) must be > 1.
  */
-void sphinx_add_modern_hop(struct sphinx_path *path, const struct pubkey *pubkey,
-			   const u8 *payload TAKES);
+void sphinx_add_hop(struct sphinx_path *path, const struct pubkey *pubkey,
+		    const u8 *payload TAKES);
 
 /**
  * Compute the size of the serialized payloads.

common/test/run-blindedpath_onion.c

@@ -204,8 +204,7 @@ int main(int argc, char *argv[])
 		payload->encrypted_data_tlv = enctlv[i];
 		onionmsg_payload[i] = tal_arr(tmpctx, u8, 0);
 		towire_tlv_onionmsg_payload(&onionmsg_payload[i], payload);
-		sphinx_add_modern_hop(sphinx_path, &alias[i],
-				      onionmsg_payload[i]);
+		sphinx_add_hop(sphinx_path, &alias[i], onionmsg_payload[i]);
 	}
 	op = create_onionpacket(tmpctx, sphinx_path, ROUTING_INFO_SIZE,
 				&path_secrets);