feat(kad): New provider record update strategy

In `MemoryStore`, the number of provider records per key is limited by `max_providers_per_key`. Former implementations keep provider records sorted by their distance to the key, and only keep those with the smallest distance. This strategy is vulnerable to Sybil attack, in which an attacker can flood the network with false identities in order to eclipse a key.

This commit change the strategy to simply keep old providers and ignore new ones. This new strategy however, can cause load imbalance, but can be mitigated by increasing `max_providers_per_key`.

In addition, old implementations failed to keep `provided` and `providers` in sync, and this commit fixes this issue.

Pull-Request: #5536.

Cargo.toml

@@ -86,7 +86,7 @@ libp2p-floodsub = { version = "0.45.0", path = "protocols/floodsub" }
 libp2p-gossipsub = { version = "0.47.0", path = "protocols/gossipsub" }
 libp2p-identify = { version = "0.45.0", path = "protocols/identify" }
 libp2p-identity = { version = "0.2.9" }
-libp2p-kad = { version = "0.46.0", path = "protocols/kad" }
+libp2p-kad = { version = "0.46.1", path = "protocols/kad" }
 libp2p-mdns = { version = "0.46.0", path = "protocols/mdns" }
 libp2p-memory-connection-limits = { version = "0.3.0", path = "misc/memory-connection-limits" }
 libp2p-metrics = { version = "0.14.2", path = "misc/metrics" }

