channeld: include proper sha value in BADONION errors.

Fortunately, we can calculate the sha256 ourselves, so the
outgoing channeld doesn't need to tell us.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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);
 	}
 }
 

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. */

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];

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)