Repository: cloudflare/boringtun (language: rust)

Issue:
Use make_array instead manually defining arrays where appropriate
https://github.com/cloudflare/boringtun/blob/3c9c01c7361f321e54c687bb8d099a0398d6692a/src/noise/mod.rs#L271

https://crates.io/crates/byteorder


Discussion / hints:
(I'm not a member of cloudfare) I don't see any strong reason to pull the byteorder crate in. Rust natively supports these functions since 1.32.0 and if you do not target lower versions of the compiler this is completly fine IMHO.
Sure but there is `u32::from_le_bytes(make_array(&self.buf[0..])),` in this crate
I try to keep dependencies that use unsafe rust to a minimum. Also using `from_le_bytes + make_array` results in a single mov instruction emitted.
Then use from_le_bytes + make_array. No need to close this issue until you have synchronized style for parsing raw bytes. You can rename my issue.
That is a valid point 

Fix patch:
diff --git a/src/crypto/chacha20poly1305/mod.rs b/src/crypto/chacha20poly1305/mod.rs
index ee6a7f34..88c93993 100644
--- a/src/crypto/chacha20poly1305/mod.rs
+++ b/src/crypto/chacha20poly1305/mod.rs
@@ -585,22 +585,22 @@ fn chacha20_block(state: [u32; 16], hchacha: bool) -> [u32; 16] {
 #[inline(always)]
 fn transmute_u8_u32(blk_in: [u8; 64]) -> [u32; 16] {
     [
-        u32::from_le_bytes([blk_in[0], blk_in[1], blk_in[2], blk_in[3]]),
-        u32::from_le_bytes([blk_in[4], blk_in[5], blk_in[6], blk_in[7]]),
-        u32::from_le_bytes([blk_in[8], blk_in[9], blk_in[10], blk_in[11]]),
-        u32::from_le_bytes([blk_in[12], blk_in[13], blk_in[14], blk_in[15]]),
-        u32::from_le_bytes([blk_in[16], blk_in[17], blk_in[18], blk_in[19]]),
-        u32::from_le_bytes([blk_in[20], blk_in[21], blk_in[22], blk_in[23]]),
-        u32::from_le_bytes([blk_in[24], blk_in[25], blk_in[26], blk_in[27]]),
-        u32::from_le_bytes([blk_in[28], blk_in[29], blk_in[30], blk_in[31]]),
-        u32::from_le_bytes([blk_in[32], blk_in[33], blk_in[34], blk_in[35]]),
-        u32::from_le_bytes([blk_in[36], blk_in[37], blk_in[38], blk_in[39]]),
-        u32::from_le_bytes([blk_in[40], blk_in[41], blk_in[42], blk_in[43]]),
-        u32::from_le_bytes([blk_in[44], blk_in[45], blk_in[46], blk_in[47]]),
-        u32::from_le_bytes([blk_in[48], blk_in[49], blk_in[50], blk_in[51]]),
-        u32::from_le_bytes([blk_in[52], blk_in[53], blk_in[54], blk_in[55]]),
-        u32::from_le_bytes([blk_in[56], blk_in[57], blk_in[58], blk_in[59]]),
-        u32::from_le_bytes([blk_in[60], blk_in[61], blk_in[62], blk_in[63]]),
+        u32::from_le_bytes(make_array(&blk_in[0..])),
+        u32::from_le_bytes(make_array(&blk_in[4..])),
+        u32::from_le_bytes(make_array(&blk_in[8..])),
+        u32::from_le_bytes(make_array(&blk_in[12..])),
+        u32::from_le_bytes(make_array(&blk_in[16..])),
+        u32::from_le_bytes(make_array(&blk_in[20..])),
+        u32::from_le_bytes(make_array(&blk_in[24..])),
+        u32::from_le_bytes(make_array(&blk_in[28..])),
+        u32::from_le_bytes(make_array(&blk_in[32..])),
+        u32::from_le_bytes(make_array(&blk_in[36..])),
+        u32::from_le_bytes(make_array(&blk_in[40..])),
+        u32::from_le_bytes(make_array(&blk_in[44..])),
+        u32::from_le_bytes(make_array(&blk_in[48..])),
+        u32::from_le_bytes(make_array(&blk_in[52..])),
+        u32::from_le_bytes(make_array(&blk_in[56..])),
+        u32::from_le_bytes(make_array(&blk_in[60..])),
     ]
 }
 
diff --git a/src/noise/mod.rs b/src/noise/mod.rs
index e57a4ab1..4ccb579d 100644
--- a/src/noise/mod.rs
+++ b/src/noise/mod.rs
@@ -20,12 +20,16 @@ use std::sync::Arc;
 const IPV4_MIN_HEADER_SIZE: usize = 20;
 const IPV4_LEN_OFF: usize = 2;
 const IPV4_SRC_IP_OFF: usize = 12;
-pub const IPV4_DST_IP_OFF: usize = 16;
+const IPV4_DST_IP_OFF: usize = 16;
+const IPV4_IP_SZ: usize = 4;
 
 const IPV6_MIN_HEADER_SIZE: usize = 40;
 const IPV6_LEN_OFF: usize = 4;
 const IPV6_SRC_IP_OFF: usize = 8;
-pub const IPV6_DST_IP_OFF: usize = 24;
+const IPV6_DST_IP_OFF: usize = 24;
+const IPV6_IP_SZ: usize = 16;
+
+const IP_LEN_SZ: usize = 2;
 
 const MAX_QUEUE_DEPTH: usize = 256;
 const N_SESSIONS: usize = 4; // number of sessions in the ring, better keep a PoT
@@ -268,12 +272,7 @@ impl Tunn {
             return TunnResult::Err(WireGuardError::InvalidPacket);
         }
         // Extract the reciever index
-        let idx = u32::from_le_bytes([
-            src[session::IDX_OFF],
-            src[session::IDX_OFF + 1],
-            src[session::IDX_OFF + 2],
-            src[session::IDX_OFF + 3],
-        ]) as usize;
+        let idx = u32::from_le_bytes(make_array(&src[session::IDX_OFF..])) as usize;
 
         // Get the (possibly) right session
         if let Some(ref session) = *self.sessions[idx % N_SESSIONS].read() {
@@ -294,74 +293,46 @@ impl Tunn {
         }
     }
 
+    /// Extract the destination address of a valid IP packet
     pub fn dst_address(packet: &[u8]) -> Option<IpAddr> {
         if packet.is_empty() {
             return None;
         }
 
         match packet[0] >> 4 {
-            4 if packet.len() >= IPV4_MIN_HEADER_SIZE => Some(IpAddr::from([
-                packet[IPV4_DST_IP_OFF],
-                packet[IPV4_DST_IP_OFF + 1],
-                packet[IPV4_DST_IP_OFF + 2],
-                packet[IPV4_DST_IP_OFF + 3],
-            ])),
-            6 if packet.len() >= IPV6_MIN_HEADER_SIZE => Some(IpAddr::from([
-                packet[IPV6_DST_IP_OFF],
-                packet[IPV6_DST_IP_OFF + 1],
-                packet[IPV6_DST_IP_OFF + 2],
-                packet[IPV6_DST_IP_OFF + 3],
-                packet[IPV6_DST_IP_OFF + 4],
-                packet[IPV6_DST_IP_OFF + 5],
-                packet[IPV6_DST_IP_OFF + 6],
-                packet[IPV6_DST_IP_OFF + 7],
-                packet[IPV6_DST_IP_OFF + 8],
-                packet[IPV6_DST_IP_OFF + 9],
-                packet[IPV6_DST_IP_OFF + 10],
-                packet[IPV6_DST_IP_OFF + 11],
-                packet[IPV6_DST_IP_OFF + 12],
-                packet[IPV6_DST_IP_OFF + 13],
-                packet[IPV6_DST_IP_OFF + 14],
-                packet[IPV6_DST_IP_OFF + 15],
-            ])),
+            4 if packet.len() >= IPV4_MIN_HEADER_SIZE => {
+                let addr_bytes: [u8; IPV4_IP_SZ] = make_array(&packet[IPV4_DST_IP_OFF..]);
+                Some(IpAddr::from(addr_bytes))
+            }
+            6 if packet.len() >= IPV6_MIN_HEADER_SIZE => {
+                let addr_bytes: [u8; IPV6_IP_SZ] = make_array(&packet[IPV6_DST_IP_OFF..]);
+                Some(IpAddr::from(addr_bytes))
+            }
             _ => None,
         }
     }
 
+    /// Check IP packet is v4 or v6, truncate to length indicated by the length field
+    /// Returns the truncated packet and the source IP as TunnResult
     fn check_decapsulated_packet<'a>(&self, packet: &'a mut [u8]) -> TunnResult<'a> {
         let (packet_len, src_ip_address) = match packet.len() {
             0 => return TunnResult::Done, // This is keepalive, and not an error
-            _ if packet[0] >> 4 == 4 && packet.len() >= IPV4_MIN_HEADER_SIZE => (
-                u16::from_be_bytes([packet[IPV4_LEN_OFF], packet[IPV4_LEN_OFF + 1]]) as usize,
-                IpAddr::from([
-                    packet[IPV4_SRC_IP_OFF],
-                    packet[IPV4_SRC_IP_OFF + 1],
-                    packet[IPV4_SRC_IP_OFF + 2],
-                    packet[IPV4_SRC_IP_OFF + 3],
-                ]),
-            ),
-            _ if packet[0] >> 4 == 6 && packet.len() >= IPV6_MIN_HEADER_SIZE => (
-                u16::from_be_bytes([packet[IPV6_LEN_OFF], packet[IPV6_LEN_OFF + 1]]) as usize
-                    + IPV6_MIN_HEADER_SIZE,
-                IpAddr::from([
-                    packet[IPV6_SRC_IP_OFF],
-                    packet[IPV6_SRC_IP_OFF + 1],
-                    packet[IPV6_SRC_IP_OFF + 2],
-                    packet[IPV6_SRC_IP_OFF + 3],
-                    packet[IPV6_SRC_IP_OFF + 4],
-                    packet[IPV6_SRC_IP_OFF + 5],
-                    packet[IPV6_SRC_IP_OFF + 6],
-                    packet[IPV6_SRC_IP_OFF + 7],
-                    packet[IPV6_SRC_IP_OFF + 8],
-                    packet[IPV6_SRC_IP_OFF + 9],
-                    packet[IPV6_SRC_IP_OFF + 10],
-                    packet[IPV6_SRC_IP_OFF + 11],
-                    packet[IPV6_SRC_IP_OFF + 12],
-                    packet[IPV6_SRC_IP_OFF + 13],
-                    packet[IPV6_SRC_IP_OFF + 14],
-                    packet[IPV6_SRC_IP_OFF + 15],
-                ]),
-            ),
+            _ if packet[0] >> 4 == 4 && packet.len() >= IPV4_MIN_HEADER_SIZE => {
+                let len_bytes: [u8; IP_LEN_SZ] = make_array(&packet[IPV4_LEN_OFF..]);
+                let addr_bytes: [u8; IPV4_IP_SZ] = make_array(&packet[IPV4_SRC_IP_OFF..]);
+                (
+                    u16::from_be_bytes(len_bytes) as usize,
+                    IpAddr::from(addr_bytes),
+                )
+            }
+            _ if packet[0] >> 4 == 6 && packet.len() >= IPV6_MIN_HEADER_SIZE => {
+                let len_bytes: [u8; IP_LEN_SZ] = make_array(&packet[IPV6_LEN_OFF..]);
+                let addr_bytes: [u8; IPV6_IP_SZ] = make_array(&packet[IPV6_SRC_IP_OFF..]);
+                (
+                    u16::from_be_bytes(len_bytes) as usize + IPV6_MIN_HEADER_SIZE,
+                    IpAddr::from(addr_bytes),
+                )
+            }
             _ => return TunnResult::Err(WireGuardError::InvalidPacket),
         };
 


Test patch: