From 55e10a60c696583edfa758a5519e1c0b4244bb60 Mon Sep 17 00:00:00 2001 From: Luca Fulchir Date: Fri, 9 Jun 2023 14:55:49 +0200 Subject: [PATCH] Fix Dnssec record serializing/deserializing Signed-off-by: Luca Fulchir --- flake.nix | 2 + src/connection/handshake/mod.rs | 32 +++----- src/dnssec/mod.rs | 42 +++++++++- src/dnssec/record.rs | 136 +++++++++++++++++++------------- src/enc/asym.rs | 16 ++-- src/enc/errors.rs | 4 +- src/inner/worker.rs | 29 +++++-- 7 files changed, 172 insertions(+), 89 deletions(-) diff --git a/flake.nix b/flake.nix index fd0713c..7a74671 100644 --- a/flake.nix +++ b/flake.nix @@ -43,6 +43,8 @@ rust-bin.stable."1.69.0".default rustfmt rust-analyzer + # fenrir deps + hwloc ]; shellHook = '' # use zsh or other custom shell diff --git a/src/connection/handshake/mod.rs b/src/connection/handshake/mod.rs index 427bff4..14ba8fd 100644 --- a/src/connection/handshake/mod.rs +++ b/src/connection/handshake/mod.rs @@ -42,36 +42,28 @@ pub enum Error { } /// List of possible handshakes -#[derive(::num_derive::FromPrimitive, Debug, Clone, Copy, PartialEq)] +#[derive( + ::num_derive::FromPrimitive, + Debug, + Clone, + Copy, + PartialEq, + ::strum_macros::EnumString, + ::strum_macros::IntoStaticStr, +)] #[repr(u8)] pub enum HandshakeID { /// 1-RTT Directory synchronized handshake. Fast, no forward secrecy + #[strum(serialize = "directory_synchronized")] DirectorySynchronized = 0, /// 2-RTT Stateful exchange. Little DDos protection + #[strum(serialize = "stateful")] Stateful, /// 3-RTT stateless exchange. Forward secrecy and ddos protection + #[strum(serialize = "stateless")] Stateless, } -impl TryFrom<&str> for HandshakeID { - type Error = ::std::io::Error; - // TODO: from actual names, not only numeric - fn try_from(raw: &str) -> Result { - if let Ok(handshake_u8) = raw.parse::() { - if handshake_u8 >= 1 { - if let Some(handshake) = HandshakeID::from_u8(handshake_u8 - 1) - { - return Ok(handshake); - } - } - } - return Err(::std::io::Error::new( - ::std::io::ErrorKind::InvalidData, - "Unknown handshake ID", - )); - } -} - pub(crate) struct HandshakeServer { pub id: KeyID, pub key: PrivKey, diff --git a/src/dnssec/mod.rs b/src/dnssec/mod.rs index 143863c..bb479ba 100644 --- a/src/dnssec/mod.rs +++ b/src/dnssec/mod.rs @@ -21,6 +21,9 @@ pub enum Error { /// Errors in establishing connection or connection drops #[error("nameserver connection: {0:?}")] NameserverConnection(String), + /// record is not valid base85 + #[error("invalid base85")] + InvalidBase85, } #[cfg(any( @@ -137,9 +140,46 @@ impl Dnssec { ::tracing::error!("Can't parse record: {}", e); Err(::std::io::Error::new( ::std::io::ErrorKind::InvalidData, - "Can't parse the record", + "Can't parse the record: ".to_owned() + &e.to_string(), )) } }; } } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_serialization() { + // The record was generated with: + // f-dnssec generate dnssec \ + // -a 1 2 42 directory_synchronized 127.0.0.1 31337 \ + // -p 42 x25519 x25519.pub \ + // -x x25519diffiehellman \ + // -c xchacha20poly1305 + const TXT_RECORD: &'static str = "v=Fenrir1 \ + 5fBgo5ovk=0Dk}g0V)6>0cKP8KO-Vna846zp@MaLF|nim_XH&nQvT-I|B9HfJpcd"; + + let record = match Dnssec::parse_txt_record(TXT_RECORD) { + Ok(record) => record, + Err(e) => { + assert!(false, "{}", e.to_string()); + return; + } + }; + let re_encoded = match record.encode() { + Ok(re_encoded) => re_encoded, + Err(e) => { + assert!(false, "{}", e.to_string()); + return; + } + }; + assert!( + TXT_RECORD[10..] == re_encoded, + "DNSSEC record decoding->encoding failed:\n{}\n{}", + TXT_RECORD, + re_encoded + ); + } +} diff --git a/src/dnssec/record.rs b/src/dnssec/record.rs index ab5065b..f5595e2 100644 --- a/src/dnssec/record.rs +++ b/src/dnssec/record.rs @@ -167,7 +167,7 @@ impl TryFrom<&str> for AddressWeight { /// * priority /// * weight within priority /// * list of supported handshakes IDs -/// * list of public keys IDs +/// * list of public keys. Indexes in the Record.public_keys #[derive(Debug, Clone)] pub struct Address { /// Ip address of server, v4 or v6 @@ -197,18 +197,17 @@ impl Address { None => None, } } - fn raw_len(&self) -> usize { - // UDP port + Priority + Weight + pubkey_len + handshake_len - let mut size = 6; + fn len(&self) -> usize { + let mut size = 4; let num_pubkey_idx = self.public_key_idx.len(); let idx_bytes = (num_pubkey_idx / 2) + (num_pubkey_idx % 2); size = size + idx_bytes + self.handshake_ids.len(); size + match self.ip { - IpAddr::V4(_) => size + 4, - IpAddr::V6(_) => size + 16, + IpAddr::V4(_) => 4, + IpAddr::V6(_) => 16, } } - fn encode_into(&self, raw: &mut Vec) { + fn serialize_into(&self, raw: &mut [u8]) { let mut bitfield: u8 = match self.ip { IpAddr::V4(_) => 0, IpAddr::V6(_) => 1, @@ -218,18 +217,19 @@ impl Address { bitfield <<= 3; bitfield |= self.weight as u8; - raw.push(bitfield); + raw[0] = bitfield; let len_combined: u8 = self.public_key_idx.len() as u8; let len_combined = len_combined << 4; let len_combined = len_combined | self.handshake_ids.len() as u8; - raw.push(len_combined); + raw[1] = len_combined; - raw.extend_from_slice( + raw[2..4].copy_from_slice( &(match self.port { Some(port) => port.get().to_le_bytes(), None => [0, 0], // oh noez, which zero goes first? }), ); + let mut written: usize = 4; // pair every idx, since the max is 16 for chunk in self.public_key_idx.chunks(2) { @@ -242,34 +242,51 @@ impl Address { }; let tmp = chunk[0].0 << 4; let tmp = tmp | second; - raw.push(tmp); + raw[written] = tmp; + written = written + 1; } for id in self.handshake_ids.iter() { - raw.push(*id as u8); + raw[written] = *id as u8; + written = written + 1; } + let next_written; match self.ip { IpAddr::V4(ip) => { + next_written = written + 4; let raw_ip = ip.octets(); - raw.extend_from_slice(&raw_ip); + raw[written..next_written].copy_from_slice(&raw_ip); } IpAddr::V6(ip) => { + next_written = written + 16; let raw_ip = ip.octets(); - raw.extend_from_slice(&raw_ip); + raw[written..next_written].copy_from_slice(&raw_ip); } }; + assert!( + next_written == raw.len(), + "write how much? {} - {}", + next_written, + raw.len() + ); } fn decode_raw(raw: &[u8]) -> Result<(Self, usize), Error> { - if raw.len() < 10 { + if raw.len() < 9 { return Err(Error::NotEnoughData(0)); } + // 3-byte bitfield let ip_type = raw[0] >> 6; let is_ipv6: bool; + let ip_len: usize; match ip_type { 0 => { is_ipv6 = false; + ip_len = 4; + } + 1 => { + is_ipv6 = true; + ip_len = 16; } - 1 => is_ipv6 = true, _ => return Err(Error::UnsupportedData(0)), } let raw_priority = (raw[0] << 2) >> 5; @@ -277,18 +294,19 @@ impl Address { let priority = AddressPriority::from_u8(raw_priority).unwrap(); let weight = AddressWeight::from_u8(raw_weight).unwrap(); + // Add publickey ids + let num_pubkey_idx = (raw[1] >> 4) as usize; + let num_handshake_ids = (raw[1] & 0x0F) as usize; + // UDP port - let raw_port = u16::from_le_bytes([raw[1], raw[2]]); + let raw_port = u16::from_le_bytes([raw[2], raw[3]]); let port = if raw_port == 0 { None } else { Some(NonZeroU16::new(raw_port).unwrap()) }; - // Add publickey ids - let num_pubkey_idx = (raw[3] >> 4) as usize; - let num_handshake_ids = (raw[3] & 0x0F) as usize; - if raw.len() <= 3 + num_pubkey_idx + num_handshake_ids { + if raw.len() <= 3 + num_pubkey_idx + num_handshake_ids + ip_len { return Err(Error::NotEnoughData(3)); } let mut bytes_parsed = 4; @@ -306,9 +324,9 @@ impl Address { } idx_added = idx_added + 2; } + bytes_parsed = bytes_parsed + idx_bytes; // add handshake ids - bytes_parsed = bytes_parsed + idx_bytes; let mut handshake_ids = Vec::with_capacity(num_handshake_ids); for raw_handshake_id in raw[bytes_parsed..(bytes_parsed + num_handshake_ids)].iter() @@ -407,57 +425,69 @@ impl Record { // everything else is all good let total_size: usize = 3 - + self.addresses.iter().map(|a| a.raw_len()).sum::() + + self.addresses.iter().map(|a| a.len()).sum::() + self .public_keys .iter() - .map(|(_, key)| 3 + key.kind().pub_len()) + .map(|(_, key)| 4 + key.kind().pub_len()) .sum::() + self.key_exchanges.len() + self.hkdfs.len() + self.ciphers.len(); let mut raw = Vec::with_capacity(total_size); + raw.resize(total_size, 0); // amount of data. addresses, then pubkeys. 4 bits each let len_combined: u8 = self.addresses.len() as u8; let len_combined = len_combined << 4; let len_combined = len_combined | self.public_keys.len() as u8; - raw.push(len_combined); + raw[0] = len_combined; // number of key exchanges and hkdfs let len_combined: u8 = self.key_exchanges.len() as u8; let len_combined = len_combined << 4; let len_combined = len_combined | self.hkdfs.len() as u8; - raw.push(len_combined); + raw[1] = len_combined; let num_of_ciphers: u8 = (self.ciphers.len() as u8) << 4; - raw.push(num_of_ciphers); + raw[2] = num_of_ciphers; + let mut written: usize = 3; for address in self.addresses.iter() { - address.encode_into(&mut raw); + let len = address.len(); + let written_next = written + len; + address.serialize_into(&mut raw[written..written_next]); + written = written_next; } for (public_key_id, public_key) in self.public_keys.iter() { let key_id_bytes = public_key_id.0.to_le_bytes(); - raw.extend_from_slice(&key_id_bytes); - raw.push(public_key.kind().pub_len() as u8); - raw.push(public_key.kind() as u8); - public_key.serialize_into(&mut raw); + let written_next = written + KeyID::len(); + raw[written..written_next].copy_from_slice(&key_id_bytes); + written = written_next; + raw[written] = public_key.kind().pub_len() as u8; + written = written + 1; + let written_next = written + public_key.len(); + public_key.serialize_into(&mut raw[written..written_next]); + written = written_next; } for k_x in self.key_exchanges.iter() { - raw.push(*k_x as u8); + raw[written] = *k_x as u8; + written = written + 1; } for h in self.hkdfs.iter() { - raw.push(*h as u8); + raw[written] = *h as u8; + written = written + 1; } for c in self.ciphers.iter() { - raw.push(*c as u8); + raw[written] = *c as u8; + written = written + 1; } Ok(::base85::encode(&raw)) } /// Decode from base85 to the actual object pub fn decode(raw: &[u8]) -> Result { - // bare minimum for lengths, (1 address), (1 key), cipher negotiation - const MIN_RAW_LENGTH: usize = 3 + (6 + 4) + (4 + 32) + 1 + 1 + 1; + // bare minimum for lengths, (1 address), (1 key),no cipher negotiation + const MIN_RAW_LENGTH: usize = 3 + (6 + 4) + (4 + 32); if raw.len() <= MIN_RAW_LENGTH { return Err(Error::NotEnoughData(0)); } @@ -492,6 +522,7 @@ impl Record { result.addresses.push(address); num_addresses = num_addresses - 1; } + while num_public_keys > 0 { if bytes_parsed + 3 >= raw.len() { return Err(Error::NotEnoughData(bytes_parsed)); @@ -503,24 +534,23 @@ impl Record { bytes_parsed = bytes_parsed + 2; let pubkey_length = raw[bytes_parsed] as usize; bytes_parsed = bytes_parsed + 1; - if pubkey_length + bytes_parsed >= raw.len() { + let bytes_next_key = bytes_parsed + 1 + pubkey_length; + if bytes_next_key > raw.len() { return Err(Error::NotEnoughData(bytes_parsed)); } - let (public_key, bytes) = match PubKey::deserialize( - &raw[bytes_parsed..(bytes_parsed + pubkey_length)], - ) { - Ok((public_key, bytes)) => (public_key, bytes), - Err(enc::Error::UnsupportedKey(_)) => { - // continue parsing. This could be a new pubkey type - // that is not supported by an older client - ::tracing::warn!("Unsupported public key type"); - bytes_parsed = bytes_parsed + pubkey_length; - continue; - } - Err(_) => { - return Err(Error::UnsupportedData(bytes_parsed)); - } - }; + let (public_key, bytes) = + match PubKey::deserialize(&raw[bytes_parsed..bytes_next_key]) { + Ok((public_key, bytes)) => (public_key, bytes), + Err(enc::Error::UnsupportedKey(_)) => { + // continue parsing. This could be a new pubkey type + // that is not supported by an older client + bytes_parsed = bytes_parsed + pubkey_length; + continue; + } + Err(e) => { + return Err(Error::UnsupportedData(bytes_parsed)); + } + }; if bytes != 1 + pubkey_length { return Err(Error::UnsupportedData(bytes_parsed)); } diff --git a/src/enc/asym.rs b/src/enc/asym.rs index 4b6c09f..5d057f2 100644 --- a/src/enc/asym.rs +++ b/src/enc/asym.rs @@ -185,7 +185,7 @@ pub enum PubKey { impl PubKey { /// Get the serialized key length pub fn len(&self) -> usize { - match self { + 1 + match self { PubKey::Exchange(ex) => ex.len(), PubKey::Signing => todo!(), } @@ -231,7 +231,7 @@ impl PubKey { /// Try to deserialize the pubkey from raw bytes /// on success returns the public key and the number of parsed bytes pub fn deserialize(raw: &[u8]) -> Result<(Self, usize), Error> { - if raw.len() < 1 + MIN_KEY_SIZE { + if raw.len() < 1 { return Err(Error::NotEnoughData(0)); } let kind: KeyKind = match KeyKind::from_u8(raw[0]) { @@ -248,14 +248,18 @@ impl PubKey { } KeyKind::X25519 => { let pub_key: ::x25519_dalek::PublicKey = - match ::bincode::deserialize(&raw[1..(1 + kind.pub_len())]) + //match ::bincode::deserialize(&raw[1..(1 + kind.pub_len())]) + match ::bincode::deserialize(&raw[1..]) { Ok(pub_key) => pub_key, - Err(_) => return Err(Error::Parsing), + Err(e) => { + ::tracing::error!("x25519 deserialize: {}", e); + return Err(Error::Parsing); + } }; Ok(( PubKey::Exchange(ExchangePubKey::X25519(pub_key)), - kind.pub_len(), + 1 + kind.pub_len(), )) } } @@ -277,7 +281,7 @@ pub enum PrivKey { impl PrivKey { /// Get the serialized key length pub fn len(&self) -> usize { - match self { + 1 + match self { PrivKey::Exchange(ex) => ex.len(), PrivKey::Signing => todo!(), } diff --git a/src/enc/errors.rs b/src/enc/errors.rs index c0591b0..a394406 100644 --- a/src/enc/errors.rs +++ b/src/enc/errors.rs @@ -7,13 +7,13 @@ pub enum Error { #[error("can't parse key")] Parsing, /// Not enough data - #[error("not enough data")] + #[error("not enough data: {0}")] NotEnoughData(usize), /// buffer too small #[error("buffer too small")] InsufficientBuffer, /// Unsupported Key type found. - #[error("unsupported key type")] + #[error("unsupported key type: {0}")] UnsupportedKey(usize), /// Unsupported key exchange for this key #[error("unsupported key exchange")] diff --git a/src/inner/worker.rs b/src/inner/worker.rs index 0daec59..6d77c03 100644 --- a/src/inner/worker.rs +++ b/src/inner/worker.rs @@ -14,7 +14,7 @@ use crate::{ }, dnssec, enc::{ - asym::{KeyID, PrivKey, PubKey}, + asym::{self, KeyID, PrivKey, PubKey}, hkdf::{self, Hkdf, HkdfKind}, sym, Random, Secret, }, @@ -202,18 +202,33 @@ impl Worker { // that supports one of the key exchanges that // *we* support for idx in addr.public_key_idx.iter() { + // for each key, + // get all the possible key exchanges let key_supported_k_x = conn_info.resolved.public_keys [idx.0 as usize] .1 .kind() .key_exchanges(); - match self - .cfg - .key_exchanges - .iter() - .find(|x| key_supported_k_x.contains(x)) - { + // consider only the key exchanges allowed + // in the dnssec record + let filtered_key_exchanges: Vec< + asym::KeyExchangeKind, + > = key_supported_k_x + .into_iter() + .filter(|k_x| { + conn_info + .resolved + .key_exchanges + .contains(k_x) + }) + .copied() + .collect(); + + // finally make sure that we support those + match self.cfg.key_exchanges.iter().find(|x| { + filtered_key_exchanges.contains(x) + }) { Some(exchange) => { return Some(( addr,