libp2p/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.54.1
+
+- Update individual crates.
+    - Update to [`libp2p-kad` `v0.46.1`](protocols/kad/CHANGELOG.md#0461).
+
 ## 0.54.0
 
 - Update individual crates.

libp2p/Cargo.toml

@@ -3,7 +3,7 @@ name = "libp2p"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Peer-to-peer networking library"
-version = "0.54.0"
+version = "0.54.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

protocols/kad/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.46.1
+
+- Use new provider record update strategy to prevent Sybil attack.
+  See [PR 5536](https://github.com/libp2p/rust-libp2p/pull/5536).
+
 ## 0.46.0
 
 - Included multiaddresses of found peers alongside peer IDs in `GetClosestPeers` query results.

protocols/kad/Cargo.toml

@@ -3,7 +3,7 @@ name = "libp2p-kad"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Kademlia protocol for libp2p"
-version = "0.46.0"
+version = "0.46.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

protocols/kad/src/record/store/memory.rs

@@ -152,38 +152,31 @@ impl RecordStore for MemoryStore {
         }
         .or_insert_with(Default::default);
 
-        if let Some(i) = providers.iter().position(|p| p.provider == record.provider) {
-            // In-place update of an existing provider record.
-            providers.as_mut()[i] = record;
-        } else {
-            // It is a new provider record for that key.
-            let local_key = self.local_key;
-            let key = kbucket::Key::new(record.key.clone());
-            let provider = kbucket::Key::from(record.provider);
-            if let Some(i) = providers.iter().position(|p| {
-                let pk = kbucket::Key::from(p.provider);
-                provider.distance(&key) < pk.distance(&key)
-            }) {
-                // Insert the new provider.
-                if local_key.preimage() == &record.provider {
+        for p in providers.iter_mut() {
+            if p.provider == record.provider {
+                // In-place update of an existing provider record.
+                if self.local_key.preimage() == &record.provider {
+                    self.provided.remove(p);
                     self.provided.insert(record.clone());
                 }
-                providers.insert(i, record);
-                // Remove the excess provider, if any.
-                if providers.len() > self.config.max_providers_per_key {
-                    if let Some(p) = providers.pop() {
-                        self.provided.remove(&p);
-                    }
-                }
-            } else if providers.len() < self.config.max_providers_per_key {
-                // The distance of the new provider to the key is larger than
-                // the distance of any existing provider, but there is still room.
-                if local_key.preimage() == &record.provider {
-                    self.provided.insert(record.clone());
-                }
-                providers.push(record);
+                *p = record;
+                return Ok(());
             }
         }
+
+        // If the providers list is full, we ignore the new provider.
+        // This strategy can mitigate Sybil attacks, in which an attacker
+        // floods the network with fake provider records.
+        if providers.len() == self.config.max_providers_per_key {
+            return Ok(());
+        }
+
+        // Otherwise, insert the new provider record.
+        if self.local_key.preimage() == &record.provider {
+            self.provided.insert(record.clone());
+        }
+        providers.push(record);
+
         Ok(())
     }
 
@@ -202,7 +195,9 @@ impl RecordStore for MemoryStore {
             let providers = e.get_mut();
             if let Some(i) = providers.iter().position(|p| &p.provider == provider) {
                 let p = providers.remove(i);
-                self.provided.remove(&p);
+                if &p.provider == self.local_key.preimage() {
+                    self.provided.remove(&p);
+                }
             }
             if providers.is_empty() {
                 e.remove();
@@ -221,11 +216,6 @@ mod tests {
     fn random_multihash() -> Multihash<64> {
         Multihash::wrap(SHA_256_MH, &rand::thread_rng().gen::<[u8; 32]>()).unwrap()
     }
-
-    fn distance(r: &ProviderRecord) -> kbucket::Distance {
-        kbucket::Key::new(r.key.clone()).distance(&kbucket::Key::from(r.provider))
-    }
-
     #[test]
     fn put_get_remove_record() {
         fn prop(r: Record) {
@@ -250,30 +240,6 @@ mod tests {
         quickcheck(prop as fn(_))
     }
 
-    #[test]
-    fn providers_ordered_by_distance_to_key() {
-        fn prop(providers: Vec<kbucket::Key<PeerId>>) -> bool {
-            let mut store = MemoryStore::new(PeerId::random());
-            let key = Key::from(random_multihash());
-
-            let mut records = providers
-                .into_iter()
-                .map(|p| ProviderRecord::new(key.clone(), p.into_preimage(), Vec::new()))
-                .collect::<Vec<_>>();
-
-            for r in &records {
-                assert!(store.add_provider(r.clone()).is_ok());
-            }
-
-            records.sort_by_key(distance);
-            records.truncate(store.config.max_providers_per_key);
-
-            records == store.providers(&key).to_vec()
-        }
-
-        quickcheck(prop as fn(_) -> _)
-    }
-
     #[test]
     fn provided() {
         let id = PeerId::random();
@@ -302,6 +268,46 @@ mod tests {
         assert_eq!(vec![rec.clone()], store.providers(&rec.key).to_vec());
     }
 
+    #[test]
+    fn update_provided() {
+        let prv = PeerId::random();
+        let mut store = MemoryStore::new(prv);
+        let key = random_multihash();
+        let mut rec = ProviderRecord::new(key, prv, Vec::new());
+        assert!(store.add_provider(rec.clone()).is_ok());
+        assert_eq!(
+            vec![Cow::Borrowed(&rec)],
+            store.provided().collect::<Vec<_>>()
+        );
+        rec.expires = Some(Instant::now());
+        assert!(store.add_provider(rec.clone()).is_ok());
+        assert_eq!(
+            vec![Cow::Borrowed(&rec)],
+            store.provided().collect::<Vec<_>>()
+        );
+    }
+
+    #[test]
+    fn max_providers_per_key() {
+        let config = MemoryStoreConfig::default();
+        let key = kbucket::Key::new(Key::from(random_multihash()));
+
+        let mut store = MemoryStore::with_config(PeerId::random(), config.clone());
+        let peers = (0..config.max_providers_per_key)
+            .map(|_| PeerId::random())
+            .collect::<Vec<_>>();
+        for peer in peers {
+            let rec = ProviderRecord::new(key.preimage().clone(), peer, Vec::new());
+            assert!(store.add_provider(rec).is_ok());
+        }
+
+        // The new provider cannot be added because the key is already saturated.
+        let peer = PeerId::random();
+        let rec = ProviderRecord::new(key.preimage().clone(), peer, Vec::new());
+        assert!(store.add_provider(rec.clone()).is_ok());
+        assert!(!store.providers(&rec.key).contains(&rec));
+    }
+
     #[test]
     fn max_provided_keys() {
         let mut store = MemoryStore::new(PeerId::random());