Merge pull request #145 from TheBlueMatt/2018-09-134-rebased
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 3 Sep 2018 22:10:51 +0000 (18:10 -0400)
committerGitHub <noreply@github.com>
Mon, 3 Sep 2018 22:10:51 +0000 (18:10 -0400)
#134 rebased

1  2 
src/ln/channel.rs
src/ln/channelmanager.rs

diff --combined src/ln/channel.rs
index a34db5dc7165dd31c49e5e7af4fae1c815ba2433,a15bbd813b706a2f7d3e4af2a8abb70a3b4540e3..0659f8261af0e52083fd7b9d5a3efceadc602c70
@@@ -2002,6 -2002,12 +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
                                if tx.txid() == self.channel_monitor.get_funding_txo().unwrap().txid {
                                        let txo_idx = self.channel_monitor.get_funding_txo().unwrap().index as usize;
                                        if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
 -                                              tx.output[txo_idx].value != self.channel_value_satoshis {
 +                                                      tx.output[txo_idx].value != self.channel_value_satoshis {
 +                                              if self.channel_outbound {
 +                                                      // If we generated the funding transaction and it doesn't match what it
 +                                                      // should, the client is really broken and we should just panic and
 +                                                      // tell them off. That said, because hash collisions happen with high
 +                                                      // probability in fuzztarget mode, if we're fuzzing we just close the
 +                                                      // channel and move on.
 +                                                      #[cfg(not(feature = "fuzztarget"))]
 +                                                      panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
 +                                              }
                                                self.channel_state = ChannelState::ShutdownComplete as u32;
                                                self.channel_update_count += 1;
                                                return Err(HandleError{err: "funding tx had wrong script/value", action: Some(ErrorAction::DisconnectPeer{msg: None})});
diff --combined src/ln/channelmanager.rs
index dfe246d547f6c1321c7302e1427d04623c38218f,2bcf90d9631952433c1654e048e976421fd6386d..956a08c98dbb2e6b00af4e5a7f894c38d2f2be78
@@@ -189,10 -189,11 +189,10 @@@ pub struct ChannelManager 
  const CLTV_EXPIRY_DELTA: u16 = 6 * 24 * 2; //TODO?
  
  macro_rules! secp_call {
 -      ( $res : expr ) => {
 +      ( $res: expr, $err_msg: expr, $action: expr ) => {
                match $res {
                        Ok(key) => key,
 -                      //TODO: Make the err a parameter!
 -                      Err(_) => return Err(HandleError{err: "Key error", action: None})
 +                      Err(_) => return Err(HandleError{err: $err_msg, action: Some($action)})
                }
        };
  }
@@@ -474,9 -475,10 +474,9 @@@ impl ChannelManager 
  
        // can only fail if an intermediary hop has an invalid public key or session_priv is invalid
        #[inline]
 -      fn construct_onion_keys_callback<T: secp256k1::Signing, FType: FnMut(SharedSecret, [u8; 32], PublicKey, &RouteHop)> (secp_ctx: &Secp256k1<T>, route: &Route, session_priv: &SecretKey, mut callback: FType) -> Result<(), HandleError> {
 +      fn construct_onion_keys_callback<T: secp256k1::Signing, FType: FnMut(SharedSecret, [u8; 32], PublicKey, &RouteHop)> (secp_ctx: &Secp256k1<T>, route: &Route, session_priv: &SecretKey, mut callback: FType) -> Result<(), secp256k1::Error> {
                let mut blinded_priv = session_priv.clone();
                let mut blinded_pub = PublicKey::from_secret_key(secp_ctx, &blinded_priv);
 -              let mut first_iteration = true;
  
                for hop in route.hops.iter() {
                        let shared_secret = SharedSecret::new(secp_ctx, &hop.pubkey, &blinded_priv);
                        let mut blinding_factor = [0u8; 32];
                        sha.result(&mut blinding_factor);
  
 -                      if first_iteration {
 -                              blinded_pub = PublicKey::from_secret_key(secp_ctx, &blinded_priv);
 -                              first_iteration = false;
 -                      }
                        let ephemeral_pubkey = blinded_pub;
  
 -                      secp_call!(blinded_priv.mul_assign(secp_ctx, &secp_call!(SecretKey::from_slice(secp_ctx, &blinding_factor))));
 +                      blinded_priv.mul_assign(secp_ctx, &SecretKey::from_slice(secp_ctx, &blinding_factor)?)?;
                        blinded_pub = PublicKey::from_secret_key(secp_ctx, &blinded_priv);
  
                        callback(shared_secret, blinding_factor, ephemeral_pubkey, hop);
        }
  
        // can only fail if an intermediary hop has an invalid public key or session_priv is invalid
 -      fn construct_onion_keys<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, route: &Route, session_priv: &SecretKey) -> Result<Vec<OnionKeys>, HandleError> {
 +      fn construct_onion_keys<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, route: &Route, session_priv: &SecretKey) -> Result<Vec<OnionKeys>, secp256k1::Error> {
                let mut res = Vec::with_capacity(route.hops.len());
  
                Self::construct_onion_keys_callback(secp_ctx, route, session_priv, |shared_secret, _blinding_factor, ephemeral_pubkey, _| {
                        }
                }
  
 -              let session_priv = secp_call!(SecretKey::from_slice(&self.secp_ctx, &{
 +              let session_priv = SecretKey::from_slice(&self.secp_ctx, &{
                        let mut session_key = [0; 32];
                        rng::fill_bytes(&mut session_key);
                        session_key
 -              }));
 +              }).expect("RNG is bad!");
  
                let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
  
 -              let onion_keys = ChannelManager::construct_onion_keys(&self.secp_ctx, &route, &session_priv)?;
 +              //TODO: This should return something other than HandleError, that's really intended for
 +              //p2p-returns only.
 +              let onion_keys = secp_call!(ChannelManager::construct_onion_keys(&self.secp_ctx, &route, &session_priv), "Pubkey along hop was maliciously selected", msgs::ErrorAction::IgnoreError);
                let (onion_payloads, htlc_msat, htlc_cltv) = ChannelManager::build_onion_payloads(&route, cur_height)?;
                let onion_packet = ChannelManager::construct_onion_packet(onion_payloads, onion_keys, &payment_hash)?;
  
@@@ -1983,10 -1987,10 +1983,10 @@@ impl ChannelMessageHandler for ChannelM
                        match channel_state.by_id.get_mut(&msg.channel_id) {
                                Some(chan) => {
                                        if chan.get_their_node_id() != *their_node_id {
 -                                              return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: None})
 +                                              return Err(HandleError{err: "Got a message for a channel from the wrong node!", action: Some(msgs::ErrorAction::IgnoreError) })
                                        }
                                        if !chan.is_usable() {
 -                                              return Err(HandleError{err: "Got an announcement_signatures before we were ready for it", action: None });
 +                                              return Err(HandleError{err: "Got an announcement_signatures before we were ready for it", action: Some(msgs::ErrorAction::IgnoreError) });
                                        }
  
                                        let our_node_id = self.get_our_node_id();
  
                                        let were_node_one = announcement.node_id_1 == our_node_id;
                                        let msghash = Message::from_slice(&Sha256dHash::from_data(&announcement.encode()[..])[..]).unwrap();
 -                                      secp_call!(self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }));
 -                                      secp_call!(self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }));
 +                                      let bad_sig_action = msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id: msg.channel_id.clone(), data: "Invalid signature in announcement_signatures".to_string() } };
 +                                      secp_call!(self.secp_ctx.verify(&msghash, &msg.node_signature, if were_node_one { &announcement.node_id_2 } else { &announcement.node_id_1 }), "Bad announcement_signatures node_signature", bad_sig_action);
 +                                      secp_call!(self.secp_ctx.verify(&msghash, &msg.bitcoin_signature, if were_node_one { &announcement.bitcoin_key_2 } else { &announcement.bitcoin_key_1 }), "Bad announcement_signatures bitcoin_signature", bad_sig_action);
  
                                        let our_node_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
  
                                                contents: announcement,
                                        }, self.get_channel_update(chan).unwrap()) // can only fail if we're not in a ready state
                                },
 -                              None => return Err(HandleError{err: "Failed to find corresponding channel", action: None})
 +                              None => return Err(HandleError{err: "Failed to find corresponding channel", action: Some(msgs::ErrorAction::IgnoreError)})
                        }
                };
                let mut pending_events = self.pending_events.lock().unwrap();
@@@ -2090,13 -2093,14 +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;
  
                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 = {
                                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 });
                }
  
                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());
+       }
  }