Fix Dnssec record serializing/deserializing

Signed-off-by: Luca Fulchir <luca.fulchir@runesauth.com>

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

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<Self, Self::Error> {
-        if let Ok(handshake_u8) = raw.parse::<u8>() {
-            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,

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
+        );
+    }
+}

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 {
+    fn len(&self) -> usize {
-        // UDP port + Priority + Weight + pubkey_len + handshake_len
+        let mut size = 4;
-        let mut size = 6;
         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::V4(_) => 4,
-            IpAddr::V6(_) => size + 16,
+            IpAddr::V6(_) => 16,
         }
     }
-    fn encode_into(&self, raw: &mut Vec<u8>) {
+    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
+        if raw.len() <= 3 + num_pubkey_idx + num_handshake_ids + ip_len {
-        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 {
             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::<usize>()
+            + self.addresses.iter().map(|a| a.len()).sum::<usize>()
             + self
                 .public_keys
                 .iter()
-                .map(|(_, key)| 3 + key.kind().pub_len())
+                .map(|(_, key)| 4 + key.kind().pub_len())
                 .sum::<usize>()
             + 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);
+            let written_next = written + KeyID::len();
-            raw.push(public_key.kind().pub_len() as u8);
+            raw[written..written_next].copy_from_slice(&key_id_bytes);
-            raw.push(public_key.kind() as u8);
+            written = written_next;
-            public_key.serialize_into(&mut raw);
+            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<Self, Error> {
-        // bare minimum for lengths, (1 address),  (1 key),  cipher negotiation
+        // bare minimum for lengths, (1 address),  (1 key),no cipher negotiation
-        const MIN_RAW_LENGTH: usize = 3 + (6 + 4) + (4 + 32) + 1 + 1 + 1;
+        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,21 +534,20 @@ 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(
+            let (public_key, bytes) =
-                &raw[bytes_parsed..(bytes_parsed + pubkey_length)],
+                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
-                    ::tracing::warn!("Unsupported public key type");
                         bytes_parsed = bytes_parsed + pubkey_length;
                         continue;
                     }
-                Err(_) => {
+                    Err(e) => {
                         return Err(Error::UnsupportedData(bytes_parsed));
                     }
                 };

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!(),
         }

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")]

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
+                                // consider only the key exchanges allowed
-                                    .cfg
+                                // 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
-                                    .iter()
+                                            .contains(k_x)
-                                    .find(|x| key_supported_k_x.contains(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,