From 5becfa6ee18127030e86aa26ed1fca2de7a871ee Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 9 Nov 2022 12:00:10 +1030 Subject: [PATCH] onion_message: don't use general secret, use per-message secret. We had a scheme where lightningd itself would put a per-node secret in the blinded path, then we'd tell the caller when it was used. Then it simply checks the alias to determine if the correct path was used. But this doesn't work when we start to offer multiple blinded paths. So go for a far simpler scheme, where the secret is generated (and stored) by the caller, and hand it back to them. We keep the split "with secret" or "without secret" API, since I'm sure callers who don't care about the secret won't check that it doesn't exist! And without that, someone can use a blinded path for a different message and get a response which may reveal the node. Signed-off-by: Rusty Russell --- common/hsm_version.h | 2 +- connectd/connectd_wire.csv | 3 +-- connectd/onion_message.c | 2 +- doc/PLUGINS.md | 16 ++++++------- hsmd/hsmd_wire.csv | 1 - hsmd/libhsmd.c | 11 +-------- lightningd/hsm_control.c | 7 +++--- lightningd/lightningd.h | 3 --- lightningd/onion_message.c | 46 +++++++++++++++++--------------------- plugins/fetchinvoice.c | 32 +++++++++++++------------- plugins/offers.c | 2 +- 11 files changed, 51 insertions(+), 74 deletions(-) diff --git a/common/hsm_version.h b/common/hsm_version.h index ea1bdff93..8acf80112 100644 --- a/common/hsm_version.h +++ b/common/hsm_version.h @@ -11,7 +11,7 @@ #define HSM_MIN_VERSION 1 /* wire/hsmd_wire.csv contents version: - * 43c435f61de3af0dd7a91514d94b3e0762c962fce5b39be430538f8c6c4b0695 + * dd89bf9323dff42200003fb864abb6608f3aa645b636fdae3ec81d804ac05196 */ #define HSM_MAX_VERSION 2 #endif /* LIGHTNING_COMMON_HSM_VERSION_H */ diff --git a/connectd/connectd_wire.csv b/connectd/connectd_wire.csv index 9f3e47f02..16c6953d8 100644 --- a/connectd/connectd_wire.csv +++ b/connectd/connectd_wire.csv @@ -118,8 +118,7 @@ msgdata,connectd_ping_reply,totlen,u16, # We tell lightningd we got an onionmsg msgtype,connectd_got_onionmsg_to_us,2145 -msgdata,connectd_got_onionmsg_to_us,node_alias,pubkey, -msgdata,connectd_got_onionmsg_to_us,self_id,?secret, +msgdata,connectd_got_onionmsg_to_us,path_secret,?secret, msgdata,connectd_got_onionmsg_to_us,reply,?blinded_path, msgdata,connectd_got_onionmsg_to_us,rawmsg_len,u16, msgdata,connectd_got_onionmsg_to_us,rawmsg,u8,rawmsg_len diff --git a/connectd/onion_message.c b/connectd/onion_message.c index a12671aa0..a20119dea 100644 --- a/connectd/onion_message.c +++ b/connectd/onion_message.c @@ -75,7 +75,7 @@ void handle_onion_message(struct daemon *daemon, towire_tlvstream_raw(&omsg, final_om->fields); daemon_conn_send(daemon->master, take(towire_connectd_got_onionmsg_to_us(NULL, - &final_alias, final_path_id, + final_path_id, final_om->reply_path, omsg))); } else { diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index d00867a77..a40e13f54 100644 --- a/doc/PLUGINS.md +++ b/doc/PLUGINS.md @@ -1650,23 +1650,22 @@ type prefix, since Core Lightning does not know how to parse the message. Because this is a chained hook, the daemon expects the result to be `{'result': 'continue'}`. It will fail if something else is returned. -### `onion_message_blinded` and `onion_message_ourpath` +### `onion_message_recv` and `onion_message_recv_secret` **(WARNING: experimental-offers only)** These two hooks are almost identical, in that they are called when an onion message is received. -`onion_message_blinded` is used for unsolicited messages (where the +`onion_message_recv` is used for unsolicited messages (where the source knows that it is sending to this node), and -`onion_message_ourpath` is used for messages which use a blinded path -we supplied (where the source doesn't know that this node is the -destination). The latter hook will have a `our_alias` field, the +`onion_message_recv_secret` is used for messages which use a blinded path +we supplied. The latter hook will have a `pathsecret` field, the former never will. These hooks are separate, because replies MUST be ignored unless they -use the correct path (i.e. `onion_message_ourpath`, with the expected -`our_alias`). This avoids the source trying to probe for responses +use the correct path (i.e. `onion_message_recv_secret`, with the expected +`pathsecret`). This avoids the source trying to probe for responses without using the designated delivery path. The payload for a call follows this format: @@ -1674,7 +1673,7 @@ The payload for a call follows this format: ```json { "onion_message": { - "our_alias": "02df5ffe895c778e10f7742a6c5b8a0cefbe9465df58b92fadeb883752c8107c8f", + "pathsecret": "0000000000000000000000000000000000000000000000000000000000000000", "reply_first_node": "02df5ffe895c778e10f7742a6c5b8a0cefbe9465df58b92fadeb883752c8107c8f", "reply_blinding": "02df5ffe895c778e10f7742a6c5b8a0cefbe9465df58b92fadeb883752c8107c8f", "reply_path": [ {"id": "02df5ffe895c778e10f7742a6c5b8a0cefbe9465df58b92fadeb883752c8107c8f", @@ -1691,7 +1690,6 @@ The payload for a call follows this format: All fields shown here are optional. We suggest just returning `{'result': 'continue'}`; any other result -Signed-off-by: Rusty Russell will cause the message not to be handed to any other hooks. ## Bitcoin backend diff --git a/hsmd/hsmd_wire.csv b/hsmd/hsmd_wire.csv index 230848ed7..e294fba4d 100644 --- a/hsmd/hsmd_wire.csv +++ b/hsmd/hsmd_wire.csv @@ -30,7 +30,6 @@ msgtype,hsmd_init_reply_v2,113 msgdata,hsmd_init_reply_v2,node_id,node_id, msgdata,hsmd_init_reply_v2,bip32,ext_key, msgdata,hsmd_init_reply_v2,bolt12,pubkey, -msgdata,hsmd_init_reply_v2,onion_reply_secret,secret, # Declare a new channel. msgtype,hsmd_new_channel,30 diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index 107909dda..3ac8452d3 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -1648,7 +1648,6 @@ u8 *hsmd_init(struct secret hsm_secret, u32 salt = 0; struct ext_key master_extkey, child_extkey; struct node_id node_id; - struct secret onion_reply_secret; /*~ Don't swap this. */ sodium_mlock(secretstuff.hsm_secret.data, @@ -1766,14 +1765,6 @@ u8 *hsmd_init(struct secret hsm_secret, hsmd_status_failed(STATUS_FAIL_INTERNAL_ERROR, "Could derive bolt12 public key."); - /*~ We derive a secret for onion_message's self_id so we can tell - * if it used a path we created (i.e. do not leak our public id!) */ - hkdf_sha256(&onion_reply_secret, sizeof(onion_reply_secret), - NULL, 0, - &secretstuff.hsm_secret, - sizeof(secretstuff.hsm_secret), - "onion reply secret", strlen("onion reply secret")); - /* We derive the derived_secret key for generating pseudorandom keys * by taking input string from the makesecret RPC */ hkdf_sha256(&secretstuff.derived_secret, sizeof(struct secret), NULL, 0, @@ -1785,5 +1776,5 @@ u8 *hsmd_init(struct secret hsm_secret, */ return take(towire_hsmd_init_reply_v2( NULL, &node_id, &secretstuff.bip32, - &bolt12, &onion_reply_secret)); + &bolt12)); } diff --git a/lightningd/hsm_control.c b/lightningd/hsm_control.c index 1efd6c518..da849e0a4 100644 --- a/lightningd/hsm_control.c +++ b/lightningd/hsm_control.c @@ -119,16 +119,17 @@ struct ext_key *hsm_init(struct lightningd *ld) msg = wire_sync_read(tmpctx, ld->hsm_fd); if (!fromwire_hsmd_init_reply_v2(msg, &ld->id, bip32_base, - &ld->bolt12_base, - &ld->onion_reply_secret)) { + &ld->bolt12_base)) { /* v1 had x-only pubkey */ u8 pubkey32[33]; + /* And gave us a secret to use for onion_reply paths */ + struct secret onion_reply_secret; pubkey32[0] = SECP256K1_TAG_PUBKEY_EVEN; if (!fromwire_hsmd_init_reply_v1(msg, &ld->id, bip32_base, pubkey32 + 1, - &ld->onion_reply_secret)) { + &onion_reply_secret)) { if (ld->config.keypass) errx(EXITCODE_HSM_BAD_PASSWORD, "Wrong password for encrypted hsm_secret."); errx(EXITCODE_HSM_GENERIC_ERROR, "HSM did not give init reply"); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index e3b037f7d..8ada62912 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -124,9 +124,6 @@ struct lightningd { /* The public base for our payer_id keys */ struct pubkey bolt12_base; - /* The secret we put in onion message paths to know it's ours. */ - struct secret onion_reply_secret; - /* Feature set we offer. */ struct feature_set *our_features; diff --git a/lightningd/onion_message.c b/lightningd/onion_message.c index a70f7ba7f..7316b4346 100644 --- a/lightningd/onion_message.c +++ b/lightningd/onion_message.c @@ -16,7 +16,7 @@ struct onion_message_hook_payload { /* Optional */ struct blinded_path *reply_path; - struct pubkey *our_alias; + struct secret *pathsecret; struct tlv_onionmsg_tlv *om; }; @@ -45,8 +45,8 @@ static void onion_message_serialize(struct onion_message_hook_payload *payload, struct plugin *plugin) { json_object_start(stream, "onion_message"); - if (payload->our_alias) - json_add_pubkey(stream, "our_alias", payload->our_alias); + if (payload->pathsecret) + json_add_secret(stream, "pathsecret", payload->pathsecret); if (payload->reply_path) json_add_blindedpath(stream, "reply_blindedpath", @@ -86,36 +86,34 @@ onion_message_hook_cb(struct onion_message_hook_payload *payload STEALS) tal_free(payload); } -/* Two hooks, because it's critical we only accept blinding if we expect that - * exact blinding key. Otherwise, we can be probed using old blinded paths. */ -REGISTER_PLUGIN_HOOK(onion_message_blinded, +/* This is for unsolicted messages */ +REGISTER_PLUGIN_HOOK(onion_message_recv, plugin_hook_continue, onion_message_hook_cb, onion_message_serialize, struct onion_message_hook_payload *); -REGISTER_PLUGIN_HOOK(onion_message_ourpath, +/* This is for messages claiming to be using our paths: caller must + * check pathsecret! */ + REGISTER_PLUGIN_HOOK(onion_message_recv_secret, plugin_hook_continue, onion_message_hook_cb, onion_message_serialize, struct onion_message_hook_payload *); + void handle_onionmsg_to_us(struct lightningd *ld, const u8 *msg) { struct onion_message_hook_payload *payload; u8 *submsg; - struct secret *self_id; size_t submsglen; const u8 *subptr; payload = tal(tmpctx, struct onion_message_hook_payload); - payload->our_alias = tal(payload, struct pubkey); - if (!fromwire_connectd_got_onionmsg_to_us(payload, msg, - payload->our_alias, - &self_id, - &payload->reply_path, - &submsg)) { + &payload->pathsecret, + &payload->reply_path, + &submsg)) { log_broken(ld->log, "bad got_onionmsg_tous: %s", tal_hex(tmpctx, msg)); return; @@ -126,12 +124,6 @@ void handle_onionmsg_to_us(struct lightningd *ld, const u8 *msg) return; #endif - /* If there's no self_id, or it's not correct, ignore alias: alias - * means we created the path it's using. */ - if (!self_id || !secret_eq_consttime(self_id, &ld->onion_reply_secret)) - payload->our_alias = tal_free(payload->our_alias); - tal_free(self_id); - submsglen = tal_bytelen(submsg); subptr = submsg; payload->om = fromwire_tlv_onionmsg_tlv(payload, &subptr, &submsglen); @@ -144,15 +136,15 @@ void handle_onionmsg_to_us(struct lightningd *ld, const u8 *msg) /* Make sure connectd gets this right. */ log_debug(ld->log, "Got onionmsg%s%s", - payload->our_alias ? " via-ourpath": "", + payload->pathsecret ? " with pathsecret": "", payload->reply_path ? " reply_path": ""); /* We'll free this on return */ tal_steal(ld, payload); - if (payload->our_alias) - plugin_hook_call_onion_message_ourpath(ld, NULL, payload); + if (payload->pathsecret) + plugin_hook_call_onion_message_recv_secret(ld, NULL, payload); else - plugin_hook_call_onion_message_blinded(ld, NULL, payload); + plugin_hook_call_onion_message_recv(ld, NULL, payload); } struct onion_hop { @@ -286,9 +278,11 @@ static struct command_result *json_blindedpath(struct command *cmd, size_t nhops; struct json_stream *response; struct tlv_encrypted_data_tlv *tlv; + struct secret *pathsecret; if (!param(cmd, buffer, params, p_req("ids", param_pubkeys, &ids), + p_req("pathsecret", param_secret, &pathsecret), NULL)) return command_param_failed(); @@ -338,8 +332,8 @@ static struct command_result *json_blindedpath(struct command *cmd, path->path[nhops-1] = tal(path->path, struct onionmsg_hop); tlv = tlv_encrypted_data_tlv_new(tmpctx); - tlv->path_id = (u8 *)tal_dup(tlv, struct secret, - &cmd->ld->onion_reply_secret); + + tlv->path_id = (u8 *)tal_dup(tlv, struct secret, pathsecret); path->path[nhops-1]->encrypted_recipient_data = encrypt_tlv_encrypted_data(path->path[nhops-1], &blinding_iter, diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c index b887a274f..f571634f2 100644 --- a/plugins/fetchinvoice.c +++ b/plugins/fetchinvoice.c @@ -29,8 +29,8 @@ static LIST_HEAD(sent_list); struct sent { /* We're in sent_invreqs, awaiting reply. */ struct list_node list; - /* The alias used by reply */ - struct pubkey *reply_alias; + /* The secret used by reply */ + struct secret *reply_secret; /* The command which sent us. */ struct command *cmd; /* The offer we are trying to get an invoice/payment for. */ @@ -48,12 +48,12 @@ struct sent { u32 wait_timeout; }; -static struct sent *find_sent_by_alias(const struct pubkey *alias) +static struct sent *find_sent_by_secret(const struct secret *pathsecret) { struct sent *i; list_for_each(&sent_list, i, list) { - if (i->reply_alias && pubkey_eq(i->reply_alias, alias)) + if (i->reply_secret && secret_eq_consttime(i->reply_secret, pathsecret)) return i; } return NULL; @@ -409,18 +409,16 @@ static struct command_result *recv_modern_onion_message(struct command *cmd, const char *buf, const jsmntok_t *params) { - const jsmntok_t *om, *aliastok; + const jsmntok_t *om, *secrettok; struct sent *sent; - struct pubkey alias; + struct secret pathsecret; struct command_result *err; om = json_get_member(buf, params, "onion_message"); - aliastok = json_get_member(buf, om, "our_alias"); - if (!aliastok || !json_to_pubkey(buf, aliastok, &alias)) - return command_hook_success(cmd); - - sent = find_sent_by_alias(&alias); + secrettok = json_get_member(buf, om, "pathsecret"); + json_to_secret(buf, secrettok, &pathsecret); + sent = find_sent_by_secret(&pathsecret); if (!sent) { plugin_log(cmd->plugin, LOG_DBG, "No match for modern onion %.*s", @@ -703,11 +701,6 @@ static struct command_result *use_reply_path(struct command *cmd, json_tok_full_len(result), json_tok_full(buf, result)); - /* Remember our alias we used so we can recognize reply */ - sending->sent->reply_alias - = tal_dup(sending->sent, struct pubkey, - &rpath->path[tal_count(rpath->path)-1]->blinded_node_id); - return send_modern_message(cmd, rpath, sending); } @@ -722,6 +715,10 @@ static struct command_result *make_reply_path(struct command *cmd, return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "Refusing to talk to ourselves"); + /* Create transient secret so we can validate reply! */ + sending->sent->reply_secret = tal(sending->sent, struct secret); + randombytes_buf(sending->sent->reply_secret, sizeof(struct secret)); + req = jsonrpc_request_start(cmd->plugin, cmd, "blindedpath", use_reply_path, forward_error, @@ -733,6 +730,7 @@ static struct command_result *make_reply_path(struct command *cmd, for (int i = nhops - 2; i >= 0; i--) json_add_pubkey(req->js, NULL, &sending->sent->path[i]); json_array_end(req->js); + json_add_secret(req->js, "pathsecret", sending->sent->reply_secret); return send_outreq(cmd->plugin, req); } @@ -1723,7 +1721,7 @@ static const char *init(struct plugin *p, const char *buf UNUSED, static const struct plugin_hook hooks[] = { { - "onion_message_ourpath", + "onion_message_recv_secret", recv_modern_onion_message }, { diff --git a/plugins/offers.c b/plugins/offers.c index e153660a1..8130feae5 100644 --- a/plugins/offers.c +++ b/plugins/offers.c @@ -127,7 +127,7 @@ static struct command_result *onion_message_modern_call(struct command *cmd, static const struct plugin_hook hooks[] = { { - "onion_message_blinded", + "onion_message_recv", onion_message_modern_call }, };