common/onion: expunge all trace of different onion styles.

In particular, remove special routines to pull length: it's there,
take it and check it yourself.

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

common/onion.c

@@ -111,57 +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:
-	 * - `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 (*len <= 1)
-		return false;
-
-	/* It's invalid if it claims to be too long! */
-	if (*len > *max)
-		return false;
-
-	if (type)
-		*type = ONION_TLV_PAYLOAD;
-	*len += (*cursor - start);
-	return true;
-}
-
-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,
@@ -210,7 +159,14 @@ 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;

common/onion.h

@@ -6,13 +6,7 @@
 
 struct route_step;
 
-enum onion_payload_type {
-	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;
@@ -45,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

@@ -601,7 +601,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 +625,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);
 
-	/* 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);
+	/* 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);
+
+	/* 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/test/run-sphinx-xor_cipher_stream.c

@@ -38,6 +38,9 @@ struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED)
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
 { fprintf(stderr, "fromwire called!\n"); abort(); }
+/* Generated stub for fromwire_bigsize */
+bigsize_t fromwire_bigsize(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_bigsize called!\n"); abort(); }
 /* Generated stub for fromwire_bool */
 bool fromwire_bool(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
 { fprintf(stderr, "fromwire_bool called!\n"); abort(); }
@@ -47,6 +50,9 @@ void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
 /* Generated stub for fromwire_hmac */
 void fromwire_hmac(const u8 **ptr UNNEEDED, size_t *max UNNEEDED, struct hmac *hmac UNNEEDED)
 { fprintf(stderr, "fromwire_hmac called!\n"); abort(); }
+/* Generated stub for fromwire_pad */
+void fromwire_pad(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, size_t num UNNEEDED)
+{ fprintf(stderr, "fromwire_pad called!\n"); abort(); }
 /* Generated stub for fromwire_secp256k1_ecdsa_signature */
 void fromwire_secp256k1_ecdsa_signature(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
 					secp256k1_ecdsa_signature *signature UNNEEDED)
@@ -88,12 +94,6 @@ void hmac_update(crypto_auth_hmacsha256_state *state UNNEEDED,
 /* Generated stub for new_onionreply */
 struct onionreply *new_onionreply(const tal_t *ctx UNNEEDED, const u8 *contents TAKES UNNEEDED)
 { fprintf(stderr, "new_onionreply called!\n"); abort(); }
-/* Generated stub for onion_payload_length */
-size_t onion_payload_length(const u8 *raw_payload UNNEEDED, size_t len UNNEEDED,
-			    bool has_realm UNNEEDED,
-			    bool *valid UNNEEDED,
-			    enum onion_payload_type *type UNNEEDED)
-{ fprintf(stderr, "onion_payload_length called!\n"); abort(); }
 /* Generated stub for pubkey_from_node_id */
 bool pubkey_from_node_id(struct pubkey *key UNNEEDED, const struct node_id *id UNNEEDED)
 { fprintf(stderr, "pubkey_from_node_id called!\n"); abort(); }

devtools/onion.c

@@ -270,18 +270,12 @@ static void runtest(const char *filename)
 	decodetok = json_get_member(buffer, toks, "decode");
 
 	json_for_each_arr(i, hop, decodetok) {
-		bool valid;
-
 		hexprivkey = json_strdup(ctx, buffer, hop);
 		printf("Processing at hop %zu\n", i);
 		step = decode_with_privkey(ctx, serialized, hexprivkey, associated_data);
 		serialized = serialize_onionpacket(ctx, step->next);
 		if (!serialized)
 			errx(1, "Error serializing message.");
-		onion_payload_length(step->raw_payload,
-				     tal_bytelen(step->raw_payload),
-				     true, &valid, NULL);
-		assert(valid);
 		printf("  Payload: %s\n", tal_hex(ctx, step->raw_payload));
 		printf("  Next onion: %s\n", tal_hex(ctx, serialized));
 		printf("  Next HMAC: %s\n",

lightningd/peer_htlcs.c

@@ -485,21 +485,6 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
 		db_commit_transaction(db);
 }
 
-static enum forward_style get_onion_style(const struct htlc_in *hin)
-{
-	/* This happens on reload from db; don't try too hard! */
-	if (!hin->payload)
-		return FORWARD_STYLE_UNKNOWN;
-
-	switch ((int)hin->payload->type) {
-	case 0:
-		return FORWARD_STYLE_LEGACY;
-	case ONION_TLV_PAYLOAD:
-		return FORWARD_STYLE_TLV;
-	}
-	abort();
-}
-
 /* This is where channeld gives us the HTLC id, and also reports if it
  * failed immediately. */
 static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED,
@@ -539,7 +524,7 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU
 			 * so set htlc field with NULL (db wants it to exist!) */
 			wallet_forwarded_payment_add(ld->wallet,
 					 hout->in,
-					 get_onion_style(hout->in),
+					 FORWARD_STYLE_TLV,
 					 channel_scid_or_local_alias(hout->key.channel), NULL,
 					 FORWARD_LOCAL_FAILED,
 						     fromwire_peektype(hout->failmsg));
@@ -713,7 +698,7 @@ static void forward_htlc(struct htlc_in *hin,
 	if (!next || !channel_active(next)) {
 		local_fail_in_htlc(hin, take(towire_unknown_next_peer(NULL)));
 		wallet_forwarded_payment_add(hin->key.channel->peer->ld->wallet,
-					 hin, get_onion_style(hin),
+					 hin, FORWARD_STYLE_TLV,
 					 forward_scid, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 WIRE_UNKNOWN_NEXT_PEER);
@@ -812,7 +797,7 @@ static void forward_htlc(struct htlc_in *hin,
 fail:
 	local_fail_in_htlc(hin, failmsg);
 	wallet_forwarded_payment_add(ld->wallet,
-				 hin, get_onion_style(hin), forward_scid, hout,
+				 hin, FORWARD_STYLE_TLV, forward_scid, hout,
 				 FORWARD_LOCAL_FAILED,
 				 fromwire_peektype(failmsg));
 }
@@ -1060,11 +1045,7 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
 
 	json_add_hex_talarr(s, "payload", rs->raw_payload);
 	if (p->payload) {
-		switch (p->payload->type) {
-		case ONION_TLV_PAYLOAD:
-			json_add_string(s, "type", "tlv");
-			break;
-		}
+		json_add_string(s, "type", "tlv");
 
 		if (p->payload->forward_channel)
 			json_add_short_channel_id(s, "short_channel_id",
@@ -1406,7 +1387,7 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout,
 	else if (hout->in) {
 		fulfill_htlc(hout->in, preimage);
 		wallet_forwarded_payment_add(ld->wallet, hout->in,
-					     get_onion_style(hout->in),
+					     FORWARD_STYLE_TLV,
 					     channel_scid_or_local_alias(hout->key.channel), hout,
 					     FORWARD_SETTLED, 0);
 	}
@@ -1534,7 +1515,7 @@ static bool peer_failed_our_htlc(struct channel *channel,
 
 	if (hout->in)
 		wallet_forwarded_payment_add(ld->wallet, hout->in,
-					     get_onion_style(hout->in),
+					     FORWARD_STYLE_TLV,
 					     channel_scid_or_local_alias(channel),
 					     hout, FORWARD_FAILED,
 					     hout->failmsg
@@ -1697,7 +1678,7 @@ void onchain_failed_our_htlc(const struct channel *channel,
 		local_fail_in_htlc(hout->in,
 				   take(towire_permanent_channel_failure(NULL)));
 		wallet_forwarded_payment_add(hout->key.channel->peer->ld->wallet,
-					 hout->in, get_onion_style(hout->in),
+					 hout->in, FORWARD_STYLE_TLV,
 					 channel_scid_or_local_alias(channel), hout,
 					 FORWARD_LOCAL_FAILED,
 					 hout->failmsg
@@ -1864,7 +1845,7 @@ static bool update_out_htlc(struct channel *channel,
 
 		if (hout->in) {
 			wallet_forwarded_payment_add(ld->wallet, hout->in,
-						     get_onion_style(hout->in),
+						     FORWARD_STYLE_TLV,
 						     channel_scid_or_local_alias(channel), hout,
 						     FORWARD_OFFERED, 0);
 		}

plugins/bkpr/test/run-bkpr_db.c

@@ -163,9 +163,6 @@ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok
 bool json_to_txid(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
 		  struct bitcoin_txid *txid UNNEEDED)
 { fprintf(stderr, "json_to_txid called!\n"); abort(); }
-/* Generated stub for json_to_u64 */
-bool json_to_u64(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, u64 *num UNNEEDED)
-{ fprintf(stderr, "json_to_u64 called!\n"); abort(); }
 /* Generated stub for json_tok_bin_from_hex */
 u8 *json_tok_bin_from_hex(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED)
 { fprintf(stderr, "json_tok_bin_from_hex called!\n"); abort(); }

plugins/bkpr/test/run-recorder.c

@@ -169,9 +169,6 @@ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok
 bool json_to_txid(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
 		  struct bitcoin_txid *txid UNNEEDED)
 { fprintf(stderr, "json_to_txid called!\n"); abort(); }
-/* Generated stub for json_to_u64 */
-bool json_to_u64(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, u64 *num UNNEEDED)
-{ fprintf(stderr, "json_to_u64 called!\n"); abort(); }
 /* Generated stub for json_tok_bin_from_hex */
 u8 *json_tok_bin_from_hex(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED)
 { fprintf(stderr, "json_tok_bin_from_hex called!\n"); abort(); }

wallet/wallet.h

@@ -165,7 +165,7 @@ bool string_to_forward_status(const char *status_str, size_t len,
  * already defined elements (adding is ok) /!\ */
 enum forward_style {
 	FORWARD_STYLE_LEGACY = 0,
-	FORWARD_STYLE_TLV = ONION_TLV_PAYLOAD,
+	FORWARD_STYLE_TLV = 1,
 	FORWARD_STYLE_UNKNOWN = 2, /* Not actually in db, safe to renumber! */
 };