Skip to content

Commit 9965292

Browse files
committed
argon2: fix big endian support
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.
1 parent d8aab76 commit 9965292

4 files changed

Lines changed: 79 additions & 20 deletions

File tree

.github/workflows/argon2.yml

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,48 @@ jobs:
4848
runs-on: ubuntu-latest
4949
strategy:
5050
matrix:
51-
rust:
52-
- 1.65.0 # MSRV
53-
- stable
51+
include:
52+
# 32-bit Linux
53+
- target: i686-unknown-linux-gnu
54+
rust: 1.65.0 # MSRV
55+
deps: sudo apt update && sudo apt install gcc-multilib
56+
- target: i686-unknown-linux-gnu
57+
rust: stable
58+
deps: sudo apt update && sudo apt install gcc-multilib
59+
60+
# 64-bit Linux
61+
- target: x86_64-unknown-linux-gnu
62+
rust: 1.65.0 # MSRV
63+
- target: x86_64-unknown-linux-gnu
64+
rust: stable
5465
steps:
5566
- uses: actions/checkout@v4
5667
- uses: RustCrypto/actions/cargo-cache@master
5768
- uses: dtolnay/rust-toolchain@master
5869
with:
5970
toolchain: ${{ matrix.rust }}
71+
targets: ${{ matrix.target }}
72+
- run: ${{ matrix.deps }}
6073
- run: cargo test --no-default-features
6174
- run: cargo test --no-default-features --features password-hash
6275
- run: cargo test
6376
- run: cargo test --all-features
77+
78+
cross:
79+
strategy:
80+
matrix:
81+
include:
82+
- target: powerpc-unknown-linux-gnu
83+
rust: 1.65.0 # MSRV
84+
- target: powerpc-unknown-linux-gnu
85+
rust: stable
86+
runs-on: ubuntu-latest
87+
steps:
88+
- uses: actions/checkout@v4
89+
- run: ${{ matrix.deps }}
90+
- uses: dtolnay/rust-toolchain@master
91+
with:
92+
toolchain: ${{ matrix.rust }}
93+
targets: ${{ matrix.target }}
94+
- uses: RustCrypto/actions/cross-install@master
95+
- run: cross test --release --target ${{ matrix.target }} --all-features

argon2/src/block.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use core::{
44
convert::{AsMut, AsRef},
55
num::Wrapping,
66
ops::{BitXor, BitXorAssign},
7+
slice,
78
};
89

910
#[cfg(feature = "zeroize")]
@@ -58,12 +59,18 @@ impl Block {
5859
Self([0u64; Self::SIZE / 8])
5960
}
6061

61-
pub(crate) fn as_bytes(&self) -> &[u8; Self::SIZE] {
62-
unsafe { &*(self.0.as_ptr() as *const [u8; Self::SIZE]) }
62+
/// Load a block from a block-sized byte slice
63+
#[inline(always)]
64+
pub(crate) fn load(&mut self, input: &[u8; Block::SIZE]) {
65+
for (i, chunk) in input.chunks(8).enumerate() {
66+
self.0[i] = u64::from_le_bytes(chunk.try_into().unwrap());
67+
}
6368
}
6469

65-
pub(crate) fn as_mut_bytes(&mut self) -> &mut [u8; Self::SIZE] {
66-
unsafe { &mut *(self.0.as_mut_ptr() as *mut [u8; Self::SIZE]) }
70+
/// Iterate over the `u64` values contained in this block
71+
#[inline(always)]
72+
pub(crate) fn iter(&self) -> slice::Iter<'_, u64> {
73+
self.0.iter()
6774
}
6875

6976
/// NOTE: do not call this directly. It should only be called via

argon2/src/lib.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,13 +305,17 @@ impl<'key> Argon2<'key> {
305305
let i = i as u32;
306306
let l = l as u32;
307307

308+
// Make the first and second block in each lane as G(H0||0||i) or
309+
// G(H0||1||i)
308310
let inputs = &[
309311
initial_hash.as_ref(),
310312
&i.to_le_bytes()[..],
311313
&l.to_le_bytes()[..],
312314
];
313315

314-
blake2b_long(inputs, block.as_mut_bytes())?;
316+
let mut hash = [0u8; Block::SIZE];
317+
blake2b_long(inputs, &mut hash)?;
318+
block.load(&hash);
315319
}
316320
}
317321

@@ -485,10 +489,20 @@ impl<'key> Argon2<'key> {
485489
blockhash ^= &memory_blocks[last_block_in_lane];
486490
}
487491

488-
blake2b_long(&[blockhash.as_bytes()], out)?;
492+
// Hash the result
493+
let mut blockhash_bytes = [0u8; Block::SIZE];
494+
495+
for (chunk, v) in blockhash_bytes.chunks_mut(8).zip(blockhash.iter()) {
496+
chunk.copy_from_slice(&v.to_le_bytes())
497+
}
498+
499+
blake2b_long(&[&blockhash_bytes], out)?;
489500

490501
#[cfg(feature = "zeroize")]
491-
blockhash.zeroize();
502+
{
503+
blockhash.zeroize();
504+
blockhash_bytes.zeroize();
505+
}
492506

493507
Ok(())
494508
}

argon2/tests/kat.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,14 @@ fn salt_bad_length() {
307307
let ret = ctx.hash_password_into(b"password", &too_short_salt, &mut out);
308308
assert_eq!(ret, Err(Error::SaltTooShort));
309309

310-
// 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
311-
// optimizations kicks in an nothing is really allocated
312-
let too_long_salt = vec![0u8; argon2::MAX_SALT_LEN + 1];
313-
let ret = ctx.hash_password_into(b"password", &too_long_salt, &mut out);
314-
assert_eq!(ret, Err(Error::SaltTooLong));
310+
#[cfg(target_pointer_width = "64")] // MAX_SALT_LEN + 1 is too big for 32-bit targets
311+
{
312+
// 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
313+
// optimizations kicks in an nothing is really allocated
314+
let too_long_salt = vec![0u8; argon2::MAX_SALT_LEN + 1];
315+
let ret = ctx.hash_password_into(b"password", &too_long_salt, &mut out);
316+
assert_eq!(ret, Err(Error::SaltTooLong));
317+
}
315318
}
316319

317320
#[test]
@@ -321,11 +324,14 @@ fn output_bad_length() {
321324
let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
322325
assert_eq!(ret, Err(Error::OutputTooShort));
323326

324-
// 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
325-
// optimizations kicks in an nothing is really allocated
326-
let mut out = vec![0u8; Params::MAX_OUTPUT_LEN + 1];
327-
let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
328-
assert_eq!(ret, Err(Error::OutputTooLong));
327+
#[cfg(target_pointer_width = "64")] // MAX_SALT_LEN + 1 is too big for 32-bit targets
328+
{
329+
// 4 GiB of RAM seems big, but as long as we ask for a zero-initialized vector
330+
// optimizations kicks in an nothing is really allocated
331+
let mut out = vec![0u8; Params::MAX_OUTPUT_LEN + 1];
332+
let ret = ctx.hash_password_into(b"password", b"diffsalt", &mut out);
333+
assert_eq!(ret, Err(Error::OutputTooLong));
334+
}
329335
}
330336

331337
// =======================================

0 commit comments

Comments
 (0)