Skip to content

Commit

Permalink
common/onion_message_parse: return string, not bool.
Browse files Browse the repository at this point in the history 
Allows for caller to log, but more importantly, when we add a command to
inject onion messages, allows for us to capture the error.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
@rustyrussell @vincenzopalazzo
rustyrussell authored and vincenzopalazzo committed Jul 10, 2024
1 parent a3c15e8 commit ba82592
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 57 deletions.
66 changes: 27 additions & 39 deletions common/onion_message_parse.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* Caller does fromwire_onion_message(), this does the rest. */
#include "config.h"
#include <assert.h>
#include <ccan/tal/str/str.h>
#include <common/blindedpath.h>
#include <common/ecdh.h>
#include <common/onion_message_parse.h>
Expand Down Expand Up @@ -70,16 +71,15 @@ static bool decrypt_forwarding_onionmsg(const struct pubkey *blinding,
}

/* Returns false on failure */
bool onion_message_parse(const tal_t *ctx,
const u8 *onion_message_packet,
const struct pubkey *blinding,
const struct node_id *peer,
const struct pubkey *me,
u8 **next_onion_msg,
struct pubkey *next_node_id,
struct tlv_onionmsg_tlv **final_om,
struct pubkey *final_alias,
struct secret **final_path_id)
const char *onion_message_parse(const tal_t *ctx,
const u8 *onion_message_packet,
const struct pubkey *blinding,
const struct pubkey *me,
u8 **next_onion_msg,
struct pubkey *next_node_id,
struct tlv_onionmsg_tlv **final_om,
struct pubkey *final_alias,
struct secret **final_path_id)
{
enum onion_wire badreason;
struct onionpacket *op;
Expand All @@ -96,47 +96,40 @@ bool onion_message_parse(const tal_t *ctx,
tal_bytelen(onion_message_packet),
&badreason);
if (!op) {
status_peer_debug(peer, "onion_message_parse: can't parse onionpacket: %s",
onion_wire_name(badreason));
return false;
return tal_fmt(ctx, "onion_message_parse: can't parse onionpacket: %s",
onion_wire_name(badreason));
}

ephemeral = op->ephemeralkey;
if (!unblind_onion(blinding, ecdh, &ephemeral, &ss)) {
status_peer_debug(peer, "onion_message_parse: can't unblind onionpacket");
return false;
return tal_fmt(ctx, "onion_message_parse: can't unblind onionpacket");
}

/* Now get onion shared secret and parse it. */
ecdh(&ephemeral, &onion_ss);
rs = process_onionpacket(tmpctx, op, &onion_ss, NULL, 0);
if (!rs) {
status_peer_debug(peer,
"onion_message_parse: can't process onionpacket ss=%s",
fmt_secret(tmpctx, &onion_ss));
return false;
return tal_fmt(ctx, "onion_message_parse: can't process onionpacket ss=%s",
fmt_secret(tmpctx, &onion_ss));
}

/* The raw payload is prepended with length in the modern onion. */
cursor = rs->raw_payload;
max = tal_bytelen(rs->raw_payload);
maxlen = fromwire_bigsize(&cursor, &max);
if (!cursor) {
status_peer_debug(peer, "onion_message_parse: Invalid hop payload %s",
tal_hex(tmpctx, rs->raw_payload));
return false;
return tal_fmt(ctx, "onion_message_parse: Invalid hop payload %s",
tal_hex(tmpctx, rs->raw_payload));
}
if (maxlen > max) {
status_peer_debug(peer, "onion_message_parse: overlong hop payload %s",
tal_hex(tmpctx, rs->raw_payload));
return false;
return tal_fmt(ctx, "onion_message_parse: overlong hop payload %s",
tal_hex(tmpctx, rs->raw_payload));
}

om = fromwire_tlv_onionmsg_tlv(tmpctx, &cursor, &maxlen);
if (!om) {
status_peer_debug(peer, "onion_message_parse: invalid onionmsg_tlv %s",
tal_hex(tmpctx, rs->raw_payload));
return false;
return tal_fmt(ctx, "onion_message_parse: invalid onionmsg_tlv %s",
tal_hex(tmpctx, rs->raw_payload));
}
if (rs->nextcase == ONION_END) {
*next_onion_msg = NULL;
Expand All @@ -149,10 +142,9 @@ bool onion_message_parse(const tal_t *ctx,
om->encrypted_recipient_data, me,
final_alias,
final_path_id)) {
status_peer_debug(peer,
return tal_fmt(ctx,
"onion_message_parse: failed to decrypt encrypted_recipient_data"
" %s", tal_hex(tmpctx, om->encrypted_recipient_data));
return false;
}
} else {
struct pubkey next_blinding;
Expand All @@ -165,19 +157,15 @@ bool onion_message_parse(const tal_t *ctx,
* - MUST ignore the message.
*/
if (tal_count(om->fields) != 1) {
status_peer_debug(peer,
"onion_message_parse: "
"disallowed tlv field");
return false;
return tal_fmt(ctx, "onion_message_parse: disallowed tlv field");
}

/* This fails as expected if no enctlv. */
if (!decrypt_forwarding_onionmsg(blinding, &ss, om->encrypted_recipient_data, next_node_id,
&next_blinding)) {
status_peer_debug(peer,
"onion_message_parse: invalid encrypted_recipient_data %s",
tal_hex(tmpctx, om->encrypted_recipient_data));
return false;
return tal_fmt(ctx,
"onion_message_parse: invalid encrypted_recipient_data %s",
tal_hex(tmpctx, om->encrypted_recipient_data));
}
*next_onion_msg = towire_onion_message(ctx,
&next_blinding,
Expand All @@ -186,5 +174,5 @@ bool onion_message_parse(const tal_t *ctx,

/* Exactly one is set */
assert(!*next_onion_msg + !*final_om == 1);
return true;
return NULL;
}
22 changes: 10 additions & 12 deletions common/onion_message_parse.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,23 @@ struct pubkey;
* @ctx: context to allocate @next_onion_msg or @final_om/@path_id off
* @onion_message_packet: Sphinx-encrypted onion
* @blinding: Blinding we were given for @onion_message_packet
* @peer: node_id of peer (for status_peer_debug msgs)
* @me: my pubkey
* @next_onion_msg (out): set if we should forward, otherwise NULL.
* @next_node_id (out): set to node id to fwd to, iff *@next_onion_msg.
* @final_om (out): set if we're the final hop, otherwise NULL.
* @final_alias (out): our alias (if *@final_om), or our own ID
* @final_path_id (out): secret enclosed, if any (iff *@final_om).
*
* Returns false if it wasn't valid.
* Returns NULL if it was valid, otherwise an error string.
*/
bool onion_message_parse(const tal_t *ctx,
const u8 *onion_message_packet,
const struct pubkey *blinding,
const struct node_id *peer,
const struct pubkey *me,
u8 **next_onion_msg,
struct pubkey *next_node_id,
struct tlv_onionmsg_tlv **final_om,
struct pubkey *final_alias,
struct secret **final_path_id);
const char *onion_message_parse(const tal_t *ctx,
const u8 *onion_message_packet,
const struct pubkey *blinding,
const struct pubkey *me,
u8 **next_onion_msg,
struct pubkey *next_node_id,
struct tlv_onionmsg_tlv **final_om,
struct pubkey *final_alias,
struct secret **final_path_id);

#endif /* LIGHTNING_COMMON_ONION_MESSAGE_PARSE_H */
4 changes: 2 additions & 2 deletions common/test/run-onion-message-test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,12 @@ int main(int argc, char *argv[])

/* For test_ecdh */
mykey = &privkey[i];
assert(onion_message_parse(tmpctx, onion_message_packet, &blinding_pub, NULL,
assert(onion_message_parse(tmpctx, onion_message_packet, &blinding_pub,
&id[i],
&onion_message, &next_node_id,
&final_om,
&final_alias,
&final_path_id));
&final_path_id) == NULL);
if (onion_message) {
json_pubkey("next_node_id", &next_node_id);
} else {
Expand Down
12 changes: 8 additions & 4 deletions connectd/onion_message.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void handle_onion_message(struct daemon *daemon,
struct tlv_onionmsg_tlv *final_om;
struct pubkey final_alias;
struct secret *final_path_id;
const char *err;

/* Ignore unless explicitly turned on. */
if (!feature_offered(daemon->our_features->bits[NODE_ANNOUNCE_FEATURE],
Expand All @@ -60,11 +61,14 @@ void handle_onion_message(struct daemon *daemon,
return;
}

if (!onion_message_parse(tmpctx, onion, &blinding, &peer->id,
&daemon->mykey,
&next_onion_msg, &next_node,
&final_om, &final_alias, &final_path_id))
err = onion_message_parse(tmpctx, onion, &blinding,
&daemon->mykey,
&next_onion_msg, &next_node,
&final_om, &final_alias, &final_path_id);
if (err) {
status_peer_debug(&peer->id, "%s", err);
return;
}

if (final_om) {
u8 *omsg;
Expand Down

0 comments on commit ba82592

Please sign in to comment.