Skip to content

Commit 1a87d62

Browse files
wille-iojsdanielh
authored andcommitted
Discard stale peers received via discovery + tests
1 parent 0f93d76 commit 1a87d62

2 files changed

Lines changed: 120 additions & 12 deletions

File tree

network-libp2p/src/discovery/peer_contacts.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -427,27 +427,40 @@ impl PeerContactBook {
427427
}
428428
}
429429

430-
// Reject contacts with timestamps in the future
431430
let current_ts = SystemTime::now()
432431
.duration_since(SystemTime::UNIX_EPOCH)
433432
.unwrap()
434433
.as_secs();
434+
435+
// Reject contacts with timestamps in the future
435436
if info.contact().timestamp > current_ts {
436437
return;
437438
}
438439

440+
// Rejects contacts that are older than the allowed age
441+
if info.exceeds_age(
442+
Duration::from_secs(PeerContactBook::MAX_PEER_AGE),
443+
Duration::from_secs(current_ts),
444+
) {
445+
return;
446+
}
447+
439448
let info = Arc::new(info);
440-
let is_new = self
441-
.peer_contacts
442-
.insert(info.peer_id, Arc::clone(&info))
443-
.is_none();
444-
if is_new {
445-
log::trace!(
446-
peer_id = %info.peer_id,
447-
services = ?info.services(),
448-
addresses = ?info.contact.inner.addresses,
449-
"Adding peer contact",
450-
);
449+
match self.peer_contacts.entry(info.peer_id) {
450+
std::collections::hash_map::Entry::Occupied(mut entry) => {
451+
if entry.get().contact().timestamp < info.contact().timestamp {
452+
entry.insert(info);
453+
}
454+
}
455+
std::collections::hash_map::Entry::Vacant(entry) => {
456+
log::trace!(
457+
peer_id = %info.peer_id,
458+
services = ?info.services(),
459+
addresses = ?info.contact.inner.addresses,
460+
"Adding peer contact",
461+
);
462+
entry.insert(info);
463+
}
451464
}
452465
}
453466

network-libp2p/tests/discovery.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,3 +594,98 @@ async fn test_duplicate_discovery_substream_closes_without_panic() {
594594
"Attacker failed to negotiate two discovery substreams"
595595
);
596596
}
597+
598+
#[test]
599+
fn test_insert_filtered_rejects_stale_timestamp() {
600+
let own_contact = random_peer_contact(1, Services::FULL_BLOCKS);
601+
let mut peer_contact_book = PeerContactBook::new(own_contact, false, true, true);
602+
603+
let stale_contact = {
604+
let keypair = Keypair::generate_ed25519();
605+
let stale_ts = SystemTime::now()
606+
.duration_since(SystemTime::UNIX_EPOCH)
607+
.unwrap()
608+
.as_secs()
609+
.saturating_sub(PeerContactBook::MAX_PEER_AGE + 1);
610+
611+
let peer_contact = PeerContact::new(
612+
Some("/dns/stale.evil.local/tcp/443/wss".parse().unwrap()),
613+
keypair.public(),
614+
Services::FULL_BLOCKS,
615+
stale_ts,
616+
)
617+
.expect("Peer contact must be creatable");
618+
619+
peer_contact.sign(&keypair)
620+
};
621+
622+
let stale_peer_id = stale_contact.public_key().clone().to_peer_id();
623+
624+
peer_contact_book.insert_filtered(stale_contact, Services::FULL_BLOCKS, false);
625+
626+
assert!(
627+
peer_contact_book.get(&stale_peer_id).is_none(),
628+
"insert_filtered must reject contacts older than MAX_PEER_AGE"
629+
);
630+
}
631+
632+
#[test]
633+
fn test_insert_filtered_only_replaces_with_newer_timestamp() {
634+
let own_contact = random_peer_contact(1, Services::FULL_BLOCKS);
635+
let mut peer_contact_book = PeerContactBook::new(own_contact, false, true, true);
636+
637+
let keypair = Keypair::generate_ed25519();
638+
let now = SystemTime::now()
639+
.duration_since(SystemTime::UNIX_EPOCH)
640+
.unwrap()
641+
.as_secs();
642+
let base_ts = now.saturating_sub(60);
643+
644+
let original_contact = PeerContact::new(
645+
Some("/dns/peer.old.local/tcp/443/wss".parse().unwrap()),
646+
keypair.public(),
647+
Services::FULL_BLOCKS,
648+
base_ts,
649+
)
650+
.expect("Peer contact must be creatable")
651+
.sign(&keypair);
652+
let same_timestamp_contact = PeerContact::new(
653+
Some("/dns/peer.same.local/tcp/443/wss".parse().unwrap()),
654+
keypair.public(),
655+
Services::FULL_BLOCKS,
656+
base_ts,
657+
)
658+
.expect("Peer contact must be creatable")
659+
.sign(&keypair);
660+
let newer_contact = PeerContact::new(
661+
Some("/dns/peer.new.local/tcp/443/wss".parse().unwrap()),
662+
keypair.public(),
663+
Services::FULL_BLOCKS,
664+
base_ts + 1,
665+
)
666+
.expect("Peer contact must be creatable")
667+
.sign(&keypair);
668+
669+
let peer_id = original_contact.public_key().clone().to_peer_id();
670+
671+
peer_contact_book.insert_filtered(original_contact.clone(), Services::FULL_BLOCKS, false);
672+
assert_eq!(
673+
peer_contact_book.get(&peer_id).unwrap().signed(),
674+
&original_contact,
675+
"fresh contact should be inserted"
676+
);
677+
678+
peer_contact_book.insert_filtered(same_timestamp_contact, Services::FULL_BLOCKS, false);
679+
assert_eq!(
680+
peer_contact_book.get(&peer_id).unwrap().signed(),
681+
&original_contact,
682+
"same timestamp must not overwrite the existing contact"
683+
);
684+
685+
peer_contact_book.insert_filtered(newer_contact.clone(), Services::FULL_BLOCKS, false);
686+
assert_eq!(
687+
peer_contact_book.get(&peer_id).unwrap().signed(),
688+
&newer_contact,
689+
"newer timestamp must replace the existing contact"
690+
);
691+
}

0 commit comments

Comments
 (0)