lightningd: make the caller set invreq_metadata and invreq_payer_id f…

…or createinvoicerequest. It's an internal undocumented interface, which makes this change less painful. We *do* check that the invreq_metadata maps to the given invreq_payer_id, which would is required for us to sign it. Signed-off-by: Rusty Russell <[email protected]>
ElementsProject · Jul 23, 2024 · 936fc1a · 936fc1a
1 parent 30f1605
commit 936fc1a
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 42 deletions.
diff --git a/lightningd/offer.c b/lightningd/offer.c
@@ -355,14 +355,13 @@ static struct command_result *param_b12_invreq(struct command *cmd,
 	if (!*invreq)
 		return command_fail_badparam(cmd, name, buffer, tok, fail);
 
-	/* We use this for testing with known payer_info */
-	if ((*invreq)->invreq_metadata && !cmd->ld->developer)
+	if (!(*invreq)->invreq_metadata)
 		return command_fail_badparam(cmd, name, buffer, tok,
-					     "must not have invreq_metadata");
+					     "must have invreq_metadata");
 
-	if ((*invreq)->invreq_payer_id)
+	if (!(*invreq)->invreq_payer_id)
 		return command_fail_badparam(cmd, name, buffer, tok,
-					     "must not have invreq_payer_id");
+					     "must have invreq_payer_id");
 	return NULL;
 }
 
@@ -407,15 +406,15 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
 	struct json_stream *response;
 	u64 *prev_basetime = NULL;
 	struct sha256 merkle;
-	bool *save, *single_use, *exposeid;
+	bool *save, *single_use;
 	enum offer_status status;
 	struct sha256 invreq_id;
 	const char *b12str;
+	const u8 *tweak;
 
 	if (!param_check(cmd, buffer, params,
 			 p_req("bolt12", param_b12_invreq, &invreq),
 			 p_req("savetodb", param_bool, &save),
-			 p_opt_def("exposeid", param_bool, &exposeid, false),
 			 p_opt("recurrence_label", param_label, &label),
 			 p_opt_def("single_use", param_bool, &single_use, true),
 			 NULL))
@@ -426,14 +425,6 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
 	else
 		status = OFFER_MULTIPLE_USE_UNUSED;
 
-	if (!invreq->invreq_metadata)
-		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-				    "invoice_request has no invreq_metadata");
-
-	if (invreq->invreq_payer_id)
-		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-				    "invoice_request already has invreq_payer_id");
-
 	/* If it's a recurring payment, we look for previous to copy basetime */
 	if (invreq->invreq_recurrence_counter) {
 		if (!label)
@@ -449,27 +440,25 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
 		}
 	}
 
-	/* BOLT-offers #12:
-	 *
-	 * `invreq_metadata` might typically contain information about
-	 * the derivation of the `invreq_payer_id`.  This should not
-	 * leak any information (such as using a simple BIP-32
-	 * derivation path); a valid system might be for a node to
-	 * maintain a base payer key and encode a 128-bit tweak here.
-	 * The payer_id would be derived by tweaking the base key with
-	 * SHA256(payer_base_pubkey || tweak).  It's also the first
-	 * entry (if present), ensuring an unpredictable nonce for
-	 * hashing.
-	 */
-	invreq->invreq_payer_id = tal(invreq, struct pubkey);
-	if (*exposeid) {
-		*invreq->invreq_payer_id = cmd->ld->our_pubkey;
-	} else if (!payer_key(cmd->ld,
-			      invreq->invreq_metadata,
-			      tal_bytelen(invreq->invreq_metadata),
-			      invreq->invreq_payer_id)) {
-		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
-				    "Invalid tweak");
+	/* If the payer_id is not our node id, we sanity check that it
+	 * correctly maps from invreq_metadata */
+	if (!pubkey_eq(invreq->invreq_payer_id, &cmd->ld->our_pubkey)) {
+		struct pubkey expected;
+		if (!payer_key(cmd->ld,
+			       invreq->invreq_metadata,
+			       tal_bytelen(invreq->invreq_metadata),
+			       &expected)) {
+			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+					    "Invalid tweak");
+		}
+		if (!pubkey_eq(invreq->invreq_payer_id, &expected)) {
+			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+					    "payer_id did not match invreq_metadata derivation %s",
+					    fmt_pubkey(tmpctx, &expected));
+		}
+		tweak = invreq->invreq_metadata;
+	} else {
+		tweak = NULL;
 	}
 
 	if (command_check_only(cmd))
@@ -484,7 +473,7 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
 	merkle_tlv(invreq->fields, &merkle);
 	invreq->signature = tal(invreq, struct bip340sig);
 	hsm_sign_b12(cmd->ld, "invoice_request", "signature",
-		     &merkle, *exposeid ? NULL : invreq->invreq_metadata,
+		     &merkle, tweak,
 		     invreq->invreq_payer_id, invreq->signature);
 
 	b12str = invrequest_encode(cmd, invreq);

diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c
@@ -766,6 +766,20 @@ static struct command_result *param_dev_reply_path(struct command *cmd, const ch
 	return NULL;
 }
 
+static bool payer_key(const u8 *public_tweak, size_t public_tweak_len,
+		      struct pubkey *key)
+{
+	struct sha256 tweakhash;
+
+	bolt12_alias_tweak(&nodealias_base, public_tweak, public_tweak_len,
+			   &tweakhash);
+
+	*key = id;
+	return secp256k1_ec_pubkey_tweak_add(secp256k1_ctx,
+					     &key->pubkey,
+					     tweakhash.u.u8) == 1;
+}
+
 /* Fetches an invoice for this offer, and makes sure it corresponds. */
 struct command_result *json_fetchinvoice(struct command *cmd,
 					 const char *buffer,
@@ -966,6 +980,16 @@ struct command_result *json_fetchinvoice(struct command *cmd,
 				tal_bytelen(invreq->invreq_metadata));
 	}
 
+	/* We derive transient payer_id from invreq_metadata */
+	invreq->invreq_payer_id = tal(invreq, struct pubkey);
+	if (!payer_key(invreq->invreq_metadata,
+		       tal_bytelen(invreq->invreq_metadata),
+		       invreq->invreq_payer_id)) {
+		/* Doesn't happen! */
+		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+				    "Invalid tweak for payer_id");
+	}
+
 	/* BOLT-offers #12:
 	 *
 	 * - if `offer_chains` is set:

diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c
@@ -674,7 +674,8 @@ struct command_result *json_invoicerequest(struct command *cmd,
 	 *...
 	 *   - MUST set `invreq_payer_id` as it would set `offer_issuer_id` for an offer.
 	 */
-	/* createinvoicerequest sets this! */
+	/* FIXME: Allow invoicerequests using aliases! */
+	invreq->invreq_payer_id = tal_dup(invreq, struct pubkey, &id);
 
 	/* BOLT-offers #12:
 	 * - if it supports bolt12 invoice request features:
@@ -685,8 +686,6 @@ struct command_result *json_invoicerequest(struct command *cmd,
 				    invreq);
 	json_add_string(req->js, "bolt12", invrequest_encode(tmpctx, invreq));
 	json_add_bool(req->js, "savetodb", true);
-	/* FIXME: Allow invoicerequests using aliases! */
-	json_add_bool(req->js, "exposeid", true);
 	json_add_bool(req->js, "single_use", *single_use);
 	if (label)
 		json_add_string(req->js, "recurrence_label", label);

diff --git a/tests/test_pay.py b/tests/test_pay.py
@@ -5254,9 +5254,21 @@ def test_payerkey(node_factory):
                      "030b68257230f7057e694222bbd54d9d108decced6b647a90da6f578360af53f7d",
                      "02f402bd7374a1304b07c7236d9c683b83f81072517195ddede8ab328026d53157"]
 
+    bolt12tool = os.path.join(os.path.dirname(__file__), "..", "devtools", "bolt12-cli")
+
+    # Returns "lnr <hexstring>" on first line
+    hexprefix = subprocess.check_output([bolt12tool, 'decodehex',
+                                         'lnr1qqgz2d7u2smys9dc5q2447e8thjlgq3qqc3xu3s3rg94nj40zfsy866mhu5vxne6tcej5878k2mneuvgjy8ssqvepgz5zsjrg3z3vggzvkm2khkgvrxj27r96c00pwl4kveecdktm29jdd6w0uwu5jgtv5v9qgqxyfhyvyg6pdvu4tcjvpp7kkal9rp57wj7xv4pl3ajku70rzy3pu']).decode('UTF-8').split('\n')[0].split()
+
+    # Now we are supposed to put invreq_payer_id inside invreq, and lightningd
+    # checks the derivation as a courtesy.  Fortunately, invreq_payer_id is last
     for n, k in zip(nodes, expected_keys):
-        b12 = n.rpc.createinvoicerequest('lnr1qqgz2d7u2smys9dc5q2447e8thjlgq3qqc3xu3s3rg94nj40zfsy866mhu5vxne6tcej5878k2mneuvgjy8ssqvepgz5zsjrg3z3vggzvkm2khkgvrxj27r96c00pwl4kveecdktm29jdd6w0uwu5jgtv5v9qgqxyfhyvyg6pdvu4tcjvpp7kkal9rp57wj7xv4pl3ajku70rzy3pu', False)['bolt12']
-        assert n.rpc.decode(b12)['invreq_payer_id'] == k
+        # BOLT-offers #12:
+        #     1. type: 88 (`invreq_payer_id`)
+        #     2. data:
+        #        * [`point`:`key`]
+        encoded = subprocess.check_output([bolt12tool, 'encodehex'] + hexprefix + ['5821', k]).decode('UTF-8').strip()
+        n.rpc.createinvoicerequest(encoded, False)['bolt12']
 
 
 def test_pay_multichannel_use_zeroconf(bitcoind, node_factory):