Skip to content

Peer port randomization #1377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions ntp-proto/src/peer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ use crate::{
time_types::{NtpDuration, NtpInstant, NtpTimestamp, PollInterval},
};
use serde::{Deserialize, Serialize};
use std::{io::Cursor, net::SocketAddr};
use std::{
io::Cursor,
net::{IpAddr, SocketAddr},
};
use tracing::{debug, info, instrument, trace, warn};

const MAX_STRATUM: u8 = 16;
Expand Down Expand Up @@ -75,7 +78,6 @@ pub struct Peer {

source_addr: SocketAddr,
source_id: ReferenceId,
our_id: ReferenceId,
reach: Reach,
tries: usize,

Expand Down Expand Up @@ -207,7 +209,6 @@ pub struct PeerSnapshot {
pub source_addr: SocketAddr,

pub source_id: ReferenceId,
pub our_id: ReferenceId,

pub poll_interval: PollInterval,
pub reach: Reach,
Expand All @@ -225,6 +226,7 @@ impl PeerSnapshot {
pub fn accept_synchronization(
&self,
local_stratum: u8,
local_ips: &[IpAddr],
#[cfg_attr(not(feature = "ntpv5"), allow(unused_variables))] system: &SystemSnapshot,
) -> Result<(), AcceptSynchronizationError> {
use AcceptSynchronizationError::*;
Expand All @@ -242,7 +244,12 @@ impl PeerSnapshot {
// if so, we shouldn't sync to them as that would create a loop.
// Note, this can only ever be an issue if the peer is not using
// hardware as its source, so ignore reference_id if stratum is 1.
if self.stratum != 1 && self.reference_id == self.our_id {

if self.stratum != 1
&& local_ips
.iter()
.any(|ip| ReferenceId::from_ip(*ip) == self.source_id)
{
info!("Peer rejected because of detected synchronization loop (ref id)");
return Err(Loop);
}
Expand All @@ -269,7 +276,6 @@ impl PeerSnapshot {
Self {
source_addr: peer.source_addr,
source_id: peer.source_id,
our_id: peer.our_id,
stratum: peer.stratum,
reference_id: peer.reference_id,
reach: peer.reach,
Expand All @@ -283,7 +289,7 @@ impl PeerSnapshot {

#[cfg(feature = "__internal-test")]
pub fn peer_snapshot() -> PeerSnapshot {
use std::net::{IpAddr, Ipv4Addr};
use std::net::Ipv4Addr;

let mut reach = crate::peer::Reach::default();
reach.received_packet();
Expand All @@ -294,7 +300,6 @@ pub fn peer_snapshot() -> PeerSnapshot {
stratum: 0,
reference_id: ReferenceId::from_int(0),

our_id: ReferenceId::from_int(1),
reach,
poll_interval: crate::time_types::PollIntervalLimits::default().min,
protocol_version: Default::default(),
Expand Down Expand Up @@ -364,7 +369,6 @@ impl Default for ProtocolVersion {
impl Peer {
#[instrument]
pub fn new(
our_addr: SocketAddr,
source_addr: SocketAddr,
local_clock_time: NtpInstant,
peer_defaults_config: SourceDefaultsConfig,
Expand All @@ -378,7 +382,6 @@ impl Peer {
remote_min_poll_interval: peer_defaults_config.poll_interval_limits.min,

current_request_identifier: None,
our_id: ReferenceId::from_ip(our_addr.ip()),
source_id: ReferenceId::from_ip(source_addr.ip()),
source_addr,
reach: Default::default(),
Expand All @@ -398,7 +401,6 @@ impl Peer {

#[instrument]
pub fn new_nts(
our_addr: SocketAddr,
source_addr: SocketAddr,
local_clock_time: NtpInstant,
peer_defaults_config: SourceDefaultsConfig,
Expand All @@ -408,7 +410,6 @@ impl Peer {
Self {
nts: Some(nts),
..Self::new(
our_addr,
source_addr,
local_clock_time,
peer_defaults_config,
Expand Down Expand Up @@ -692,12 +693,12 @@ impl Peer {
// make sure in-flight messages are ignored
self.current_request_identifier = None;

info!(our_id = ?self.our_id, source_id = ?self.source_id, "Source reset");
info!(source_id = ?self.source_id, "Source reset");
}

#[cfg(test)]
pub(crate) fn test_peer() -> Self {
use std::net::{IpAddr, Ipv4Addr};
use std::net::Ipv4Addr;

Peer {
nts: None,
Expand All @@ -710,7 +711,6 @@ impl Peer {

source_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), 0),
source_id: ReferenceId::from_int(0),
our_id: ReferenceId::from_int(0),
reach: Reach::default(),
tries: 0,

Expand Down Expand Up @@ -896,15 +896,14 @@ mod test {
macro_rules! accept {
() => {{
let snapshot = PeerSnapshot::from_peer(&peer);
snapshot.accept_synchronization(16, &system)
snapshot.accept_synchronization(16, &["127.0.0.1".parse().unwrap()], &system)
}};
}

// by default, the packet id and the peer's id are the same, indicating a loop
peer.source_id = ReferenceId::from_ip("127.0.0.1".parse().unwrap());
assert_eq!(accept!(), Err(Loop));

peer.our_id = ReferenceId::from_int(42);

peer.source_id = ReferenceId::from_ip("127.0.1.1".parse().unwrap());
assert_eq!(accept!(), Err(ServerUnreachable));

peer.reach.received_packet();
Expand Down
2 changes: 0 additions & 2 deletions ntp-proto/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ mod tests {
PeerSnapshot {
source_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), 0),
source_id: ReferenceId::KISS_DENY,
our_id: ReferenceId::NONE,
poll_interval: PollIntervalLimits::default().max,
reach: Default::default(),
stratum: 2,
Expand All @@ -146,7 +145,6 @@ mod tests {
PeerSnapshot {
source_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::UNSPECIFIED), 0),
source_id: ReferenceId::KISS_RATE,
our_id: ReferenceId::KISS_RSTR,
poll_interval: PollIntervalLimits::default().max,
reach: Default::default(),
stratum: 3,
Expand Down
35 changes: 35 additions & 0 deletions ntpd/src/daemon/local_ip_provider.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use std::{net::IpAddr, sync::Arc};

use timestamped_socket::interface::{interfaces, ChangeDetector};
use tokio::sync::watch;

pub fn spawn() -> std::io::Result<watch::Receiver<Arc<[IpAddr]>>> {
let mut change_listener = ChangeDetector::new()?;
let local_ips: Arc<[IpAddr]> = interfaces()?
.iter()
.flat_map(|(_, interface)| interface.ips())
.collect();

let (writer, reader) = watch::channel(local_ips);

tokio::spawn(async move {
loop {
change_listener.wait_for_change().await;
match interfaces() {
Ok(interfaces) => {
let _ = writer.send(
interfaces
.iter()
.flat_map(|(_, interface)| interface.ips())
.collect(),
);
}
Err(e) => {
tracing::warn!("Could not get new list of which ip addresses the interfaces in the system have: {}", e);
}
}
}
});

Ok(reader)
}
1 change: 1 addition & 0 deletions ntpd/src/daemon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod clock;
pub mod config;
mod ipfilter;
pub mod keyexchange;
mod local_ip_provider;
pub mod nts_key_provider;
pub mod observer;
mod peer;
Expand Down
Loading