Skip to content

Commit

Permalink
Add handling of NTPv5 to the server (#1153)
Browse files Browse the repository at this point in the history 
* WIP v5 server test

* Added WIP v5 server test

* Add draft id to every packet and only allow v5 with draft id to respond

This also fixed NTPv5 extension field handling along the way...

* Implement NTPv5 Negotiation in NTPv4 for the server side

* Fix clippy warning

* Fix error in parsing NTPv5 extensions without padding

* Rename feature to reflect its unstablenes

* Only respond to upgrade request if they also include the correct draftid

---------

Co-authored-by: Marlon Baeten <marlon@tweedegolf.com>
  • Loading branch information
@tdittr @marlonbaeten
tdittr and marlonbaeten authored Oct 31, 2023
1 parent 284ce54 commit 39a15e9
Show file tree
Hide file tree
Showing 5 changed files with 340 additions and 74 deletions.
18 changes: 16 additions & 2 deletions ntp-proto/src/packet/extension_fields.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,12 @@ impl<'a> ExtensionFieldData<'a> {
let mut it = self.untrusted.iter().peekable();
while let Some(field) = it.next() {
let is_last = it.peek().is_none();
let minimum_size = if is_last { 28 } else { 16 };
let minimum_size = match version {
ExtensionHeaderVersion::V4 if is_last => 28,
ExtensionHeaderVersion::V4 => 16,
#[cfg(feature = "ntpv5")]
ExtensionHeaderVersion::V5 => 4,
};
field.serialize(w, minimum_size, version)?;
}

Expand All @@ -554,10 +559,15 @@ impl<'a> ExtensionFieldData<'a> {
let mut size = 0;
let mut is_valid_nts = true;
let mut cookie = None;
let mac_size = match version {
ExtensionHeaderVersion::V4 => Mac::MAXIMUM_SIZE,
#[cfg(feature = "ntpv5")]
ExtensionHeaderVersion::V5 => 0,
};

for field in RawExtensionField::deserialize_sequence(
&data[header_size..],
Mac::MAXIMUM_SIZE,
mac_size,
RawExtensionField::V4_UNENCRYPTED_MINIMUM_SIZE,
version,
) {
Expand Down Expand Up @@ -776,6 +786,10 @@ impl<'a> RawExtensionField<'a> {
return Err(IncorrectLength);
}

// In NTPv5: There must still be enough room in the packet for data + padding
data.get(4..next_multiple_of_usize(field_length, 4))
.ok_or(IncorrectLength)?;

// because the field length includes padding, the message bytes may not exactly match the input
let message_bytes = data.get(4..field_length).ok_or(IncorrectLength)?;

Expand Down
168 changes: 145 additions & 23 deletions ntp-proto/src/packet/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ impl<'a> NtpPacket<'a> {
};

// TODO: Check extension field handling in V5
match ExtensionFieldData::deserialize(
let res_packet = match ExtensionFieldData::deserialize(
data,
header_size,
cipher,
Expand All @@ -407,6 +407,20 @@ impl<'a> NtpPacket<'a> {

Err(ParsingError::DecryptError(packet))
}
};

let (packet, cookie) = res_packet?;

match packet.draft_id() {
Some(id) if id == v5::DRAFT_VERSION => Ok((packet, cookie)),
received @ (Some(_) | None) => {
tracing::error!(
expected = v5::DRAFT_VERSION,
received,
"Mismatched draft ID ignoring packet!"
);
Err(ParsingError::V5(v5::V5Error::InvalidDraftIdentification))
}
}
}
_ => Err(PacketParsingError::InvalidVersion(version)),
Expand Down Expand Up @@ -505,6 +519,27 @@ impl<'a> NtpPacket<'a> {
)
}

#[cfg(feature = "ntpv5")]
pub fn poll_message_v5(poll_interval: PollInterval) -> (Self, RequestIdentifier) {
let (header, id) = v5::NtpHeaderV5::poll_message(poll_interval);

let draft_id = ExtensionField::DraftIdentification(Cow::Borrowed(v5::DRAFT_VERSION));

(
NtpPacket {
header: NtpHeader::V5(header),
efdata: ExtensionFieldData {
authenticated: vec![],
encrypted: vec![],
untrusted: vec![draft_id],
},
mac: None,
},
id,
)
}

#[cfg_attr(not(feature = "ntpv5"), allow(unused_mut))]
pub fn timestamp_response<C: NtpClock>(
system: &SystemSnapshot,
input: Self,
Expand All @@ -522,27 +557,38 @@ impl<'a> NtpPacket<'a> {
efdata: Default::default(),
mac: None,
},
NtpHeader::V4(header) => NtpPacket {
header: NtpHeader::V4(NtpHeaderV3V4::timestamp_response(
system,
header,
recv_timestamp,
clock,
)),
efdata: ExtensionFieldData {
authenticated: vec![],
encrypted: vec![],
// Ignore encrypted so as not to accidentaly leak anything
untrusted: input
.efdata
.untrusted
.into_iter()
.chain(input.efdata.authenticated)
.filter(|ef| matches!(ef, ExtensionField::UniqueIdentifier(_)))
.collect(),
},
mac: None,
},
NtpHeader::V4(header) => {
let mut response_header =
NtpHeaderV3V4::timestamp_response(system, header, recv_timestamp, clock);

#[cfg(feature = "ntpv5")]
{
// Respond with the upgrade timestamp (NTP5NTP5) iff the input had it and the packet
// had the correct draft identification
if let (v5::UPGRADE_TIMESTAMP, Some(v5::DRAFT_VERSION)) =
(header.reference_timestamp, input.draft_id())
{
response_header.reference_timestamp = v5::UPGRADE_TIMESTAMP;
};
}

NtpPacket {
header: NtpHeader::V4(response_header),
efdata: ExtensionFieldData {
authenticated: vec![],
encrypted: vec![],
// Ignore encrypted so as not to accidentally leak anything
untrusted: input
.efdata
.untrusted
.into_iter()
.chain(input.efdata.authenticated)
.filter(|ef| matches!(ef, ExtensionField::UniqueIdentifier(_)))
.collect(),
},
mac: None,
}
}
#[cfg(feature = "ntpv5")]
NtpHeader::V5(header) => NtpPacket {
// TODO deduplicate extension handling with V4
Expand All @@ -555,20 +601,31 @@ impl<'a> NtpPacket<'a> {
efdata: ExtensionFieldData {
authenticated: vec![],
encrypted: vec![],
// Ignore encrypted so as not to accidentaly leak anything
// Ignore encrypted so as not to accidentally leak anything
untrusted: input
.efdata
.untrusted
.into_iter()
.chain(input.efdata.authenticated)
.filter(|ef| matches!(ef, ExtensionField::UniqueIdentifier(_)))
.chain(std::iter::once(ExtensionField::DraftIdentification(
Cow::Borrowed(v5::DRAFT_VERSION),
)))
.collect(),
},
mac: None,
},
}
}

#[cfg(feature = "ntpv5")]
fn draft_id(&self) -> Option<&'_ str> {
self.efdata.untrusted.iter().find_map(|ef| match ef {
ExtensionField::DraftIdentification(id) => Some(&**id),
_ => None,
})
}

pub fn nts_timestamp_response<C: NtpClock>(
system: &SystemSnapshot,
input: Self,
Expand Down Expand Up @@ -760,6 +817,15 @@ impl<'a> NtpPacket<'a> {
})
}

pub fn version(&self) -> u8 {
match self.header {
NtpHeader::V3(_) => 3,
NtpHeader::V4(_) => 4,
#[cfg(feature = "ntpv5")]
NtpHeader::V5(_) => 5,
}
}

pub fn leap(&self) -> NtpLeapIndicator {
match self.header {
NtpHeader::V3(header) => header.leap,
Expand Down Expand Up @@ -1442,6 +1508,44 @@ mod tests {
assert!(!response.valid_server_response(id, true));
}

#[test]
fn v5_upgrade_packet() {
let (mut packet, _) = NtpPacket::poll_message(PollInterval::default());
let NtpHeader::V4(header) = &mut packet.header else {
panic!("wrong version");
};
header.reference_timestamp = NtpTimestamp::from_fixed_int(0x4E5450354E545035);

#[cfg(feature = "ntpv5")]
packet
.efdata
.untrusted
.push(ExtensionField::DraftIdentification(Cow::Borrowed(
v5::DRAFT_VERSION,
)));

let response = NtpPacket::timestamp_response(
&SystemSnapshot::default(),
packet,
NtpTimestamp::from_fixed_int(0),
&TestClock {
now: NtpTimestamp::from_fixed_int(1),
},
);

let NtpHeader::V4(header) = response.header else {
panic!("wrong version");
};

let expect = if cfg!(feature = "ntpv5") {
NtpTimestamp::from_fixed_int(0x4E5450354E545035)
} else {
NtpTimestamp::from_fixed_int(0)
};

assert_eq!(header.reference_timestamp, expect);
}

#[test]
fn test_timestamp_response() {
let decoded = DecodedServerCookie {
Expand Down Expand Up @@ -2029,4 +2133,22 @@ mod tests {
];
assert!(NtpPacket::deserialize(&input, &NoCipher).is_err());
}

#[cfg(feature = "ntpv5")]
#[test]
fn ef_with_missing_padding_v5() {
let (packet, _) = NtpPacket::poll_message_v5(PollInterval::default());
let mut data = packet.serialize_without_encryption_vec().unwrap();
data.extend([
0, 0, // Type = Unknown
0, 6, // Length = 5
1, 2, // Data
// Missing 2 padding bytes
]);

assert!(matches!(
NtpPacket::deserialize(&data, &NoCipher),
Err(ParsingError::IncorrectLength)
));
}
}
57 changes: 53 additions & 4 deletions ntp-proto/src/packet/v5/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::missing_const_for_fn)]
use crate::{NtpClock, NtpDuration, NtpLeapIndicator, NtpTimestamp, SystemSnapshot};
use crate::{NtpClock, NtpDuration, NtpLeapIndicator, NtpTimestamp, PollInterval, SystemSnapshot};
use rand::random;

mod error;
Expand All @@ -9,8 +9,11 @@ pub mod extension_fields;
use crate::packet::error::ParsingError;
pub use error::V5Error;

use super::RequestIdentifier;

#[allow(dead_code)]
pub(crate) const DRAFT_VERSION: &str = "draft-ietf-ntp-ntpv5-00";
pub(crate) const UPGRADE_TIMESTAMP: NtpTimestamp = NtpTimestamp::from_bits(*b"NTP5NTP5");

#[repr(u8)]
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
Expand Down Expand Up @@ -108,7 +111,7 @@ impl NtpFlags {
pub struct NtpServerCookie(pub [u8; 8]);

impl NtpServerCookie {
fn new_random() -> NtpServerCookie {
fn new_random() -> Self {
// TODO does this match entropy handling of the rest of the system?
Self(random())
}
Expand All @@ -118,9 +121,18 @@ impl NtpServerCookie {
pub struct NtpClientCookie(pub [u8; 8]);

impl NtpClientCookie {
fn new_random() -> Self {
// TODO does this match entropy handling of the rest of the system?
Self(random())
}

pub const fn from_ntp_timestamp(ts: NtpTimestamp) -> Self {
Self(ts.to_bits())
}

pub const fn into_ntp_timestamp(self) -> NtpTimestamp {
NtpTimestamp::from_bits(self.0)
}
}

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand All @@ -144,6 +156,28 @@ pub struct NtpHeaderV5 {
}

impl NtpHeaderV5 {
fn new() -> Self {
Self {
leap: NtpLeapIndicator::NoWarning,
mode: NtpMode::Request,
stratum: 0,
poll: 0,
precision: 0,
root_delay: NtpDuration::default(),
root_dispersion: NtpDuration::default(),
receive_timestamp: NtpTimestamp::default(),
transmit_timestamp: NtpTimestamp::default(),
timescale: NtpTimescale::Utc,
era: NtpEra(0),
flags: NtpFlags {
unknown_leap: false,
interleaved_mode: false,
},
server_cookie: NtpServerCookie([0; 8]),
client_cookie: NtpClientCookie([0; 8]),
}
}

pub(crate) fn timestamp_response<C: NtpClock>(
system: &SystemSnapshot,
input: Self,
Expand Down Expand Up @@ -174,9 +208,7 @@ impl NtpHeaderV5 {
transmit_timestamp: clock.now().expect("Failed to read time"),
}
}
}

impl NtpHeaderV5 {
const WIRE_LENGTH: usize = 48;
const VERSION: u8 = 5;

Expand Down Expand Up @@ -228,6 +260,23 @@ impl NtpHeaderV5 {
w.write_all(&self.transmit_timestamp.to_bits())?;
Ok(())
}

pub fn poll_message(poll_interval: PollInterval) -> (Self, RequestIdentifier) {
let mut packet = Self::new();
packet.poll = poll_interval.as_log();
packet.mode = NtpMode::Request;

let client_cookie = NtpClientCookie::new_random();
packet.client_cookie = client_cookie;

(
packet,
RequestIdentifier {
expected_origin_timestamp: client_cookie.into_ntp_timestamp(),
uid: None,
},
)
}
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 39a15e9

Please sign in to comment.