From b62230edae17bc6958c39dc02366c3a5f4b4e553 Mon Sep 17 00:00:00 2001 From: Mivik <54128043+Mivik@users.noreply.github.com> Date: Wed, 14 Aug 2024 00:27:40 +0800 Subject: [PATCH] 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.lock | 4 +- Cargo.toml | 2 +- libp2p/CHANGELOG.md | 5 + libp2p/Cargo.toml | 2 +- protocols/kad/CHANGELOG.md | 5 + protocols/kad/Cargo.toml | 2 +- protocols/kad/src/record/store/memory.rs | 122 ++++++++++++----------- 7 files changed, 79 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65a3f27bbdf..c228c7e31ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2623,7 +2623,7 @@ checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c" [[package]] name = "libp2p" -version = "0.54.0" +version = "0.54.1" dependencies = [ "async-std", "async-trait", @@ -2928,7 +2928,7 @@ dependencies = [ [[package]] name = "libp2p-kad" -version = "0.46.0" +version = "0.46.1" dependencies = [ "arrayvec", "async-std", diff --git a/Cargo.toml b/Cargo.toml index 92562489ed5..802779774cb 100644 --- a/Cargo.toml +++ b/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" } diff --git a/libp2p/CHANGELOG.md b/libp2p/CHANGELOG.md index 7e1b95c6e75..f2041399042 100644 --- a/libp2p/CHANGELOG.md +++ b/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. diff --git a/libp2p/Cargo.toml b/libp2p/Cargo.toml index 68a76e52b58..b1017f5958c 100644 --- a/libp2p/Cargo.toml +++ b/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 "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index cbb5a4decf2..a41d6b9a131 100644 --- a/protocols/kad/CHANGELOG.md +++ b/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. diff --git a/protocols/kad/Cargo.toml b/protocols/kad/Cargo.toml index 72b29d00ef7..a00959fced6 100644 --- a/protocols/kad/Cargo.toml +++ b/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 "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/kad/src/record/store/memory.rs b/protocols/kad/src/record/store/memory.rs index 3bd3c56e30c..3fb6d2be3e8 100644 --- a/protocols/kad/src/record/store/memory.rs +++ b/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>) -> 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::>(); - - 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::>() + ); + rec.expires = Some(Instant::now()); + assert!(store.add_provider(rec.clone()).is_ok()); + assert_eq!( + vec![Cow::Borrowed(&rec)], + store.provided().collect::>() + ); + } + + #[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::>(); + 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());