lightningd: Add extra_tlvs to htlc_accepted_hook
What changed, and why it matters
This commit adds a new plugin hook feature to Core Lightning that lets plugins inspect and replace optional extra data (TLV fields) attached to forwarded Lightning payments. It is a feature addition, not a direct bug fix. The code includes validation of those extra fields and a note that they are not saved to the database. There is no claim in the commit that this fixes a security vulnerability.
Treat as a normal feature review. Verify that validation of plugin-returned extra_tlvs correctly rejects malformed or odd-type TLVs, that memory allocation failures in tal_dup_talarr are handled, and that the documented non-persistence of extra_tlvs does not violate protocol requirements for forwarded HTLCs after restart.
Security signals we found
New plugin hook surface added for TLV replacement
Validation of plugin-supplied extra_tlvs against known BOLT#1 TLV schema
Database persistence deliberately omitted for extra_tlvs with spam/size concern noted
No vendor claim of security relevance in commit message or diff
Evidence from the diff
The commit extends the htlc_accepted_hook to serialize/deserialize extra TLV streams from update_add_htlc messages. It stores extra_tlvs on htlc_in/htlc_out, passes them through forwards, and exposes them to plugins via JSON. Plugins can return replacement extra_tlvs, which are validated against the known update_add_htlc_tlvs schema (restricting to known even types). The commit also notes that extra_tlvs are not persisted to the wallet database, which is acceptable because they matter only during an in-flight forward. No memory corruption, authentication bypass, or cryptographic weakness is evident in the diff.
Changed components
lightningd/htlc_end.clightningd/htlc_end.hlightningd/pay.clightningd/peer_htlcs.clightningd/peer_htlcs.hwallet/wallet.cwallet/test/run-wallet.cInspect captured patch +147 / −11
diff --git a/lightningd/htlc_end.c b/lightningd/htlc_end.c
index 46083353..b040f3b6 100644
--- a/lightningd/htlc_end.c
+++ b/lightningd/htlc_end.c
@@ -6,6 +6,7 @@
#include <common/pseudorand.h>
#include <lightningd/htlc_end.h>
#include <lightningd/log.h>
+#include <wire/tlvstream.h>
size_t hash_htlc_key(const struct htlc_key *k)
{
@@ -130,6 +131,7 @@ struct htlc_in *new_htlc_in(const tal_t *ctx,
const struct secret *shared_secret TAKES,
const struct pubkey *path_key TAKES,
const u8 *onion_routing_packet,
+ const struct tlv_field *extra_tlvs,
bool fail_immediate)
{
struct htlc_in *hin = tal(ctx, struct htlc_in);
@@ -146,6 +148,15 @@ struct htlc_in *new_htlc_in(const tal_t *ctx,
hin->path_key = tal_dup_or_null(hin, struct pubkey, path_key);
memcpy(hin->onion_routing_packet, onion_routing_packet,
sizeof(hin->onion_routing_packet));
+ if (extra_tlvs) {
+ hin->extra_tlvs = tal_dup_talarr(hin, 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 */
+ hin->extra_tlvs[i].value = tal_dup_talarr(hin, u8, hin->extra_tlvs[i].value);
+ }
+ } else {
+ hin->extra_tlvs = NULL;
+ }
hin->hstate = RCVD_ADD_COMMIT;
hin->badonion = 0;
@@ -265,6 +276,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
const struct pubkey *path_key,
+ const struct tlv_field* extra_tlvs,
bool am_origin,
struct amount_msat final_msat,
u64 partid,
@@ -291,6 +303,17 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
hout->timeout = NULL;
hout->path_key = tal_dup_or_null(hout, struct pubkey, path_key);
+
+ if (extra_tlvs) {
+ hout->extra_tlvs = tal_dup_talarr(hout, 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 */
+ hout->extra_tlvs[i].value = tal_dup_talarr(hout, u8, hout->extra_tlvs[i].value);
+ }
+ } else {
+ hout->extra_tlvs = NULL;
+ }
+
hout->am_origin = am_origin;
if (am_origin) {
hout->partid = partid;
diff --git a/lightningd/htlc_end.h b/lightningd/htlc_end.h
index 6b106cac..5d193251 100644
--- a/lightningd/htlc_end.h
+++ b/lightningd/htlc_end.h
@@ -58,6 +58,9 @@ struct htlc_in {
/* The decoded onion payload after hooks processed it. */
struct onion_payload *payload;
+
+ /* Incommimg extra update_add_htlc_tlv tlvs */
+ struct tlv_field *extra_tlvs;
};
struct htlc_out {
@@ -106,6 +109,9 @@ struct htlc_out {
/* Timer we use in case they don't add an HTLC in a timely manner. */
struct oneshot *timeout;
+
+ /* Extra tlvs that are extended to the update_add_htlc_tlvs */
+ struct tlv_field *extra_tlvs;
};
static inline const struct htlc_key *keyof_htlc_in(const struct htlc_in *in)
@@ -158,6 +164,7 @@ struct htlc_in *new_htlc_in(const tal_t *ctx,
const struct secret *shared_secret TAKES,
const struct pubkey *path_key TAKES,
const u8 *onion_routing_packet,
+ const struct tlv_field *extra_tlvs TAKES,
bool fail_immediate);
/* You need to set the ID, then connect_htlc_out this! */
@@ -168,6 +175,7 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
const struct pubkey *path_key,
+ const struct tlv_field *extra_tlvs,
bool am_origin,
struct amount_msat final_msat,
u64 partid,
diff --git a/lightningd/pay.c b/lightningd/pay.c
index 5b17f92a..f7a26781 100644
--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@@ -783,8 +783,8 @@ static const u8 *send_onion(const tal_t *ctx, struct lightningd *ld,
return send_htlc_out(ctx, channel, first_hop->amount,
base_expiry + first_hop->delay,
final_amount, payment_hash,
- path_key, partid, groupid, onion, NULL, hout);
-}
+ path_key, NULL, partid, groupid, onion, NULL, hout);
+ }
static struct command_result *check_invoice_request_usage(struct command *cmd,
const struct sha256 *local_invreq_id)
@@ -2093,7 +2093,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
failmsg = send_htlc_out(tmpctx, next, *msat,
*cltv, *destination_msat,
payment_hash,
- next_path_key, *partid, *groupid,
+ next_path_key, NULL, *partid, *groupid,
serialize_onionpacket(tmpctx, rs->next),
NULL, &hout);
if (failmsg) {
diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
index f21d796b..6c946f0b 100644
--- a/lightningd/peer_htlcs.c
+++ b/lightningd/peer_htlcs.c
@@ -3,6 +3,7 @@
#include <ccan/mem/mem.h>
#include <ccan/tal/str/str.h>
#include <channeld/channeld_wiregen.h>
+#include <common/bigsize.h>
#include <common/blinding.h>
#include <common/configdir.h>
#include <common/ecdh.h>
@@ -23,6 +24,10 @@
#include <lightningd/plugin_hook.h>
#include <lightningd/subd.h>
#include <onchaind/onchaind_wiregen.h>
+#include <stdio.h>
+#include <wire/onion_wiregen.h>
+#include <wire/peer_wiregen.h>
+#include <wire/tlvstream.h>
#ifndef SUPERVERBOSE
#define SUPERVERBOSE(...)
@@ -695,6 +700,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
struct amount_msat final_msat,
const struct sha256 *payment_hash,
const struct pubkey *path_key,
+ const struct tlv_field *extra_tlvs,
u64 partid,
u64 groupid,
const u8 *onion_routing_packet,
@@ -729,7 +735,8 @@ const u8 *send_htlc_out(const tal_t *ctx,
/* Make peer's daemon own it, catch if it dies. */
*houtp = new_htlc_out(out->owner, out, amount, cltv,
payment_hash, onion_routing_packet,
- path_key, in == NULL,
+ path_key, extra_tlvs,
+ in == NULL,
final_msat,
partid, groupid, in);
tal_add_destructor(*houtp, destroy_hout_subd_died);
@@ -742,6 +749,13 @@ const u8 *send_htlc_out(const tal_t *ctx,
*houtp);
}
+ if (extra_tlvs) {
+ raw_tlvs = tal_arr(tmpctx, u8, 0);
+ towire_tlvstream_raw(&raw_tlvs,
+ tal_dup_talarr(tmpctx, struct tlv_field,
+ extra_tlvs));
+ }
+
msg = towire_channeld_offer_htlc(out, amount, cltv, payment_hash,
onion_routing_packet, path_key,
raw_tlvs);
@@ -797,7 +811,8 @@ static void forward_htlc(struct htlc_in *hin,
const struct short_channel_id *forward_scid,
const struct channel_id *forward_to,
const u8 next_onion[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)],
- const struct pubkey *next_path_key)
+ const struct pubkey *next_path_key,
+ const struct tlv_field *extra_tlvs)
{
const u8 *failmsg;
struct lightningd *ld = hin->key.channel->peer->ld;
@@ -912,7 +927,7 @@ static void forward_htlc(struct htlc_in *hin,
failmsg = send_htlc_out(tmpctx, next, amt_to_forward,
outgoing_cltv_value, AMOUNT_MSAT(0),
&hin->payment_hash,
- next_path_key, 0 /* partid */, 0 /* groupid */,
+ next_path_key, extra_tlvs, 0 /* partid */, 0 /* groupid */,
next_onion, hin, &hout);
if (!failmsg)
return;
@@ -942,6 +957,7 @@ struct htlc_accepted_hook_payload {
u64 failtlvtype;
size_t failtlvpos;
const char *failexplanation;
+ u8 *extra_tlvs_raw;
};
static void
@@ -998,8 +1014,8 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
struct htlc_in *hin = request->hin;
struct lightningd *ld = request->ld;
struct preimage payment_preimage;
- const jsmntok_t *resulttok, *paykeytok, *payloadtok, *fwdtok;
- u8 *failonion;
+ const jsmntok_t *resulttok, *paykeytok, *payloadtok, *fwdtok, *extra_tlvs_tok;
+ u8 *failonion, *raw_tlvs;
if (!toks || !buffer)
return true;
@@ -1013,6 +1029,49 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
json_strdup(tmpctx, buffer, toks));
}
+ extra_tlvs_tok = json_get_member(buffer, toks, "extra_tlvs");
+ if (extra_tlvs_tok) {
+ size_t max;
+ struct tlv_update_add_htlc_tlvs *check_extra_tlvs;
+
+ raw_tlvs = json_tok_bin_from_hex(tmpctx, buffer,
+ extra_tlvs_tok);
+ if (!raw_tlvs)
+ fatal("Bad custom tlvs for htlc_accepted"
+ " hook: %.*s",
+ extra_tlvs_tok->end - extra_tlvs_tok->start,
+ buffer + extra_tlvs_tok->start);
+
+ max = tal_bytelen(raw_tlvs);
+
+ /* We check if the custom tlvs are still valid BOLT#1 tlvs.
+ * As these are appended to forwarded htlcs we check for valid
+ * update_add_htlc_tlvs (restricts to known even types).
+ * NOTE: We may be less strict and allow unknown evens .*/
+ const u8 *cursor = raw_tlvs;
+ check_extra_tlvs = fromwire_tlv_update_add_htlc_tlvs(tmpctx,
+ &cursor,
+ &max);
+ if (!check_extra_tlvs) {
+ fatal("htlc_accepted_hook returned bad extra_tlvs %s",
+ tal_hex(tmpctx, raw_tlvs));
+ }
+
+ /* If we got a blinded path key we replace the next path key
+ * with it. */
+ if (check_extra_tlvs->blinded_path) {
+ tal_free(request->next_path_key);
+ request->next_path_key
+ = tal_steal(request,
+ check_extra_tlvs->blinded_path);
+ }
+
+ /* We made it and got a valid extra_tlvs: Replace the current
+ * extra_tlvs with it. */
+ tal_free(request->extra_tlvs_raw);
+ request->extra_tlvs_raw = tal_steal(request, raw_tlvs);
+ }
+
payloadtok = json_get_member(buffer, toks, "payload");
if (payloadtok) {
u8 *payload = json_tok_bin_from_hex(rs, buffer, payloadtok);
@@ -1170,6 +1229,9 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
json_add_u32(s, "cltv_expiry", expiry);
json_add_s32(s, "cltv_expiry_relative", expiry - blockheight);
json_add_sha256(s, "payment_hash", &hin->payment_hash);
+ if (p->extra_tlvs_raw) {
+ json_add_hex_talarr(s, "extra_tlvs", p->extra_tlvs_raw);
+ }
json_object_end(s);
}
@@ -1200,13 +1262,25 @@ htlc_accepted_hook_final(struct htlc_accepted_hook_payload *request STEALS)
NULL, request->failtlvtype,
request->failtlvpos)));
} else if (rs->nextcase == ONION_FORWARD) {
+ struct tlv_field *extra_tlvs;
+
+ if (request->extra_tlvs_raw) {
+ const u8 *cursor = request->extra_tlvs_raw;
+ size_t max = tal_bytelen(cursor);
+ extra_tlvs = tal_arr(request, struct tlv_field, 0);
+ fromwire_tlv(&cursor, &max, NULL, 0, request,
+ &extra_tlvs, NULL, NULL, NULL);
+ } else {
+ extra_tlvs = NULL;
+ }
+
forward_htlc(hin, hin->cltv_expiry,
request->payload->amt_to_forward,
request->payload->outgoing_cltv,
request->payload->forward_channel,
request->fwd_channel_id,
serialize_onionpacket(tmpctx, rs->next),
- request->next_path_key);
+ request->next_path_key, extra_tlvs);
} else
handle_localpay(hin,
request->payload->amt_to_forward,
@@ -1484,6 +1558,14 @@ static bool peer_accepted_htlc(const tal_t *ctx,
hook_payload->fwd_channel_id
= calc_forwarding_channel(ld, hook_payload);
+ if(hin->extra_tlvs) {
+ hook_payload->extra_tlvs_raw = tal_arr(hook_payload, u8, 0);
+ towire_tlvstream_raw(&hook_payload->extra_tlvs_raw,
+ hin->extra_tlvs);
+ } else {
+ hook_payload->extra_tlvs_raw = NULL;
+ }
+
plugin_hook_call_htlc_accepted(ld, NULL, hook_payload);
/* Falling through here is ok, after all the HTLC locked */
@@ -2210,6 +2292,7 @@ static bool channel_added_their_htlc(struct channel *channel,
op ? &shared_secret : NULL,
added->path_key,
added->onion_routing_packet,
+ added->extra_tlvs,
added->fail_immediate);
/* Save an incoming htlc to the wallet */
@@ -2641,13 +2724,15 @@ const struct existing_htlc **peer_htlcs(const tal_t *ctx,
else
f = NULL;
+
existing = new_existing_htlc(htlcs, hin->key.id, hin->hstate,
hin->msat, &hin->payment_hash,
hin->cltv_expiry,
hin->onion_routing_packet,
hin->path_key,
hin->preimage,
- f, NULL);
+ f,
+ hin->extra_tlvs);
tal_arr_expand(&htlcs, existing);
}
@@ -2679,7 +2764,8 @@ const struct existing_htlc **peer_htlcs(const tal_t *ctx,
hout->onion_routing_packet,
hout->path_key,
hout->preimage,
- f, NULL);
+ f,
+ hout->extra_tlvs);
tal_arr_expand(&htlcs, existing);
}
diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h
index 0ac26821..a9c73e47 100644
--- a/lightningd/peer_htlcs.h
+++ b/lightningd/peer_htlcs.h
@@ -33,6 +33,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
struct amount_msat final_msat,
const struct sha256 *payment_hash,
const struct pubkey *path_key,
+ const struct tlv_field *extra_tlvs,
u64 partid,
u64 groupid,
const u8 *onion_routing_packet,
diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
index 6f187e72..52eedf8e 100644
--- a/wallet/test/run-wallet.c
+++ b/wallet/test/run-wallet.c
@@ -365,6 +365,10 @@ bool fromwire_onchaind_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNE
/* Generated stub for fromwire_openingd_dev_memleak_reply */
bool fromwire_openingd_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED)
{ fprintf(stderr, "fromwire_openingd_dev_memleak_reply called!\n"); abort(); }
+/* Generated stub for fromwire_tlv_update_add_htlc_tlvs */
+struct tlv_update_add_htlc_tlvs *fromwire_tlv_update_add_htlc_tlvs(const tal_t *ctx UNNEEDED,
+ const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
+{ fprintf(stderr, "fromwire_tlv_update_add_htlc_tlvs called!\n"); abort(); }
/* Generated stub for get_network_blockheight */
u32 get_network_blockheight(const struct chain_topology *topo UNNEEDED)
{ fprintf(stderr, "get_network_blockheight called!\n"); abort(); }
diff --git a/wallet/wallet.c b/wallet/wallet.c
index 1b2bcaa7..3687959b 100644
--- a/wallet/wallet.c
+++ b/wallet/wallet.c
@@ -3415,6 +3415,13 @@ static bool wallet_stmt2htlc_in(struct channel *channel,
/* FIXME: save path_key in db !*/
in->path_key = NULL;
in->payload = NULL;
+ /* FIXME: save extra_tlvs in db! But: check the implications that a
+ * spammy peer - giving us big extra tlvs - would have on our database.
+ * Right now, not saving the extra tlvs in the db seems OK as it is
+ * only relevant in the case that I forward but restart in the middle
+ * of a payment.
+ */
+ in->extra_tlvs = NULL;
db_col_sha256(stmt, "payment_hash", &in->payment_hash);
@@ -3487,6 +3494,13 @@ static bool wallet_stmt2htlc_out(struct wallet *wallet,
db_col_sha256(stmt, "payment_hash", &out->payment_hash);
/* FIXME: save path_key in db !*/
out->path_key = NULL;
+ /* FIXME: save extra_tlvs in db! But: check the implications that a
+ * spammy peer - giving us big extra tlvs - would have on our database.
+ * Right now, not saving the extra tlvs in the db seems OK as it is
+ * only relevant in the case that I forward but restart in the middle
+ * of a payment.
+ */
+ out->extra_tlvs = NULL;
out->preimage = db_col_optional(out, stmt, "payment_key", preimage);
Why this scored 38/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.