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

[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

traits/cipher/src/stream/wrapper.rs

Lines 176 to 202 in 67131af

	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.

[newpavlov]

As explained here, it's an artifact of how we generically detect wrapping of keystream. We could change the code to properly detect wrapping around 0, but it has to be done in the cipher crate.