fix(stun-types): remove MessageId & remove padding "fix"

crates/stun-types/src/attributes/addr.rs

@@ -91,12 +91,12 @@ impl Attribute<'_> for XorMappedAddress {
     const TYPE: u16 = 0x0020;
 
     fn decode(_: Self::Context, msg: &mut Message, attr: AttrSpan) -> Result<Self, Error> {
-        let xor128 = msg.id().0;
+        let xor128 = msg.id();
         decode_addr(attr.get_value(msg.buffer()), XOR16, COOKIE, xor128).map(Self)
     }
 
     fn encode(&self, _: Self::Context, builder: &mut MessageBuilder) -> Result<(), Error> {
-        let xor128 = builder.id().0;
+        let xor128 = builder.id();
         encode_addr(self.0, builder.buffer(), XOR16, COOKIE, xor128);
         Ok(())
     }

crates/stun-types/src/attributes/error_code.rs

@@ -80,7 +80,7 @@ mod test {
     #[test]
     fn error_code() {
         let mut builder =
-            MessageBuilder::new(Class::Error, Method::Binding, TransactionId::new(128));
+            MessageBuilder::new(Class::Error, Method::Binding, TransactionId::new([0; 12]));
         builder
             .add_attr(&ErrorCode {
                 number: 400,

crates/stun-types/src/attributes/integrity.rs

@@ -172,7 +172,7 @@ mod test {
         let password = "abc123";
 
         let mut message =
-            MessageBuilder::new(Class::Request, Method::Binding, TransactionId::new(123));
+            MessageBuilder::new(Class::Request, Method::Binding, TransactionId::new([0; 12]));
 
         message.add_attr(&Software::new("ezk-stun")).unwrap();
         message
@@ -196,7 +196,7 @@ mod test {
         let password = "abc123";
 
         let mut message =
-            MessageBuilder::new(Class::Request, Method::Binding, TransactionId::new(123));
+            MessageBuilder::new(Class::Request, Method::Binding, TransactionId::new([0; 12]));
 
         message.add_attr(&Software::new("ezk-stun")).unwrap();
         message

crates/stun-types/src/builder.rs

@@ -1,12 +1,12 @@
 use crate::attributes::Attribute;
-use crate::header::{Class, MessageHead, MessageId, Method};
-use crate::{padding_u16, padding_usize, Error, TransactionId};
+use crate::header::{Class, MessageHead, Method, STUN_HEADER_LENGTH};
+use crate::{padding_u16, padding_usize, Error, TransactionId, COOKIE};
 use bytes::BufMut;
 
 /// Builder for a STUN message
 pub struct MessageBuilder {
     head: MessageHead,
-    id: MessageId,
+    transaction_id: TransactionId,
 
     padding_in_value_len: bool,
 
@@ -26,14 +26,13 @@ impl MessageBuilder {
         head.set_typ(typ);
         buffer.put_u32(head.0);
 
-        let mut id = MessageId::new();
-        id.set_transaction_id(transaction_id.0);
-        buffer.put_u128(id.0);
+        buffer.put_u32(COOKIE);
+        buffer.put_slice(&transaction_id.0);
 
         Self {
             head,
+            transaction_id,
             padding_in_value_len: true,
-            id,
             buffer,
         }
     }
@@ -42,10 +41,6 @@ impl MessageBuilder {
         self.padding_in_value_len = b;
     }
 
-    pub fn id(&self) -> &MessageId {
-        &self.id
-    }
-
     /// Set the length of the message
     pub fn set_len(&mut self, len: u16) {
         self.head.set_len(len);
@@ -90,8 +85,20 @@ impl MessageBuilder {
         Ok(())
     }
 
+    pub fn id(&self) -> u128 {
+        let cookie = COOKIE.to_be_bytes();
+        let tsx = self.transaction_id.0;
+
+        let mut id = [0u8; 16];
+
+        id[..4].copy_from_slice(&cookie);
+        id[4..].copy_from_slice(&tsx);
+
+        u128::from_be_bytes(id)
+    }
+
     pub fn finish(mut self) -> Vec<u8> {
-        self.set_len((self.buffer.len() - 20).try_into().unwrap());
+        self.set_len((self.buffer.len() - STUN_HEADER_LENGTH).try_into().unwrap());
         self.buffer
     }
 

crates/stun-types/src/header.rs

@@ -1,4 +1,4 @@
-use crate::{Error, COOKIE};
+use crate::Error;
 use bitfield::bitfield;
 use std::convert::TryFrom;
 
@@ -18,26 +18,6 @@ bitfield! {
     pub len, set_len: 15, 0;
 }
 
-bitfield! {
-    /// Internal bitfield representing the cookie + transaction id
-    #[derive(Debug, Clone, Copy)]
-    pub struct MessageId(u128);
-
-    u32;
-    pub cookie, set_cookie: 127,  96;
-
-    u128;
-    pub transaction_id, set_transaction_id: 95, 0;
-}
-
-impl MessageId {
-    pub(crate) fn new() -> Self {
-        let mut new = Self(0);
-        new.set_cookie(COOKIE);
-        new
-    }
-}
-
 /// STUN class
 #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
 pub enum Class {

crates/stun-types/src/lib.rs

@@ -1,7 +1,6 @@
 #![warn(unreachable_pub)]
 
 use byteorder::ReadBytesExt;
-use rand::Rng;
 use std::io::{self, Cursor};
 use std::num::TryFromIntError;
 use std::str::Utf8Error;
@@ -12,7 +11,7 @@ mod header;
 mod parse;
 
 pub use builder::MessageBuilder;
-pub use header::{Class, MessageHead, MessageId, Method};
+pub use header::{Class, MessageHead, Method};
 pub use parse::{AttrSpan, Message};
 
 type NE = byteorder::NetworkEndian;
@@ -60,29 +59,17 @@ fn padding_usize(n: usize) -> usize {
 
 /// 96 bit STUN transaction id
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct TransactionId(u128);
+pub struct TransactionId([u8; 12]);
 
 impl TransactionId {
-    const MAX: u128 = (1 << 96) - 1;
-
-    /// Returns the inner numeric representation of the transaction id
-    pub fn as_u128(self) -> u128 {
-        self.0
-    }
-
     /// Create a new transaction id from the given value
-    ///
-    /// # Panics
-    ///
-    /// Panics if the given value is larger than 96 bits
-    pub fn new(v: u128) -> Self {
-        assert!(v <= Self::MAX);
+    pub fn new(v: [u8; 12]) -> Self {
         Self(v)
     }
 
     /// Generate a new random transaction id
     pub fn random() -> Self {
-        Self(rand::thread_rng().gen_range(0..=Self::MAX))
+        Self(rand::random())
     }
 }
 
@@ -123,10 +110,9 @@ pub fn is_stun_message(i: &[u8]) -> IsStunMessageInfo {
         return IsStunMessageInfo::No;
     }
 
-    let id = cursor.read_u128::<NE>().unwrap();
-    let id = MessageId(id);
+    let cookie = cursor.read_u32::<NE>().unwrap();
 
-    if id.cookie() != COOKIE {
+    if cookie != COOKIE {
         return IsStunMessageInfo::No;
     }
 

crates/stun-types/src/parse.rs

@@ -1,10 +1,10 @@
 use crate::attributes::{Attribute, Fingerprint, MessageIntegrity, MessageIntegritySha256};
-use crate::header::{Class, MessageHead, MessageId, Method};
+use crate::header::{Class, MessageHead, Method};
 use crate::{padding_usize, Error, TransactionId, COOKIE, NE};
 use byteorder::ReadBytesExt;
 use bytes::Buf;
 use std::convert::TryFrom;
-use std::io::Cursor;
+use std::io::{Cursor, Read};
 
 #[derive(Debug, Clone, Copy)]
 pub struct AttrSpan {
@@ -31,7 +31,7 @@ pub struct Message {
     buffer: Vec<u8>,
 
     head: MessageHead,
-    id: MessageId,
+    id: u128,
 
     class: Class,
     method: Method,
@@ -53,6 +53,10 @@ impl Message {
         self.transaction_id
     }
 
+    pub(crate) fn id(&self) -> u128 {
+        self.id
+    }
+
     pub fn parse(buffer: impl Into<Vec<u8>>) -> Result<Message, Error> {
         let mut cursor = Cursor::new(buffer.into());
 
@@ -64,9 +68,16 @@ impl Message {
         }
 
         let id = cursor.read_u128::<NE>()?;
-        let id = MessageId(id);
 
-        if id.cookie() != COOKIE {
+        let (cookie, transaction_id) = {
+            let mut cursor = Cursor::new(id.to_be_bytes());
+            let cookie = cursor.read_u32::<NE>()?;
+            let mut transaction_id = [0u8; 12];
+            cursor.read_exact(&mut transaction_id)?;
+            (cookie, transaction_id)
+        };
+
+        if cookie != COOKIE {
             return Err(Error::InvalidData("not a stun message"));
         }
 
@@ -81,7 +92,7 @@ impl Message {
             let padding = padding_usize(attr_len);
 
             let value_begin = usize::try_from(cursor.position())?;
-            let mut value_end = value_begin + attr_len;
+            let value_end = value_begin + attr_len;
             let padding_end = value_end + padding;
 
             if padding_end > cursor.get_ref().len() {
@@ -90,20 +101,6 @@ impl Message {
                 ));
             }
 
-            // https://datatracker.ietf.org/doc/html/rfc8489#section-14
-            // explicitly states that the length field must contain the
-            // value length __prior__ to padding. Some stun agents have
-            // the padding included in the length anyway. This double
-            // checks and removes all bytes from the end of the value.
-            if padding == 0 {
-                let value = &cursor.get_ref()[value_begin..value_end];
-
-                // count all zero bytes at the end of the value
-                let counted_padding = value.iter().rev().take_while(|&&b| b == 0).count();
-
-                value_end -= counted_padding;
-            }
-
             let attr = AttrSpan {
                 begin: value_begin,
                 end: value_end,
@@ -122,7 +119,7 @@ impl Message {
             id,
             class,
             method,
-            transaction_id: TransactionId(id.transaction_id()),
+            transaction_id: TransactionId(transaction_id),
             attributes,
         })
     }
@@ -201,9 +198,4 @@ impl Message {
     pub fn head(&self) -> &MessageHead {
         &self.head
     }
-
-    /// ID of the STUN message (cookie + transaction)
-    pub fn id(&self) -> &MessageId {
-        &self.id
-    }
 }