Merge pull request #145 from TheBlueMatt/2018-09-134-rebased

#134 rebased

fuzz/fuzz_targets/full_stack_target.rs

@@ -232,12 +232,12 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
 		Err(_) => return,
 	};
 
-	let watch = Arc::new(ChainWatchInterfaceUtil::new(Arc::clone(&logger)));
+	let watch = Arc::new(ChainWatchInterfaceUtil::new(Network::Bitcoin, Arc::clone(&logger)));
 	let broadcast = Arc::new(TestBroadcaster{});
 	let monitor = channelmonitor::SimpleManyChannelMonitor::new(watch.clone(), broadcast.clone());
 
 	let channelmanager = ChannelManager::new(our_network_key, slice_to_be32(get_slice!(4)), get_slice!(1)[0] != 0, Network::Bitcoin, fee_est.clone(), monitor.clone(), watch.clone(), broadcast.clone(), Arc::clone(&logger)).unwrap();
-	let router = Arc::new(Router::new(PublicKey::from_secret_key(&secp_ctx, &our_network_key), Arc::clone(&logger)));
+	let router = Arc::new(Router::new(PublicKey::from_secret_key(&secp_ctx, &our_network_key), watch.clone(), Arc::clone(&logger)));
 
 	let peers = RefCell::new([false; 256]);
 	let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler {

fuzz/fuzz_targets/router_target.rs

@@ -2,6 +2,11 @@ extern crate bitcoin;
 extern crate lightning;
 extern crate secp256k1;
 
+use bitcoin::util::hash::Sha256dHash;
+use bitcoin::blockdata::script::{Script, Builder};
+use bitcoin::blockdata::opcodes;
+
+use lightning::chain::chaininterface::{ChainError,ChainWatchInterface, ChainListener};
 use lightning::ln::channelmanager::ChannelDetails;
 use lightning::ln::msgs;
 use lightning::ln::msgs::{MsgDecodable, RoutingMessageHandler};
@@ -16,7 +21,8 @@ mod utils;
 
 use utils::test_logger;
 
-use std::sync::Arc;
+use std::sync::{Weak, Arc};
+use std::sync::atomic::{AtomicUsize, Ordering};
 
 #[inline]
 pub fn slice_to_be16(v: &[u8]) -> u16 {
@@ -44,27 +50,71 @@ pub fn slice_to_be64(v: &[u8]) -> u64 {
 	((v[7] as u64) << 8*0)
 }
 
+
+struct InputData {
+	data: Vec<u8>,
+	read_pos: AtomicUsize,
+}
+impl InputData {
+	fn get_slice(&self, len: usize) -> Option<&[u8]> {
+		let old_pos = self.read_pos.fetch_add(len, Ordering::AcqRel);
+		if self.data.len() < old_pos + len {
+			return None;
+		}
+		Some(&self.data[old_pos..old_pos + len])
+	}
+	fn get_slice_nonadvancing(&self, len: usize) -> Option<&[u8]> {
+		let old_pos = self.read_pos.load(Ordering::Acquire);
+		if self.data.len() < old_pos + len {
+			return None;
+		}
+		Some(&self.data[old_pos..old_pos + len])
+	}
+}
+
+struct DummyChainWatcher {
+	input: Arc<InputData>,
+}
+
+impl ChainWatchInterface for DummyChainWatcher {
+	fn install_watch_script(&self, _script_pub_key: &Script) { }
+	fn install_watch_outpoint(&self, _outpoint: (Sha256dHash, u32), _out_script: &Script) { }
+	fn watch_all_txn(&self) { }
+	fn register_listener(&self, _listener: Weak<ChainListener>) { }
+
+	fn get_chain_utxo(&self, _genesis_hash: Sha256dHash, _unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError> {
+		match self.input.get_slice(2) {
+			Some(&[0, _]) => Err(ChainError::NotSupported),
+			Some(&[1, _]) => Err(ChainError::NotWatched),
+			Some(&[2, _]) => Err(ChainError::UnknownTx),
+			Some(&[_, x]) => Ok((Builder::new().push_int(x as i64).into_script().to_v0_p2wsh(), 0)),
+			None => Err(ChainError::UnknownTx),
+			_ => unreachable!(),
+		}
+	}
+}
+
 #[inline]
 pub fn do_test(data: &[u8]) {
 	reset_rng_state();
 
-	let mut read_pos = 0;
+	let input = Arc::new(InputData {
+		data: data.to_vec(),
+		read_pos: AtomicUsize::new(0),
+	});
 	macro_rules! get_slice_nonadvancing {
 		($len: expr) => {
-			{
+			match input.get_slice_nonadvancing($len as usize) {
-				if data.len() < read_pos + $len as usize {
+				Some(slice) => slice,
-					return;
+				None => return,
-				}
-				&data[read_pos..read_pos + $len as usize]
 			}
 		}
 	}
 	macro_rules! get_slice {
 		($len: expr) => {
-			{
+			match input.get_slice($len as usize) {
-				let res = get_slice_nonadvancing!($len);
+				Some(slice) => slice,
-				read_pos += $len;
+				None => return,
-				res
 			}
 		}
 	}
@@ -107,9 +157,12 @@ pub fn do_test(data: &[u8]) {
 	}
 
 	let logger: Arc<Logger> = Arc::new(test_logger::TestLogger{});
+	let chain_monitor = Arc::new(DummyChainWatcher {
+		input: Arc::clone(&input),
+	});
 
 	let our_pubkey = get_pubkey!();
-	let router = Router::new(our_pubkey.clone(), Arc::clone(&logger));
+	let router = Router::new(our_pubkey.clone(), chain_monitor, Arc::clone(&logger));
 
 	loop {
 		match get_slice!(1)[0] {

src/chain/chaininterface.rs

@@ -1,11 +1,24 @@
 use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::blockdata::transaction::Transaction;
 use bitcoin::blockdata::script::Script;
+use bitcoin::blockdata::constants::genesis_block;
 use bitcoin::util::hash::Sha256dHash;
+use bitcoin::network::constants::Network;
+use bitcoin::network::serialize::BitcoinHash;
 use util::logger::Logger;
 use std::sync::{Mutex,Weak,MutexGuard,Arc};
 use std::sync::atomic::{AtomicUsize, Ordering};
 
+/// Used to give chain error details upstream
+pub enum ChainError {
+	/// Client doesn't support UTXO lookup (but the chain hash matches our genesis block hash)
+	NotSupported,
+	/// Chain isn't the one watched
+	NotWatched,
+	/// Tx doesn't exist or is unconfirmed
+	UnknownTx,
+}
+
 /// An interface to request notification of certain scripts as they appear the
 /// chain.
 /// Note that all of the functions implemented here *must* be reentrant-safe (obviously - they're
@@ -24,6 +37,12 @@ pub trait ChainWatchInterface: Sync + Send {
 
 	fn register_listener(&self, listener: Weak<ChainListener>);
 	//TODO: unregister
+
+	/// Gets the script and value in satoshis for a given unspent transaction output given a
+	/// short_channel_id (aka unspent_tx_output_identier). For BTC/tBTC channels the top three
+	/// bytes are the block height, the next 3 the transaction index within the block, and the
+	/// final two the output within the transaction.
+	fn get_chain_utxo(&self, genesis_hash: Sha256dHash, unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError>;
 }
 
 /// An interface to send a transaction to the Bitcoin network.
@@ -69,6 +88,7 @@ pub trait FeeEstimator: Sync + Send {
 /// Utility to capture some common parts of ChainWatchInterface implementors.
 /// Keeping a local copy of this in a ChainWatchInterface implementor is likely useful.
 pub struct ChainWatchInterfaceUtil {
+	network: Network,
 	watched: Mutex<(Vec<Script>, Vec<(Sha256dHash, u32)>, bool)>, //TODO: Something clever to optimize this
 	listeners: Mutex<Vec<Weak<ChainListener>>>,
 	reentered: AtomicUsize,
@@ -99,11 +119,19 @@ impl ChainWatchInterface for ChainWatchInterfaceUtil {
 		let mut vec = self.listeners.lock().unwrap();
 		vec.push(listener);
 	}
+
+	fn get_chain_utxo(&self, genesis_hash: Sha256dHash, _unspent_tx_output_identifier: u64) -> Result<(Script, u64), ChainError> {
+		if genesis_hash != genesis_block(self.network).header.bitcoin_hash() {
+			return Err(ChainError::NotWatched);
+		}
+		Err(ChainError::NotSupported)
+	}
 }
 
 impl ChainWatchInterfaceUtil {
-	pub fn new(logger: Arc<Logger>) -> ChainWatchInterfaceUtil {
+	pub fn new(network: Network, logger: Arc<Logger>) -> ChainWatchInterfaceUtil {
 		ChainWatchInterfaceUtil {
+			network: network,
 			watched: Mutex::new((Vec::new(), Vec::new(), false)),
 			listeners: Mutex::new(Vec::new()),
 			reentered: AtomicUsize::new(1),

src/chain/transaction.rs

@@ -13,7 +13,7 @@ pub struct OutPoint {
 }
 
 impl OutPoint {
-	/// Creates a new `OutPoint` from the txid an the index.
+	/// Creates a new `OutPoint` from the txid and the index.
 	pub fn new(txid: Sha256dHash, index: u16) -> OutPoint {
 		OutPoint { txid, index }
 	}

src/ln/channel.rs

@@ -2002,6 +2002,12 @@ impl Channel {
 		self.channel_value_satoshis
 	}
 
+	//TODO: Testing purpose only, should be changed in another way after #81
+	#[cfg(test)]
+	pub fn get_local_keys(&self) -> &ChannelKeys {
+		&self.local_keys
+	}
+
 	/// Allowed in any state (including after shutdown)
 	pub fn get_channel_update_count(&self) -> u32 {
 		self.channel_update_count

src/ln/channelmanager.rs

@@ -2090,13 +2090,14 @@ mod tests {
 	use bitcoin::util::hash::Sha256dHash;
 	use bitcoin::blockdata::block::{Block, BlockHeader};
 	use bitcoin::blockdata::transaction::{Transaction, TxOut};
+	use bitcoin::blockdata::constants::genesis_block;
 	use bitcoin::network::constants::Network;
 	use bitcoin::network::serialize::serialize;
 	use bitcoin::network::serialize::BitcoinHash;
 
 	use hex;
 
-	use secp256k1::Secp256k1;
+	use secp256k1::{Secp256k1, Message};
 	use secp256k1::key::{PublicKey,SecretKey};
 
 	use crypto::sha2::Sha256;
@@ -2813,7 +2814,7 @@ mod tests {
 
 		for _ in 0..node_count {
 			let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
-			let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Arc::clone(&logger)));
+			let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger)));
 			let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
 			let chan_monitor = Arc::new(test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone()));
 			let node_id = {
@@ -2822,7 +2823,7 @@ mod tests {
 				SecretKey::from_slice(&secp_ctx, &key_slice).unwrap()
 			};
 			let node = ChannelManager::new(node_id.clone(), 0, true, Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger)).unwrap();
-			let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &node_id), Arc::clone(&logger));
+			let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &node_id), chain_monitor.clone(), Arc::clone(&logger));
 			nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router });
 		}
 
@@ -3232,4 +3233,78 @@ mod tests {
 		assert_eq!(channel_state.by_id.len(), 0);
 		assert_eq!(channel_state.short_to_id.len(), 0);
 	}
+
+	#[test]
+	fn test_invalid_channel_announcement() {
+		//Test BOLT 7 channel_announcement msg requirement for final node, gather data to build customed channel_announcement msgs
+		let secp_ctx = Secp256k1::new();
+		let nodes = create_network(2);
+
+		let chan_announcement = create_chan_between_nodes(&nodes[0], &nodes[1]);
+
+		let a_channel_lock = nodes[0].node.channel_state.lock().unwrap();
+		let b_channel_lock = nodes[1].node.channel_state.lock().unwrap();
+		let as_chan = a_channel_lock.by_id.get(&chan_announcement.3).unwrap();
+		let bs_chan = b_channel_lock.by_id.get(&chan_announcement.3).unwrap();
+
+		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap() } );
+
+		let as_bitcoin_key = PublicKey::from_secret_key(&secp_ctx, &as_chan.get_local_keys().funding_key);
+		let bs_bitcoin_key = PublicKey::from_secret_key(&secp_ctx, &bs_chan.get_local_keys().funding_key);
+
+		let as_network_key = nodes[0].node.get_our_node_id();
+		let bs_network_key = nodes[1].node.get_our_node_id();
+
+		let were_node_one = as_bitcoin_key.serialize()[..] < bs_bitcoin_key.serialize()[..];
+
+		let mut chan_announcement;
+
+		macro_rules! dummy_unsigned_msg {
+			() => {
+				msgs::UnsignedChannelAnnouncement {
+					features: msgs::GlobalFeatures::new(),
+					chain_hash: genesis_block(Network::Testnet).header.bitcoin_hash(),
+					short_channel_id: as_chan.get_short_channel_id().unwrap(),
+					node_id_1: if were_node_one { as_network_key } else { bs_network_key },
+					node_id_2: if !were_node_one { bs_network_key } else { as_network_key },
+					bitcoin_key_1: if were_node_one { as_bitcoin_key } else { bs_bitcoin_key },
+					bitcoin_key_2: if !were_node_one { bs_bitcoin_key } else { as_bitcoin_key },
+					excess_data: Vec::new(),
+				};
+			}
+		}
+
+		macro_rules! sign_msg {
+			($unsigned_msg: expr) => {
+				let msghash = Message::from_slice(&Sha256dHash::from_data(&$unsigned_msg.encode()[..])[..]).unwrap();
+				let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().funding_key);
+				let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().funding_key);
+				let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].node.our_network_key);
+				let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].node.our_network_key);
+				chan_announcement = msgs::ChannelAnnouncement {
+					node_signature_1 : if were_node_one { as_node_sig } else { bs_node_sig},
+					node_signature_2 : if !were_node_one { bs_node_sig } else { as_node_sig},
+					bitcoin_signature_1: if were_node_one { as_bitcoin_sig } else { bs_bitcoin_sig },
+					bitcoin_signature_2 : if !were_node_one { bs_bitcoin_sig } else { as_bitcoin_sig },
+					contents: $unsigned_msg
+				}
+			}
+		}
+
+		let unsigned_msg = dummy_unsigned_msg!();
+		sign_msg!(unsigned_msg);
+		assert_eq!(nodes[0].router.handle_channel_announcement(&chan_announcement).unwrap(), true);
+		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap() } );
+
+		// Configured with Network::Testnet
+		let mut unsigned_msg = dummy_unsigned_msg!();
+		unsigned_msg.chain_hash = genesis_block(Network::Bitcoin).header.bitcoin_hash();
+		sign_msg!(unsigned_msg);
+		assert!(nodes[0].router.handle_channel_announcement(&chan_announcement).is_err());
+
+		let mut unsigned_msg = dummy_unsigned_msg!();
+		unsigned_msg.chain_hash = Sha256dHash::from_data(&[1,2,3,4,5,6,7,8,9]);
+		sign_msg!(unsigned_msg);
+		assert!(nodes[0].router.handle_channel_announcement(&chan_announcement).is_err());
+	}
 }

