mirrors/rust-lightning
1
0
Fork 0
mirror of https://github.com/lightningdevkit/rust-lightning.git synced 2025-02-24 15:02:20 +01:00
Code Issues Projects Releases Packages Wiki Activity

Move UTXO-lookup into pub utility function from RoutingMsgHandler

Browse source 
This makes the public utility methods in `NetworkGraph` able to do
UTXO lookups ala `NetworkMsgHandler`'s `RoutingMessageHandler`
implementation, slightly simplifying the public interface.

We also take this opportunity to verify signatures before calling
out to UTXO lookups, under the "do actions in order of
cheapest-to-most-expensive to reduce DoS surface" principle.
This commit is contained in:
Matt Corallo 2020-11-24 12:40:11 -05:00
parent 9642356d9a
commit d9c03f26f9
2 changed files with 39 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
fuzz/src/router.rs
View file

@ -175,14 +175,13 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
let msg = decode_msg_with_len16!(msgs::ChannelAnnouncement, 64*4, 32+8+33*4);
node_pks.insert(msg.contents.node_id_1);
node_pks.insert(msg.contents.node_id_2);
let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly>(&msg, None, None);
let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly, &FuzzChainSource>(&msg, &None, None);
},
2 => {
let msg = decode_msg_with_len16!(msgs::ChannelAnnouncement, 64*4, 32+8+33*4);
node_pks.insert(msg.contents.node_id_1);
node_pks.insert(msg.contents.node_id_2);
let val = slice_to_be64(get_slice!(8));
let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly>(&msg, Some(val), None);
let _ = net_graph.update_channel_from_announcement::<secp256k1::VerifyOnly, &FuzzChainSource>(&msg, &Some(&FuzzChainSource { input: Arc::clone(&input) }), None);
},
3 => {
let _ = net_graph.update_channel(&decode_msg!(msgs::ChannelUpdate, 136), None);

81
lightning/src/routing/network_graph.rs
View file

@ -125,40 +125,7 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
}
fn handle_channel_announcement(&self, msg: &msgs::ChannelAnnouncement) -> Result<bool, LightningError> {
if msg.contents.node_id_1 == msg.contents.node_id_2 || msg.contents.bitcoin_key_1 == msg.contents.bitcoin_key_2 {
return Err(LightningError{err: "Channel announcement node had a channel with itself".to_owned(), action: ErrorAction::IgnoreError});
}
let utxo_value = match &self.chain_access {
&None => {
// Tentatively accept, potentially exposing us to DoS attacks
None
},
&Some(ref chain_access) => {
match chain_access.get_utxo(&msg.contents.chain_hash, msg.contents.short_channel_id) {
Ok(TxOut { value, script_pubkey }) => {
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(LightningError{err: format!("Channel announcement key ({}) didn't match on-chain script ({})", script_pubkey.to_hex(), expected_script.to_hex()), action: 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
Some(value)
},
Err(chain::AccessError::UnknownChain) => {
return Err(LightningError{err: format!("Channel announced on an unknown chain ({})", msg.contents.chain_hash.encode().to_hex()), action: ErrorAction::IgnoreError});
},
Err(chain::AccessError::UnknownTx) => {
return Err(LightningError{err: "Channel announced without corresponding UTXO entry".to_owned(), action: ErrorAction::IgnoreError});
},
}
},
};
let result = self.network_graph.write().unwrap().update_channel_from_announcement(msg, utxo_value, Some(&self.secp_ctx));
let result = self.network_graph.write().unwrap().update_channel_from_announcement(msg, &self.chain_access, Some(&self.secp_ctx));
log_trace!(self.logger, "Added channel_announcement for {}{}", msg.contents.short_channel_id, if !msg.contents.excess_data.is_empty() { " with excess uninterpreted data!" } else { "" });
result
}
@ -605,17 +572,14 @@ impl NetworkGraph {
/// RoutingMessageHandler implementation to call it indirectly. This may be useful to accept
/// routing messages without checking their signatures.
///
/// If the channel has been confirmed to exist on chain (with correctly-formatted scripts on
/// chain), set utxo_value to the value of the output on chain, otherwise leave it as None.
/// The UTXO value is then used in routing calculation if we have no better information on the
/// maximum HTLC value that can be sent over the channel.
///
/// Further, setting utxo_value to Some indicates that the announcement message is genuine,
/// allowing us to update existing channel data in the case of reorgs or to replace bogus
/// channel data generated by a DoS attacker.
/// If a `chain::Access` object is provided via `chain_access`, it will be called to verify
/// the corresponding UTXO exists on chain and is correctly-formatted.
///
/// Announcement signatures are checked here only if Secp256k1 object is provided.
pub fn update_channel_from_announcement<T: secp256k1::Verification>(&mut self, msg: &msgs::ChannelAnnouncement, utxo_value: Option<u64>, secp_ctx: Option<&Secp256k1<T>>) -> Result<bool, LightningError> {
pub fn update_channel_from_announcement<T: secp256k1::Verification, C: Deref>
(&mut self, msg: &msgs::ChannelAnnouncement, chain_access: &Option<C>, secp_ctx: Option<&Secp256k1<T>>)
-> Result<bool, LightningError>
where C::Target: chain::Access {
if msg.contents.node_id_1 == msg.contents.node_id_2 || msg.contents.bitcoin_key_1 == msg.contents.bitcoin_key_2 {
return Err(LightningError{err: "Channel announcement node had a channel with itself".to_owned(), action: ErrorAction::IgnoreError});
}
@ -628,8 +592,37 @@ impl NetworkGraph {
secp_verify_sig!(sig_verifier, &msg_hash, &msg.bitcoin_signature_2, &msg.contents.bitcoin_key_2);
}
let should_relay = msg.contents.excess_data.is_empty();
let utxo_value = match &chain_access {
&None => {
// Tentatively accept, potentially exposing us to DoS attacks
None
},
&Some(ref chain_access) => {
match chain_access.get_utxo(&msg.contents.chain_hash, msg.contents.short_channel_id) {
Ok(TxOut { value, script_pubkey }) => {
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(LightningError{err: format!("Channel announcement key ({}) didn't match on-chain script ({})", script_pubkey.to_hex(), expected_script.to_hex()), action: 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
Some(value)
},
Err(chain::AccessError::UnknownChain) => {
return Err(LightningError{err: format!("Channel announced on an unknown chain ({})", msg.contents.chain_hash.encode().to_hex()), action: ErrorAction::IgnoreError});
},
Err(chain::AccessError::UnknownTx) => {
return Err(LightningError{err: "Channel announced without corresponding UTXO entry".to_owned(), action: ErrorAction::IgnoreError});
},
}
},
};
let should_relay = msg.contents.excess_data.is_empty();
let chan_info = ChannelInfo {
features: msg.contents.features.clone(),
node_one: msg.contents.node_id_1.clone(),