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

Merge pull request #132 from TheBlueMatt/2018-08-bolt-4-spec-return-fail

Browse source 
Return a malformed HTLC message when ephemeral pubkey is garbage
This commit is contained in:
Matt Corallo 2018-08-27 12:44:13 -04:00 • committed by GitHub
commit 765987759a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 88 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
fuzz/Cargo.toml
View file

@ -64,6 +64,10 @@ path = "fuzz_targets/msg_pong_target.rs"
name = "msg_error_message_target"
path = "fuzz_targets/msg_error_message_target.rs"
[[bin]]
name = "msg_update_add_htlc_target"
path = "fuzz_targets/msg_update_add_htlc_target.rs"
[[bin]]
name = "msg_accept_channel_target"
path = "fuzz_targets/msg_targets/msg_accept_channel_target.rs"
@ -100,10 +104,6 @@ path = "fuzz_targets/msg_targets/msg_revoke_and_ack_target.rs"
name = "msg_shutdown_target"
path = "fuzz_targets/msg_targets/msg_shutdown_target.rs"
[[bin]]
name = "msg_update_add_htlc_target"
path = "fuzz_targets/msg_targets/msg_update_add_htlc_target.rs"
[[bin]]
name = "msg_update_fail_malformed_htlc_target"
path = "fuzz_targets/msg_targets/msg_update_fail_malformed_htlc_target.rs"

2
fuzz/fuzz_targets/channel_target.rs
View file

