Skip to content

Commit

Permalink
Fix nts parsing (#825)
Browse files Browse the repository at this point in the history 
* Added additional tests for nts basic functionality.

* Revert "simplify offset calculation"

This reverts commit a52cdf2.
  • Loading branch information
@davidv1992
davidv1992 authored Jun 30, 2023
1 parent c6ec607 commit b1451a8
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 13 deletions.
26 changes: 14 additions & 12 deletions ntp-proto/src/packet/extensionfields.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,8 @@ impl<'a> ExtensionFieldData<'a> {

#[allow(clippy::type_complexity)]
pub(super) fn deserialize(
extension_field_bytes: &'a [u8],
data: &'a [u8],
header_size: usize,
cipher: &impl CipherProvider,
) -> Result<DeserializedExtensionField<'a>, ParsingError<InvalidNtsExtensionField<'a>>> {
use ExtensionField::InvalidNtsEncryptedField;
Expand All @@ -438,13 +439,12 @@ impl<'a> ExtensionFieldData<'a> {
let mut cookie = None;

for field in RawExtensionField::deserialize_sequence(
extension_field_bytes,
&data[header_size..],
Mac::MAXIMUM_SIZE,
RawExtensionField::V4_UNENCRYPTED_MINIMUM_SIZE,
) {
let field = field.map_err(|e| e.generalize())?;
size += field.wire_length();

let (offset, field) = field.map_err(|e| e.generalize())?;
size = offset + field.wire_length();
match field.type_id {
ExtensionFieldTypeId::NtsEncryptedField => {
let encrypted = RawEncryptedField::from_message_bytes(field.message_bytes)
Expand All @@ -460,7 +460,7 @@ impl<'a> ExtensionFieldData<'a> {
};

let encrypted_fields =
match encrypted.decrypt(cipher.as_ref(), field.message_bytes) {
match encrypted.decrypt(cipher.as_ref(), &data[..header_size + offset]) {
Ok(encrypted_fields) => encrypted_fields,
Err(e) => {
// early return if it's anything but a decrypt error
Expand Down Expand Up @@ -488,7 +488,7 @@ impl<'a> ExtensionFieldData<'a> {
}
}

let remaining_bytes = &extension_field_bytes[size..];
let remaining_bytes = &data[header_size + size..];

if is_valid_nts {
let result = DeserializedExtensionField {
Expand Down Expand Up @@ -559,7 +559,7 @@ impl<'a> RawEncryptedField<'a> {

RawExtensionField::deserialize_sequence(&plaintext, 0, RawExtensionField::BARE_MINIMUM_SIZE)
.map(|encrypted_field| {
let encrypted_field = encrypted_field.map_err(|e| e.generalize())?;
let encrypted_field = encrypted_field.map_err(|e| e.generalize())?.1;
if encrypted_field.type_id == ExtensionFieldTypeId::NtsEncryptedField {
// TODO: Discuss whether we want this check
Err(ParsingError::MalformedNtsExtensionFields)
Expand Down Expand Up @@ -631,8 +631,9 @@ impl<'a> RawExtensionField<'a> {
buffer: &'a [u8],
cutoff: usize,
minimum_size: usize,
) -> impl Iterator<Item = Result<RawExtensionField<'a>, ParsingError<std::convert::Infallible>>> + 'a
{
) -> impl Iterator<
Item = Result<(usize, RawExtensionField<'a>), ParsingError<std::convert::Infallible>>,
> + 'a {
ExtensionFieldStreamer {
buffer,
cutoff,
Expand All @@ -649,7 +650,7 @@ struct ExtensionFieldStreamer<'a> {
}

impl<'a> Iterator for ExtensionFieldStreamer<'a> {
type Item = Result<RawExtensionField<'a>, ParsingError<std::convert::Infallible>>;
type Item = Result<(usize, RawExtensionField<'a>), ParsingError<std::convert::Infallible>>;

fn next(&mut self) -> Option<Self::Item> {
if self.buffer.len() - self.offset <= self.cutoff {
Expand All @@ -658,8 +659,9 @@ impl<'a> Iterator for ExtensionFieldStreamer<'a> {

match RawExtensionField::deserialize(&self.buffer[self.offset..], self.minimum_size) {
Ok(field) => {
let offset = self.offset;
self.offset += field.wire_length();
Some(Ok(field))
Some(Ok((offset, field)))
}
Err(error) => {
self.offset = self.buffer.len();
Expand Down
50 changes: 49 additions & 1 deletion ntp-proto/src/packet/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl<'a> NtpPacket<'a> {
Ok::<_, ParsingError<std::convert::Infallible>>(packet)
};

match ExtensionFieldData::deserialize(&data[header_size..], cipher) {
match ExtensionFieldData::deserialize(data, header_size, cipher) {
Ok(decoded) => {
let packet = contruct_packet(decoded.remaining_bytes, decoded.efdata)
.map_err(|e| e.generalize())?;
Expand Down Expand Up @@ -1068,6 +1068,54 @@ mod tests {
}
}

#[test]
fn test_nts_roundtrip() {
let cookie = [0; 16];
let (packet1, _) =
NtpPacket::nts_poll_message(&cookie, 1, PollIntervalLimits::default().min);
let cipher = AesSivCmac512::new(std::array::from_fn::<_, 64, _>(|i| i as u8).into());

let mut buffer = [0u8; 2048];
let mut cursor = Cursor::new(buffer.as_mut());
packet1.serialize(&mut cursor, &cipher).unwrap();
let (packet2, _) =
NtpPacket::deserialize(&cursor.get_ref()[..cursor.position() as usize], &cipher)
.unwrap();
assert_eq!(packet1, packet2);
}

#[test]
fn test_nts_captured_server() {
let packet = b"\x24\x01\x04\xe8\x00\x00\x00\x00\x00\x00\x00\x60\x54\x4d\x4e\x4c\xe8\x49\x48\x92\xf9\x29\x57\x9e\x62\x87\xdb\x47\x3f\xf7\x5f\x58\xe8\x49\x48\xb2\xb6\x40\xd7\x01\xe8\x49\x48\xb2\xb6\x44\xbf\xf8\x01\x04\x00\x24\xe4\x83\x3a\x8d\x60\x0e\x13\x42\x43\x5c\xb2\x9d\xe5\x50\xac\xc0\xf8\xd8\xfa\x16\xe5\xc5\x37\x0a\x62\x0b\x15\x5f\x58\x6a\xda\xd6\x04\x04\x00\xd4\x00\x10\x00\xbc\x6a\x1d\xe3\xc2\x6e\x13\xeb\x10\xc7\x39\xd7\x0b\x84\x1f\xad\x1b\x86\xe2\x30\xc6\x3e\x9e\xa5\xf7\x1b\x62\xa8\xa7\x98\x81\xce\x7c\x6b\x17\xcb\x31\x32\x49\x0f\xde\xcf\x21\x10\x56\x4e\x36\x88\x92\xdd\xee\xf1\xf4\x23\xf6\x55\x53\x41\xc2\xc9\x17\x61\x20\xa5\x18\xdc\x1a\x7e\xdc\x5e\xe3\xc8\x3b\x05\x08\x7b\x73\x03\xf7\xab\x86\xd5\x2c\xc7\x49\x0c\xe8\x29\x39\x72\x23\xdc\xef\x2d\x94\xfa\xf8\xd7\x1d\x12\x80\xda\x03\x2d\xd7\x04\x69\xe9\xac\x5f\x82\xef\x57\x81\xd2\x07\xfb\xac\xb4\xa8\xb6\x31\x91\x14\xd5\xf5\x6f\xb2\x2a\x0c\xb6\xd7\xdc\xf7\x7d\xf0\x21\x46\xf6\x7e\x46\x01\xb5\x3b\x21\x7c\xa8\xac\x1a\x4d\x97\xd5\x9b\xce\xeb\x98\x33\x99\x7f\x10\x0e\xd4\x69\x85\x8b\xcd\x73\x52\x01\xad\xec\x38\xcf\x8c\xb2\xc6\xd0\x54\x1a\x97\x67\xdd\xb3\xea\x09\x1d\x63\xd9\x8d\x03\xdd\x6e\x48\x15\x3d\xc9\xb6\x1f\xe5\xd9\x1d\x74\xae\x35\x48";
let cipher = AesSivCmac512::new(
[
244, 6, 63, 13, 47, 226, 180, 25, 104, 212, 47, 14, 186, 70, 187, 93, 134, 140, 2,
82, 238, 254, 113, 79, 90, 31, 135, 138, 123, 210, 121, 47, 228, 208, 243, 76, 126,
213, 196, 233, 65, 15, 33, 163, 196, 30, 6, 197, 222, 105, 40, 14, 73, 138, 200,
45, 235, 127, 48, 248, 171, 8, 141, 180,
]
.into(),
);

assert!(NtpPacket::deserialize(packet, &cipher).is_ok());
}

#[test]
fn test_nts_captured_client() {
let packet = b"\x23\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x62\x87\xdb\x47\x3f\xf7\x5f\x58\x01\x04\x00\x24\xe4\x83\x3a\x8d\x60\x0e\x13\x42\x43\x5c\xb2\x9d\xe5\x50\xac\xc0\xf8\xd8\xfa\x16\xe5\xc5\x37\x0a\x62\x0b\x15\x5f\x58\x6a\xda\xd6\x02\x04\x00\xac\x1c\xc4\x0a\x94\xda\x3f\x94\xa4\xd1\x2a\xc2\xd6\x09\xf1\x6f\x72\x11\x59\x6a\x0a\xce\xfc\x62\xd1\x1f\x28\x3a\xd1\x08\xd8\x01\xb5\x91\x38\x5d\x9b\xf5\x07\xf9\x0d\x21\x82\xe6\x81\x2a\x58\xa7\x35\xdc\x49\xc4\xd3\xe9\xb7\x9c\x72\xb7\xf6\x44\x64\xf8\xfc\x0d\xed\x25\xea\x1f\x7c\x9b\x31\x5c\xd8\x60\x86\xfd\x67\x74\x90\xf5\x0e\x61\xe6\x68\x0e\x29\x0d\x49\x77\x0c\xed\x44\xd4\x2f\x2d\x9b\xa8\x9f\x4d\x5d\xce\x4f\xdd\x57\x49\x51\x49\x5a\x1f\x38\xdb\xc7\xec\x1b\x86\x5b\xa5\x8f\x23\x1e\xdd\x76\xee\x1d\xaf\xdd\x66\xb2\xb2\x64\x1f\x03\xc6\x47\x9b\x42\x9c\x7f\xf6\x59\x6b\x82\x44\xcf\x67\xb5\xa2\xcd\x20\x9d\x39\xbb\xe6\x40\x2b\xf6\x20\x45\xdf\x95\x50\xf0\x38\x77\x06\x89\x79\x12\x18\x04\x04\x00\x28\x00\x10\x00\x10\xce\x89\xee\x97\x34\x42\xbc\x0f\x43\xaa\xce\x49\x99\xbd\xf5\x8e\x8f\xee\x7b\x1a\x2d\x58\xaf\x6d\xe9\xa2\x0e\x56\x1f\x7f\xf0\x6a";
let cipher = AesSivCmac512::new(
[
170, 111, 161, 118, 7, 200, 232, 128, 145, 250, 170, 186, 87, 143, 171, 252, 110,
241, 170, 179, 13, 150, 134, 147, 211, 248, 62, 207, 122, 155, 198, 109, 167, 15,
18, 118, 146, 63, 186, 146, 212, 188, 175, 27, 89, 3, 237, 212, 52, 113, 28, 21,
203, 200, 230, 17, 8, 186, 126, 1, 52, 230, 86, 40,
]
.into(),
);

assert!(NtpPacket::deserialize(packet, &cipher).is_ok());
}

#[test]
fn test_nts_poll_message() {
let cookie = [0; 16];
Expand Down

0 comments on commit b1451a8

Please sign in to comment.