Skip to content

Commit 1e43c55

Browse files
committed
Fix pre-noise peer disconnect panic on non-Err disconnect
366e796 fixed the same crash for Errs that come up during handshake, but was incomplete and should have just dropped the node_id being different based on inbound/outbound. This patch does so and actually fixes the issue. Found by fuzzer.
1 parent 74f59a2 commit 1e43c55

File tree

2 files changed

+8
-13
lines changed

2 files changed

+8
-13
lines changed

src/ln/peer_channel_encryptor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ impl PeerChannelEncryptor {
279279
self.process_act_one_with_ephemeral_key(act_one, our_node_secret, our_ephemeral_key)
280280
}
281281

282-
pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<[u8; 66], HandleError> {
282+
pub fn process_act_two(&mut self, act_two: &[u8], our_node_secret: &SecretKey) -> Result<([u8; 66], PublicKey), HandleError> {
283283
assert_eq!(act_two.len(), 50);
284284

285285
let mut final_hkdf = [0; 64];
@@ -334,7 +334,7 @@ impl PeerChannelEncryptor {
334334
rck: ck,
335335
};
336336

337-
Ok(res)
337+
Ok((res, self.their_node_id.unwrap().clone()))
338338
}
339339

340340
pub fn process_act_three(&mut self, act_three: &[u8]) -> Result<PublicKey, HandleError> {

src/ln/peer_handler.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
210210
if peers.peers.insert(descriptor, Peer {
211211
channel_encryptor: peer_encryptor,
212212
outbound: true,
213-
their_node_id: Some(their_node_id),
213+
their_node_id: None,
214214
their_global_features: None,
215215
their_local_features: None,
216216

@@ -359,9 +359,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
359359

360360
macro_rules! try_potential_handleerror {
361361
($thing: expr) => {
362-
try_potential_handleerror!($thing, false);
363-
};
364-
($thing: expr, $pre_noise: expr) => {
365362
match $thing {
366363
Ok(x) => x,
367364
Err(e) => {
@@ -370,9 +367,6 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
370367
msgs::ErrorAction::DisconnectPeer { msg: _ } => {
371368
//TODO: Try to push msg
372369
log_trace!(self, "Got Err handling message, disconnecting peer because {}", e.err);
373-
if $pre_noise {
374-
peer.their_node_id = None; // Unset so that we don't generate a peer_disconnected event
375-
}
376370
return Err(PeerHandleError{ no_connection_possible: false });
377371
},
378372
msgs::ErrorAction::IgnoreError => {
@@ -438,16 +432,17 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
438432
let next_step = peer.channel_encryptor.get_noise_step();
439433
match next_step {
440434
NextNoiseStep::ActOne => {
441-
let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
435+
let act_two = try_potential_handleerror!(peer.channel_encryptor.process_act_one_with_key(&peer.pending_read_buffer[..], &self.our_node_secret)).to_vec();
442436
peer.pending_outbound_buffer.push_back(act_two);
443437
peer.pending_read_buffer = [0; 66].to_vec(); // act three is 66 bytes long
444438
},
445439
NextNoiseStep::ActTwo => {
446-
let act_three = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret), true).to_vec();
447-
peer.pending_outbound_buffer.push_back(act_three);
440+
let (act_three, their_node_id) = try_potential_handleerror!(peer.channel_encryptor.process_act_two(&peer.pending_read_buffer[..], &self.our_node_secret));
441+
peer.pending_outbound_buffer.push_back(act_three.to_vec());
448442
peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
449443
peer.pending_read_is_header = true;
450444

445+
peer.their_node_id = Some(their_node_id);
451446
insert_node_id!();
452447
let mut local_features = msgs::LocalFeatures::new();
453448
if self.initial_syncs_sent.load(Ordering::Acquire) < INITIAL_SYNCS_TO_SEND {
@@ -460,7 +455,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
460455
}, 16);
461456
},
462457
NextNoiseStep::ActThree => {
463-
let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]), true);
458+
let their_node_id = try_potential_handleerror!(peer.channel_encryptor.process_act_three(&peer.pending_read_buffer[..]));
464459
peer.pending_read_buffer = [0; 18].to_vec(); // Message length header is 18 bytes
465460
peer.pending_read_is_header = true;
466461
peer.their_node_id = Some(their_node_id);

0 commit comments

Comments
 (0)