Skip to content

Check channel_announcement is on Bitcoin Mainnet or Testnet + test_invalid_channel_announcement #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/full_stack_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
57 changes: 55 additions & 2 deletions fuzz/fuzz_targets/router_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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 {
Expand Down Expand Up @@ -44,6 +50,46 @@ 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])
}
}

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(1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't generate full coverage of possible return values, best to expand the return set.

Some(slice) => Ok((Builder::new().push_opcode(opcodes::All::OP_PUSHBYTES_0).into_script().to_v0_p2wsh(), 0)),
None => Err(ChainError::UnknownTx),
}
}
}

#[inline]
pub fn do_test(data: &[u8]) {
reset_rng_state();
Expand Down Expand Up @@ -107,9 +153,16 @@ pub fn do_test(data: &[u8]) {
}

let logger: Arc<Logger> = Arc::new(test_logger::TestLogger{});
let input = Arc::new(InputData {
data: data.to_vec(),
read_pos: AtomicUsize::new(0),
});
let chain_monitor = Arc::new(DummyChainWatcher {
input: 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] {
Expand Down
30 changes: 29 additions & 1 deletion src/chain/chaininterface.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/chain/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand Down
6 changes: 6 additions & 0 deletions src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
81 changes: 78 additions & 3 deletions src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2093,13 +2093,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;
Expand Down Expand Up @@ -2816,7 +2817,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 = {
Expand All @@ -2825,7 +2826,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 });
}

Expand Down Expand Up @@ -3235,4 +3236,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() {
Copy link
Author

@ariard ariard Aug 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little bit plumbing but I needed to separate msg content from its signatures to tweak fields freely so can't reuse create_announced_chan_between_nodes here

//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());
}
}
Loading