From 554c3ec7e5a4a223b0e8eb881e8dfc69cc2a3b48 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 8 Jan 2019 11:22:13 +1030 Subject: [PATCH] channeld: process onion packet ourselves. This covers all the cases where an onion can be malformed; this means we know in advance that it's bad. That allows us to distinguish two cases: where lightningd rejects the onion as bad, and where the next peer rejects the next onion as bad. Both of those (will) set failcode to one of the BADONION values. Signed-off-by: Rusty Russell --- channeld/channeld.c | 24 ++++++++++++++++++------ channeld/channeld_htlc.h | 2 ++ lightningd/peer_htlcs.c | 9 +++++++-- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index c56735cdc..6cac0f1c4 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -528,18 +528,18 @@ static void handle_peer_announcement_signatures(struct peer *peer, const u8 *msg } static struct secret *get_shared_secret(const tal_t *ctx, - const struct htlc *htlc) + const struct htlc *htlc, + enum onion_type *why_bad) { struct pubkey ephemeral; struct onionpacket *op; struct secret *secret = tal(ctx, struct secret); const u8 *msg; - /* FIXME: Use this! */ - enum onion_type why_bad; + struct route_step *rs; /* We unwrap the onion now. */ op = parse_onionpacket(tmpctx, htlc->routing, TOTAL_PACKET_SIZE, - &why_bad); + why_bad); if (!op) return tal_free(secret); @@ -549,6 +549,16 @@ static struct secret *get_shared_secret(const tal_t *ctx, 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, + htlc->rhash.u.u8, + sizeof(htlc->rhash)); + if (!rs) { + *why_bad = WIRE_INVALID_ONION_HMAC; + return tal_free(secret); + } + return secret; } @@ -581,7 +591,8 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg) /* If this is wrong, we don't complain yet; when it's confirmed we'll * send it to the master which handles all HTLC failures. */ - htlc->shared_secret = get_shared_secret(htlc, htlc); + htlc->shared_secret = get_shared_secret(htlc, htlc, + &htlc->why_bad_onion); } static void handle_peer_feechange(struct peer *peer, const u8 *msg) @@ -2593,7 +2604,8 @@ static void init_shared_secrets(struct channel *channel, continue; htlc = channel_get_htlc(channel, REMOTE, htlcs[i].id); - htlc->shared_secret = get_shared_secret(htlc, htlc); + htlc->shared_secret = get_shared_secret(htlc, htlc, + &htlc->why_bad_onion); } } diff --git a/channeld/channeld_htlc.h b/channeld/channeld_htlc.h index 18a50b600..46dc4baa4 100644 --- a/channeld/channeld_htlc.h +++ b/channeld/channeld_htlc.h @@ -23,6 +23,8 @@ struct htlc { /* The routing shared secret (only for incoming) */ struct secret *shared_secret; + /* If incoming HTLC has shared_secret, this is which BADONION error */ + enum onion_type why_bad_onion; /* FIXME: We could union these together: */ /* Routing information sent with this HTLC. */ diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 7a9cfac5c..0433ed7da 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -646,6 +646,8 @@ static bool peer_accepted_htlc(struct channel *channel, goto out; } + /* FIXME: Have channeld hand through just the route_step! */ + /* channeld tests this, so it should pass. */ op = parse_onionpacket(tmpctx, hin->onion_routing_packet, sizeof(hin->onion_routing_packet), @@ -663,8 +665,11 @@ static bool peer_accepted_htlc(struct channel *channel, hin->payment_hash.u.u8, sizeof(hin->payment_hash)); if (!rs) { - *failcode = WIRE_INVALID_ONION_HMAC; - goto out; + channel_internal_error(channel, + "bad process_onionpacket in got_revoke: %s", + tal_hexstr(channel, hin->onion_routing_packet, + sizeof(hin->onion_routing_packet))); + return false; } /* Unknown realm isn't a bad onion, it's a normal failure. */