Nat hole punching for discv5.2 (#176)

* Plug-in hole punching * Notification message container * Plug relaying into service * Move hole punch trait to extern crate * Adjust impl of hole punch trait * fixup! Plug relaying into service * fixup! Plug-in hole punching * Update trait impl * Consolidate hole punching in Handler * fixup! Consolidate hole punching in Handler * fixup! Consolidate hole punching in Handler * Add trace info * fixup! Consolidate hole punching in Handler * Reinsert active request on hole punch attempt * Update node address conversion * RO access kbuckets from handler for hole punch target peer lookup * Replace RO access to kbuckets with channels * Add trace messages * fixup! Add trace messages * Handle each realy init separately * Receive responses in notification packets * Reset renaming of packet kind * Send responses in notification * fixup! Reset renaming of packet kind * fixup! Reinsert active request on hole punch attempt * Plug in mechanism for keeping hole punched for peer * Only give work of keeping hole punched to nodes that aren't WAN reachable * fixup! Only give work of keeping hole punched to nodes that aren't WAN reachable * Correct comment * Include nodes that discover reachable address earlier in small networks * fixup! Include nodes that discover reachable address earlier in small networks * Limit sessions with unreachable enrs * Set limit for sessions with peers with unreachable ENRs in config * Allow unlimited sessions with unreachable ENRs like in discv5.0 * Clean up fs * Update naming to match wire * Lint fixes * Doc links fix * Bug fix * Log messages fixes * Updates from nat hole punch crate * fixup! Updates from nat hole punch crate * Incorporate notification * notif sep * Incorporate notification from nat_hole_punch crate * Move hole punch trait from nat_hole_punch crate into crate * fixup! Move hole punch trait from nat_hole_punch crate into crate * Make log message more human readbable * Clean up * Stop punching holes for disconnected peers * Fix session limiter constructor bug to not write to logs on no limit and add limiter test * Fix wrong comments on message types and collect message types * Check for unused deps in CI (#1262) * Check for unused deps in CI * Bump slashing protection parking_lot version * Cherry-pick 6ee4f7f * Update github CI to latest version * Remove unused dependencies * Remove unused varaiable assignment * Restore and improve config comments * Use derive From macro * fixup! Restore and improve config comments * Restore drive-by commit to master * Return earlier from enr unreachable check * Replace parse display with dep used in lighthouse * Safeguard with const evaluation * Fix typo Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> * Mark unreachable code * Allow assertions on constants * Fix clippy warnings * fixup! Allow assertions on constants * fixup! Return earlier from enr unreachable check * fixup! Fix session limiter constructor bug to not write to logs on no limit and add limiter test * fixup! Return earlier from enr unreachable check * Shorten debug message * Correct comment correction * Revert symbol * Nitpick Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> * Clarify comment * Reinsert Cargo.lock in gitignore * Fix curly brace bug * remove lockfile * reduce diff * clippy * Fix typo Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> * Check limit in config instead * Restore comment * Simplify docs * Drop notification already at service layer * Improve rust idiomacy * fixup! Drop notification already at service layer * Nitpick * Remove hole punch trait * Simplify bind Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> * Expose unused ports range parameter in config * Fix typo Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> * Fix ci for ipv6 tests * Name consistently with specs Co-authored-by: Age Manning <Age@AgeManning.com> * Fix typo Co-authored-by: Age Manning <Age@AgeManning.com> * Only track outgoing packets if node is behind nat * Only process relay messages if we are behind a nat * Catch malicious relay init * Clear hole punch tracker set for remaining entries * Run lint * Introduce bound on relay store * Remove redundant parameter * Keep session but drop packet that could be spoofed * Add debug info * Revert layout of hole punch code to use trait * Fix backwards compatibility of packet type Message * Remove duplicate guard * Use lru time cach for tracking sent packages * Adjust visibility, handler not alone-standing crate * Add signature for using session limiter on cache insert * Only send expired entries on channel * Remove duplicate code * Move assertion on constant outside of function * no unreachables in session enr * Avoid breaking change to lru time cache api * Change name 'session index' to name of session index type * Update docs * Update comment Co-authored-by: Kolby Moroz Liebl <31669092+KolbyML@users.noreply.github.com> * fixup! Update docs * Lint fixes * Lint fixes * fixup! Lint fixes * Fix docs * Tweak incorporation of SessionLimiter * Avoid clone * Lint fixes * Enable ipv6 tests * fixup! Fix merge conflicts with discv5.2 * fixup! Enable ipv6 tests * Fix test parallelisation * Add relay test for handler * Drive-by, fix param name * Drive-by, clean up test * Fix handler test parallelisation * Skip unnecessary interaction with service task * Add test for target * Remove unnecessary memory re-allocation for sessions cache * Reset previous commit * Improve type safety for notifications * Reset commit b32e750 and check sequence of initiator's enr against kbucket entry * Refactor initiator abbreviation * Trigger ping all peers on upgrade to reachable enr * Modify session management to inside the lrutimecache * Add reviewers comments * Simplify the code, name changes group handler API * Old mate fmt * Fix bug broken invariant active-requests and hole punch attempts * Update src/handler/mod.rs Co-authored-by: Emilia Hane <emiliaha95@gmail.com> * Update src/service.rs Co-authored-by: Emilia Hane <emiliaha95@gmail.com> * Update src/handler/mod.rs Co-authored-by: Emilia Hane <emiliaha95@gmail.com> * Complete the renaming * Fmt --------- Co-authored-by: Emilia Hane <emilia.hane@ethereum.org> Co-authored-by: Michael Sproul <michael@sigmaprime.io> Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com> Co-authored-by: Diva M <divma@protonmail.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: Kolby Moroz Liebl <31669092+KolbyML@users.noreply.github.com>
sigp · Jan 24, 2024 · 04f2dc6 · 04f2dc6
1 parent 3732b3f
commit 04f2dc6
Show file tree

Hide file tree

Showing 23 changed files with 2,308 additions and 735 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -19,7 +19,7 @@ jobs:
       - name: Get latest version of stable rust
         run: rustup update stable
       - name: Lint code for quality and style with Clippy
-        run: cargo clippy --workspace --tests --all-features -- -D warnings
+        run: cargo clippy --workspace --tests --all-features -- -D warnings -A clippy::assertions_on_constants 
   release-tests-ubuntu:
     runs-on: ubuntu-latest
     needs: cargo-fmt
@@ -50,7 +50,7 @@ jobs:
       - name: Get latest version of stable rust
         run: rustup update stable
       - name: Check rustdoc links
-        run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items
+        run: RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items
   cargo-udeps:
     name: cargo-udeps
     runs-on: ubuntu-latest

diff --git a/Cargo.toml b/Cargo.toml
@@ -34,10 +34,11 @@ aes = { version = "0.7.5", features = ["ctr"] }
 aes-gcm = "0.9.4"
 tracing = { version = "0.1.29", features = ["log"] }
 tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
-lru = "0.7.1"
+lru = {version = "0.7.1", default-features = false }
 hashlink = "0.7.0"
 delay_map = "0.3.0"
 more-asserts = "0.2.2"
+derive_more = { version = "0.99.17", default-features = false, features = ["from", "display", "deref", "deref_mut"] }
 
 [dev-dependencies]
 rand_07 = { package = "rand", version = "0.7" }

diff --git a/src/config.rs b/src/config.rs
@@ -1,9 +1,14 @@
 //! A set of configuration parameters to tune the discovery protocol.
 use crate::{
-    kbucket::MAX_NODES_PER_BUCKET, socket::ListenConfig, Enr, Executor, PermitBanList, RateLimiter,
+    kbucket::MAX_NODES_PER_BUCKET, Enr, Executor, ListenConfig, PermitBanList, RateLimiter,
     RateLimiterBuilder,
 };
-use std::time::Duration;
+
+/// The minimum number of unreachable Sessions a node must allow. This enables the network to
+/// boostrap.
+const MIN_SESSIONS_UNREACHABLE_ENR: usize = 10;
+
+use std::{ops::RangeInclusive, time::Duration};
 
 /// Configuration parameters that define the performance of the discovery network.
 #[derive(Clone)]
@@ -96,6 +101,15 @@ pub struct Discv5Config {
     /// timing support. By default, the executor that created the discv5 struct will be used.
     pub executor: Option<Box<dyn Executor + Send + Sync>>,
 
+    /// The max limit for peers with unreachable ENRs. Benevolent examples of such peers are peers
+    /// that are discovering their externally reachable socket, nodes must assist at least one
+    /// such peer in discovering their reachable socket via ip voting, and peers behind symmetric
+    /// NAT. Default is no limit. Minimum is 10.
+    pub unreachable_enr_limit: Option<usize>,
+
+    /// The unused port range to try and bind to when testing if this node is behind NAT based on
+    /// observed address reported at runtime by peers.
+    pub unused_port_range: Option<RangeInclusive<u16>>,
     /// Configuration for the sockets to listen on.
     pub listen_config: ListenConfig,
 }
@@ -142,6 +156,8 @@ impl Discv5ConfigBuilder {
             permit_ban_list: PermitBanList::default(),
             ban_duration: Some(Duration::from_secs(3600)), // 1 hour
             executor: None,
+            unreachable_enr_limit: None,
+            unused_port_range: None,
             listen_config,
         };
 
@@ -302,13 +318,33 @@ impl Discv5ConfigBuilder {
         self
     }
 
+    /// Sets the maximum number of sessions with peers with unreachable ENRs to allow. Minimum is 1
+    /// peer. Default is no limit.
+    pub fn unreachable_enr_limit(&mut self, peer_limit: Option<usize>) -> &mut Self {
+        self.config.unreachable_enr_limit = peer_limit;
+        self
+    }
+
+    /// Sets the unused port range for testing if node is behind a NAT. Default is the range
+    /// covering user and dynamic ports.
+    pub fn unused_port_range(
+        &mut self,
+        unused_port_range: Option<RangeInclusive<u16>>,
+    ) -> &mut Self {
+        self.config.unused_port_range = unused_port_range;
+        self
+    }
+
     pub fn build(&mut self) -> Discv5Config {
         // If an executor is not provided, assume a current tokio runtime is running.
         if self.config.executor.is_none() {
             self.config.executor = Some(Box::<crate::executor::TokioExecutor>::default());
         };
 
         assert!(self.config.incoming_bucket_limit <= MAX_NODES_PER_BUCKET);
+        if let Some(limit) = self.config.unreachable_enr_limit {
+            assert!(limit >= MIN_SESSIONS_UNREACHABLE_ENR);
+        }
 
         self.config.clone()
     }

diff --git a/src/error.rs b/src/error.rs
@@ -1,15 +1,24 @@
-use crate::{handler::Challenge, node_info::NonContactable};
+use crate::{
+    handler::Challenge,
+    node_info::{NodeAddress, NonContactable},
+};
+use derive_more::From;
 use rlp::DecoderError;
 use std::fmt;
 
-#[derive(Debug)]
+#[derive(Debug, From)]
 /// A general error that is used throughout the Discv5 library.
 pub enum Discv5Error {
+    /// An invalid message type was received.
+    InvalidMessage,
     /// An invalid ENR was received.
     InvalidEnr,
+    /// The limit for sessions with peers that have an unreachable ENR is reached.
+    LimitSessionsUnreachableEnr,
     /// The public key type is known.
     UnknownPublicKey,
     /// The ENR key used is not supported.
+    #[from(ignore)]
     KeyTypeNotSupported(&'static str),
     /// Failed to derive an ephemeral public key.
     KeyDerivationFailed,
@@ -27,25 +36,48 @@ pub enum Discv5Error {
     ServiceAlreadyStarted,
     /// A session could not be established with the remote.
     SessionNotEstablished,
+    /// A session to the given peer is already established.
+    SessionAlreadyEstablished(NodeAddress),
     /// An RLP decoding error occurred.
     RLPError(DecoderError),
     /// Failed to encrypt a message.
+    #[from(ignore)]
     EncryptionFail(String),
     /// Failed to decrypt a message.
+    #[from(ignore)]
     DecryptionFailed(String),
     /// The custom error has occurred.
+    #[from(ignore)]
     Custom(&'static str),
     /// A generic dynamic error occurred.
+    #[from(ignore)]
     Error(String),
     /// An IO error occurred.
     Io(std::io::Error),
 }
 
-impl From<std::io::Error> for Discv5Error {
-    fn from(err: std::io::Error) -> Discv5Error {
-        Discv5Error::Io(err)
-    }
+/// An error occurred whilst attempting to hole punch NAT.
+#[derive(Debug)]
+pub enum NatError {
+    /// Initiator error.
+    Initiator(Discv5Error),
+    /// Relayer error.
+    Relay(Discv5Error),
+    /// Target error.
+    Target(Discv5Error),
+}
+
+macro_rules! impl_from_variant {
+    ($(<$($generic: ident,)+>)*, $from_type: ty, $to_type: ty, $variant: path) => {
+        impl$(<$($generic,)+>)* From<$from_type> for $to_type {
+            fn from(_e: $from_type) -> Self {
+                $variant
+            }
+        }
+    };
 }
+impl_from_variant!(<T,>, tokio::sync::mpsc::error::SendError<T>, Discv5Error, Self::ServiceChannelClosed);
+impl_from_variant!(, NonContactable, Discv5Error, Self::InvalidEnr);
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 /// Types of packet errors.
@@ -111,6 +143,9 @@ pub enum RequestError {
     InvalidMultiaddr(&'static str),
     /// Failure generating random numbers during request.
     EntropyFailure(&'static str),
+    /// Malicious peer tried to initiate nat hole punching for another peer. todo(emhane): this is
+    /// notification error.
+    MaliciousRelayInit,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]