From d4408a128e4b163d18e281505afd817b573916b4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:18:38 +1030 Subject: [PATCH 01/10] ccan: update to fix recent gcc "comparison will always evaluate as 'false'" warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` lightningd/jsonrpc.c: In function ‘destroy_json_command’: lightningd/jsonrpc.c:1180:63: error: the comparison will always evaluate as ‘false’ for the address of ‘canary’ will never be NULL [-Werror=address] lightningd/jsonrpc.c:108:53: note: ‘canary’ declared here ``` Signed-off-by: Rusty Russell --- ccan/README | 2 +- ccan/ccan/tcon/tcon.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ccan/README b/ccan/README index ad5b8d61fff5..17f9a28d1668 100644 --- a/ccan/README +++ b/ccan/README @@ -1,3 +1,3 @@ CCAN imported from http://ccodearchive.net. -CCAN version: init-2548-gab87e56b +CCAN version: init-2549-gba79e21b diff --git a/ccan/ccan/tcon/tcon.h b/ccan/ccan/tcon/tcon.h index e0f84b383976..df3aac88b785 100644 --- a/ccan/ccan/tcon/tcon.h +++ b/ccan/ccan/tcon/tcon.h @@ -147,8 +147,7 @@ * It evaluates to @x so you can chain it. */ #define tcon_check_ptr(x, canary, expr) \ - (sizeof(&(x)->_tcon[0].canary == (expr)) ? (x) : (x)) - + (sizeof((expr) ? (expr) : &(x)->_tcon[0].canary) ? (x) : (x)) /** * tcon_type - the type within a container (or void *) From 2d33fbd131b288faa847f94640ba3c73a36244ba Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:19:38 +1030 Subject: [PATCH 02/10] common/test: remove unused padding in bolt04/blinded-onion-message-onion-test.json This was reported by @valentinewallace: Dave would only use padding to make all his own encrypted_recipient_data equal-length. We did it across the entire path, which includes the hop added by Alice, which Dave wouldn't know about. Signed-off-by: Rusty Russell --- common/test/run-onion-message-test.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/common/test/run-onion-message-test.c b/common/test/run-onion-message-test.c index a2e3bba977d1..676397eb75ef 100644 --- a/common/test/run-onion-message-test.c +++ b/common/test/run-onion-message-test.c @@ -126,8 +126,8 @@ void ecdh(const struct pubkey *point, struct secret *ss) abort(); } -/* This established by trial and error! */ -#define LARGEST_TLV_SIZE 70 +/* This established by trial and error. */ +#define LARGEST_DAVE_TLV_SIZE 42 /* Generic, ugly, function to calc encrypted_recipient_data, alias and next blinding, and print out JSON */ @@ -175,17 +175,22 @@ static u8 *add_hop(const char *name, enctlv = tal_arr(tmpctx, u8, 0); towire_tlv_encrypted_data_tlv(&enctlv, tlv); - /* Now create padding, and reencode */ - if (tal_bytelen(enctlv) + tal_bytelen(additional) != LARGEST_TLV_SIZE) + /* Now create padding (in Dave's path) */ + if (tal_bytelen(enctlv) + tal_bytelen(additional) + < LARGEST_DAVE_TLV_SIZE) { + /* Add padding: T and L take 2 bytes, even before V */ + assert(tal_bytelen(enctlv) + tal_bytelen(additional) + 2 + <= LARGEST_DAVE_TLV_SIZE); tlv->padding = tal_arrz(tlv, u8, - LARGEST_TLV_SIZE + LARGEST_DAVE_TLV_SIZE - tal_bytelen(enctlv) - - tal_bytelen(additional) - - 2); + - tal_bytelen(additional) - 2); + } enctlv = tal_arr(tmpctx, u8, 0); towire_tlv_encrypted_data_tlv(&enctlv, tlv); towire(&enctlv, additional, tal_bytelen(additional)); - assert(tal_bytelen(enctlv) == LARGEST_TLV_SIZE); + if (!override_blinding) + assert(tal_bytelen(enctlv) == LARGEST_DAVE_TLV_SIZE); json_start("tlvs", '{'); if (tlv->padding) From 3c2a6379fb9289abaf12b3dd1e6be13923c07d6a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:20:38 +1030 Subject: [PATCH 03/10] wire: use correct number of update_add_tlvs blinding field. See: #5823 Reported-by: @t-bast Signed-off-by: Rusty Russell Changelog-EXPERIMENTAL: `offers` breaking blinded payments change (update_add_tlvs fix, Eclair compat) --- wire/extracted_peer_add_htlc-plus-blinding.patch | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wire/extracted_peer_add_htlc-plus-blinding.patch b/wire/extracted_peer_add_htlc-plus-blinding.patch index 8f0ba00c985e..0c2c02ec00f7 100644 --- a/wire/extracted_peer_add_htlc-plus-blinding.patch +++ b/wire/extracted_peer_add_htlc-plus-blinding.patch @@ -7,7 +7,7 @@ index 5a2a8c23f..7b26242e3 100644 msgdata,update_add_htlc,cltv_expiry,u32, msgdata,update_add_htlc,onion_routing_packet,byte,1366 +msgdata,update_add_htlc,tlvs,update_add_tlvs, -+tlvtype,update_add_tlvs,blinding,2 ++tlvtype,update_add_tlvs,blinding,0 +tlvdata,update_add_tlvs,blinding,blinding,point, msgtype,update_fulfill_htlc,130 msgdata,update_fulfill_htlc,channel_id,channel_id, From c02796998256922e521fede3d4fa53e692e8647f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:21:38 +1030 Subject: [PATCH 04/10] tools/check-bolt.c: don't leak open directory. Thanks valgrind! Signed-off-by: Rusty Russell --- tools/check-bolt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/check-bolt.c b/tools/check-bolt.c index 2149caf898cb..3e60510e8321 100644 --- a/tools/check-bolt.c +++ b/tools/check-bolt.c @@ -79,6 +79,7 @@ static bool get_files(const char *dir, const char *subdir, e->d_name))); tal_arr_expand(files, bf); } + closedir(d); return true; } From f5510869331f3fc8c3a82186f001959d3b7de3b5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:22:38 +1030 Subject: [PATCH 05/10] common/onion_decode: put final flag in onion_payload. You can use rs->nextcase, but we don't always keep that around, so keep a flag in onion_payload. We'll use this in the "do we need to return a blinded error code" patch. Signed-off-by: Rusty Russell --- common/onion_decode.c | 6 ++++-- common/onion_encode.h | 2 ++ lightningd/peer_htlcs.c | 9 ++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common/onion_decode.c b/common/onion_decode.c index a17c8e87ebe5..0f6419d3a991 100644 --- a/common/onion_decode.c +++ b/common/onion_decode.c @@ -151,6 +151,8 @@ struct onion_payload *onion_decode(const tal_t *ctx, const u8 *cursor = rs->raw_payload; size_t max = tal_bytelen(cursor), len; + p->final = (rs->nextcase == ONION_END); + /* BOLT-remove-legacy-onion #4: * 1. type: `hop_payloads` * 2. data: @@ -276,7 +278,7 @@ struct onion_payload *onion_decode(const tal_t *ctx, goto field_bad; } - if (rs->nextcase == ONION_FORWARD) { + if (!p->final) { if (!handle_blinded_forward(p, amount_in, cltv_expiry, p->tlv, enc, failtlvtype)) goto field_bad; @@ -333,7 +335,7 @@ struct onion_payload *onion_decode(const tal_t *ctx, * - For every non-final node: * - MUST include `short_channel_id` */ - if (rs->nextcase == ONION_FORWARD) { + if (!p->final) { if (!p->tlv->short_channel_id) { *failtlvtype = TLV_TLV_PAYLOAD_SHORT_CHANNEL_ID; goto field_bad; diff --git a/common/onion_encode.h b/common/onion_encode.h index ba48211917af..ac2129325719 100644 --- a/common/onion_encode.h +++ b/common/onion_encode.h @@ -14,6 +14,8 @@ enum onion_payload_type { struct onion_payload { enum onion_payload_type type; + /* Is this the final hop? */ + bool final; struct amount_msat amt_to_forward; u32 outgoing_cltv; diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 0ab918db9ecf..d85149788351 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -1201,17 +1201,16 @@ REGISTER_PLUGIN_HOOK(htlc_accepted, /* Figures out how to fwd, allocating return off hp */ static struct channel_id *calc_forwarding_channel(struct lightningd *ld, - struct htlc_accepted_hook_payload *hp, - const struct route_step *rs) + struct htlc_accepted_hook_payload *hp) { const struct onion_payload *p = hp->payload; struct peer *peer; struct channel *c, *best; - if (rs->nextcase != ONION_FORWARD) + if (!p) return NULL; - if (!p) + if (p->final) return NULL; if (p->forward_channel) { @@ -1402,7 +1401,7 @@ static bool peer_accepted_htlc(const tal_t *ctx, /* We don't store actual channel as it could vanish while * we're in hook */ hook_payload->fwd_channel_id - = calc_forwarding_channel(ld, hook_payload, rs); + = calc_forwarding_channel(ld, hook_payload); plugin_hook_call_htlc_accepted(ld, NULL, hook_payload); From 10b40d583342ceec8ba75260571bee532a0acedf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:23:38 +1030 Subject: [PATCH 06/10] common: update to latest route-blinding spec. ``` make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-route-blinding" BOLTVERSION=guilt/offers ``` Other than textual changes, this does: 1. Ensures we put total_amount_msat in onion final hop (reported by @t-bast). 2. Require that they put total_amount_msat in onion final hop. 3. Return `invalid_onion_blinding` exactly as defined by the spec (i.e. less aggressive when we're the final hop) (also reported by @t-bast, but I already knew). See: #5823 Signed-off-by: Rusty Russell Changelog-EXPERIMENTAL: `offers` breaking blinded payments change (total_amount_sat required, Eclair compat) --- common/blindedpay.c | 5 +-- common/blindedpay.h | 2 ++ common/onion_decode.c | 9 ++++-- common/onion_encode.c | 6 ++++ common/onion_encode.h | 3 +- common/test/run-route_blinding_onion_test.c | 2 +- lightningd/peer_htlcs.c | 36 +++++++++++++-------- plugins/libplugin-pay.c | 3 +- plugins/test/run-route-overlong.c | 1 + 9 files changed, 46 insertions(+), 21 deletions(-) diff --git a/common/blindedpay.c b/common/blindedpay.c index b542bb26a621..d959f8f55d94 100644 --- a/common/blindedpay.c +++ b/common/blindedpay.c @@ -7,6 +7,7 @@ u8 **blinded_onion_hops(const tal_t *ctx, struct amount_msat final_amount, u32 final_cltv, + struct amount_msat total_amount, const struct blinded_path *path) { u8 **onions = tal_arr(ctx, u8 *, tal_count(path->path)); @@ -25,12 +26,12 @@ u8 **blinded_onion_hops(const tal_t *ctx, * - MUST include the `blinding_point` provided by the * recipient in `current_blinding_point` * - If it is the final node: - * - MUST include `amt_to_forward` and `outgoing_cltv_value`. - * - MUST include `total_amount_msat` when using `basic_mpp`. + * - MUST include `amt_to_forward`, `outgoing_cltv_value` and `total_amount_msat`. * - MUST NOT include any other tlv field. */ onions[i] = onion_blinded_hop(onions, final ? &final_amount : NULL, + final ? &total_amount : NULL, final ? &final_cltv : NULL, path->path[i]->encrypted_recipient_data, first ? &path->blinding : NULL); diff --git a/common/blindedpay.h b/common/blindedpay.h index 5ef1b0ddf6a6..fa99483aa899 100644 --- a/common/blindedpay.h +++ b/common/blindedpay.h @@ -12,6 +12,7 @@ struct blinded_path; * @ctx: context to allocate from * @final_amount: amount we want to reach the end * @final_cltv: cltv we want to at end + * @total_amount: amount of all parts together. * @payinfo: fee and other restriction info * * This calls onion_nonfinal_hop and onion_final_hop to create onion @@ -20,6 +21,7 @@ struct blinded_path; u8 **blinded_onion_hops(const tal_t *ctx, struct amount_msat final_amount, u32 final_cltv, + struct amount_msat total_amount, const struct blinded_path *path); #endif /* LIGHTNING_COMMON_BLINDEDPAY_H */ diff --git a/common/onion_decode.c b/common/onion_decode.c index 0f6419d3a991..544ad864f75a 100644 --- a/common/onion_decode.c +++ b/common/onion_decode.c @@ -102,8 +102,8 @@ static bool handle_blinded_terminal(struct onion_payload *p, } /* BOLT-route-blinding #4: - * - MUST return an error if `amt_to_forward` or - * `outgoing_cltv_value` are not present. + * - MUST return an error if `amt_to_forward`, `outgoing_cltv_value` + * or `total_amount_msat` are not present. * - MUST return an error if `amt_to_forward` is below what it expects * for the payment. */ @@ -117,6 +117,11 @@ static bool handle_blinded_terminal(struct onion_payload *p, return false; } + if (!tlv->total_amount_msat) { + *failtlvtype = TLV_TLV_PAYLOAD_TOTAL_AMOUNT_MSAT; + return false; + } + p->amt_to_forward = amount_msat(*tlv->amt_to_forward); p->outgoing_cltv = *tlv->outgoing_cltv_value; diff --git a/common/onion_encode.c b/common/onion_encode.c index 711b8dc9c76a..f5facf75ec7f 100644 --- a/common/onion_encode.c +++ b/common/onion_encode.c @@ -103,6 +103,7 @@ u8 *onion_final_hop(const tal_t *ctx, u8 *onion_blinded_hop(const tal_t *ctx, const struct amount_msat *amt_to_forward, + const struct amount_msat *total_amount_msat, const u32 *outgoing_cltv_value, const u8 *enctlv, const struct pubkey *blinding) @@ -114,6 +115,11 @@ u8 *onion_blinded_hop(const tal_t *ctx, = cast_const(u64 *, &amt_to_forward->millisatoshis); /* Raw: TLV convert */ } + if (total_amount_msat) { + tlv->total_amount_msat + = cast_const(u64 *, + &total_amount_msat->millisatoshis); /* Raw: TLV convert */ + } tlv->outgoing_cltv_value = cast_const(u32 *, outgoing_cltv_value); tlv->encrypted_recipient_data = cast_const(u8 *, enctlv); tlv->blinding_point = cast_const(struct pubkey *, blinding); diff --git a/common/onion_encode.h b/common/onion_encode.h index ac2129325719..2892cbc8c748 100644 --- a/common/onion_encode.h +++ b/common/onion_encode.h @@ -53,8 +53,9 @@ u8 *onion_final_hop(const tal_t *ctx, * generic interface, as used by blindedpay.h */ u8 *onion_blinded_hop(const tal_t *ctx, const struct amount_msat *amt_to_forward, + const struct amount_msat *total_amount_msat, const u32 *outgoing_cltv_value, const u8 *enctlv, const struct pubkey *blinding) - NON_NULL_ARGS(4); + NON_NULL_ARGS(5); #endif /* LIGHTNING_COMMON_ONION_ENCODE_H */ diff --git a/common/test/run-route_blinding_onion_test.c b/common/test/run-route_blinding_onion_test.c index b2239772774e..f3eec3f17172 100644 --- a/common/test/run-route_blinding_onion_test.c +++ b/common/test/run-route_blinding_onion_test.c @@ -106,7 +106,7 @@ int main(int argc, char *argv[]) } /* FIXME: These amounts / scid should be in test vectors! */ - onionhops = blinded_onion_hops(tmpctx, AMOUNT_MSAT(200), 700, bpath); + onionhops = blinded_onion_hops(tmpctx, AMOUNT_MSAT(200), 700, AMOUNT_MSAT(200), bpath); assert(mk_short_channel_id(&initscid, 0, 0, 10)); /* Prepend Alice: poor thing doesn't speak blinding! */ diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index d85149788351..039a2c931116 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -92,20 +92,33 @@ static bool htlc_out_update_state(struct channel *channel, return true; } +/* BOLT-route-blinding #4: + * - if `blinding_point` is set in the incoming `update_add_htlc`: + * - MUST return an `invalid_onion_blinding` error. + * - if `current_blinding_point` is set in the onion payload and it is not the + * final node: + * - MUST return an `invalid_onion_blinding` error. + */ +static bool blind_error_return(const struct htlc_in *hin) +{ + if (hin->blinding) + return true; + + if (hin->payload + && hin->payload->blinding + && !hin->payload->final) + return true; + + return false; +} + static struct failed_htlc *mk_failed_htlc_badonion(const tal_t *ctx, const struct htlc_in *hin, enum onion_wire badonion) { struct failed_htlc *f = tal(ctx, struct failed_htlc); - /* BOLT-route-blinding #4: - * - If `blinding_point` is set in the incoming `update_add_htlc`: - * - MUST return `invalid_onion_blinding` for any local error or - * other downstream errors. - */ - /* FIXME: That's not enough! Don't leak information about forward - * failures either! */ - if (hin->blinding || (hin->payload && hin->payload->blinding)) + if (blind_error_return(hin)) badonion = WIRE_INVALID_ONION_BLINDING; f->id = hin->key.id; @@ -123,12 +136,7 @@ static struct failed_htlc *mk_failed_htlc(const tal_t *ctx, { struct failed_htlc *f = tal(ctx, struct failed_htlc); - /* BOLT-route-blinding #4: - * - If `blinding_point` is set in the incoming `update_add_htlc`: - * - MUST return `invalid_onion_blinding` for any local error or - * other downstream errors. - */ - if (hin->blinding) { + if (blind_error_return(hin)) { return mk_failed_htlc_badonion(ctx, hin, WIRE_INVALID_ONION_BLINDING); } diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 30ac1f700c7e..06fd7123d704 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -1671,7 +1671,8 @@ static void payment_add_blindedpath(const tal_t *ctx, { /* It's a bit of a weird API for us, so we convert it back to * the struct tlv_tlv_payload */ - u8 **tlvs = blinded_onion_hops(tmpctx, final_amt, final_cltv, bpath); + u8 **tlvs = blinded_onion_hops(tmpctx, final_amt, final_cltv, + final_amt, bpath); for (size_t i = 0; i < tal_count(tlvs); i++) { const u8 *cursor = tlvs[i]; diff --git a/plugins/test/run-route-overlong.c b/plugins/test/run-route-overlong.c index a20778629a36..adeace84a13f 100644 --- a/plugins/test/run-route-overlong.c +++ b/plugins/test/run-route-overlong.c @@ -12,6 +12,7 @@ u8 **blinded_onion_hops(const tal_t *ctx UNNEEDED, struct amount_msat final_amount UNNEEDED, u32 final_cltv UNNEEDED, + struct amount_msat total_amount UNNEEDED, const struct blinded_path *path UNNEEDED) { fprintf(stderr, "blinded_onion_hops called!\n"); abort(); } /* Generated stub for command_finished */ From 5fc68e784d344bb2906c3246fdb7e96b4d69c0a0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:24:38 +1030 Subject: [PATCH 07/10] common/test: fix up name of test file to match latest version. Otherwise it's skipped! Signed-off-by: Rusty Russell --- common/test/run-route_blinding_onion_test.c | 100 +++++++++++++++++--- 1 file changed, 89 insertions(+), 11 deletions(-) diff --git a/common/test/run-route_blinding_onion_test.c b/common/test/run-route_blinding_onion_test.c index f3eec3f17172..df9452c3f01a 100644 --- a/common/test/run-route_blinding_onion_test.c +++ b/common/test/run-route_blinding_onion_test.c @@ -44,6 +44,18 @@ bool pubkey_from_node_id(struct pubkey *key UNNEEDED, const struct node_id *id U { fprintf(stderr, "pubkey_from_node_id called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ +#if 0 +/* Updated each time, as we pretend to be Alice, Bob, Carol */ +static struct secret mykey; + +static void test_ecdh(const struct pubkey *point, struct secret *ss) +{ + if (secp256k1_ecdh(secp256k1_ctx, ss->data, &point->pubkey, + mykey.data, NULL, NULL) != 1) + abort(); +} +#endif + static bool json_to_tok(const char *buffer, const jsmntok_t *tok, const jsmntok_t **tokp) { @@ -64,6 +76,9 @@ int main(int argc, char *argv[]) struct short_channel_id initscid; struct sphinx_path *sp; struct secret session_key, *path_secrets; + u32 final_cltv; + struct amount_msat initial_amount, final_amount; + u32 path_fee_base_msat, path_fee_proportional_millionths, path_cltv_delta; common_setup(argv[0]); @@ -74,7 +89,7 @@ int main(int argc, char *argv[]) json = grab_file(tmpctx, path_join(tmpctx, dir ? dir : "../bolts", - "bolt04/onion-route-blinding-test.json")); + "bolt04/blinded-payment-onion-test.json")); if (!json) { printf("test file not found, skipping\n"); goto out; @@ -87,9 +102,20 @@ int main(int argc, char *argv[]) bpath = tal(tmpctx, struct blinded_path); - assert(json_scan(tmpctx, json, toks, "{generate:{session_key:%,associated_data:%,blinded_route:{introduction_node_id:%,blinding:%,hops:%}}}", + assert(json_scan(tmpctx, json, toks, + "{generate:{session_key:%," + "associated_data:%," + "final_amount_msat:%," + "final_cltv:%," + "blinded_payinfo:{fee_base_msat:%,fee_proportional_millionths:%,cltv_expiry_delta:%}," + "blinded_route:{introduction_node_id:%,blinding:%,hops:%}}}", JSON_SCAN(json_to_secret, &session_key), JSON_SCAN_TAL(tmpctx, json_tok_bin_from_hex, &associated_data), + JSON_SCAN(json_to_msat, &final_amount), + JSON_SCAN(json_to_u32, &final_cltv), + JSON_SCAN(json_to_u32, &path_fee_base_msat), + JSON_SCAN(json_to_u32, &path_fee_proportional_millionths), + JSON_SCAN(json_to_u32, &path_cltv_delta), JSON_SCAN(json_to_pubkey, &bpath->first_node_id), JSON_SCAN(json_to_pubkey, &bpath->blinding), JSON_SCAN(json_to_tok, &hops_tok)) == NULL); @@ -105,19 +131,28 @@ int main(int argc, char *argv[]) &bpath->path[i]->encrypted_recipient_data)) == NULL); } - /* FIXME: These amounts / scid should be in test vectors! */ - onionhops = blinded_onion_hops(tmpctx, AMOUNT_MSAT(200), 700, AMOUNT_MSAT(200), bpath); - assert(mk_short_channel_id(&initscid, 0, 0, 10)); + assert(json_scan(tmpctx, json, toks, "{generate:{full_route:{hops:%}}}", + JSON_SCAN(json_to_tok, &hops_tok)) == NULL); + + /* We have to read scid from first hop contents, since it's made up */ + assert(json_scan(tmpctx, json, hops_tok + 1, "{tlvs:{outgoing_channel_id:%}}", + JSON_SCAN(json_to_short_channel_id, &initscid)) == NULL); + + initial_amount = final_amount; + assert(amount_msat_add_fee(&initial_amount, + path_fee_base_msat, path_fee_proportional_millionths)); + + /* FIXME: Test vector actually claims total_amount_msat is 150000msat! */ + struct amount_msat total_amount; + assert(amount_msat_add(&total_amount, final_amount, AMOUNT_MSAT(50000))); + onionhops = blinded_onion_hops(tmpctx, final_amount, final_cltv, total_amount, bpath); - /* Prepend Alice: poor thing doesn't speak blinding! */ + /* Prepend Alice: poor thing doesn't speak blinding! (But doesn't charge fees!) */ tal_resize(&onionhops, tal_count(onionhops) + 1); memmove(onionhops + 1, onionhops, (tal_count(onionhops) - 1) * sizeof(*onionhops)); onionhops[0] = onion_nonfinal_hop(onionhops, &initscid, - AMOUNT_MSAT(500), 1000); - - assert(json_scan(tmpctx, json, toks, "{generate:{full_route:{hops:%}}}", - JSON_SCAN(json_to_tok, &hops_tok)) == NULL); + initial_amount, final_cltv + path_cltv_delta); ids = tal_arr(tmpctx, struct pubkey, hops_tok->size); json_for_each_arr(i, t, hops_tok) { @@ -134,7 +169,7 @@ int main(int argc, char *argv[]) /* Now, create onion! */ sp = sphinx_path_new_with_key(tmpctx, associated_data, &session_key); for (i = 0; i < tal_count(ids); i++) - sphinx_add_hop(sp, &ids[i], onionhops[i]); + sphinx_add_hop_has_length(sp, &ids[i], onionhops[i]); onion = serialize_onionpacket(tmpctx, create_onionpacket(tmpctx, sp, ROUTING_INFO_SIZE, @@ -147,6 +182,49 @@ int main(int argc, char *argv[]) onion, tal_bytelen(onion))); /* FIXME: unwrap and test! */ +#if 0 + struct onionpacket *op; + struct pubkey *blinding; + + assert(json_scan(tmpctx, json, toks, "{decrypt:{hops:%}}", + JSON_SCAN(json_to_tok, &hops_tok)) == NULL); + op = parse_onionpacket(tmpctx, expected_onion, tal_bytelen(expected_onion), NULL); + blinding = NULL; + json_for_each_arr(i, t, hops_tok) { + struct route_step *rs; + struct secret ss; + const u8 *serialized; + + assert(json_scan(tmpctx, json, t, "{node_privkey:%,onion:%}", + JSON_SCAN(json_to_secret, &mykey), + JSON_SCAN_TAL(tmpctx, json_tok_bin_from_hex, &expected_onion)) + == NULL); + serialized = serialize_onionpacket(tmpctx, op); + assert(memeq(expected_onion, tal_bytelen(expected_onion), + serialized, tal_bytelen(serialized))); + + if (blinding) { + assert(unblind_onion(blinding, test_ecdh, + &op->ephemeralkey, &ss)); + } else { + test_ecdh(&op->ephemeralkey, &ss); + } + rs = process_onionpacket(tmpctx, op, &ss, associated_data, + tal_bytelen(associated_data), true); + assert(memeq(rs->raw_payload, tal_bytelen(rs->raw_payload), + onionhops[i], tal_bytelen(onionhops[i]))); + if (rs->nextcase == ONION_FORWARD) + op = rs->next; + else + op = NULL; + blinding = tal(tmpctx, struct pubkey); + /* Alice doesn't have a blinding! */ + if (json_scan(tmpctx, json, t, "{next_blinding:%}", + JSON_SCAN(json_to_pubkey, blinding)) != NULL) + blinding = NULL; + } + assert(!op); +#endif out: common_shutdown(); From a202ff32889a97ea2a5411b5481c562618df4c7b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:25:38 +1030 Subject: [PATCH 08/10] common: update to latest onion-message spec. ``` make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-onion-message" BOLTVERSION=guilt/offers ``` Mainly textual, though I neatened the extra fields check for TLVs with blinding, and implemented the "no other fields" requirement for non-final onion message hops. Signed-off-by: Rusty Russell --- common/blindedpath.c | 5 +- common/onion_decode.c | 80 ++++++++++++++++++---------- common/onion_message_parse.c | 32 ++++++----- common/test/run-onion-message-test.c | 2 +- lightningd/onion_message.c | 2 +- 5 files changed, 75 insertions(+), 46 deletions(-) diff --git a/common/blindedpath.c b/common/blindedpath.c index 46176eab1360..44a1d7ed9827 100644 --- a/common/blindedpath.c +++ b/common/blindedpath.c @@ -205,8 +205,9 @@ struct tlv_encrypted_data_tlv *decrypt_encrypted_data(const tal_t *ctx, /* BOLT-onion-message #4: * - * - if the `enctlv` is not a valid TLV... - * - MUST drop the message. + * - MUST return an error if `encrypted_recipient_data` does not decrypt + * using the blinding point as described in + * [Route Blinding](#route-blinding). */ /* Note: our parser consider nothing is a valid TLV, but decrypt_encmsg_raw * returns NULL if it couldn't decrypt. */ diff --git a/common/onion_decode.c b/common/onion_decode.c index 544ad864f75a..77b78a25103b 100644 --- a/common/onion_decode.c +++ b/common/onion_decode.c @@ -9,6 +9,54 @@ #include #include +/* BOLT-route-blinding #4: + * - If `encrypted_recipient_data` is present: + *... + * - If it is not the final node: + * - MUST return an error if the payload contains other tlv fields than + * `encrypted_recipient_data` and `current_blinding_point`. + */ +static bool check_nonfinal_tlv(const struct tlv_tlv_payload *tlv, + u64 *failtlvtype) +{ + for (size_t i = 0; i < tal_count(tlv->fields); i++) { + switch (tlv->fields[i].numtype) { + case TLV_TLV_PAYLOAD_BLINDING_POINT: + case TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA: + continue; + } + *failtlvtype = tlv->fields[i].numtype; + return false; + } + return true; +} + +/* BOLT-route-blinding #4: + * - If `encrypted_recipient_data` is present: + *... + * - If it is the final node: + * - MUST return an error if the payload contains other tlv fields than + * `encrypted_recipient_data`, `current_blinding_point`, `amt_to_forward`, + * `outgoing_cltv_value` and `total_amount_msat`. + */ +static bool check_final_tlv(const struct tlv_tlv_payload *tlv, + u64 *failtlvtype) +{ + for (size_t i = 0; i < tal_count(tlv->fields); i++) { + switch (tlv->fields[i].numtype) { + case TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA: + case TLV_TLV_PAYLOAD_BLINDING_POINT: + case TLV_TLV_PAYLOAD_AMT_TO_FORWARD: + case TLV_TLV_PAYLOAD_OUTGOING_CLTV_VALUE: + case TLV_TLV_PAYLOAD_TOTAL_AMOUNT_MSAT: + continue; + } + *failtlvtype = tlv->fields[i].numtype; + return false; + } + return true; +} + static u64 ceil_div(u64 a, u64 b) { return (a + b - 1) / b; @@ -23,18 +71,8 @@ static bool handle_blinded_forward(struct onion_payload *p, { u64 amt = amount_in.millisatoshis; /* Raw: allowed to wrap */ - /* BOLT-route-blinding #4: - * - If it is not the final node: - * - MUST return an error if the payload contains other tlv fields - * than `encrypted_recipient_data` and `current_blinding_point`. - */ - for (size_t i = 0; i < tal_count(tlv->fields); i++) { - if (tlv->fields[i].numtype != TLV_TLV_PAYLOAD_BLINDING_POINT - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA) { - *failtlvtype = tlv->fields[i].numtype; - return false; - } - } + if (!check_nonfinal_tlv(tlv, failtlvtype)) + return false; /* BOLT-route-blinding #4: * - If it is not the final node: @@ -84,22 +122,8 @@ static bool handle_blinded_terminal(struct onion_payload *p, const struct tlv_encrypted_data_tlv *enc, u64 *failtlvtype) { - /* BOLT-route-blinding #4: - * - If it is the final node: - * - MUST return an error if the payload contains other tlv fields than - * `encrypted_recipient_data`, `current_blinding_point`, `amt_to_forward`, - * `outgoing_cltv_value` and `total_amount_msat`. - */ - for (size_t i = 0; i < tal_count(tlv->fields); i++) { - if (tlv->fields[i].numtype != TLV_TLV_PAYLOAD_BLINDING_POINT - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_AMT_TO_FORWARD - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_OUTGOING_CLTV_VALUE - && tlv->fields[i].numtype != TLV_TLV_PAYLOAD_TOTAL_AMOUNT_MSAT) { - *failtlvtype = tlv->fields[i].numtype; - return false; - } - } + if (!check_final_tlv(tlv, failtlvtype)) + return false; /* BOLT-route-blinding #4: * - MUST return an error if `amt_to_forward`, `outgoing_cltv_value` diff --git a/common/onion_message_parse.c b/common/onion_message_parse.c index 311c33ef5ec1..8bf4ef82ce44 100644 --- a/common/onion_message_parse.c +++ b/common/onion_message_parse.c @@ -50,25 +50,19 @@ static bool decrypt_forwarding_onionmsg(const struct pubkey *blinding, return false; /* BOLT-onion-message #4: - * - * The reader: * - if it is not the final node according to the onion encryption: *... - * - if the `enctlv` ... does not contain - * `next_node_id`: - * - MUST drop the message. + * - if the `encrypted_data_tlv` contains `path_id`: + * - MUST ignore the message. */ - if (!encmsg->next_node_id) + if (encmsg->path_id) return false; /* BOLT-onion-message #4: - * The reader: - * - if it is not the final node according to the onion encryption: - *... - * - if the `enctlv` contains `path_id`: - * - MUST drop the message. + * - SHOULD forward the message using `onion_message` to the next peer + * indicated by `next_node_id`. */ - if (encmsg->path_id) + if (!encmsg->next_node_id) return false; *next_node = *encmsg->next_node_id; @@ -145,7 +139,6 @@ bool onion_message_parse(const tal_t *ctx, tal_hex(tmpctx, rs->raw_payload)); return false; } - if (rs->nextcase == ONION_END) { *next_onion_msg = NULL; *final_om = tal_steal(ctx, om); @@ -167,6 +160,18 @@ bool onion_message_parse(const tal_t *ctx, *final_om = NULL; + /* BOLT-onion-message #4: + * - if it is not the final node according to the onion encryption: + * - if the `onionmsg_tlv` contains other tlv fields than `encrypted_recipient_data`: + * - MUST ignore the message. + */ + if (tal_count(om->fields) != 1) { + status_peer_debug(peer, + "onion_message_parse: " + "disallowed tlv field"); + return false; + } + /* This fails as expected if no enctlv. */ if (!decrypt_forwarding_onionmsg(blinding, &ss, om->encrypted_recipient_data, next_node_id, &next_blinding)) { @@ -175,7 +180,6 @@ bool onion_message_parse(const tal_t *ctx, tal_hex(tmpctx, om->encrypted_recipient_data)); return false; } - *next_onion_msg = towire_onion_message(ctx, &next_blinding, serialize_onionpacket(tmpctx, rs->next)); diff --git a/common/test/run-onion-message-test.c b/common/test/run-onion-message-test.c index 676397eb75ef..c3ef156b8786 100644 --- a/common/test/run-onion-message-test.c +++ b/common/test/run-onion-message-test.c @@ -337,7 +337,7 @@ int main(int argc, char *argv[]) sphinx_path->session_key = &session_key; /* BOLT-onion-message #4: - * - SHOULD set `len` to 1366 or 32834. + * - SHOULD set `onion_message_packet` `len` to 1366 or 32834. */ op = create_onionpacket(tmpctx, sphinx_path, ROUTING_INFO_SIZE, &path_secrets); diff --git a/lightningd/onion_message.c b/lightningd/onion_message.c index 7316b4346f4b..58b693da4943 100644 --- a/lightningd/onion_message.c +++ b/lightningd/onion_message.c @@ -216,7 +216,7 @@ static struct command_result *json_sendonionmessage(struct command *cmd, sphinx_add_hop(sphinx_path, &hops[i].node, hops[i].tlv); /* BOLT-onion-message #4: - * - SHOULD set `len` to 1366 or 32834. + * - SHOULD set `onion_message_packet` `len` to 1366 or 32834. */ if (sphinx_path_payloads_size(sphinx_path) <= ROUTING_INFO_SIZE) onion_size = ROUTING_INFO_SIZE; From a15e9ed489227242023562fec3a6479e28e453b3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:26:38 +1030 Subject: [PATCH 09/10] plugins: update to match latest offers text. ``` make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-offers" BOLTVERSION=guilt/offers ``` In this case, only trivial mods. Signed-off-by: Rusty Russell --- plugins/fetchinvoice.c | 2 +- plugins/offers_invreq_hook.c | 2 +- plugins/offers_offer.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c index a70577065ce8..89f7b38d41d4 100644 --- a/plugins/fetchinvoice.c +++ b/plugins/fetchinvoice.c @@ -1478,7 +1478,7 @@ static struct command_result *json_sendinvoice(struct command *cmd, /* BOLT-offers #12: * - MUST set `invoice_created_at` to the number of seconds since Midnight 1 - * January 1970, UTC when the offer was created. + * January 1970, UTC when the invoice was created. * - MUST set `invoice_amount` to the minimum amount it will accept, in units of * the minimal lightning-payable unit (e.g. milli-satoshis for bitcoin) for * `invreq_chain`. diff --git a/plugins/offers_invreq_hook.c b/plugins/offers_invreq_hook.c index 9d79895685b8..8091759bcf63 100644 --- a/plugins/offers_invreq_hook.c +++ b/plugins/offers_invreq_hook.c @@ -963,7 +963,7 @@ static struct command_result *listoffers_done(struct command *cmd, /* BOLT-offers #12: * - MUST set `invoice_created_at` to the number of seconds since - * Midnight 1 January 1970, UTC when the offer was created. + * Midnight 1 January 1970, UTC when the invoice was created. */ ir->inv->invoice_created_at = tal(ir->inv, u64); *ir->inv->invoice_created_at = time_now().ts.tv_sec; diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c index e2ba3d81d91f..d4ec501df236 100644 --- a/plugins/offers_offer.c +++ b/plugins/offers_offer.c @@ -415,9 +415,9 @@ struct command_result *json_invoicerequest(struct command *cmd, /* BOLT-offers #12: * - otherwise (not responding to an offer): - * - MUST set (or not set) `offer_metadata`, `offer_description`, `offer_absolute_expiry`, `offer_paths` and `offer_issuer` as it would for an offer. + * - MUST set (or not set) `offer_description`, `offer_absolute_expiry`, `offer_paths` and `offer_issuer` as it would for an offer. * - MUST set `invreq_payer_id` as it would set `offer_node_id` for an offer. - * - MUST NOT include `signature`, `offer_chains`, `offer_amount`, `offer_currency`, `offer_features`, `offer_quantity_max` or `offer_node_id` + * - MUST NOT include `signature`, `offer_metadata`, `offer_chains`, `offer_amount`, `offer_currency`, `offer_features`, `offer_quantity_max` or `offer_node_id` * - if the chain for the invoice is not solely bitcoin: * - MUST specify `invreq_chain` the offer is valid for. * - MUST set `invreq_amount`. From 3d1f620d83ff4feaaff305ef1a34f2f84153acc5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 12 Jan 2023 14:28:03 +1030 Subject: [PATCH 10/10] decode: fix handling of blinded_payinfo. Our pay code handles this correctly, but decode was still using an old model where there was a payinfo per hop, not per path. Reported-by: @t-bast See: #5823 Signed-off-by: Rusty Russell --- doc/lightning-decode.7.md | 11 +++++---- doc/schemas/decode.schema.json | 45 ++++++++++++++++++++++------------ plugins/offers.c | 39 +++++++++++++++-------------- 3 files changed, 55 insertions(+), 40 deletions(-) diff --git a/doc/lightning-decode.7.md b/doc/lightning-decode.7.md index a8a78096c1d5..78a5578ddaa8 100644 --- a/doc/lightning-decode.7.md +++ b/doc/lightning-decode.7.md @@ -148,13 +148,14 @@ If **type** is "bolt12 invoice", and **valid** is *true*: - **invoice\_paths** (array of objects): Paths to pay the destination: - **first\_node\_id** (pubkey): the (presumably well-known) public key of the start of the path - **blinding** (pubkey): blinding factor for this path + - **payinfo** (object): + - **fee\_base\_msat** (msat): basefee for path + - **fee\_proportional\_millionths** (u32): proportional fee for path + - **cltv\_expiry\_delta** (u32): CLTV delta for path + - **features** (hex): features allowed for path - **path** (array of objects): an individual path: - **blinded\_node\_id** (pubkey): node\_id of the hop - **encrypted\_recipient\_data** (hex): encrypted TLV entry for this hop - - **fee\_base\_msat** (msat, optional): basefee for path - - **fee\_proportional\_millionths** (u32, optional): proportional fee for path - - **cltv\_expiry\_delta** (u32, optional): CLTV delta for path - - **features** (hex, optional): features allowed for path - **invoice\_created\_at** (u64): the UNIX timestamp of invoice creation - **invoice\_payment\_hash** (hex): the hash of the *payment\_preimage* (always 64 characters) - **invoice\_amount\_msat** (msat): the amount required to fulfill invoice @@ -302,4 +303,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:eadd9b06e6cb495a794568f3a9e2371c09718311c0b61ad05b0fca3014c429d3) +[comment]: # ( SHA256STAMP:a8843027b18a1d54efcf0ca423fc6fbe0c2136d32fa3d8c552a875b6844dd950) diff --git a/doc/schemas/decode.schema.json b/doc/schemas/decode.schema.json index ca5d94358e7a..9635b5693bad 100644 --- a/doc/schemas/decode.schema.json +++ b/doc/schemas/decode.schema.json @@ -927,6 +927,7 @@ "required": [ "first_node_id", "blinding", + "payinfo", "path" ], "additionalProperties": false, @@ -939,6 +940,34 @@ "type": "pubkey", "description": "blinding factor for this path" }, + "payinfo": { + "type": "object", + "required": [ + "fee_base_msat", + "fee_proportional_millionths", + "cltv_expiry_delta", + "features" + ], + "additionalProperties": false, + "properties": { + "fee_base_msat": { + "type": "msat", + "description": "basefee for path" + }, + "fee_proportional_millionths": { + "type": "u32", + "description": "proportional fee for path" + }, + "cltv_expiry_delta": { + "type": "u32", + "description": "CLTV delta for path" + }, + "features": { + "type": "hex", + "description": "features allowed for path" + } + } + }, "path": { "type": "array", "description": "an individual path", @@ -957,22 +986,6 @@ "encrypted_recipient_data": { "type": "hex", "description": "encrypted TLV entry for this hop" - }, - "fee_base_msat": { - "type": "msat", - "description": "basefee for path" - }, - "fee_proportional_millionths": { - "type": "u32", - "description": "proportional fee for path" - }, - "cltv_expiry_delta": { - "type": "u32", - "description": "CLTV delta for path" - }, - "features": { - "type": "hex", - "description": "features allowed for path" } } } diff --git a/plugins/offers.c b/plugins/offers.c index a34e0f07a7d2..5dadd97ed1af 100644 --- a/plugins/offers.c +++ b/plugins/offers.c @@ -249,21 +249,11 @@ static void json_add_chains(struct json_stream *js, static void json_add_onionmsg_path(struct json_stream *js, const char *fieldname, - const struct onionmsg_hop *hop, - const struct blinded_payinfo *payinfo) + const struct onionmsg_hop *hop) { json_object_start(js, fieldname); json_add_pubkey(js, "blinded_node_id", &hop->blinded_node_id); json_add_hex_talarr(js, "encrypted_recipient_data", hop->encrypted_recipient_data); - if (payinfo) { - json_add_amount_msat_only(js, "fee_base_msat", - amount_msat(payinfo->fee_base_msat)); - json_add_u32(js, "fee_proportional_millionths", - payinfo->fee_proportional_millionths); - json_add_u32(js, "cltv_expiry_delta", - payinfo->cltv_expiry_delta); - json_add_hex_talarr(js, "features", payinfo->features); - } json_object_end(js); } @@ -273,18 +263,28 @@ static bool json_add_blinded_paths(struct json_stream *js, struct blinded_path **paths, struct blinded_payinfo **blindedpay) { - size_t n = 0; json_array_start(js, fieldname); for (size_t i = 0; i < tal_count(paths); i++) { json_object_start(js, NULL); json_add_pubkey(js, "first_node_id", &paths[i]->first_node_id); json_add_pubkey(js, "blinding", &paths[i]->blinding); + + /* Don't crash if we're short a payinfo! */ + if (i < tal_count(blindedpay)) { + json_object_start(js, "payinfo"); + json_add_amount_msat_only(js, "fee_base_msat", + amount_msat(blindedpay[i]->fee_base_msat)); + json_add_u32(js, "fee_proportional_millionths", + blindedpay[i]->fee_proportional_millionths); + json_add_u32(js, "cltv_expiry_delta", + blindedpay[i]->cltv_expiry_delta); + json_add_hex_talarr(js, "features", blindedpay[i]->features); + json_object_end(js); + } + json_array_start(js, "path"); for (size_t j = 0; j < tal_count(paths[i]->path); j++) { - json_add_onionmsg_path(js, NULL, paths[i]->path[j], - n < tal_count(blindedpay) - ? blindedpay[n] : NULL); - n++; + json_add_onionmsg_path(js, NULL, paths[i]->path[j]); } json_array_end(js); json_object_end(js); @@ -295,9 +295,10 @@ static bool json_add_blinded_paths(struct json_stream *js, * - MUST reject the invoice if `invoice_blindedpay` does not contain * exactly one `blinded_payinfo` per `invoice_paths`.`blinded_path`. */ - if (blindedpay && n != tal_count(blindedpay)) { - json_add_string(js, "warning_invalid_invoice_blindedpay", - "invoice does not have correct number of blinded_payinfo"); + if (blindedpay && tal_count(blindedpay) != tal_count(paths)) { + json_add_str_fmt(js, "warning_invalid_invoice_blindedpay", + "invoice has %zu blinded_payinfo but %zu paths", + tal_count(blindedpay), tal_count(paths)); return false; }