Skip to content

Commit

Permalink
Security: Move the Verack response after the version check (#2121)
Browse files Browse the repository at this point in the history
We should do as many local checks as possible, before sending further
messages.
  • Loading branch information
teor2345 authored May 17, 2021
1 parent e0fdada commit 7969459
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions zebra-network/src/peer/handshake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,18 +493,6 @@ pub async fn negotiate_version(
Err(HandshakeError::NonceReuse)?;
}

peer_conn.send(Message::Verack).await?;

let remote_msg = peer_conn
.next()
.await
.ok_or(HandshakeError::ConnectionClosed)??;
if let Message::Verack = remote_msg {
debug!("got verack from remote peer");
} else {
Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg)))?;
}

// XXX in zcashd remote peer can only send one version message and
// we would disconnect here if it received a second one. Is it even possible
// for that to happen to us here?
Expand Down Expand Up @@ -532,6 +520,18 @@ pub async fn negotiate_version(
Err(HandshakeError::ObsoleteVersion(remote_version))?;
}

peer_conn.send(Message::Verack).await?;

let remote_msg = peer_conn
.next()
.await
.ok_or(HandshakeError::ConnectionClosed)??;
if let Message::Verack = remote_msg {
debug!("got verack from remote peer");
} else {
Err(HandshakeError::UnexpectedMessage(Box::new(remote_msg)))?;
}

Ok((remote_version, remote_services, remote_canonical_addr))
}

Expand Down

0 comments on commit 7969459

Please sign in to comment.