sphinx: fix potential data leak.

common/sphinx.c

@@ -511,6 +511,7 @@ struct onionpacket *create_onionpacket(
 			      sphinx_hop_size(&sp->hops[num_hops - 1]);
 	u8 filler[fillerSize];
 	struct keyset keys;
+	u8 padkey[KEY_LEN];
 	u8 nexthmac[HMAC_SIZE];
 	u8 stream[ROUTING_INFO_SIZE];
 	struct hop_params *params;
@@ -529,7 +530,16 @@ struct onionpacket *create_onionpacket(
 	}
 	packet->version = 0;
 	memset(nexthmac, 0, HMAC_SIZE);
-	memset(packet->routinginfo, 0, ROUTING_INFO_SIZE);
+
+	/* BOLT-e116441ee836447ac3f24cdca62bac1e0f223d5f #4:
+	 *
+	 * The packet is initialized with 1366 _random_ bytes derived from a
+	 * CSPRNG.
+	 */
+	/* Note that this is just hop_payloads: the rest of the packet is
+	 * overwritten below or above anyway. */
+	generate_key(padkey, "pad", 3, sp->session_key->data);
+	generate_cipher_stream(stream, padkey, ROUTING_INFO_SIZE);
 
 	generate_header_padding(filler, sizeof(filler), sp, params);
 

devtools/gossipwith.c

@@ -221,7 +221,7 @@ int main(int argc, char *argv[])
 			   "Stream complete gossip history at start");
 	opt_register_arg("--max-messages", opt_set_ulongval, opt_show_ulongval,
 			 &max_messages,
-			 "Terminate after reading this many messages (> 0)");
+			 "Terminate after reading this many messages");
 	opt_register_noarg("--stdin", opt_set_bool, &stream_stdin,
 			   "Stream gossip messages from stdin.");
 	opt_register_noarg("--no-init", opt_set_bool, &no_init,

gossipd/gossipd.c

@@ -1517,6 +1517,8 @@ static struct io_plan *handle_outpoint_spent(struct io_conn *conn,
 		    "Deleting channel %s due to the funding outpoint being "
 		    "spent",
 		    type_to_string(msg, struct short_channel_id, &scid));
+		/* Suppress any now-obsolete updates/announcements */
+		add_to_txout_failures(rstate, &scid);
 		remove_channel_from_store(rstate, chan);
 		/* Freeing is sufficient since everything else is allocated off
 		 * of the channel and this takes care of unregistering

gossipd/routing.c

@@ -258,8 +258,8 @@ static void txout_failure_age(struct routing_state *rstate)
 						   txout_failure_age, rstate);
 }
 
-static void add_to_txout_failures(struct routing_state *rstate,
-				  const struct short_channel_id *scid)
+void add_to_txout_failures(struct routing_state *rstate,
+			   const struct short_channel_id *scid)
 {
 	if (uintmap_add(&rstate->txout_failures, scid->u64, true)
 	    && ++rstate->num_txout_failures == 10000) {
@@ -1721,9 +1721,8 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
 	}
 
 	/* If a prior txout lookup failed there is little point it trying
-	 * again. Just drop the announcement and walk away whistling. Any non-0
-	 * result means this failed before. */
-	if (uintmap_get(&rstate->txout_failures, pending->short_channel_id.u64)) {
+	 * again. Just drop the announcement and walk away whistling. */
+	if (in_txout_failures(rstate, &pending->short_channel_id)) {
 		SUPERVERBOSE(
 		    "Ignoring channel_announcement of %s due to a prior txout "
 		    "query failure. The channel was likely closed on-chain.",

gossipd/routing.h

@@ -529,4 +529,8 @@ void remove_channel_from_store(struct routing_state *rstate,
 const char *unfinalized_entries(const tal_t *ctx, struct routing_state *rstate);
 
 void remove_all_gossip(struct routing_state *rstate);
+
+/* This scid is dead to us. */
+void add_to_txout_failures(struct routing_state *rstate,
+			   const struct short_channel_id *scid);
 #endif /* LIGHTNING_GOSSIPD_ROUTING_H */

tests/test_gossip.py

@@ -1094,8 +1094,10 @@ def test_gossipwith(node_factory):
 
 
 def test_gossip_notices_close(node_factory, bitcoind):
-    # We want IO logging so we can replay a channel_announce to l1.
-    l1 = node_factory.get_node(options={'log-level': 'io'})
+    # We want IO logging so we can replay a channel_announce to l1;
+    # We also *really* do feed it bad gossip!
+    l1 = node_factory.get_node(options={'log-level': 'io'},
+                               allow_bad_gossip=True)
     l2, l3 = node_factory.line_graph(2)
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     # FIXME: sending SIGUSR1 immediately may kill it before handler installed.
@@ -1121,17 +1123,13 @@ def test_gossip_notices_close(node_factory, bitcoind):
     wait_for(lambda: l1.rpc.listchannels()['channels'] == [])
     wait_for(lambda: l1.rpc.listnodes()['nodes'] == [])
 
-    # FIXME: This is a hack: we should have a framework for canned conversations
-    # This doesn't naturally terminate, so we give it 5 seconds.
-    try:
-        subprocess.run(['devtools/gossipwith',
-                        '{}@localhost:{}'.format(l1.info['id'], l1.port),
-                        channel_announcement,
-                        channel_update,
-                        node_announcement],
-                       timeout=5, stdout=subprocess.PIPE)
-    except subprocess.TimeoutExpired:
-        pass
+    subprocess.run(['devtools/gossipwith',
+                    '--max-messages=0',
+                    '{}@localhost:{}'.format(l1.info['id'], l1.port),
+                    channel_announcement,
+                    channel_update,
+                    node_announcement],
+                   timeout=TIMEOUT)
 
     # l1 should reject it.
     assert(l1.rpc.listchannels()['channels'] == [])