From ff5f7b194f73e33593dada5a1b0ec58d22de84c5 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Fri, 29 Nov 2019 21:20:18 +0100 Subject: [PATCH] sphinx: Return the error in parse_onionpacket As suggested by @niftynei here: https://github.com/ElementsProject/lightning/pull/3260#discussion_r347543999 Suggested-by: Lisa Neigut <@niftynei> Suggested-by: Rusty Russell <@rustyrussell> Signed-off-by: Christian Decker <@cdecker> --- channeld/channeld.c | 11 +++++------ common/sphinx.c | 28 +++++++++++----------------- common/sphinx.h | 10 ++++------ devtools/onion.c | 10 +++++----- lightningd/pay.c | 8 ++++---- lightningd/peer_htlcs.c | 12 ++++++------ wallet/test/run-wallet.c | 7 +++---- 7 files changed, 38 insertions(+), 48 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 4e03bc9b4..379c3ef93 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -584,25 +584,24 @@ static struct secret *get_shared_secret(const tal_t *ctx, enum onion_type *why_bad, struct sha256 *next_onion_sha) { - struct onionpacket *op; + struct onionpacket op; struct secret *secret = tal(ctx, struct secret); const u8 *msg; struct route_step *rs; /* We unwrap the onion now. */ - op = parse_onionpacket(tmpctx, htlc->routing, TOTAL_PACKET_SIZE, - why_bad); - if (!op) + *why_bad = parse_onionpacket(htlc->routing, TOTAL_PACKET_SIZE, &op); + if (*why_bad != 0) return tal_free(secret); /* Because wire takes struct pubkey. */ - msg = hsm_req(tmpctx, towire_hsm_ecdh_req(tmpctx, &op->ephemeralkey)); + msg = hsm_req(tmpctx, towire_hsm_ecdh_req(tmpctx, &op.ephemeralkey)); if (!fromwire_hsm_ecdh_resp(msg, secret)) status_failed(STATUS_FAIL_HSM_IO, "Reading ecdh response"); /* We make sure we can parse onion packet, so we know if shared secret * is actually valid (this checks hmac). */ - rs = process_onionpacket(tmpctx, op, secret->data, + rs = process_onionpacket(tmpctx, &op, secret->data, htlc->rhash.u.u8, sizeof(htlc->rhash)); if (!rs) { diff --git a/common/sphinx.c b/common/sphinx.c index f4babdd7b..8d9157132 100644 --- a/common/sphinx.c +++ b/common/sphinx.c @@ -136,36 +136,30 @@ u8 *serialize_onionpacket( return dst; } -struct onionpacket *parse_onionpacket(const tal_t *ctx, - const void *src, - const size_t srclen, - enum onion_type *why_bad) +enum onion_type parse_onionpacket(const u8 *src, + const size_t srclen, + struct onionpacket *dest) { - struct onionpacket *m; int p = 0; u8 rawEphemeralkey[PUBKEY_CMPR_LEN]; assert(srclen == TOTAL_PACKET_SIZE); - m = talz(ctx, struct onionpacket); - - read_buffer(&m->version, src, 1, &p); - if (m->version != 0x00) { + read_buffer(&dest->version, src, 1, &p); + if (dest->version != 0x00) { // FIXME add logging - *why_bad = WIRE_INVALID_ONION_VERSION; - return tal_free(m); + return WIRE_INVALID_ONION_VERSION; } read_buffer(rawEphemeralkey, src, sizeof(rawEphemeralkey), &p); if (!pubkey_from_der(rawEphemeralkey, sizeof(rawEphemeralkey), - &m->ephemeralkey)) { - *why_bad = WIRE_INVALID_ONION_KEY; - return tal_free(m); + &dest->ephemeralkey)) { + return WIRE_INVALID_ONION_KEY; } - read_buffer(&m->routinginfo, src, ROUTING_INFO_SIZE, &p); - read_buffer(&m->mac, src, HMAC_SIZE, &p); - return m; + read_buffer(&dest->routinginfo, src, ROUTING_INFO_SIZE, &p); + read_buffer(&dest->mac, src, HMAC_SIZE, &p); + return 0; } static void xorbytes(uint8_t *d, const uint8_t *a, const uint8_t *b, size_t len) diff --git a/common/sphinx.h b/common/sphinx.h index bad91c643..30078b929 100644 --- a/common/sphinx.h +++ b/common/sphinx.h @@ -148,15 +148,13 @@ u8 *serialize_onionpacket( /** * parse_onionpacket - Parse an onionpacket from a buffer. * - * @ctx: tal context to allocate from * @src: buffer to read the packet from * @srclen: length of the @src (must be TOTAL_PACKET_SIZE) - * @why_bad: if NULL return, this is what was wrong with the packet. + * @dest: the destination into which we should parse the packet */ -struct onionpacket *parse_onionpacket(const tal_t *ctx, - const void *src, - const size_t srclen, - enum onion_type *why_bad); +enum onion_type parse_onionpacket(const u8 *src, + const size_t srclen, + struct onionpacket *dest); struct onionreply { /* Node index in the path that is replying */ diff --git a/devtools/onion.c b/devtools/onion.c index 4bec8b933..a4ef5d934 100644 --- a/devtools/onion.c +++ b/devtools/onion.c @@ -103,21 +103,21 @@ static struct route_step *decode_with_privkey(const tal_t *ctx, const u8 *onion, { struct privkey seckey; struct route_step *step; - struct onionpacket *packet; + struct onionpacket packet; enum onion_type why_bad; u8 shared_secret[32]; if (!hex_decode(hexprivkey, strlen(hexprivkey), &seckey, sizeof(seckey))) errx(1, "Invalid private key hex '%s'", hexprivkey); - packet = parse_onionpacket(ctx, onion, TOTAL_PACKET_SIZE, &why_bad); + why_bad = parse_onionpacket(onion, TOTAL_PACKET_SIZE, &packet); - if (!packet) + if (why_bad != 0) errx(1, "Error parsing message: %s", onion_type_name(why_bad)); - if (!onion_shared_secret(shared_secret, packet, &seckey)) + if (!onion_shared_secret(shared_secret, &packet, &seckey)) errx(1, "Error creating shared secret."); - step = process_onionpacket(ctx, packet, shared_secret, assocdata, + step = process_onionpacket(ctx, &packet, shared_secret, assocdata, tal_bytelen(assocdata)); return step; diff --git a/lightningd/pay.c b/lightningd/pay.c index b8a8675b0..525ec955b 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -969,7 +969,7 @@ static struct command_result *json_sendonion(struct command *cmd, const jsmntok_t *params) { u8 *onion; - struct onionpacket *packet; + struct onionpacket packet; enum onion_type failcode; struct htlc_out *hout; struct route_hop *first_hop; @@ -989,9 +989,9 @@ static struct command_result *json_sendonion(struct command *cmd, NULL)) return command_param_failed(); - packet = parse_onionpacket(cmd, onion, tal_bytelen(onion), &failcode); + failcode = parse_onionpacket(onion, tal_bytelen(onion), &packet); - if (!packet) + if (failcode != 0) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Could not parse the onion. Parsing failed " "with failcode=%d", @@ -1032,7 +1032,7 @@ static struct command_result *json_sendonion(struct command *cmd, wallet_local_htlc_out_delete(ld->wallet, channel, payment_hash); } - failcode = send_onion(cmd->ld, packet, first_hop, payment_hash, channel, + failcode = send_onion(cmd->ld, &packet, first_hop, payment_hash, channel, &hout); payment = tal(hout, struct wallet_payment); diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 867b5ed7c..4eb53ea04 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -894,7 +894,7 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id, { struct htlc_in *hin; struct route_step *rs; - struct onionpacket *op; + struct onionpacket op; struct lightningd *ld = channel->peer->ld; struct htlc_accepted_hook_payload *hook_payload; @@ -947,10 +947,10 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id, /* channeld calls both parse_onionpacket and process_onionpacket, * so they should succeed.. */ - op = parse_onionpacket(tmpctx, hin->onion_routing_packet, - sizeof(hin->onion_routing_packet), - failcode); - if (!op) { + *failcode = parse_onionpacket(hin->onion_routing_packet, + sizeof(hin->onion_routing_packet), + &op); + if (*failcode != 0) { channel_internal_error(channel, "bad onion in got_revoke: %s", tal_hexstr(channel, hin->onion_routing_packet, @@ -958,7 +958,7 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id, return false; } - rs = process_onionpacket(tmpctx, op, hin->shared_secret->data, + rs = process_onionpacket(tmpctx, &op, hin->shared_secret->data, hin->payment_hash.u.u8, sizeof(hin->payment_hash)); if (!rs) { diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 228b8ab8b..56c359e8a 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -477,10 +477,9 @@ struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name const jsmntok_t **out UNNEEDED) { fprintf(stderr, "param_tok called!\n"); abort(); } /* Generated stub for parse_onionpacket */ -struct onionpacket *parse_onionpacket(const tal_t *ctx UNNEEDED, - const void *src UNNEEDED, - const size_t srclen UNNEEDED, - enum onion_type *why_bad UNNEEDED) +enum onion_type parse_onionpacket(const u8 *src UNNEEDED, + const size_t srclen UNNEEDED, + struct onionpacket *dest UNNEEDED) { fprintf(stderr, "parse_onionpacket called!\n"); abort(); } /* Generated stub for payment_failed */ void payment_failed(struct lightningd *ld UNNEEDED, const struct htlc_out *hout UNNEEDED,