From: Valentine Wallace <vwallace@protonmail.com>
Date: Fri, 15 Sep 2023 20:55:12 +0000 (-0400)
Subject: Blame outbound channel on UPDATE onion failure with 0-len update
X-Git-Tag: v0.0.117-alpha2~8^2
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=6299f7d14f4a6b47e133c6abaa3a05ed50177b0f;p=rust-lightning

Blame outbound channel on UPDATE onion failure with 0-len update

We've run into this several times in the wild, likely due to
https://github.com/ElementsProject/lightning/issues/6200 wherein a node on the
path will error with 0x1000 but not provide a channel update (a spec
violation).

Previously, we would blame the inbound edge even though the buggy peer wanted
us to blame the outbound edge. Since this issue seems to be recurring and our
blaming the inbound edge is causing us to punish innocent channels, trust the
peer that the outbound edge is the one to blame.
---

diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs
index 3731c31c5..c124360d1 100644
--- a/lightning/src/ln/onion_route_tests.rs
+++ b/lightning/src/ln/onion_route_tests.rs
@@ -645,8 +645,34 @@ fn test_onion_failure() {
 		}, || nodes[2].node.fail_htlc_backwards(&payment_hash), false, None,
 		Some(NetworkUpdate::NodeFailure { node_id: route.paths[0].hops[1].pubkey, is_permanent: true }),
 		Some(channels[1].0.contents.short_channel_id));
-	run_onion_failure_test_with_fail_intercept("0-length channel update in UPDATE onion failure", 200, &nodes,
-		&route, &payment_hash, &payment_secret, |_msg| {}, |msg| {
+	run_onion_failure_test_with_fail_intercept("0-length channel update in intermediate node UPDATE onion failure",
+		100, &nodes, &route, &payment_hash, &payment_secret, |msg| {
+			msg.amount_msat -= 1;
+		}, |msg| {
+			let session_priv = SecretKey::from_slice(&[3; 32]).unwrap();
+			let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
+			let mut decoded_err_packet = msgs::DecodedOnionErrorPacket {
+				failuremsg: vec![
+					0x10, 0x7, // UPDATE|7
+					0x0, 0x0 // 0-len channel update
+				],
+				pad: vec![0; 255 - 4 /* 4-byte error message */],
+				hmac: [0; 32],
+			};
+			let um = onion_utils::gen_um_from_shared_secret(&onion_keys[0].shared_secret.as_ref());
+			let mut hmac = HmacEngine::<Sha256>::new(&um);
+			hmac.input(&decoded_err_packet.encode()[32..]);
+			decoded_err_packet.hmac = Hmac::from_engine(hmac).into_inner();
+			msg.reason = onion_utils::encrypt_failure_packet(
+				&onion_keys[0].shared_secret.as_ref(), &decoded_err_packet.encode()[..])
+		}, || {}, true, Some(0x1000|7),
+		Some(NetworkUpdate::ChannelFailure {
+			short_channel_id: channels[1].0.contents.short_channel_id,
+			is_permanent: false,
+		}),
+		Some(channels[1].0.contents.short_channel_id));
+	run_onion_failure_test_with_fail_intercept("0-length channel update in final node UPDATE onion failure",
+		200, &nodes, &route, &payment_hash, &payment_secret, |_msg| {}, |msg| {
 			let session_priv = SecretKey::from_slice(&[3; 32]).unwrap();
 			let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
 			let mut decoded_err_packet = msgs::DecodedOnionErrorPacket {
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index f3f860860..390826e4f 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -641,8 +641,9 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
 							} else {
 								// The node in question intentionally encoded a 0-length channel update. This is
 								// likely due to https://github.com/ElementsProject/lightning/issues/6200.
+								short_channel_id = Some(failing_route_hop.short_channel_id);
 								network_update = Some(NetworkUpdate::ChannelFailure {
-									short_channel_id: route_hop.short_channel_id,
+									short_channel_id: failing_route_hop.short_channel_id,
 									is_permanent: false,
 								});
 							}