argon2: fix big endian support (#482)

This was broken in #247, which added unsafe pointer casts to access the contents of a block, which only works on little endian targets. This reverts back to something closer to the previous code, which used `u64::{from_le_bytes, to_le_bytes}` instead. It also adds cross tests for PPC32 to spot future regressions.
RustCrypto · Jan 20, 2024 · 28dfc27 · 28dfc27
1 parent 63df8fa
commit 28dfc27
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 20 deletions.
diff --git a/.github/workflows/argon2.yml b/.github/workflows/argon2.yml
@@ -48,16 +48,48 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        rust:
-          - 1.65.0 # MSRV
-          - stable
+        include:
+          # 32-bit Linux
+          - target: i686-unknown-linux-gnu
+            rust: 1.65.0 # MSRV
+            deps: sudo apt update && sudo apt install gcc-multilib
+          - target: i686-unknown-linux-gnu
+            rust: stable
+            deps: sudo apt update && sudo apt install gcc-multilib
+
+          # 64-bit Linux
+          - target: x86_64-unknown-linux-gnu
+            rust: 1.65.0 # MSRV
+          - target: x86_64-unknown-linux-gnu
+            rust: stable
     steps:
       - uses: actions/checkout@v4
       - uses: RustCrypto/actions/cargo-cache@master
       - uses: dtolnay/rust-toolchain@master
         with:
           toolchain: ${{ matrix.rust }}
+          targets: ${{ matrix.target }}
+      - run: ${{ matrix.deps }}
       - run: cargo test --no-default-features
       - run: cargo test --no-default-features --features password-hash
       - run: cargo test
       - run: cargo test --all-features
+
+  cross:
+    strategy:
+      matrix:
+        include:
+          - target: powerpc-unknown-linux-gnu
+            rust: 1.65.0 # MSRV
+          - target: powerpc-unknown-linux-gnu
+            rust: stable
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - run: ${{ matrix.deps }}
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ matrix.rust }}
+          targets: ${{ matrix.target }}
+      - uses: RustCrypto/actions/cross-install@master
+      - run: cross test --release --target ${{ matrix.target }} --all-features
diff --git a/argon2/src/block.rs b/argon2/src/block.rs
@@ -4,6 +4,7 @@ use core::{
     convert::{AsMut, AsRef},
     num::Wrapping,
     ops::{BitXor, BitXorAssign},
+    slice,
 };
 
 #[cfg(feature = "zeroize")]
@@ -58,12 +59,18 @@ impl Block {
         Self([0u64; Self::SIZE / 8])
     }
 
-    pub(crate) fn as_bytes(&self) -> &[u8; Self::SIZE] {
-        unsafe { &*(self.0.as_ptr() as *const [u8; Self::SIZE]) }
+    /// Load a block from a block-sized byte slice
+    #[inline(always)]
+    pub(crate) fn load(&mut self, input: &[u8; Block::SIZE]) {
+        for (i, chunk) in input.chunks(8).enumerate() {
+            self.0[i] = u64::from_le_bytes(chunk.try_into().expect("should be 8 bytes"));
+        }
     }
 
-    pub(crate) fn as_mut_bytes(&mut self) -> &mut [u8; Self::SIZE] {
-        unsafe { &mut *(self.0.as_mut_ptr() as *mut [u8; Self::SIZE]) }
+    /// Iterate over the `u64` values contained in this block
+    #[inline(always)]
+    pub(crate) fn iter(&self) -> slice::Iter<'_, u64> {
+        self.0.iter()
     }
 
     /// NOTE: do not call this directly. It should only be called via

diff --git a/argon2/src/lib.rs b/argon2/src/lib.rs
@@ -305,13 +305,17 @@ impl<'key> Argon2<'key> {
                 let i = i as u32;
                 let l = l as u32;
 
+                // Make the first and second block in each lane as G(H0||0||i) or
+                // G(H0||1||i)
                 let inputs = &[
                     initial_hash.as_ref(),
                     &i.to_le_bytes()[..],
                     &l.to_le_bytes()[..],
                 ];
 
-                blake2b_long(inputs, block.as_mut_bytes())?;
+                let mut hash = [0u8; Block::SIZE];
+                blake2b_long(inputs, &mut hash)?;
+                block.load(&hash);
             }
         }
 
@@ -485,10 +489,20 @@ impl<'key> Argon2<'key> {
             blockhash ^= &memory_blocks[last_block_in_lane];
         }
 
-        blake2b_long(&[blockhash.as_bytes()], out)?;
+        // Hash the result
+        let mut blockhash_bytes = [0u8; Block::SIZE];
+
+        for (chunk, v) in blockhash_bytes.chunks_mut(8).zip(blockhash.iter()) {
+            chunk.copy_from_slice(&v.to_le_bytes())
+        }
+
+        blake2b_long(&[&blockhash_bytes], out)?;
 
         #[cfg(feature = "zeroize")]
-        blockhash.zeroize();
+        {
+            blockhash.zeroize();
+            blockhash_bytes.zeroize();
+        }
 
         Ok(())
     }

diff --git a/argon2/tests/kat.rs b/argon2/tests/kat.rs
@@ -307,11 +307,14 @@ fn salt_bad_length() {
     let ret = ctx.hash_password_into(b"password", &too_short_salt, &mut out);
     assert_eq!(ret, Err(Error::SaltTooShort));
 
-    // 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
-    // optimizations kicks in an nothing is really allocated
-    let too_long_salt = vec![0u8; argon2::MAX_SALT_LEN + 1];
-    let ret = ctx.hash_password_into(b"password", &too_long_salt, &mut out);
-    assert_eq!(ret, Err(Error::SaltTooLong));
+    #[cfg(target_pointer_width = "64")] // MAX_SALT_LEN + 1 is too big for 32-bit targets
+    {
+        // 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
+        // optimizations kicks in an nothing is really allocated
+        let too_long_salt = vec![0u8; argon2::MAX_SALT_LEN + 1];
+        let ret = ctx.hash_password_into(b"password", &too_long_salt, &mut out);
+        assert_eq!(ret, Err(Error::SaltTooLong));
+    }
 }
 
 #[test]
@@ -321,11 +324,14 @@ fn output_bad_length() {
     let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
     assert_eq!(ret, Err(Error::OutputTooShort));
 
-    // 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
-    // optimizations kicks in an nothing is really allocated
-    let mut out = vec![0u8; Params::MAX_OUTPUT_LEN + 1];
-    let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
-    assert_eq!(ret, Err(Error::OutputTooLong));
+    #[cfg(target_pointer_width = "64")] // MAX_SALT_LEN + 1 is too big for 32-bit targets
+    {
+        // 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
+        // optimizations kicks in an nothing is really allocated
+        let mut out = vec![0u8; Params::MAX_OUTPUT_LEN + 1];
+        let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
+        assert_eq!(ret, Err(Error::OutputTooLong));
+    }
 }
 
 // =======================================