diff --git a/channeld/channeld.c b/channeld/channeld.c index e8faa36f9..e71c54159 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -529,7 +529,8 @@ 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, - enum onion_type *why_bad) + enum onion_type *why_bad, + struct sha256 *next_onion_sha) { struct pubkey ephemeral; struct onionpacket *op; @@ -559,6 +560,10 @@ static struct secret *get_shared_secret(const tal_t *ctx, return tal_free(secret); } + /* Calculate sha256 we'll hand to next peer, in case they complain. */ + msg = serialize_onionpacket(tmpctx, rs->next); + sha256(next_onion_sha, msg, tal_bytelen(msg)); + return secret; } @@ -592,7 +597,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->why_bad_onion); + &htlc->why_bad_onion, + &htlc->next_onion_sha); } static void handle_peer_feechange(struct peer *peer, const u8 *msg) @@ -1799,12 +1805,10 @@ static void send_fail_or_fulfill(struct peer *peer, const struct htlc *h) } else if (h->failcode || h->fail) { const u8 *onion; if (h->failcode) { - /* FIXME: we need sha256_of_onion from peer. */ - struct sha256 dummy; - memset(&dummy, 0, sizeof(dummy)); /* Local failure, make a message. */ u8 *failmsg = make_failmsg(tmpctx, peer, h, h->failcode, - h->failed_scid, &dummy); + h->failed_scid, + &h->next_onion_sha); onion = create_onionreply(tmpctx, h->shared_secret, failmsg); } else /* Remote failure, just forward. */ @@ -2613,7 +2617,8 @@ static void init_shared_secrets(struct channel *channel, htlc = channel_get_htlc(channel, REMOTE, htlcs[i].id); htlc->shared_secret = get_shared_secret(htlc, htlc, - &htlc->why_bad_onion); + &htlc->why_bad_onion, + &htlc->next_onion_sha); } } diff --git a/channeld/channeld_htlc.h b/channeld/channeld_htlc.h index 46dc4baa4..2e6ef2cc1 100644 --- a/channeld/channeld_htlc.h +++ b/channeld/channeld_htlc.h @@ -25,6 +25,8 @@ struct htlc { struct secret *shared_secret; /* If incoming HTLC has shared_secret, this is which BADONION error */ enum onion_type why_bad_onion; + /* sha256 of next_onion, in case peer says it was malformed. */ + struct sha256 next_onion_sha; /* FIXME: We could union these together: */ /* Routing information sent with this HTLC. */ diff --git a/lightningd/pay.c b/lightningd/pay.c index 11df40669..7fd7f850a 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -359,10 +359,8 @@ remote_routing_failure(const tal_t *ctx, /* If the error is a BADONION, then it's on behalf of the * following node. */ if (failcode & BADONION) { - log_debug(log, "failcode %u => erring_node %s", - failcode, - type_to_string(tmpctx, struct pubkey, - &route_nodes[origin_index + 1])); + log_debug(log, "failcode %u from onionreply %s", + failcode, tal_hex(tmpctx, failure->msg)); erring_node = &route_nodes[origin_index + 1]; } else erring_node = &route_nodes[origin_index]; diff --git a/tests/test_misc.py b/tests/test_misc.py index 3cef21ccd..33fbdd034 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -8,6 +8,7 @@ from ephemeral_port_reserve import reserve import json import os import pytest +import re import shutil import signal import socket @@ -1137,9 +1138,11 @@ def test_check_command(node_factory): sock.close() +@unittest.skipIf(not DEVELOPER, "need log_all_io") def test_bad_onion(node_factory, bitcoind): """Test that we get a reasonable error from sendpay when an onion is bad""" - l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True) + l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True, + opts={'log_all_io': True}) h = l4.rpc.invoice(123000, 'test_bad_onion', 'description')['payment_hash'] route = l1.rpc.getroute(l4.info['id'], 123000, 1)['route'] @@ -1163,6 +1166,16 @@ def test_bad_onion(node_factory, bitcoind): assert err.value.error['data']['erring_node'] == mangled_nodeid assert err.value.error['data']['erring_channel'] == route[2]['channel'] + # We should see a WIRE_UPDATE_FAIL_MALFORMED_HTLC from l4. + line = l4.daemon.is_in_log(r'\[OUT\] 0087') + # 008739d3149a5c37e95f9dae718ce46efc60248e110e10117d384870a6762e8e33030000000000000000d7fc52f6c32773aabca55628fe616058aecc44a384e0abfa85c0c48b449dd38dc005 + # type<--------------channelid---------------------------------------><--htlc-id-----><--------------------------------------------- sha_of_onion --->code + sha = re.search(r' 0087.{64}.{16}(.{64})', line).group(1) + + # Should see same sha in onionreply + line = l1.daemon.is_in_log(r'failcode .* from onionreply .*') + assert re.search(r'onionreply .*{}'.format(sha), line) + # Replace id with a different pubkey, so onion encoded badly at second hop. route[1]['id'] = mangled_nodeid l1.rpc.sendpay(route, h)