channeld: Add extra_tlvs to wire htlcs
What the developer wrote
Authored by Peter Neuroth
channeld: Add extra_tlvs to wire htlcs
This appends the extra_tlvs to the internal wire htlcs "added" and
"existing" for the extra tlvs to be handed to lightningd.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
What changed, and why it matters
This commit extends the internal message format used between parts of a Core Lightning node so that extra optional data fields (TLVs) attached to forwarded payments are preserved when HTLCs are passed between sub-daemons. The change itself is a data plumbing/plumbing extension, not a fix for a known vulnerability. There is no vendor statement that this is a security patch, and no independent researcher is credited.
Treat as a routine feature commit. If auditing, verify that the new fromwire_tlv path cannot be abused to cause memory exhaustion or parsing failures across the channeld/lightningd boundary, and consider whether the noted TODO about restricting to known TLV types should be enforced.
Security signals we found
New TLV parsing path introduced in inter-daemon wire messages
Deserializer currently allows arbitrary TLV types (FROMWIRE_TLV_ANY_TYPE)
No bounds/length validation beyond the u16 length prefix is visible in the diff
No vendor disclosure of security relevance
Evidence from the diff
The patch adds an extra_tlvs field to the added_htlc and existing_htlc wire structures and updates their serialization (towire_) and deserialization (fromwire_) routines. It also copies the field in channeld when marshalling HTLCs and when force-loading HTLCs. The deserialization accepts any TLV type (FROMWIRE_TLV_ANY_TYPE) and includes a code comment noting that stricter filtering to known tlvs_tlv_update_add_htlc_tlvs types could be considered later. The change is additive and does not alter consensus or cryptographic checks.
Changed components
channeld/channeld.cchanneld/full_channel.ccommon/htlc_wire.ccommon/htlc_wire.hInspect captured patch +69 / −3
diff --git a/channeld/channeld.c b/channeld/channeld.c
index 97b4216e..2ead09e6 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -1590,6 +1590,7 @@ static void marshall_htlc_info(const tal_t *ctx,
htlc->routing,
sizeof(a.onion_routing_packet));
a.path_key = htlc->path_key;
+ a.extra_tlvs = htlc->extra_tlvs;
a.fail_immediate = htlc->fail_immediate;
tal_arr_expand(added, a);
} else if (htlc->state == RCVD_REMOVE_COMMIT) {
diff --git a/channeld/full_channel.c b/channeld/full_channel.c
index 2bcc4d58..699ccdf4 100644
--- a/channeld/full_channel.c
+++ b/channeld/full_channel.c
@@ -1632,7 +1632,8 @@ bool channel_force_htlcs(struct channel *channel,
&htlcs[i]->payment_hash,
htlcs[i]->onion_routing_packet,
htlcs[i]->path_key,
- &htlc, false, NULL, NULL, false);
+ &htlc, false, NULL,
+ htlcs[i]->extra_tlvs, false);
if (e != CHANNEL_ERR_ADD_OK) {
status_broken("%s HTLC %"PRIu64" failed error %u",
htlc_state_owner(htlcs[i]->state) == LOCAL
diff --git a/common/htlc_wire.c b/common/htlc_wire.c
index aa3ef92f..10893156 100644
--- a/common/htlc_wire.c
+++ b/common/htlc_wire.c
@@ -4,6 +4,7 @@
#include <ccan/crypto/shachain/shachain.h>
#include <common/htlc_wire.h>
#include <common/onionreply.h>
+#include <wire/tlvstream.h>
static struct failed_htlc *failed_htlc_dup(const tal_t *ctx,
const struct failed_htlc *f TAKES)
@@ -33,7 +34,8 @@ struct existing_htlc *new_existing_htlc(const tal_t *ctx,
const u8 onion_routing_packet[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)],
const struct pubkey *path_key TAKES,
const struct preimage *preimage TAKES,
- const struct failed_htlc *failed TAKES)
+ const struct failed_htlc *failed TAKES,
+ const struct tlv_field *extra_tlvs TAKES)
{
struct existing_htlc *existing = tal(ctx, struct existing_htlc);
@@ -51,6 +53,17 @@ struct existing_htlc *new_existing_htlc(const tal_t *ctx,
existing->failed = failed_htlc_dup(existing, failed);
else
existing->failed = NULL;
+ if (extra_tlvs) {
+ existing->extra_tlvs = tal_dup_talarr(existing, struct tlv_field, extra_tlvs);
+ for (size_t i = 0; i < tal_count(extra_tlvs); i++) {
+ /* We need to attach the value to the correct parent */
+ existing->extra_tlvs[i].value
+ = tal_dup_talarr(existing, u8,
+ existing->extra_tlvs[i].value);
+ }
+ } else {
+ existing->extra_tlvs = NULL;
+ }
return existing;
}
@@ -70,6 +83,16 @@ void towire_added_htlc(u8 **pptr, const struct added_htlc *added)
towire_pubkey(pptr, added->path_key);
} else
towire_bool(pptr, false);
+ if (added->extra_tlvs) {
+ u8 *tmp_pptr = tal_arr(tmpctx, u8, 0);
+ towire_tlvstream_raw(&tmp_pptr, added->extra_tlvs);
+
+ towire_bool(pptr, true);
+ towire_u16(pptr, tal_bytelen(tmp_pptr));
+ towire_u8_array(pptr, tmp_pptr,
+ tal_bytelen(tmp_pptr));
+ } else
+ towire_bool(pptr, false);
towire_bool(pptr, added->fail_immediate);
}
@@ -97,6 +120,16 @@ void towire_existing_htlc(u8 **pptr, const struct existing_htlc *existing)
towire_pubkey(pptr, existing->path_key);
} else
towire_bool(pptr, false);
+ if (existing->extra_tlvs) {
+ u8 *tmp_pptr = tal_arr(tmpctx, u8, 0);
+ towire_tlvstream_raw(&tmp_pptr, existing->extra_tlvs);
+
+ towire_bool(pptr, true);
+ towire_u16(pptr, tal_bytelen(tmp_pptr));
+ towire_u8_array(pptr, tmp_pptr,
+ tal_bytelen(tmp_pptr));
+ } else
+ towire_bool(pptr, false);
}
void towire_fulfilled_htlc(u8 **pptr, const struct fulfilled_htlc *fulfilled)
@@ -163,6 +196,20 @@ void fromwire_added_htlc(const u8 **cursor, size_t *max,
fromwire_pubkey(cursor, max, added->path_key);
} else
added->path_key = NULL;
+ if (fromwire_bool(cursor, max)) {
+ size_t tlv_len = fromwire_u16(cursor, max);
+ /* NOTE: We might consider to be more strict and only allow for
+ * known tlv types from the tlvs_tlv_update_add_htlc_tlvs
+ * record. */
+ const u64 *allowed = cast_const(u64 *, FROMWIRE_TLV_ANY_TYPE);
+ added->extra_tlvs = tal_arr(added, struct tlv_field, 0);
+ if (!fromwire_tlv(cursor, &tlv_len, NULL, 0, added,
+ &added->extra_tlvs, allowed, NULL, NULL)) {
+ tal_free(added->extra_tlvs);
+ added->extra_tlvs = NULL;
+ }
+ } else
+ added->extra_tlvs = NULL;
added->fail_immediate = fromwire_bool(cursor, max);
}
@@ -192,6 +239,20 @@ struct existing_htlc *fromwire_existing_htlc(const tal_t *ctx,
fromwire_pubkey(cursor, max, existing->path_key);
} else
existing->path_key = NULL;
+ if (fromwire_bool(cursor, max)) {
+ size_t tlv_len = fromwire_u16(cursor, max);
+ /* NOTE: We might consider to be more strict and only allow for
+ * known tlv types from the tlvs_tlv_update_add_htlc_tlvs
+ * record. */
+ const u64 *allowed = cast_const(u64 *, FROMWIRE_TLV_ANY_TYPE);
+ existing->extra_tlvs = tal_arr(existing, struct tlv_field, 0);
+ if (!fromwire_tlv(cursor, &tlv_len, NULL, 0, existing,
+ &existing->extra_tlvs, allowed, NULL, NULL)) {
+ tal_free(existing->extra_tlvs);
+ existing->extra_tlvs = NULL;
+ }
+ } else
+ existing->extra_tlvs = NULL;
return existing;
}
diff --git a/common/htlc_wire.h b/common/htlc_wire.h
index 4d758a64..c50fece3 100644
--- a/common/htlc_wire.h
+++ b/common/htlc_wire.h
@@ -17,6 +17,7 @@ struct added_htlc {
u8 onion_routing_packet[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)];
bool fail_immediate;
struct pubkey *path_key;
+ struct tlv_field *extra_tlvs;
};
/* This is how lightningd tells us about HTLCs which already exist at startup */
@@ -33,6 +34,7 @@ struct existing_htlc {
struct preimage *payment_preimage;
/* If failed, this is set */
const struct failed_htlc *failed;
+ struct tlv_field *extra_tlvs;
};
struct fulfilled_htlc {
@@ -69,7 +71,8 @@ struct existing_htlc *new_existing_htlc(const tal_t *ctx,
const u8 onion_routing_packet[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)],
const struct pubkey *path_key TAKES,
const struct preimage *preimage TAKES,
- const struct failed_htlc *failed TAKES);
+ const struct failed_htlc *failed TAKES,
+ const struct tlv_field *extra_tlvs TAKES);
void towire_added_htlc(u8 **pptr, const struct added_htlc *added);
void towire_existing_htlc(u8 **pptr, const struct existing_htlc *existing);
Why this scored 20/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.