From 7969459b198765600c440c6c7ba57468008a5c2f Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 18 May 2021 06:39:44 +1000 Subject: [PATCH] Security: Move the Verack response after the version check (#2121) We should do as many local checks as possible, before sending further messages. --- zebra-network/src/peer/handshake.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/zebra-network/src/peer/handshake.rs b/zebra-network/src/peer/handshake.rs index d66e4f42917..c276dbd0981 100644 --- a/zebra-network/src/peer/handshake.rs +++ b/zebra-network/src/peer/handshake.rs @@ -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? @@ -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)) }