src/ln/router.rs

@@ -3,7 +3,10 @@ use secp256k1::{Secp256k1,Message};
 use secp256k1;
 
 use bitcoin::util::hash::Sha256dHash;
+use bitcoin::blockdata::script::Builder;
+use bitcoin::blockdata::opcodes;
 
+use chain::chaininterface::{ChainError, ChainWatchInterface};
 use ln::channelmanager;
 use ln::msgs::{ErrorAction,HandleError,RoutingMessageHandler,MsgEncodable,NetAddress,GlobalFeatures};
 use ln::msgs;
@@ -155,6 +158,7 @@ pub struct RouteHint {
 pub struct Router {
 	secp_ctx: Secp256k1<secp256k1::VerifyOnly>,
 	network_map: RwLock<NetworkMap>,
+	chain_monitor: Arc<ChainWatchInterface>,
 	logger: Arc<Logger>,
 }
 
@@ -201,13 +205,33 @@ impl RoutingMessageHandler for Router {
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_1, &msg.contents.bitcoin_key_1);
 		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
 
-		//TODO: Call blockchain thing to ask if the short_channel_id is valid
-		//TODO: Only allow bitcoin chain_hash
-
 		if msg.contents.features.requires_unknown_bits() {
 			panic!("Unknown-required-features ChannelAnnouncements should never deserialize!");
 		}
 
+		match self.chain_monitor.get_chain_utxo(msg.contents.chain_hash, msg.contents.short_channel_id) {
+			Ok((script_pubkey, _value)) => {
+				let expected_script = Builder::new().push_opcode(opcodes::All::OP_PUSHNUM_2)
+				                                    .push_slice(&msg.contents.bitcoin_key_1.serialize())
+				                                    .push_slice(&msg.contents.bitcoin_key_2.serialize())
+				                                    .push_opcode(opcodes::All::OP_PUSHNUM_2).push_opcode(opcodes::All::OP_CHECKMULTISIG).into_script().to_v0_p2wsh();
+				if script_pubkey != expected_script {
+					return Err(HandleError{err: "Channel announcement keys didn't match on-chain script", action: Some(ErrorAction::IgnoreError)});
+				}
+				//TODO: Check if value is worth storing, use it to inform routing, and compare it
+				//to the new HTLC max field in channel_update
+			},
+			Err(ChainError::NotSupported) => {
+				// Tentatively accept, potentially exposing us to DoS attacks
+			},
+			Err(ChainError::NotWatched) => {
+				return Err(HandleError{err: "Channel announced on an unknown chain", action: Some(ErrorAction::IgnoreError)});
+			},
+			Err(ChainError::UnknownTx) => {
+				return Err(HandleError{err: "Channel announced without corresponding UTXO entry", action: Some(ErrorAction::IgnoreError)});
+			},
+		}
+
 		let mut network = self.network_map.write().unwrap();
 
 		match network.channels.entry(NetworkMap::get_key(msg.contents.short_channel_id, msg.contents.chain_hash)) {
@@ -388,7 +412,7 @@ struct DummyDirectionalChannelInfo {
 }
 
 impl Router {
-	pub fn new(our_pubkey: PublicKey, logger: Arc<Logger>) -> Router {
+	pub fn new(our_pubkey: PublicKey, chain_monitor: Arc<ChainWatchInterface>, logger: Arc<Logger>) -> Router {
 		let mut nodes = HashMap::new();
 		nodes.insert(our_pubkey.clone(), NodeInfo {
 			channels: Vec::new(),
@@ -407,6 +431,7 @@ impl Router {
 				our_node_id: our_pubkey,
 				nodes: nodes,
 			}),
+			chain_monitor,
 			logger,
 		}
 	}
@@ -632,6 +657,7 @@ impl Router {
 
 #[cfg(test)]
 mod tests {
+	use chain::chaininterface;
 	use ln::channelmanager;
 	use ln::router::{Router,NodeInfo,NetworkMap,ChannelInfo,DirectionalChannelInfo,RouteHint};
 	use ln::msgs::GlobalFeatures;
@@ -639,6 +665,7 @@ mod tests {
 	use util::logger::Logger;
 
 	use bitcoin::util::hash::Sha256dHash;
+	use bitcoin::network::constants::Network;
 
 	use hex;
 
@@ -652,7 +679,8 @@ mod tests {
 		let secp_ctx = Secp256k1::new();
 		let our_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap());
 		let logger: Arc<Logger> = Arc::new(test_utils::TestLogger::new());
-		let router = Router::new(our_id, Arc::clone(&logger));
+		let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger)));
+		let router = Router::new(our_id, chain_monitor, Arc::clone(&logger));
 
 		// Build network from our_id to node8:
 		//