From 88024fa8d6c0e8171fb8f3c52ebee1800d6f91cb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Jul 2024 11:04:55 +0930 Subject: [PATCH] common/bolt12, offers plugin: handle experimental ranges in bolt12 correctly. The latest draft allows these experimental ranges, which involves more changes than I expected. Signed-off-by: Rusty Russell Changelog-EXPERIMENTAL: offers: handle experimental ranges in offers/invoice_requests/invoices. --- common/bolt12.c | 117 ++++++++++++++++++++++----- common/bolt12.h | 14 ++++ common/test/run-bolt12-encode-test.c | 2 +- plugins/fetchinvoice.c | 33 ++++---- plugins/offers_inv_hook.c | 4 +- plugins/offers_invreq_hook.c | 17 ++-- tests/test_pay.py | 21 +++++ 7 files changed, 160 insertions(+), 48 deletions(-) diff --git a/common/bolt12.c b/common/bolt12.c index 86f74a77a..5e81b1de1 100644 --- a/common/bolt12.c +++ b/common/bolt12.c @@ -171,6 +171,7 @@ struct tlv_offer *offer_decode(const tal_t *ctx, struct tlv_offer *offer; const u8 *data; size_t dlen; + const struct tlv_field *badf; data = string_to_data(tmpctx, b12, b12len, "lno", &dlen, fail); if (!data) @@ -193,7 +194,7 @@ struct tlv_offer *offer_decode(const tal_t *ctx, /* BOLT-offers #12: * A reader of an offer: - * - if the offer contains any TLV fields greater or equal to 80: + * - if the offer contains any TLV fields outside the inclusive ranges: 1 to 79 and 1000000000 to 1999999999: * - MUST NOT respond to the offer. * - if `offer_features` contains unknown _odd_ bits that are non-zero: * - MUST ignore the bit. @@ -201,13 +202,14 @@ struct tlv_offer *offer_decode(const tal_t *ctx, * - MUST NOT respond to the offer. * - SHOULD indicate the unknown bit to the user. */ - for (size_t i = 0; i < tal_count(offer->fields); i++) { - if (offer->fields[i].numtype > 80) { - *fail = tal_fmt(ctx, - "Offer %"PRIu64" field >= 80", - offer->fields[i].numtype); - return tal_free(offer); - } + badf = any_field_outside_range(offer->fields, false, + 1, 79, + 1000000000, 1999999999); + if (badf) { + *fail = tal_fmt(ctx, + "Offer %"PRIu64" field outside offer range", + badf->numtype); + return tal_free(offer); } /* BOLT-offers #12: @@ -248,6 +250,7 @@ struct tlv_invoice_request *invrequest_decode(const tal_t *ctx, struct tlv_invoice_request *invrequest; const u8 *data; size_t dlen; + const struct tlv_field *badf; data = string_to_data(tmpctx, b12, b12len, "lnr", &dlen, fail); if (!data) @@ -267,6 +270,21 @@ struct tlv_invoice_request *invrequest_decode(const tal_t *ctx, if (*fail) return tal_free(invrequest); + /* BOLT-offers #12: + * The reader: + *... + * - MUST fail the request if any non-signature TLV fields outside the inclusive ranges: 0 to 159 and 1000000000 to 2999999999 + */ + badf = any_field_outside_range(invrequest->fields, true, + 0, 159, + 1000000000, 2999999999); + if (badf) { + *fail = tal_fmt(ctx, + "Invoice request %"PRIu64" field outside invoice request range", + badf->numtype); + return tal_free(invrequest); + } + return invrequest; } @@ -497,14 +515,20 @@ size_t tlv_span(const u8 *tlvstream, u64 minfield, u64 maxfield, static void calc_offer(const u8 *tlvstream, struct sha256 *id) { - size_t start, len; + size_t start1, len1, start2, len2; + struct sha256_ctx ctx; /* BOLT-offers #12: * A writer of an offer: - * - MUST NOT set any tlv fields greater or equal to 80, or tlv field 0. + * - MUST NOT set any TLV fields outside the inclusive ranges: 1 to 79 and 1000000000 to 1999999999. */ - len = tlv_span(tlvstream, 1, 79, &start); - sha256(id, tlvstream + start, len); + len1 = tlv_span(tlvstream, 1, 79, &start1); + len2 = tlv_span(tlvstream, 1000000000, 1999999999, &start2); + + sha256_init(&ctx); + sha256_update(&ctx, tlvstream + start1, len1); + sha256_update(&ctx, tlvstream + start2, len2); + sha256_done(&ctx, id); } void offer_offer_id(const struct tlv_offer *offer, struct sha256 *id) @@ -533,15 +557,21 @@ void invoice_offer_id(const struct tlv_invoice *invoice, struct sha256 *id) static void calc_invreq(const u8 *tlvstream, struct sha256 *id) { - size_t start, len; + size_t start1, len1, start2, len2; + struct sha256_ctx ctx; /* BOLT-offers #12: - * - if the invoice is a response to an `invoice_request`: - * - MUST reject the invoice if all fields less than type 160 - * do not exactly match the `invoice_request`. + * The writer: + *... + * - MUST NOT set any non-signature TLV fields outside the inclusive ranges: 0 to 159 and 1000000000 to 2999999999 */ - len = tlv_span(tlvstream, 0, 159, &start); - sha256(id, tlvstream + start, len); + len1 = tlv_span(tlvstream, 0, 159, &start1); + len2 = tlv_span(tlvstream, 1000000000, 2999999999, &start2); + + sha256_init(&ctx); + sha256_update(&ctx, tlvstream + start1, len1); + sha256_update(&ctx, tlvstream + start2, len2); + sha256_done(&ctx, id); } void invreq_invreq_id(const struct tlv_invoice_request *invreq, struct sha256 *id) @@ -588,8 +618,9 @@ struct tlv_invoice *invoice_for_invreq(const tal_t *ctx, const struct tlv_invoice_request *invreq) { const u8 *cursor; - size_t start, len; + size_t start1, len1, start2, len2; u8 *wire = tal_arr(tmpctx, u8, 0); + towire_tlv_invoice_request(&wire, invreq); /* BOLT-offers #12: @@ -599,8 +630,50 @@ struct tlv_invoice *invoice_for_invreq(const tal_t *ctx, * - MUST copy all non-signature fields from the `invoice_request` (including * unknown fields). */ - len = tlv_span(wire, 0, 159, &start); - cursor = wire + start; - return fromwire_tlv_invoice(ctx, &cursor, &len); + len1 = tlv_span(wire, 0, 159, &start1); + len2 = tlv_span(wire, 1000000000, 2999999999, &start2); + + /* Move second span adjacent first span */ + memmove(wire + start1 + len1, wire + start2, len2); + + /* Unmarshal combined result */ + len1 = len1 + len2; + cursor = wire + start1; + return fromwire_tlv_invoice(ctx, &cursor, &len1); } +bool is_bolt12_signature_field(u64 typenum) +{ + /* BOLT-offers #12: + * Each form is signed using one or more *signature TLV elements*: TLV + * types 240 through 1000 (inclusive). */ + return typenum >= 240 && typenum <= 1000; +} + +static bool in_ranges(u64 numtype, + u64 r1_start, u64 r1_end, + u64 r2_start, u64 r2_end) +{ + if (numtype >= r1_start && numtype <= r1_end) + return true; + if (numtype >= r2_start && numtype <= r2_end) + return true; + return false; +} + +const struct tlv_field *any_field_outside_range(const struct tlv_field *fields, + bool ignore_signature_fields, + size_t r1_start, size_t r1_end, + size_t r2_start, size_t r2_end) +{ + for (size_t i = 0; i < tal_count(fields); i++) { + if (ignore_signature_fields + && is_bolt12_signature_field(fields[i].numtype)) + continue; + if (!in_ranges(fields[i].numtype, + r1_start, r1_end, + r2_start, r2_end)) + return &fields[i]; + } + return NULL; +} diff --git a/common/bolt12.h b/common/bolt12.h index 4ad89bd4f..cd272e8d2 100644 --- a/common/bolt12.h +++ b/common/bolt12.h @@ -154,4 +154,18 @@ struct tlv_invoice_request *invoice_request_for_offer(const tal_t *ctx, struct tlv_invoice *invoice_for_invreq(const tal_t *ctx, const struct tlv_invoice_request *invreq); +/* BOLT-offers #12: + * Each form is signed using one or more *signature TLV elements*: TLV + * types 240 through 1000 (inclusive). */ +bool is_bolt12_signature_field(u64 typenum); + +/** + * Return the first field (if any) outside the inclusive ranges. + */ +const struct tlv_field *any_field_outside_range(const struct tlv_field *fields, + bool ignore_signature_fields, + size_t r1_start, size_t r1_end, + size_t r2_start, size_t r2_end); + + #endif /* LIGHTNING_COMMON_BOLT12_H */ diff --git a/common/test/run-bolt12-encode-test.c b/common/test/run-bolt12-encode-test.c index d73d81092..ef7a14306 100644 --- a/common/test/run-bolt12-encode-test.c +++ b/common/test/run-bolt12-encode-test.c @@ -365,7 +365,7 @@ int main(int argc, char *argv[]) /* Now these are simply invalid, not bad encodings */ /* BOLT-offers #12: * A reader of an offer: - * - if the offer contains any TLV fields greater or equal to 80: + * - if the offer contains any TLV fields outside the inclusive ranges: 1 to 79 and 1000000000 to 1999999999: * - MUST NOT respond to the offer. */ print_malformed_tlv("lno", diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c index c85a3feba..70f48d2de 100644 --- a/plugins/fetchinvoice.c +++ b/plugins/fetchinvoice.c @@ -136,23 +136,28 @@ static struct command_result *handle_error(struct command *cmd, /* BOLT-offers #12: * - if the invoice is a response to an `invoice_request`: - * - MUST reject the invoice if all fields less than type 160 do not - * exactly match the `invoice_request`. + * - MUST reject the invoice if all fields in ranges 0 to 159 and 1000000000 to 2999999999 (inclusive) do not exactly match the `invoice_request`. */ static bool invoice_matches_request(struct command *cmd, const u8 *invbin, const struct tlv_invoice_request *invreq) { - size_t len1, len2; + size_t ir_len1, ir_len2, ir_start1, ir_start2; + size_t inv_len1, inv_len2, inv_start1, inv_start2; u8 *wire; /* We linearize then strip signature. This is dumb! */ wire = tal_arr(tmpctx, u8, 0); towire_tlv_invoice_request(&wire, invreq); - len1 = tlv_span(wire, 0, 159, NULL); + ir_len1 = tlv_span(wire, 0, 159, &ir_start1); + ir_len2 = tlv_span(wire, 1000000000, 2999999999, &ir_start2); - len2 = tlv_span(invbin, 0, 159, NULL); - return memeq(wire, len1, invbin, len2); + inv_len1 = tlv_span(invbin, 0, 159, &inv_start1); + inv_len2 = tlv_span(invbin, 1000000000, 2999999999, &inv_start2); + return memeq(wire + ir_start1, ir_len1, + invbin + ir_start1, inv_len1) + && memeq(wire + ir_start2, ir_len2, + invbin + ir_start2, inv_len2); } static struct command_result *handle_invreq_response(struct command *cmd, @@ -202,8 +207,9 @@ static struct command_result *handle_invreq_response(struct command *cmd, /* BOLT-offers #12: * - if the invoice is a response to an `invoice_request`: - * - MUST reject the invoice if all fields less than type 160 do not - * exactly match the `invoice_request`. + * - MUST reject the invoice if all fields in ranges 0 to 159 and + * 1000000000 to 2999999999 (inclusive) do not exactly match the + * `invoice_request`. */ if (!invoice_matches_request(cmd, invbin, sent->invreq)) { badfield = "invoice_request match"; @@ -1064,7 +1070,6 @@ static struct command_result *param_invreq(struct command *cmd, { char *fail; int badf; - u8 *wire; struct sha256 merkle, sighash; /* BOLT-offers #12: @@ -1088,8 +1093,7 @@ static struct command_result *param_invreq(struct command *cmd, * The reader: * - MUST fail the request if `invreq_payer_id` or `invreq_metadata` * are not present. - * - MUST fail the request if any non-signature TLV fields greater or - * equal to 160. + * - MUST fail the request if any non-signature TLV fields are outside the inclusive ranges: 0 to 159 and 1000000000 to 2999999999. * - if `invreq_features` contains unknown _odd_ bits that are * non-zero: * - MUST ignore the bit. @@ -1105,10 +1109,9 @@ static struct command_result *param_invreq(struct command *cmd, return command_fail_badparam(cmd, name, buffer, tok, "Missing invreq_metadata"); - wire = tal_arr(tmpctx, u8, 0); - towire_tlv_invoice_request(&wire, *invreq); - if (tlv_span(wire, 160, 239, NULL) != 0 - || tlv_span(wire, 1001, UINT64_MAX, NULL) != 0) { + if (any_field_outside_range((*invreq)->fields, true, + 0, 159, + 1000000000, 2999999999)) { return command_fail_badparam(cmd, name, buffer, tok, "Invalid high-numbered fields"); } diff --git a/plugins/offers_inv_hook.c b/plugins/offers_inv_hook.c index 09aab00fa..6f1c7761e 100644 --- a/plugins/offers_inv_hook.c +++ b/plugins/offers_inv_hook.c @@ -133,7 +133,7 @@ static struct command_result *listinvreqs_done(struct command *cmd, * A reader of an invoice: *... * - if the invoice is a response to an `invoice_request`: - * - MUST reject the invoice if all fields less than type 160 do not exactly match the `invoice_request`. + * - MUST reject the invoice if all fields in ranges 0 to 159 and 1000000000 to 2999999999 (inclusive) do not exactly match the `invoice_request`. * - if `offer_node_id` is present (invoice_request for an offer): * - MUST reject the invoice if `invoice_node_id` is not equal to `offer_node_id`. * - otherwise (invoice_request without an offer): @@ -142,7 +142,7 @@ static struct command_result *listinvreqs_done(struct command *cmd, * - otherwise: (a invoice presented without being requested, eg. scanned by user): */ - /* Since the invreq_id hashes all fields < 160, we know it matches */ + /* Since the invreq_id hashes all fields in those ranges, we know it matches */ if (arr->size == 0) return fail_inv(cmd, inv, "Unknown invoice_request %s", fmt_sha256(tmpctx, &inv->invreq_id)); diff --git a/plugins/offers_invreq_hook.c b/plugins/offers_invreq_hook.c index d065961f6..73bdee42d 100644 --- a/plugins/offers_invreq_hook.c +++ b/plugins/offers_invreq_hook.c @@ -984,22 +984,23 @@ struct command_result *handle_invoice_request(struct command *cmd, ir->secret = tal_dup_or_null(ir, struct secret, secret); ir->invreq = fromwire_tlv_invoice_request(cmd, &cursor, &len); + if (!ir->invreq) { + return fail_invreq(cmd, ir, "Invalid invreq"); + } + /* BOLT-offers #12: * The reader: * ... - * - MUST fail the request if any non-signature TLV fields greater or - * equal to 160. + * - MUST fail the request if any non-signature TLV fields are outside the inclusive ranges: 0 to 159 and 1000000000 to 2999999999 */ /* BOLT-offers #12: * Each form is signed using one or more *signature TLV elements*: * TLV types 240 through 1000 (inclusive) */ - if (tlv_span(invreqbin, 0, 159, NULL) - + tlv_span(invreqbin, 240, 1000, NULL) != tal_bytelen(invreqbin)) - return fail_invreq(cmd, ir, "Fields beyond 160"); - - if (!ir->invreq) { - return fail_invreq(cmd, ir, "Invalid invreq"); + if (any_field_outside_range(ir->invreq->fields, true, + 0, 159, + 1000000000, 2999999999)) { + return fail_invreq(cmd, ir, "Invalid high fields"); } /* BOLT-offers #12: diff --git a/tests/test_pay.py b/tests/test_pay.py index dd09c6885..7086f77fb 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5883,6 +5883,27 @@ def test_decryptencrypteddata(node_factory): assert dec['decrypted'].startswith('0421' + l3.info['id']) +def test_offer_experimental_fields(node_factory): + l1, l2 = node_factory.line_graph(2, opts={'experimental-offers': None}) + + # Append experimental type 1000000001, length 1 + offer = l1.rpc.offer(amount='2msat', description='test_offer_path_self')['bolt12'] + bolt12tool = os.path.join(os.path.dirname(__file__), "..", "devtools", "bolt12-cli") + # Returns HRP and hex + as_hex = subprocess.check_output([bolt12tool, 'decodehex', offer]).decode('UTF-8').split() + mangled = subprocess.check_output([bolt12tool, 'encodehex', as_hex[0], as_hex[1] + 'FE3B9ACA01' '01' '00']).decode('UTF-8').strip() + + assert l1.rpc.decode(mangled)['unknown_offer_tlvs'] == [{'type': 1000000001, 'length': 1, 'value': '00'}] + + # This will fail (offer has added field!) + with pytest.raises(RpcError, match="Unknown offer"): + l2.rpc.fetchinvoice(mangled) + + # invice request contains the unknown field + m = re.search(r'invoice_request: \\"([a-z0-9]*)\\"', l2.daemon.is_in_log('invoice_request:')) + assert l1.rpc.decode(m.group(1))['unknown_invoice_request_tlvs'] == [{'type': 1000000001, 'length': 1, 'value': '00'}] + + def test_fetch_no_description_offer(node_factory): """Reproducing the issue: https://github.com/ElementsProject/lightning/issues/7405""" l1, l2 = node_factory.line_graph(2, opts={'experimental-offers': None,