diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 4edbd5fc4..cc4342b3b 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -70,6 +70,7 @@ msgdata,channel_init,remote_ann_node_sig,?secp256k1_ecdsa_signature, msgdata,channel_init,remote_ann_bitcoin_sig,?secp256k1_ecdsa_signature, msgdata,channel_init,option_static_remotekey,bool, msgdata,channel_init,dev_fast_gossip,bool, +msgdata,channel_init,dev_fail_process_onionpacket,bool, # master->channeld funding hit new depth(funding locked if >= lock depth) msgtype,channel_funding_depth,1002 diff --git a/channeld/channeld.c b/channeld/channeld.c index 8d083e355..4dbb8b6a7 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -3007,6 +3007,9 @@ static void init_channel(struct peer *peer) secp256k1_ecdsa_signature *remote_ann_node_sig; secp256k1_ecdsa_signature *remote_ann_bitcoin_sig; bool option_static_remotekey; +#if !DEVELOPER + bool dev_fail_process_onionpacket; /* Ignored */ +#endif assert(!(fcntl(MASTER_FD, F_GETFL) & O_NONBLOCK)); @@ -3066,8 +3069,9 @@ static void init_channel(struct peer *peer) &remote_ann_node_sig, &remote_ann_bitcoin_sig, &option_static_remotekey, - &dev_fast_gossip)) { - master_badmsg(WIRE_CHANNEL_INIT, msg); + &dev_fast_gossip, + &dev_fail_process_onionpacket)) { + master_badmsg(WIRE_CHANNEL_INIT, msg); } /* stdin == requests, 3 == peer, 4 = gossip, 5 = gossip_store, 6 = HSM */ per_peer_state_set_fds(peer->pps, 3, 4, 5); diff --git a/common/sphinx.c b/common/sphinx.c index 4ffa61b74..66a735780 100644 --- a/common/sphinx.c +++ b/common/sphinx.c @@ -459,6 +459,10 @@ struct onionpacket *create_onionpacket( return packet; } +#if DEVELOPER +bool dev_fail_process_onionpacket; +#endif + /* * Given an onionpacket msg extract the information for the current * node and unwrap the remainder so that the node can forward it. @@ -487,7 +491,8 @@ struct route_step *process_onionpacket( compute_packet_hmac(msg, assocdata, assocdatalen, keys.mu, hmac); - if (memcmp(msg->mac, hmac, sizeof(hmac)) != 0) { + if (memcmp(msg->mac, hmac, sizeof(hmac)) != 0 + || IFDEV(dev_fail_process_onionpacket, false)) { /* Computed MAC does not match expected MAC, the message was modified. */ return tal_free(step); } diff --git a/common/sphinx.h b/common/sphinx.h index 1f687bd7b..045167d12 100644 --- a/common/sphinx.h +++ b/common/sphinx.h @@ -227,4 +227,9 @@ void sphinx_add_hop(struct sphinx_path *path, const struct pubkey *pubkey, */ size_t sphinx_path_payloads_size(const struct sphinx_path *path); +#if DEVELOPER +/* Override to force us to reject valid onion packets */ +extern bool dev_fail_process_onionpacket; +#endif + #endif /* LIGHTNING_COMMON_SPHINX_H */ diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index 1cb3e84e4..ddbedb70b 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -492,7 +492,8 @@ void peer_start_channeld(struct channel *channel, /* Set at channel open, even if not * negotiated now! */ channel->option_static_remotekey, - IFDEV(ld->dev_fast_gossip, false)); + IFDEV(ld->dev_fast_gossip, false), + IFDEV(dev_fail_process_onionpacket, false)); /* We don't expect a response: we are triggered by funding_depth_cb. */ subd_send_msg(channel->owner, take(initmsg)); diff --git a/lightningd/options.c b/lightningd/options.c index 7a874a8fc..17b065eb7 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -526,6 +526,9 @@ static void dev_register_opts(struct lightningd *ld) opt_register_noarg("--dev-no-htlc-timeout", opt_set_bool, &ld->dev_no_htlc_timeout, "Don't kill channeld if HTLCs not confirmed within 30 seconds"); + opt_register_noarg("--dev-fail-process-onionpacket", opt_set_bool, + &dev_fail_process_onionpacket, + "Force all processing of onion packets to fail"); } #endif /* DEVELOPER */ diff --git a/tests/test_misc.py b/tests/test_misc.py index e55c9dddb..8323e430a 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1591,6 +1591,28 @@ def test_bad_onion(node_factory, bitcoind): assert err.value.error['data']['erring_channel'] == route[1]['channel'] +@pytest.mark.xfail(strict=True) +@unittest.skipIf(not DEVELOPER, "Needs DEVELOPER=1 to force onion fail") +def test_bad_onion_immediate_peer(node_factory, bitcoind): + """Test that we handle the malformed msg when we're the origin""" + l1, l2 = node_factory.line_graph(2, opts={'dev-fail-process-onionpacket': None}) + + h = l2.rpc.invoice(123000, 'test_bad_onion_immediate_peer', 'description')['payment_hash'] + route = l1.rpc.getroute(l2.info['id'], 123000, 1)['route'] + assert len(route) == 1 + + l1.rpc.sendpay(route, h) + with pytest.raises(RpcError) as err: + l1.rpc.waitsendpay(h) + + # FIXME: #define PAY_UNPARSEABLE_ONION 202 + PAY_UNPARSEABLE_ONION = 202 + assert err.value.error['code'] == PAY_UNPARSEABLE_ONION + # FIXME: WIRE_INVALID_ONION_HMAC = BADONION|PERM|5 + WIRE_INVALID_ONION_HMAC = 0x8000 | 0x4000 | 5 + assert err.value.error['data']['failcode'] == WIRE_INVALID_ONION_HMAC + + def test_newaddr(node_factory, chainparams): l1 = node_factory.get_node() p2sh = l1.rpc.newaddr('p2sh-segwit')