Repository: RustCrypto/password-hashes (language: rust)

Issue:
[argon2] many tests fail on s390x, likely due to endianness issues
I'm packaging `argon2` in Fedora, and many tests fail on s390x:

https://koji.fedoraproject.org/koji/taskinfo?taskID=112000146
https://kojipkgs.fedoraproject.org/work/tasks/231/112000231/build.log
[build.log](https://github.com/RustCrypto/password-hashes/files/13994452/build.log)

Full build.log uploaded since Fedora's build system would garbage collect scratch builds after a couple of weeks.

It succeeds on other architectures - x86_64, aarch64, ppc64le


Discussion / hints:
Reproduced, thanks.

That's unfortunate. Normally we run our crates through `cross` tests to look for this sort of thing, but they were never configured for `argon2`.
[These methods](https://github.com/RustCrypto/password-hashes/blob/master/argon2/src/block.rs#L61-L67) introduced in #247 are obviously not endianess-portable.

For BE testing it would be nice to use MIRI instead of `cross`.

cc @Pjottos
> [These methods](https://github.com/RustCrypto/password-hashes/blob/master/argon2/src/block.rs#L61-L67) introduced in https://github.com/RustCrypto/password-hashes/pull/247 are obviously not endianess-portable.

Aha! Good catch!

> For BE testing it would be nice to use MIRI instead of cross.

I don't think that's going to work. The `argon2` test suite exercises the full Argon2 algorithm repeatedly with KATs. Miri is going to be way too slow for those to complete in a reasonable timeframe.
Okay, got it fixed locally. Will add some cross config in CI and push up a PR.

FWIW I tried running the tests under Miri. I gave it about 10 minutes to get through one of the nontrivial ones and it couldn't complete that, so I don't think it's going to work out in CI.

cross with `powerpc-unknown-linux-gnu` as the target seems like the fastest option to me.

Fix patch:
diff --git a/.github/workflows/argon2.yml b/.github/workflows/argon2.yml
index a566ae07..5d372d97 100644
--- 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
index ea7809a6..fcf89d71 100644
--- 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
index be373c86..a8b801f2 100644
--- 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(())
     }


Test patch:
diff --git a/argon2/tests/kat.rs b/argon2/tests/kat.rs
index cfabcfe1..dd70df84 100644
--- 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));
+    }
 }
 
 // =======================================