Security: Move the Verack response after the version check (#2121)
We should do as many local checks as possible, before sending further messages.
This commit is contained in:
parent
e0fdadaca2
commit
7969459b19
|
|
@ -493,18 +493,6 @@ pub async fn negotiate_version(
|
||||||
Err(HandshakeError::NonceReuse)?;
|
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
|
// 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
|
// we would disconnect here if it received a second one. Is it even possible
|
||||||
// for that to happen to us here?
|
// for that to happen to us here?
|
||||||
|
|
@ -532,6 +520,18 @@ pub async fn negotiate_version(
|
||||||
Err(HandshakeError::ObsoleteVersion(remote_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))
|
Ok((remote_version, remote_services, remote_canonical_addr))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue