Merge latest master

.github/workflows/build.yml

@@ -6,11 +6,22 @@ jobs:
  cargo-fmt:
  runs-on: ubuntu-latest
  steps:
- - uses: actions/checkout@v2
- - name: Get latest version of stable rust
- run: rustup update stable
- - name: Check formatting with cargofmt
- run: cargo fmt --all -- --check --config imports_granularity=Crate
+ - uses: actions/checkout@v2
+ - name: Get latest version of stable rust
+ run: rustup update stable
+ - name: Check formatting with cargofmt
+ run: cargo fmt --all -- --check --config imports_granularity=Crate
+ clippy:
+ runs-on: ubuntu-latest
+ needs: cargo-fmt
+ steps:
+ - uses: actions/checkout@v2
+ - name: Get latest version of stable rust
+ run: rustup update stable
+ - name: Install protobuf compiler for the libp2p-core dependency
+ uses: arduino/setup-protoc@v1
+ - name: Lint code for quality and style with Clippy
+ run: cargo clippy --workspace --tests --all-features -- -D warnings
  release-tests-ubuntu:
  runs-on: ubuntu-latest
  needs: cargo-fmt
@@ -29,21 +40,18 @@ jobs:
  - uses: actions/checkout@v2
  - name: Get latest version of stable rust
  run: rustup update stable
+ - name: Install protobuf compiler for the libp2p-core dependency
+ uses: arduino/setup-protoc@v1
  - name: Run tests in release
  run: cargo test --all --release --all-features --tests
- clippy:
- runs-on: ubuntu-latest
- needs: cargo-fmt
- steps:
- - uses: actions/checkout@v2
- - name: Lint code for quality and style with Clippy
- run: cargo clippy
  check-rustdoc-links:
  name: Check rustdoc intra-doc links
  runs-on: ubuntu-latest
  container:
  image: rust
  steps:
- - uses: actions/checkout@v2
- - name: Check rustdoc links
- run: RUSTDOCFLAGS="--deny broken_intra_doc_links" cargo doc --verbose --workspace --no-deps --document-private-items
+ - uses: actions/checkout@v2
+ - 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

Cargo.toml

@@ -2,7 +2,7 @@
 name = "discv5"
 authors = ["Age Manning <[email protected]>"]
 edition = "2018"
-version = "0.1.0-beta.13"
+version = "0.1.0"
 description = "Implementation of the p2p discv5 discovery protocol"
 license = "Apache-2.0"
 repository = "https:/sigp/discv5"
@@ -12,22 +12,22 @@ categories = ["network-programming", "asynchronous"]
 exclude = [".gitignore", ".github/*"]
 
 [dependencies]
-enr = { version = "0.5.1", features = ["k256", "ed25519"] }
+enr = { version = "0.6.2", features = ["k256", "ed25519"] }
 tokio = { version = "1.15.0", features = ["net", "sync", "macros", "rt"] }
 tokio-stream = "0.1.8"
 tokio-util = { version = "0.6.9", features = ["time"] }
-libp2p-core = { version = "0.30.0", optional = true }
+libp2p-core = { version = "0.36.0", optional = true }
 zeroize = { version = "1.4.3", features = ["zeroize_derive"] }
 futures = "0.3.19"
 uint = { version = "0.9.1", default-features = false }
 rlp = "0.5.1"
-sha2 = "0.9.5"
-hkdf = "0.11.0"
+# This version must be kept up to date do it uses the same dependencies as ENR
+hkdf = "0.12.3" 
 hex = "0.4.3"
 fnv = "1.0.7"
 arrayvec = "0.7.2"
-digest = "0.10.1"
 rand = { version = "0.8.4", package = "rand" }
+socket2 = "0.4.4"
 smallvec = "1.7.0"
 parking_lot = "0.11.2"
 lazy_static = "1.4.0"
@@ -37,7 +37,8 @@ tracing = { version = "0.1.29", features = ["log"] }
 tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
 lru = "0.7.1"
 hashlink = "0.7.0"
-delay_map = "0.1.0"
+delay_map = "0.1.1"
+more-asserts = "0.2.2"
 
 [dev-dependencies]
 rand_07 = { package = "rand", version = "0.7" }
@@ -49,6 +50,7 @@ tokio-util = { version = "0.6.9", features = ["time"] }
 tokio = { version = "1.15.0", features = ["full"] }
 rand_xorshift = "0.3.0"
 rand_core = "0.6.3"
+clap = { version = "3.1", features = ["derive"] }
 
 [features]
 libp2p = ["libp2p-core"]

