Skip to content

cipher: Seeking near the end of the keystream causes try_current_pos to return an error #1808

@jpdoyle

Description

@jpdoyle

The following test fails on the last line:

    #[test]
    fn test_current_pos() {
        let offset: u64 = 256u64 << 30;

        let mut cipher = chacha20::ChaCha20::new(&Default::default(), &Default::default());
        cipher
            .try_seek(offset - 64)
            .expect("Couldn't seek to nearly 256GB");
        assert_eq!(cipher.try_current_pos::<u64>().unwrap(),
            offset - ((1<<6))
        );
        cipher
            .try_seek(offset - 63)
            .expect("Couldn't seek to nearly 256GB");
        cipher.try_current_pos::<u64>().unwrap();
    }

The root cause appears to be a combination of the incorrect looping counter behavior highlighted in RustCrypto/stream-ciphers#391 and from the behavior of try_seek in StreamCipherCoreWrapper. try_seek sets the block position to (((256u64 << 30)-63) / 64) as u32, which is u32::MAX, and then calls write_keystream_block, which increments the counter, causing it to loop back to 0. Then the try_current_pos calls get_block_pos(), which returns 0 -- see

fn try_current_pos<SN: SeekNum>(&self) -> Result<SN, OverflowError> {
let pos = self.get_pos();
SN::from_block_byte(self.core.get_block_pos(), pos, T::BlockSize::U8)
}
fn try_seek<SN: SeekNum>(&mut self, new_pos: SN) -> Result<(), StreamCipherError> {
let (block_pos, byte_pos) = new_pos.into_block_byte(T::BlockSize::U8)?;
// For correct implementations of `SeekNum` compiler should be able to
// eliminate this assert
assert!(byte_pos < T::BlockSize::U8);
self.core.set_block_pos(block_pos);
let new_pos = if byte_pos != 0 {
// See comment in `try_apply_keystream_inout` for use of `write_keystream_block`
self.core.write_keystream_block(&mut self.buffer);
byte_pos.into()
} else {
T::BlockSize::USIZE
};
// SAFETY: we assert that `byte_pos` is always smaller than block size.
// If `byte_pos` is zero, we replace it with block size. Thus the invariant
// required by `set_pos_unchecked` is satisfied.
unsafe {
self.set_pos_unchecked(new_pos);
}
Ok(())
}

SeekNum::get_block_byte is then called with a value of 0 for the block parameter, and the checked subtraction on this line returns None.

Metadata

Metadata

Assignees

No one assigned

    Labels

    cipherBlock and stream cipher crate

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions