common/bolt12, offers plugin: handle experimental ranges in bolt12 co…

…rrectly.

The latest draft allows these experimental ranges, which involves more
changes than I expected.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: offers: handle experimental ranges in offers/invoice_requests/invoices.

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,21 +194,22 @@ 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.
 	 * - if `offer_features` contains unknown _even_ bits that are non-zero:
 	 *   - 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;
+}

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 */

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",

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);
-
-	len2 = tlv_span(invbin, 0, 159, NULL);
-	return memeq(wire, len1, invbin, len2);
+	ir_len1 = tlv_span(wire, 0, 159, &ir_start1);
+	ir_len2 = tlv_span(wire, 1000000000, 2999999999, &ir_start2);
+
+	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");
 	}

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));

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:

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,