Add checks that amt_to_forward and outgoing_cltv_value are within

acceptable ranges

add log_route macro

Author: Antoine Riard

src/ln/channelmanager.rs

@@ -1625,7 +1625,18 @@ impl ChannelMessageHandler for ChannelManager {
 					hmac: next_hop_data.hmac.clone(),
 				};
 
-				//TODO: Check amt_to_forward and outgoing_cltv_value are within acceptable ranges!
+				let channel_state = self.channel_state.lock().unwrap();
+				let chan = channel_state.by_id.get(&msg.channel_id).unwrap();
+				let fee = chan.get_our_fee_base_msat(&*self.fee_estimator) + (next_hop_data.data.amt_to_forward * self.fee_proportional_millionths as u64 / 1000000) as u32;
+				if (msg.amount_msat - fee as u64) < next_hop_data.data.amt_to_forward {
+					log_debug!(self, "HTLC {} incorrect amount: in {} out {} fee required {}", msg.htlc_id, msg.amount_msat, next_hop_data.data.amt_to_forward, fee);
+					//TODO: give back a "channel_update" in failure onion packet ?
+					return_err!("Prior hop has deviated from specified parameters", 0x1000 | 12, &[0;0]);
+				}
+				if (msg.cltv_expiry - CLTV_EXPIRY_DELTA as u32) < next_hop_data.data.outgoing_cltv_value {
+					log_debug!(self, "HTLC {} incorrect CLTV: in {} out {} delta required {}", msg.htlc_id, msg.cltv_expiry, next_hop_data.data.outgoing_cltv_value, CLTV_EXPIRY_DELTA);
+					return_err!("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, &[0;0]);
+				}
 
 				PendingForwardHTLCInfo {
 					onion_packet: Some(outgoing_packet),
@@ -2595,7 +2606,6 @@ mod tests {
 		for (node, hop) in expected_route.iter().zip(route.hops.iter()) {
 			assert_eq!(hop.pubkey, node.node.get_our_node_id());
 		}
-
 		send_along_route(origin_node, route, expected_route, recv_value)
 	}
 
@@ -2804,15 +2814,15 @@ mod tests {
 			pubkey: nodes[2].node.get_our_node_id(),
 			short_channel_id: chan_3.0.contents.short_channel_id,
 			fee_msat: 0,
-			cltv_expiry_delta: chan_2.1.contents.cltv_expiry_delta as u32
+			cltv_expiry_delta: chan_2.0.contents.cltv_expiry_delta as u32
 		});
 		hops.push(RouteHop {
 			pubkey: nodes[1].node.get_our_node_id(),
 			short_channel_id: chan_2.0.contents.short_channel_id,
 			fee_msat: 1000000,
 			cltv_expiry_delta: TEST_FINAL_CLTV,
 		});
-		hops[1].fee_msat = chan_2.1.contents.fee_base_msat as u64 + chan_2.1.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
+		hops[1].fee_msat = chan_2.0.contents.fee_base_msat as u64 + chan_2.0.contents.fee_proportional_millionths as u64 * hops[2].fee_msat as u64 / 1000000;
 		hops[0].fee_msat = chan_3.1.contents.fee_base_msat as u64 + chan_3.1.contents.fee_proportional_millionths as u64 * hops[1].fee_msat as u64 / 1000000;
 		let payment_hash_2 = send_along_route(&nodes[1], Route { hops }, &vec!(&nodes[3], &nodes[2], &nodes[1])[..], 1000000).1;
 

src/ln/router.rs

@@ -258,29 +258,29 @@ impl RoutingMessageHandler for Router {
 			None => return Err(HandleError{err: "Couldn't find channel for update", action: Some(ErrorAction::IgnoreError)}),
 			Some(channel) => {
 				macro_rules! maybe_update_channel_info {
-					( $target: expr) => {
-						if $target.last_update >= msg.contents.timestamp {
+					( $first_side: expr, $other_side: expr) => {
+						if $first_side.last_update >= msg.contents.timestamp {
 							return Err(HandleError{err: "Update older than last processed update", action: Some(ErrorAction::IgnoreError)});
 						}
-						chan_was_enabled = $target.enabled;
-						$target.last_update = msg.contents.timestamp;
-						$target.enabled = chan_enabled;
-						$target.cltv_expiry_delta = msg.contents.cltv_expiry_delta;
-						$target.htlc_minimum_msat = msg.contents.htlc_minimum_msat;
-						$target.fee_base_msat = msg.contents.fee_base_msat;
-						$target.fee_proportional_millionths = msg.contents.fee_proportional_millionths;
+						chan_was_enabled = $first_side.enabled;
+						$first_side.last_update = msg.contents.timestamp;
+						$first_side.enabled = chan_enabled;
+						$other_side.cltv_expiry_delta = msg.contents.cltv_expiry_delta;
+						$other_side.htlc_minimum_msat = msg.contents.htlc_minimum_msat;
+						$other_side.fee_base_msat = msg.contents.fee_base_msat;
+						$other_side.fee_proportional_millionths = msg.contents.fee_proportional_millionths;
 					}
 				}
 
 				let msg_hash = Message::from_slice(&Sha256dHash::from_data(&msg.contents.encode()[..])[..]).unwrap();
 				if msg.contents.flags & 1 == 1 {
 					dest_node_id = channel.one_to_two.src_node_id.clone();
 					secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &channel.two_to_one.src_node_id);
-					maybe_update_channel_info!(channel.two_to_one);
+					maybe_update_channel_info!(channel.two_to_one, channel.one_to_two);
 				} else {
 					dest_node_id = channel.two_to_one.src_node_id.clone();
 					secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &channel.one_to_two.src_node_id);
-					maybe_update_channel_info!(channel.one_to_two);
+					maybe_update_channel_info!(channel.one_to_two, channel.two_to_one);
 				}
 			}
 		}

src/util/macro_logger.rs

@@ -3,6 +3,8 @@ use chain::transaction::OutPoint;
 use bitcoin::util::hash::Sha256dHash;
 use secp256k1::key::PublicKey;
 
+use ln::router::Route;
+
 use std;
 
 pub(crate) struct DebugPubKey<'a>(pub &'a PublicKey);
@@ -50,6 +52,21 @@ macro_rules! log_funding_channel_id {
 	}
 }
 
+pub(crate) struct DebugRoute<'a>(pub &'a Route);
+impl<'a> std::fmt::Display for DebugRoute<'a> {
+	fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
+		for (i,h) in self.0.hops.iter().enumerate() {
+			write!(f, "Hop {}\n pubkey {}\n short_channel_id {}\n fee_msat {}\n cltv_expiry_delta {}\n\n", i, log_pubkey!(h.pubkey), h.short_channel_id, h.fee_msat, h.cltv_expiry_delta)?;
+		}
+		Ok(())
+	}
+}
+macro_rules! log_route {
+	($obj: expr) => {
+		::util::macro_logger::DebugRoute(&$obj)
+	}
+}
+
 macro_rules! log_internal {
 	($self: ident, $lvl:expr, $($arg:tt)+) => (
 		&$self.logger.log(&Record::new($lvl, format_args!($($arg)+), module_path!(), file!(), line!()));