README.md

@@ -73,3 +73,58 @@ A simple example of creating this service is as follows:
  println!("Found nodes: {:?}", found_nodes);
  });
 ```
+
+# Addresses in ENRs 
+
+This protocol will drop messages (i.e not respond to requests) from peers that
+advertise non-contactable address in their ENR (e.g `127.0.0.1` when connecting
+to non-local nodes). This section
+explains the rationale behind this design decision.
+
+An ENR is a signed record which is primarily used in this protocol for
+identifying and connecting to peers. ENRs have **OPTIONAL** `ip` and `port`
+fields.
+
+If a node does not know its contactable address (i.e if it is behind a NAT), it should leave these fields
+empty. This is done for the following reasons:
+1. When we receive an ENR we must decide whether to add it to our local routing
+ table and advertise it to other peers. If a node has put some
+ non-contactable address in the ENR (e.g `127.0.0.1` when connecting to
+ non-local nodes) we cannot use this ENR
+ to contact the node and we therefore do not wish to advertise it to other
+ nodes. Putting a non-contactable address is therefore functionally
+ equivalent to leaving the fields empty.
+2. For every new inbound connection, we do not wish to check that the address
+ given to us in an `ENR` is contactable. We do not want the scenario, where
+ any peer can give us any address and force us to attempt a connection to
+ arbitrary addresses (to check their validity) as it consumes unnecessary
+ bandwidth and we want to avoid DOS attacks where malicious users spam many
+ nodes attempting them all to send messages to a victim IP.
+
+## How this protocol handles advertised IPs in ENRs
+
+To handle the above two cases this protocol filters out and only advertises
+contactable ENRs. It doesn't make sense for a discovery protocol to advertise
+non-contactable peers.
+
+This is done in the following way:
+
+1. If a connecting node provides an ENR without specifying an address (this
+ should be the default case for most nodes behind a NAT, or ones that have
+ just started) we consider this valid. Typically this will occur when a node
+ has yet to determine its external IP address via PONG responses and has not
+ updated its ENR to a contactable address. In this case, we respond to all
+ requests this peer asks for but we do not store or add its ENR to our
+ routing table.
+2. If a peer connects to us with an ENR that specifies an IP address that
+ matches the src address we received the packet from, we consider this peer
+ valid and attempt to add it to our local routing table and therefore may advertise
+ its ENR to others.
+3. If a peer connects to us with an ENR that specifies an IP address that does
+ not match the src socket it connects to us on (e.g `127.0.0.1`, or
+ potentially some internal subnet IP that is unreachable from our current
+ network) we consider this peer malicious/faulty
+ and drop all packets. This way we can efficiently drop peers that may try to
+ get us to send messages to arbitrary remote IPs, and we can be sure that all
+ ENRs in our routing table are contactable (at least by our local node at
+ some point in time).

examples/custom_executor.rs

@@ -1,6 +1,6 @@
 //! Demonstrates how to run a basic Discovery v5 Service with a custom tokio executor.
 //!
-//! Discv5 requires a Tokio executor with all features. A custom exuector can be passed via the
+//! Discv5 requires a Tokio executor with all features. A custom executor can be passed via the
 //! configuration parameters. If none is passed, it will use the current runtime that build the
 //! `Discv5` struct.
 //!
@@ -43,16 +43,16 @@ fn main() {
 
  // if we know of another peer's ENR, add it known peers
  if let Some(base64_enr) = std::env::args().nth(1) {
- match base64_enr.parse::<enr::Enr<enr::CombinedKey>>() {
+ match base64_enr.parse::<enr::Enr<CombinedKey>>() {
  Ok(enr) => {
  println!(
  "ENR Read. ip: {:?}, udp_port {:?}, tcp_port: {:?}",
- enr.ip(),
- enr.udp(),
- enr.tcp()
+ enr.ip4(),
+ enr.udp4(),
+ enr.tcp4()
  );
  if let Err(e) = discv5.add_enr(enr) {
- println!("ENR was not added: {}", e);
+ println!("ENR was not added: {e}");
  }
  }
  Err(e) => panic!("Decoding ENR failed: {}", e),
@@ -70,7 +70,7 @@ fn main() {
  loop {
  match event_stream.recv().await {
  Some(Discv5Event::SocketUpdated(addr)) => {
- println!("Nodes ENR socket address has been updated to: {:?}", addr);
+ println!("Nodes ENR socket address has been updated to: {addr:?}");
  }
  Some(Discv5Event::Discovered(enr)) => {
  println!("A peer has been discovered: {}", enr.node_id());