@ -269,7 +269,7 @@ pub fn do_test(data: &[u8]) {
0 => {
test_err!(channel.send_htlc(slice_to_be64(get_slice!(8)), [42; 32], slice_to_be32(get_slice!(4)), msgs::OnionPacket {
version: get_slice!(1)[0],
public_key: get_pubkey!(),
public_key: PublicKey::from_slice(&secp_ctx, get_slice!(33)),
hop_data: [0; 20*65],
hmac: [0; 32],
}));

3
fuzz/fuzz_targets/msg_error_message_target.rs
View file

@ -1,6 +1,3 @@
// This file is auto-generated by gen_target.sh based on msg_target_template.txt
// To modify it, modify msg_target_template.txt and run gen_target.sh instead.
extern crate lightning;
use lightning::ln::msgs;

2
fuzz/fuzz_targets/msg_targets/gen_target.sh
View file

@ -1,4 +1,4 @@
for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateAddHTLC UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish; do
for target in CommitmentSigned FundingCreated FundingLocked FundingSigned OpenChannel RevokeAndACK Shutdown UpdateFailHTLC UpdateFailMalformedHTLC UpdateFee UpdateFulfillHTLC AcceptChannel ClosingSigned ChannelReestablish; do
tn=$(echo $target | sed 's/\([a-z0-9]\)\([A-Z]\)/\1_\2/g')
fn=msg_$(echo $tn | tr '[:upper:]' '[:lower:]')_target.rs
cat msg_target_template.txt | sed s/MSG_TARGET/$target/ > $fn

11
fuzz/fuzz_targets/msg_targets/msg_update_add_htlc_target.rs → fuzz/fuzz_targets/msg_update_add_htlc_target.rs
View file

@ -1,6 +1,3 @@
// This file is auto-generated by gen_target.sh based on msg_target_template.txt
// To modify it, modify msg_target_template.txt and run gen_target.sh instead.
extern crate lightning;
use lightning::ln::msgs;
@ -8,12 +5,14 @@ use lightning::util::reset_rng_state;
use lightning::ln::msgs::{MsgEncodable, MsgDecodable};
mod utils;
#[inline]
pub fn do_test(data: &[u8]) {
reset_rng_state();
test_msg!(msgs::UpdateAddHTLC, data);
if let Ok(msg) = msgs::UpdateAddHTLC::decode(data){
let enc = msg.encode();
assert_eq!(&data[0..85], &enc[0..85]);
assert_eq!(&data[85+33..enc.len()], &enc[85+33..]);
}
}
#[cfg(feature = "afl")]

22
src/ln/channel.rs
View file

@ -16,7 +16,7 @@ use crypto::hkdf::{hkdf_extract,hkdf_expand};
use ln::msgs;
use ln::msgs::{ErrorAction, HandleError, MsgEncodable};
use ln::channelmonitor::ChannelMonitor;
use ln::channelmanager::{PendingHTLCStatus, PendingForwardHTLCInfo, HTLCFailReason};
use ln::channelmanager::{PendingHTLCStatus, PendingForwardHTLCInfo, HTLCFailReason, HTLCFailureMsg};
use ln::chan_utils::{TxCreationKeys,HTLCOutputInCommitment,HTLC_SUCCESS_TX_WEIGHT,HTLC_TIMEOUT_TX_WEIGHT};
use ln::chan_utils;
use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
@ -1643,6 +1643,7 @@ impl Channel {
update_add_htlcs,
update_fulfill_htlcs,
update_fail_htlcs,
update_fail_malformed_htlcs: Vec::new(),
commitment_signed,
}, monitor_update)))
},
@ -1680,7 +1681,8 @@ impl Channel {
let mut to_forward_infos = Vec::new();
let mut revoked_htlcs = Vec::new();
let mut failed_htlcs = Vec::new();
let mut update_fail_htlcs = Vec::new();
let mut update_fail_malformed_htlcs = Vec::new();
let mut require_commitment = false;
let mut value_to_self_msat_diff: i64 = 0;
// We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
@ -1708,7 +1710,10 @@ impl Channel {
PendingHTLCStatus::Fail(fail_msg) => {
htlc.state = HTLCState::LocalRemoved;
require_commitment = true;
failed_htlcs.push(fail_msg);
match fail_msg {
HTLCFailureMsg::Relay(msg) => update_fail_htlcs.push(msg),
HTLCFailureMsg::Malformed(msg) => update_fail_malformed_htlcs.push(msg),
}
},
PendingHTLCStatus::Forward(forward_info) => {
to_forward_infos.push(forward_info);
@ -1727,10 +1732,14 @@ impl Channel {
match self.free_holding_cell_htlcs()? {
Some(mut commitment_update) => {
commitment_update.0.update_fail_htlcs.reserve(failed_htlcs.len());
for fail_msg in failed_htlcs.drain(..) {
commitment_update.0.update_fail_htlcs.reserve(update_fail_htlcs.len());
for fail_msg in update_fail_htlcs.drain(..) {
commitment_update.0.update_fail_htlcs.push(fail_msg);
}
commitment_update.0.update_fail_malformed_htlcs.reserve(update_fail_malformed_htlcs.len());
for fail_msg in update_fail_malformed_htlcs.drain(..) {
commitment_update.0.update_fail_malformed_htlcs.push(fail_msg);
}
Ok((Some(commitment_update.0), to_forward_infos, revoked_htlcs, commitment_update.1))
},
None => {
@ -1739,7 +1748,8 @@ impl Channel {
Ok((Some(msgs::CommitmentUpdate {
update_add_htlcs: Vec::new(),
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: failed_htlcs,
update_fail_htlcs,
update_fail_malformed_htlcs,
commitment_signed
}), to_forward_infos, revoked_htlcs, monitor_update))
} else {

76
src/ln/channelmanager.rs
View file

@ -50,11 +50,17 @@ mod channel_held_info {
pub(super) outgoing_cltv_value: u32,
}
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
pub enum HTLCFailureMsg {
Relay(msgs::UpdateFailHTLC),
Malformed(msgs::UpdateFailMalformedHTLC),
}
/// Stores whether we can't forward an HTLC or relevant forwarding info
#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
pub enum PendingHTLCStatus {
Forward(PendingForwardHTLCInfo),
Fail(msgs::UpdateFailHTLC),
Fail(HTLCFailureMsg),
}
#[cfg(feature = "fuzztarget")]
@ -619,7 +625,7 @@ impl ChannelManager {
Ok(msgs::OnionPacket{
version: 0,
public_key: onion_keys.first().unwrap().ephemeral_pubkey,
public_key: Ok(onion_keys.first().unwrap().ephemeral_pubkey),
hop_data: packet_data,
hmac: hmac_res,
})
@ -675,10 +681,7 @@ impl ChannelManager {
ChannelManager::encrypt_failure_packet(shared_secret, &failure_packet.encode()[..])
}
fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, SharedSecret, MutexGuard<ChannelHolder>) {
let shared_secret = SharedSecret::new(&self.secp_ctx, &msg.onion_routing_packet.public_key, &self.our_network_key);
let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret);
fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, Option<SharedSecret>, MutexGuard<ChannelHolder>) {
macro_rules! get_onion_hash {
() => {
{
@ -691,6 +694,19 @@ impl ChannelManager {
}
}
if let Err(_) = msg.onion_routing_packet.public_key {
log_info!(self, "Failed to accept/forward incoming HTLC with invalid ephemeral pubkey");
return (PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
sha256_of_onion: get_onion_hash!(),
failure_code: 0x8000 | 0x4000 | 6,
})), None, self.channel_state.lock().unwrap());
}
let shared_secret = SharedSecret::new(&self.secp_ctx, &msg.onion_routing_packet.public_key.unwrap(), &self.our_network_key);
let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret);
let mut channel_state = None;
macro_rules! return_err {
($msg: expr, $err_code: expr, $data: expr) => {
@ -699,11 +715,11 @@ impl ChannelManager {
if channel_state.is_none() {
channel_state = Some(self.channel_state.lock().unwrap());
}
return (PendingHTLCStatus::Fail(msgs::UpdateFailHTLC {
return (PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: ChannelManager::build_first_hop_failure_packet(&shared_secret, $err_code, $data),
}), shared_secret, channel_state.unwrap());
})), Some(shared_secret), channel_state.unwrap());
}
}
}
@ -770,7 +786,7 @@ impl ChannelManager {
chacha.process(&msg.onion_routing_packet.hop_data[65..], &mut new_packet_data[0..19*65]);
chacha.process(&ChannelManager::ZERO[0..65], &mut new_packet_data[19*65..]);
let mut new_pubkey = msg.onion_routing_packet.public_key.clone();
let mut new_pubkey = msg.onion_routing_packet.public_key.unwrap();
let blinding_factor = {
let mut sha = Sha256::new();
@ -780,26 +796,19 @@ impl ChannelManager {
sha.result(&mut res);
match SecretKey::from_slice(&self.secp_ctx, &res) {
Err(_) => {
// Return temporary node failure as its technically our issue, not the
// channel's issue.
return_err!("Blinding factor is an invalid private key", 0x2000 | 2, &[0;0]);
return_err!("Blinding factor is an invalid private key", 0x8000 | 0x4000 | 6, &get_onion_hash!());
},
Ok(key) => key
}
};
match new_pubkey.mul_assign(&self.secp_ctx, &blinding_factor) {
Err(_) => {
// Return temporary node failure as its technically our issue, not the
// channel's issue.
return_err!("New blinding factor is an invalid private key", 0x2000 | 2, &[0;0]);
},
Ok(_) => {}
};
if let Err(_) = new_pubkey.mul_assign(&self.secp_ctx, &blinding_factor) {
return_err!("New blinding factor is an invalid private key", 0x8000 | 0x4000 | 6, &get_onion_hash!());
}
let outgoing_packet = msgs::OnionPacket {
version: 0,
public_key: new_pubkey,
public_key: Ok(new_pubkey),
hop_data: new_packet_data,
hmac: next_hop_data.hmac.clone(),
};
@ -846,7 +855,7 @@ impl ChannelManager {
}
}
(pending_forward_info, shared_secret, channel_state.unwrap())
(pending_forward_info, Some(shared_secret), channel_state.unwrap())
}
/// only fails if the channel does not yet have an assigned short_id
@ -958,6 +967,7 @@ impl ChannelManager {
update_add_htlcs: vec![update_add],
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: Vec::new(),
update_fail_malformed_htlcs: Vec::new(),
commitment_signed,
},
});
@ -1102,6 +1112,7 @@ impl ChannelManager {
update_add_htlcs: add_htlc_msgs,
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: Vec::new(),
update_fail_malformed_htlcs: Vec::new(),
commitment_signed: commitment_msg,
},
}));
@ -1225,6 +1236,7 @@ impl ChannelManager {
update_add_htlcs: Vec::new(),
update_fulfill_htlcs: Vec::new(),
update_fail_htlcs: vec![msg],
update_fail_malformed_htlcs: Vec::new(),
commitment_signed: commitment_msg,
},
});
@ -1324,6 +1336,7 @@ impl ChannelManager {
update_add_htlcs: Vec::new(),
update_fulfill_htlcs: vec![msg],
update_fail_htlcs: Vec::new(),
update_fail_malformed_htlcs: Vec::new(),
commitment_signed: commitment_msg,
}
});
@ -1722,11 +1735,11 @@ impl ChannelMessageHandler for ChannelManager {
}
if !acceptable_cycle {
log_info!(self, "Failed to accept incoming HTLC: Payment looped through us twice");
pending_forward_info = PendingHTLCStatus::Fail(msgs::UpdateFailHTLC {
pending_forward_info = PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: ChannelManager::build_first_hop_failure_packet(&shared_secret, 0x4000 | 0x2000 | 2, &[0;0]),
});
reason: ChannelManager::build_first_hop_failure_packet(&shared_secret.unwrap(), 0x4000 | 0x2000 | 2, &[0;0]),
}));
} else {
will_forward = true;
}
@ -1764,7 +1777,7 @@ impl ChannelMessageHandler for ChannelManager {
};
*outbound_route = PendingOutboundHTLC::CycledRoute {
source_short_channel_id,
incoming_packet_shared_secret: shared_secret,
incoming_packet_shared_secret: shared_secret.unwrap(),
route,
session_priv,
};
@ -1772,7 +1785,7 @@ impl ChannelMessageHandler for ChannelManager {
hash_map::Entry::Vacant(e) => {
e.insert(PendingOutboundHTLC::IntermediaryHopData {
source_short_channel_id,
incoming_packet_shared_secret: shared_secret,
incoming_packet_shared_secret: shared_secret.unwrap(),
});
}
}
@ -2487,9 +2500,10 @@ mod tests {
impl SendEvent {
fn from_event(event: Event) -> SendEvent {
match event {
Event::UpdateHTLCs { node_id, updates: msgs::CommitmentUpdate { update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, commitment_signed } } => {
Event::UpdateHTLCs { node_id, updates: msgs::CommitmentUpdate { update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, commitment_signed } } => {
assert!(update_fulfill_htlcs.is_empty());
assert!(update_fail_htlcs.is_empty());
assert!(update_fail_malformed_htlcs.is_empty());
SendEvent { node_id: node_id, msgs: update_add_htlcs, commitment_msg: commitment_signed }
},
_ => panic!("Unexpected event type!"),
@ -2646,10 +2660,11 @@ mod tests {
let events = node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed } } => {
assert!(update_add_htlcs.is_empty());
assert_eq!(update_fulfill_htlcs.len(), 1);
assert!(update_fail_htlcs.is_empty());
assert!(update_fail_malformed_htlcs.is_empty());
expected_next_node = node_id.clone();
next_msgs = Some((update_fulfill_htlcs[0].clone(), commitment_signed.clone()));
},
@ -2770,10 +2785,11 @@ mod tests {
let events = node.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed } } => {
assert!(update_add_htlcs.is_empty());
assert!(update_fulfill_htlcs.is_empty());
assert_eq!(update_fail_htlcs.len(), 1);
assert!(update_fail_malformed_htlcs.is_empty());
expected_next_node = node_id.clone();
next_msgs = Some((update_fail_htlcs[0].clone(), commitment_signed.clone()));
},

14
src/ln/msgs.rs
View file

@ -1,5 +1,6 @@
use secp256k1::key::PublicKey;
use secp256k1::{Secp256k1, Signature};
use secp256k1;
use bitcoin::util::hash::Sha256dHash;
use bitcoin::network::serialize::{deserialize,serialize};
use bitcoin::blockdata::script::Script;
@ -399,6 +400,7 @@ pub struct CommitmentUpdate {
pub update_add_htlcs: Vec<UpdateAddHTLC>,
pub update_fulfill_htlcs: Vec<UpdateFulfillHTLC>,
pub update_fail_htlcs: Vec<UpdateFailHTLC>,
pub update_fail_malformed_htlcs: Vec<UpdateFailMalformedHTLC>,
pub commitment_signed: CommitmentSigned,
}
@ -475,7 +477,10 @@ unsafe impl internal_traits::NoDealloc for OnionHopData{}
#[derive(Clone)]
pub struct OnionPacket {
pub version: u8,
pub public_key: PublicKey,
/// In order to ensure we always return an error on Onion decode in compliance with BOLT 4, we
/// have to deserialize OnionPackets contained in UpdateAddHTLCs even if the ephemeral public
/// key (here) is bogus, so we hold a Result instead of a PublicKey as we'd like.
pub public_key: Result<PublicKey, secp256k1::Error>,
pub hop_data: [u8; 20*65],
pub hmac: [u8; 32],
}
@ -1537,7 +1542,7 @@ impl MsgDecodable for OnionPacket {
let secp_ctx = Secp256k1::without_caps();
Ok(Self {
version: v[0],
public_key: secp_pubkey!(&secp_ctx, &v[1..34]),
public_key: PublicKey::from_slice(&secp_ctx, &v[1..34]),
hop_data,
hmac,
})
@ -1547,7 +1552,10 @@ impl MsgEncodable for OnionPacket {
fn encode(&self) -> Vec<u8> {
let mut res = Vec::with_capacity(1 + 33 + 20*65 + 32);
res.push(self.version);
res.extend_from_slice(&self.public_key.serialize());
match self.public_key {
Ok(pubkey) => res.extend_from_slice(&pubkey.serialize()),
Err(_) => res.extend_from_slice(&[0; 33]),
}
res.extend_from_slice(&self.hop_data);
res.extend_from_slice(&self.hmac);
res

5
src/ln/peer_handler.rs
View file

@ -697,7 +697,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
Self::do_attempt_write_data(&mut descriptor, peer);
continue;
},
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref commitment_signed } } => {
Event::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, ref commitment_signed } } => {
log_trace!(self, "Handling UpdateHTLCs event in peer_handler for node {} with {} adds, {} fulfills, {} fails for channel {}",
log_pubkey!(node_id),
update_add_htlcs.len(),
@ -716,6 +716,9 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
for msg in update_fail_htlcs {
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 131)));
}
for msg in update_fail_malformed_htlcs {
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 135)));
}
peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(commitment_signed, 132)));
Self::do_attempt_write_data(&mut descriptor, peer);
continue;