Skip to content

Commit

Permalink
Rename handshake module and functions to provide clarity (#2929)
Browse files Browse the repository at this point in the history
Since multi-stream connections have been introduced, the `handshake`
module doesn't have a great name, as it only concerns single-stream
connections handshakes.

This PR renames it to `single_stream_handshake`.

Additionally, the `new` function is now named `noise_yamux` in order to
make it clear what the handshake does. This provides consistency with
the fact that the WebRTC connection is created with a `webrtc` function.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
tomaka and mergify[bot] authored Oct 27, 2022
1 parent ffd0bdf commit 70681cb
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 53 deletions.
28 changes: 15 additions & 13 deletions bin/fuzz/fuzz_targets/network-connection-encrypted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use smoldot::libp2p::{
connection::{
established::{Config, ConfigRequestResponse, ConfigRequestResponseIn, Event},
handshake, noise,
noise, single_stream_handshake,
},
read_write::ReadWrite,
};
Expand All @@ -47,8 +47,8 @@ libfuzzer_sys::fuzz_target!(|data: &[u8]| {
is_initiator
};

let mut local = handshake::Handshake::new(local_is_initiator);
let mut remote = handshake::Handshake::new(!local_is_initiator);
let mut local = single_stream_handshake::Handshake::noise_yamux(local_is_initiator);
let mut remote = single_stream_handshake::Handshake::noise_yamux(!local_is_initiator);

// Note that the noise keys and randomness are constant rather than being derived from the
// fuzzing data. This is because we're not here to fuzz the cryptographic code (which we
Expand All @@ -66,16 +66,16 @@ libfuzzer_sys::fuzz_target!(|data: &[u8]| {
while !matches!(
(&local, &remote),
(
handshake::Handshake::Success { .. },
handshake::Handshake::Success { .. }
single_stream_handshake::Handshake::Success { .. },
single_stream_handshake::Handshake::Success { .. }
)
) {
match local {
handshake::Handshake::Success { .. } => {}
handshake::Handshake::NoiseKeyRequired(key_req) => {
single_stream_handshake::Handshake::Success { .. } => {}
single_stream_handshake::Handshake::NoiseKeyRequired(key_req) => {
local = key_req.resume(&local_key).into()
}
handshake::Handshake::Healthy(nego) => {
single_stream_handshake::Handshake::Healthy(nego) => {
let local_to_remote_buffer_len = local_to_remote_buffer.len();
if local_to_remote_buffer_len < local_to_remote_buffer.capacity() {
let cap = local_to_remote_buffer.capacity();
Expand Down Expand Up @@ -103,11 +103,11 @@ libfuzzer_sys::fuzz_target!(|data: &[u8]| {
}

match remote {
handshake::Handshake::Success { .. } => {}
handshake::Handshake::NoiseKeyRequired(key_req) => {
single_stream_handshake::Handshake::Success { .. } => {}
single_stream_handshake::Handshake::NoiseKeyRequired(key_req) => {
remote = key_req.resume(&remote_key).into()
}
handshake::Handshake::Healthy(nego) => {
single_stream_handshake::Handshake::Healthy(nego) => {
let remote_to_local_buffer_len = remote_to_local_buffer.len();
if remote_to_local_buffer_len < remote_to_local_buffer.capacity() {
let cap = remote_to_local_buffer.capacity();
Expand Down Expand Up @@ -138,7 +138,7 @@ libfuzzer_sys::fuzz_target!(|data: &[u8]| {
// Handshake successful.
// Turn `local` and `remote` into state machines corresponding to the established connection.
let mut local = match local {
handshake::Handshake::Success { connection, .. } => connection
single_stream_handshake::Handshake::Success { connection, .. } => connection
.into_connection::<_, (), ()>(Config {
first_out_ping: Duration::new(60, 0),
max_inbound_substreams: 10,
Expand All @@ -157,7 +157,9 @@ libfuzzer_sys::fuzz_target!(|data: &[u8]| {
_ => unreachable!(),
};
let mut remote = match remote {
handshake::Handshake::Success { connection, .. } => connection.into_noise_state_machine(),
single_stream_handshake::Handshake::Success { connection, .. } => {
connection.into_noise_state_machine()
}
_ => unreachable!(),
};

Expand Down
4 changes: 2 additions & 2 deletions src/libp2p/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
//! the calls to [`Network::inject_connection_message`].
//!
use super::connection::{established, handshake, NoiseKey};
use super::connection::{established, single_stream_handshake, NoiseKey};
use alloc::{
collections::{BTreeMap, BTreeSet, VecDeque},
string::String,
Expand All @@ -65,7 +65,7 @@ use rand_chacha::{rand_core::SeedableRng as _, ChaCha20Rng};
pub use super::peer_id::PeerId;
pub use super::read_write::ReadWrite;
pub use established::{ConfigRequestResponse, ConfigRequestResponseIn, InboundError};
pub use handshake::HandshakeError;
pub use single_stream_handshake::HandshakeError;

pub use multi_stream::MultiStreamConnectionTask;
pub use single_stream::SingleStreamConnectionTask;
Expand Down
14 changes: 7 additions & 7 deletions src/libp2p/collection/single_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use super::{
super::{
connection::{
established::{self, ConfigRequestResponse},
handshake, NoiseKey,
single_stream_handshake, NoiseKey,
},
read_write::ReadWrite,
},
Expand Down Expand Up @@ -49,7 +49,7 @@ pub struct SingleStreamConnectionTask<TNow> {
enum SingleStreamConnectionTaskInner<TNow> {
/// Connection is still in its handshake phase.
Handshake {
handshake: handshake::HealthyHandshake,
handshake: single_stream_handshake::HealthyHandshake,

/// Seed that will be used to initialize randomness when building the
/// [`established::SingleStream`].
Expand Down Expand Up @@ -151,7 +151,7 @@ where

SingleStreamConnectionTask {
connection: SingleStreamConnectionTaskInner::Handshake {
handshake: handshake::HealthyHandshake::new(is_initiator),
handshake: single_stream_handshake::HealthyHandshake::noise_yamux(is_initiator),
randomness_seed,
timeout: handshake_timeout,
noise_key,
Expand Down Expand Up @@ -724,7 +724,7 @@ where
};

match result {
handshake::Handshake::Healthy(updated_handshake)
single_stream_handshake::Handshake::Healthy(updated_handshake)
if (read_before, written_before)
== (read_write.read_bytes, read_write.written_bytes) =>
{
Expand All @@ -740,10 +740,10 @@ where
};
break;
}
handshake::Handshake::Healthy(updated_handshake) => {
single_stream_handshake::Handshake::Healthy(updated_handshake) => {
handshake = updated_handshake;
}
handshake::Handshake::Success {
single_stream_handshake::Handshake::Success {
remote_peer_id,
connection,
} => {
Expand Down Expand Up @@ -784,7 +784,7 @@ where
};
break;
}
handshake::Handshake::NoiseKeyRequired(key) => {
single_stream_handshake::Handshake::NoiseKeyRequired(key) => {
handshake = key.resume(&noise_key);
}
}
Expand Down
21 changes: 11 additions & 10 deletions src/libp2p/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
//! > for more information.
//!
//! Once a TCP connection has been established, the *handshake* phase starts. This handshake
//! is described in details in the documentation [`handshake`] module.
//! is described in details in the documentation [`single_stream_handshake`] module.
//!
//! The handshake consists in negotiating with the remote an encryption layer, thanks to which all
//! communications are encrypted, and a multiplexing layer, thanks to which the stream of data can
Expand Down Expand Up @@ -61,15 +61,16 @@
//! > **Note**: This module only contains *no_std*-friendly code, and creating TCP connections
//! > isn't handled by it.
//!
//! After a TCP connection is established, use [`handshake::HealthyHandshake::new`] to initialize
//! the state machine that needs to be maintained in parallel of the connection. The data send and
//! received over the socket must respectively be obtained or injected using
//! [`handshake::HealthyHandshake::read_write`]. See the [`handshake`] module documentation for
//! more details.
//! After a TCP connection is established, use
//! [`single_stream_handshake::HealthyHandshake::noise_yamux`] to initialize the state machine that
//! needs to be maintained in parallel of the connection. The data sent and received over the
//! socket must respectively be obtained or injected using
//! [`single_stream_handshake::HealthyHandshake::read_write`]. See the [`single_stream_handshake`]
//! module documentation for more details.
//!
//! Keep in mind that the [`handshake`] module doesn't provide any timeout. Users are strongly
//! encouraged to add one, in order to detect situations where the remote is, intentionally or
//! not, unresponsive.
//! Keep in mind that the [`single_stream_handshake`] module doesn't provide any timeout. Users
//! are strongly encouraged to add one, in order to detect situations where the remote is,
//! intentionally or not, unresponsive.
//!
//! After the handshake is successful, use [`established::ConnectionPrototype::into_connection`]
//! to obtain an [`established::SingleStream`]. Similar to the handshake, use
Expand All @@ -79,7 +80,7 @@
pub use noise::{NoiseKey, UnsignedNoiseKey};

pub mod established;
pub mod handshake;
pub mod multistream_select;
pub mod noise;
pub mod single_stream_handshake;
pub mod yamux;
26 changes: 13 additions & 13 deletions src/libp2p/connection/established/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ fn perform_handshake(
alice_config: Config<Duration>,
bob_config: Config<Duration>,
) -> TwoEstablished {
use super::super::{handshake, NoiseKey};
use super::super::{single_stream_handshake, NoiseKey};

assert_ne!(alice_to_bob_buffer_size, 0);
assert_ne!(bob_to_alice_buffer_size, 0);

let mut alice = handshake::Handshake::new(true);
let mut bob = handshake::Handshake::new(false);
let mut alice = single_stream_handshake::Handshake::noise_yamux(true);
let mut bob = single_stream_handshake::Handshake::noise_yamux(false);

let alice_key = NoiseKey::new(&rand::random());
let bob_key = NoiseKey::new(&rand::random());
Expand All @@ -61,16 +61,16 @@ fn perform_handshake(
while !matches!(
(&alice, &bob),
(
handshake::Handshake::Success { .. },
handshake::Handshake::Success { .. }
single_stream_handshake::Handshake::Success { .. },
single_stream_handshake::Handshake::Success { .. }
)
) {
match alice {
handshake::Handshake::Success { .. } => {}
handshake::Handshake::NoiseKeyRequired(key_req) => {
single_stream_handshake::Handshake::Success { .. } => {}
single_stream_handshake::Handshake::NoiseKeyRequired(key_req) => {
alice = key_req.resume(&alice_key).into()
}
handshake::Handshake::Healthy(nego) => {
single_stream_handshake::Handshake::Healthy(nego) => {
let alice_to_bob_buffer_len = alice_to_bob_buffer.len();
if alice_to_bob_buffer_len < alice_to_bob_buffer.capacity() {
let cap = alice_to_bob_buffer.capacity();
Expand Down Expand Up @@ -98,11 +98,11 @@ fn perform_handshake(
}

match bob {
handshake::Handshake::Success { .. } => {}
handshake::Handshake::NoiseKeyRequired(key_req) => {
single_stream_handshake::Handshake::Success { .. } => {}
single_stream_handshake::Handshake::NoiseKeyRequired(key_req) => {
bob = key_req.resume(&bob_key).into()
}
handshake::Handshake::Healthy(nego) => {
single_stream_handshake::Handshake::Healthy(nego) => {
let bob_to_alice_buffer_len = bob_to_alice_buffer.len();
if bob_to_alice_buffer_len < bob_to_alice_buffer.capacity() {
let cap = bob_to_alice_buffer.capacity();
Expand Down Expand Up @@ -132,13 +132,13 @@ fn perform_handshake(

TwoEstablished {
alice: match alice {
handshake::Handshake::Success { connection, .. } => {
single_stream_handshake::Handshake::Success { connection, .. } => {
connection.into_connection(alice_config)
}
_ => unreachable!(),
},
bob: match bob {
handshake::Handshake::Success { connection, .. } => {
single_stream_handshake::Handshake::Success { connection, .. } => {
connection.into_connection(bob_config)
}
_ => unreachable!(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
//! TCP handshake), depending on the strategy used for the multistream-select protocol.
// TODO: finish commenting on the number of round trips
// TODO: this module's name ("handshake") is a bit vague considering that there are multiple different handshakes based on the protocol

use super::{
super::peer_id::PeerId,
Expand Down Expand Up @@ -64,9 +63,9 @@ pub enum Handshake {
}

impl Handshake {
/// Shortcut for [`HealthyHandshake::new`] wrapped in a [`Handshake`].
pub fn new(is_initiator: bool) -> Self {
HealthyHandshake::new(is_initiator).into()
/// Shortcut for [`HealthyHandshake::noise_yamux`] wrapped in a [`Handshake`].
pub fn noise_yamux(is_initiator: bool) -> Self {
HealthyHandshake::noise_yamux(is_initiator).into()
}
}

Expand All @@ -91,11 +90,11 @@ enum NegotiationState {
}

impl HealthyHandshake {
/// Initializes a new state machine.
/// Initializes a new state machine for a Noise + Yamux handshake.
///
/// Must pass `true` if the connection has been opened by the local machine, or `false` if it
/// has been opened by the remote.
pub fn new(is_initiator: bool) -> Self {
pub fn noise_yamux(is_initiator: bool) -> Self {
let negotiation = multistream_select::InProgress::new(if is_initiator {
multistream_select::Config::Dialer {
requested_protocol: noise::PROTOCOL_NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ fn handshake_basic_works() {
let key1 = NoiseKey::new(&rand::random());
let key2 = NoiseKey::new(&rand::random());

let mut handshake1 = Handshake::new(true);
let mut handshake2 = Handshake::new(false);
let mut handshake1 = Handshake::noise_yamux(true);
let mut handshake2 = Handshake::noise_yamux(false);

let mut buf_1_to_2 = Vec::new();
let mut buf_2_to_1 = Vec::new();
Expand Down

0 comments on commit 70681cb

Please sign in to comment.