fix: correct checksum for misaligned slices

.github/workflows/rust.yml

@@ -100,6 +100,8 @@ jobs:
       - run: cargo install cargo-tarpaulin@0.27.1
       - name: Install SCION and run local topology
         uses: "./.github/actions/setup-scion"
+        with:
+          scion-ref: v0.11.0
         id: scion
 
       - name: Run tests (including integration tests) and record coverage

crates/scion-proto/src/packet/checksum.rs

@@ -118,35 +118,48 @@ impl ChecksumDigest {
     ///
     /// If the slice is not a multiple of 2-bytes, then it is zero-padded
     /// before being added to the checksum.
-    pub fn add_slice(&mut self, data: &[u8]) -> &mut Self {
+    pub fn add_slice(&mut self, mut data: &[u8]) -> &mut Self {
         if data.is_empty() {
             return self;
         }
+        let mut initial_sum = 0;
+
+        // Before converting to a `&[u16]` we need to make sure the slice is aligned.
+        let is_aligned = data.as_ptr().align_offset(2) == 0;
+        if !is_aligned {
+            // We want to zero-prepend the value, i.e., for slice where we pair the elements, we
+            // have [0, A], [B, C], ... Storing [0, X] on a little endian architecture gets written
+            // as [X, 0] to memory, so we need to swap it with `to_be()`.
+            initial_sum = (data[0] as u16).to_be() as u32;
+            data = &data[1..];
+        };
+        let ptr: *const u8 = data.as_ptr();
 
-        // Converting to a &[u16] requires an even number of elements in the slice
-        let (data, initial_sum) = if data.len() % 2 == 0 {
-            (data, 0u32)
-        } else {
-            (
-                &data[..data.len() - 1],
-                // We want to zero pad the value, i.e., for slice where we pair the elements,
-                // we have [A, B], [C, D], ... [X, 0]. Since all the values are currently in
-                // memory in the order [A, B] storing [0, X] on a little endian architecture
-                // gets written as [X, 0] to memory. On big-endian this would get written as
-                // [0, X] so we swap it only on that big-endian architectures with to_le()
-                (data[data.len() - 1] as u16).to_le() as u32,
-            )
+        // Converting to a `&[u16]` requires an even number of elements in the slice.
+        if data.len() % 2 != 0 {
+            // We want to zero pad the value, i.e., for slice where we pair the elements,
+            // we have [A, B], [C, D], ... [X, 0]. Since all the values are currently in
+            // memory in the order [A, B] storing [0, X] on a little endian architecture
+            // gets written as [X, 0] to memory. On big-endian this would get written as
+            // [0, X] so we swap it only on that big-endian architectures with to_le().
+            initial_sum += (data[data.len() - 1] as u16).to_le() as u32;
+            data = &data[..data.len() - 1];
         };
 
-        let ptr: *const u8 = data.as_ptr();
         let data_u16 = unsafe { slice::from_raw_parts(ptr as *const u16, data.len() / 2) };
 
         let sum_with_overflow = data_u16
             .iter()
             .fold(initial_sum, |sum, value| sum + (*value as u32));
 
         // Already incorporate the overflow, as it simplifies the endian conversion below
-        let sum = Self::fold_checksum(sum_with_overflow) as u16;
+        let mut sum = Self::fold_checksum(sum_with_overflow) as u16;
+
+        // If the original slice was not aligned, we need to swap the bytes to get the correct
+        // checksum.
+        if !is_aligned {
+            sum = sum.swap_bytes();
+        }
 
         // The above sum is actually in big-endian but stored in big/little endian depending
         // on the platform. If the platform is little endian, this call will swap the byte-order
@@ -252,6 +265,24 @@ mod tests {
         assert_eq!(checksum, !0xddf2);
     }
 
+    #[test]
+    fn rfc1071_example_slice_unaligned() {
+        // Construct a slice that is not 2B aligned.
+        let mut data = b"\0\0\x01\xf2\x03\xf4\xf5\xf6\xf7".to_vec();
+        let slice = if data.as_ptr().align_offset(2) == 0 {
+            &data[1..]
+        } else {
+            data.rotate_left(1);
+            &data[..data.len() - 1]
+        };
+
+        assert_eq!(slice.as_ptr().align_offset(2), 1);
+        assert_eq!(
+            ChecksumDigest::default().add_slice(slice).checksum(),
+            !0xddf2
+        );
+    }
+
     macro_rules! test_checksum {
         (
             name: $name:ident,

crates/scion-proto/src/path/standard.rs

@@ -126,9 +126,7 @@ where
     ///
     /// There are always at most 3 segments.
     pub fn segment(&self, segment_index: usize) -> Option<Segment> {
-        let Some(info_field) = self.info_field(segment_index) else {
-            return None;
-        };
+        let info_field = self.info_field(segment_index)?;
 
         // Get the index of the first hop field in the segment.
         // This is equivalent to the index after all preceding hop fields.

crates/scion/src/pan/path_strategy/refresher.rs

@@ -341,9 +341,7 @@ impl<'a> Iterator for PathsTo<'a> {
     type Item = &'a Path;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let Some(paths) = self.paths else {
-            return None;
-        };
+        let paths = self.paths?;
 
         #[allow(clippy::while_let_on_iterator)]
         while let Some(info) = self.inner.next() {