cipher: fix seeking implementation in the stream cipher wrapper (#2052)

The change forbids seeking to the last keystream block and its application.
Additionally, unchecked methods added as an escape hatch.

Author: newpavlov

cipher/src/stream.rs

@@ -1,6 +1,6 @@
 //! Traits which define functionality of stream ciphers.
 //!
-//! See [RustCrypto/stream-ciphers](https://github.com/RustCrypto/stream-ciphers)
+//! See the [RustCrypto/stream-ciphers](https://github.com/RustCrypto/stream-ciphers) repository
 //! for ciphers implementation.
 
 use crate::block::{BlockModeDecrypt, BlockModeEncrypt};
@@ -20,7 +20,7 @@ pub use errors::{OverflowError, StreamCipherError};
 #[cfg(feature = "stream-wrapper")]
 pub use wrapper::StreamCipherCoreWrapper;
 
-/// Marker trait for block-level asynchronous stream ciphers
+/// Asynchronous stream cipher trait.
 pub trait AsyncStreamCipher: Sized {
     /// Encrypt data using `InOutBuf`.
     fn encrypt_inout(mut self, data: InOutBuf<'_, '_, u8>)
@@ -86,34 +86,127 @@ pub trait AsyncStreamCipher: Sized {
     }
 }
 
-/// Synchronous stream cipher core trait.
+/// Stream cipher trait.
+///
+/// This trait applies only to synchronous stream ciphers, which generate a keystream and
+/// XOR data with it during both encryption and decryption. Therefore, instead of separate methods
+/// for encryption and decryption, this trait provides methods for keystream application.
+///
+/// # Notes on Keystream Repetition
+/// All stream ciphers have a finite state, so the generated keystream inevitably repeats itself,
+/// making the cipher vulnerable to chosen plaintext attack. Typically, the repetition period is
+/// astronomically large, rendering keystream repetition impossible to encounter in practice.
+///
+/// However, counter-based stream ciphers allow seeking across the keystream, and some also use
+/// small counters (e.g. 32 bits). This can result in triggering keystream repetition in practice.
+///
+/// To guard against this, methods either panic (e.g. [`StreamCipher::apply_keystream`]) or
+/// return [`StreamCipherError`] (e.g. [`StreamCipher::try_apply_keystream`]) when
+/// keystream repetition occurs. We also provide a number of "unchecked" methods
+/// (e.g. [`StreamCipher::unchecked_apply_keystream`]), but they should be used with extreme care.
+///
+/// For efficiency reasons, the check for keystream repetition is typically implemented by
+/// forbidding the generation of the last keystream block in both the keystream application methods
+/// and the seeking methods defined in the [`StreamCipherSeek`] trait.
 pub trait StreamCipher {
+    /// Check that the cipher can generate a keystream with a length of `data_len` bytes.
+    fn check_remaining(&self, data_len: usize) -> Result<(), StreamCipherError>;
+
+    /// Apply keystream to `inout` without checking for keystream repetition.
+    ///
+    /// # WARNING
+    /// This method should be used with extreme caution! Triggering keystream repetition can expose
+    /// the stream cipher to chosen plaintext attacks.
+    fn unchecked_apply_keystream_inout(&mut self, buf: InOutBuf<'_, '_, u8>);
+
+    /// Apply keystream to `buf` without checking for keystream repetition.
+    ///
+    /// # WARNING
+    /// This method should be used with extreme caution! Triggering keystream repetition can expose
+    /// the stream cipher to chosen plaintext attacks.
+    fn unchecked_write_keystream(&mut self, buf: &mut [u8]);
+
+    /// Apply keystream to data behind `buf` without checking for keystream repetition.
+    ///
+    /// # WARNING
+    /// This method should be used with extreme caution! Triggering keystream repetition can expose
+    /// the stream cipher to chosen plaintext attacks.
+    #[inline]
+    fn unchecked_apply_keystream(&mut self, buf: &mut [u8]) {
+        self.unchecked_apply_keystream_inout(buf.into())
+    }
+
+    /// Apply keystream to data buffer-to-buffer without checking for keystream repetition.
+    ///
+    /// It will XOR generated keystream with data from the `input` buffer
+    /// and will write result to the `output` buffer.
+    ///
+    /// Returns [`NotEqualError`] if the `input` and `output` buffers have different lengths.
+    ///
+    /// # WARNING
+    /// This method should be used with extreme caution! Triggering keystream repetition can expose
+    /// the stream cipher to chosen plaintext attacks.
+    #[inline]
+    fn unchecked_apply_keystream_b2b(
+        &mut self,
+        input: &[u8],
+        output: &mut [u8],
+    ) -> Result<(), NotEqualError> {
+        let buf = InOutBuf::new(input, output)?;
+        self.unchecked_apply_keystream_inout(buf);
+        Ok(())
+    }
+
     /// Apply keystream to `inout` data.
     ///
-    /// If end of the keystream will be achieved with the given data length,
-    /// method will return [`StreamCipherError`] without modifying provided `data`.
+    /// If the end of the keystream is reached with the given buffer length,
+    /// the method will return [`StreamCipherError`] without modifying `buf`.
     fn try_apply_keystream_inout(
         &mut self,
         buf: InOutBuf<'_, '_, u8>,
-    ) -> Result<(), StreamCipherError>;
+    ) -> Result<(), StreamCipherError> {
+        self.check_remaining(buf.len())?;
+        self.unchecked_apply_keystream_inout(buf);
+        Ok(())
+    }
 
     /// Apply keystream to data behind `buf`.
     ///
-    /// If end of the keystream will be achieved with the given data length,
-    /// method will return [`StreamCipherError`] without modifying provided `data`.
+    /// If the end of the keystream is reached with the given buffer length,
+    /// the method will return [`StreamCipherError`] without modifying `buf`.
     #[inline]
     fn try_apply_keystream(&mut self, buf: &mut [u8]) -> Result<(), StreamCipherError> {
         self.try_apply_keystream_inout(buf.into())
     }
 
+    /// Apply keystream to data buffer-to-buffer.
+    ///
+    /// It will XOR generated keystream with data from the `input` buffer
+    /// and will write result to the `output` buffer.
+    ///
+    /// Returns [`StreamCipherError`] without modifying the buffers if the `input` and `output`
+    /// buffers have different lengths, or if the end of the keystream is reached with
+    /// the given data length.
+    #[inline]
+    fn try_apply_keystream_b2b(
+        &mut self,
+        input: &[u8],
+        output: &mut [u8],
+    ) -> Result<(), StreamCipherError> {
+        InOutBuf::new(input, output)
+            .map_err(|_| StreamCipherError)
+            .and_then(|buf| self.try_apply_keystream_inout(buf))
+    }
+
     /// Write keystream to `buf`.
     ///
-    /// If end of the keystream will be achieved with the given data length,
-    /// method will return [`StreamCipherError`] without modifying provided `data`.
+    /// If the end of the keystream is reached with the given buffer length,
+    /// the method will return [`StreamCipherError`] without modifying `buf`.
     #[inline]
     fn try_write_keystream(&mut self, buf: &mut [u8]) -> Result<(), StreamCipherError> {
-        buf.fill(0);
-        self.try_apply_keystream(buf)
+        self.check_remaining(buf.len())?;
+        self.unchecked_write_keystream(buf);
+        Ok(())
     }
 
     /// Apply keystream to `inout` data.
@@ -122,8 +215,7 @@ pub trait StreamCipher {
     /// and will write result to `out` pointer.
     ///
     /// # Panics
-    /// If end of the keystream will be reached with the given data length,
-    /// method will panic without modifying the provided `data`.
+    /// If the end of the keystream is reached with the given buffer length.
     #[inline]
     fn apply_keystream_inout(&mut self, buf: InOutBuf<'_, '_, u8>) {
         self.try_apply_keystream_inout(buf).unwrap();
@@ -135,84 +227,84 @@ pub trait StreamCipher {
     /// to the same buffer.
     ///
     /// # Panics
-    /// If end of the keystream will be reached with the given data length,
-    /// method will panic without modifying the provided `data`.
+    /// If the end of the keystream is reached with the given buffer length.
     #[inline]
     fn apply_keystream(&mut self, buf: &mut [u8]) {
         self.try_apply_keystream(buf).unwrap();
     }
 
-    /// Write keystream to `buf`.
+    /// Apply keystream to data buffer-to-buffer.
+    ///
+    /// It will XOR generated keystream with data from the `input` buffer
+    /// and will write result to the `output` buffer.
     ///
     /// # Panics
-    /// If end of the keystream will be reached with the given data length,
-    /// method will panic without modifying the provided `data`.
+    /// If the end of the keystream is reached with the given buffer length,
+    /// of if the `input` and `output` buffers have different lengths.
     #[inline]
-    fn write_keystream(&mut self, buf: &mut [u8]) {
-        self.try_write_keystream(buf).unwrap();
+    fn apply_keystream_b2b(&mut self, input: &[u8], output: &mut [u8]) {
+        let Ok(buf) = InOutBuf::new(input, output) else {
+            panic!("Lengths of input and output buffers are not equal to each other!");
+        };
+        self.apply_keystream_inout(buf)
     }
 
-    /// Apply keystream to data buffer-to-buffer.
-    ///
-    /// It will XOR generated keystream with data from the `input` buffer
-    /// and will write result to the `output` buffer.
+    /// Write keystream to `buf`.
     ///
-    /// Returns [`StreamCipherError`] if provided `in_blocks` and `out_blocks`
-    /// have different lengths or if end of the keystream will be reached with
-    /// the given input data length.
+    /// # Panics
+    /// If the end of the keystream is reached with the given buffer length.
     #[inline]
-    fn apply_keystream_b2b(
-        &mut self,
-        input: &[u8],
-        output: &mut [u8],
-    ) -> Result<(), StreamCipherError> {
-        InOutBuf::new(input, output)
-            .map_err(|_| StreamCipherError)
-            .and_then(|buf| self.try_apply_keystream_inout(buf))
+    fn write_keystream(&mut self, buf: &mut [u8]) {
+        self.try_write_keystream(buf).unwrap();
     }
 }
 
 /// Trait for seekable stream ciphers.
 ///
-/// Methods of this trait are generic over the [`SeekNum`] trait, which is
-/// implemented for primitive numeric types, i.e.: `i32`, `u32`, `u64`,
-/// `u128`, and `usize`.
+/// Methods of this trait are generic over the [`SeekNum`] trait,
+/// i.e. they can be used with `i32`, `u32`, `u64`, `u128`, and `usize`.
 pub trait StreamCipherSeek {
-    /// Try to get current keystream position
+    /// Try to get current keystream position in bytes.
     ///
-    /// Returns [`OverflowError`] if position can not be represented by type `T`
+    /// Returns [`OverflowError`] if the position value can not be represented by type `T`.
     fn try_current_pos<T: SeekNum>(&self) -> Result<T, OverflowError>;
 
-    /// Try to seek to the given position
+    /// Try to seek to the provided position in bytes.
     ///
-    /// Returns [`StreamCipherError`] if provided position value is bigger than
-    /// keystream length.
+    /// Returns [`StreamCipherError`] if the position value is bigger than keystream length.
     fn try_seek<T: SeekNum>(&mut self, pos: T) -> Result<(), StreamCipherError>;
 
-    /// Get current keystream position
+    /// Get current keystream position in bytes.
     ///
     /// # Panics
-    /// If position can not be represented by type `T`
+    /// If the position value can not be represented by type `T`.
     fn current_pos<T: SeekNum>(&self) -> T {
         self.try_current_pos().unwrap()
     }
 
-    /// Seek to the given position
+    /// Seek to the provided keystream position in bytes.
     ///
     /// # Panics
-    /// If provided position value is bigger than keystream length
+    /// If the position value is bigger than keystream length.
     fn seek<T: SeekNum>(&mut self, pos: T) {
         self.try_seek(pos).unwrap()
     }
 }
 
 impl<C: StreamCipher> StreamCipher for &mut C {
     #[inline]
-    fn try_apply_keystream_inout(
-        &mut self,
-        buf: InOutBuf<'_, '_, u8>,
-    ) -> Result<(), StreamCipherError> {
-        C::try_apply_keystream_inout(self, buf)
+    fn check_remaining(&self, data_len: usize) -> Result<(), StreamCipherError> {
+        C::check_remaining(self, data_len)
+    }
+
+    #[inline]
+    fn unchecked_apply_keystream_inout(&mut self, buf: InOutBuf<'_, '_, u8>) {
+        C::unchecked_apply_keystream_inout(self, buf)
+    }
+
+    #[inline]
+    fn unchecked_write_keystream(&mut self, buf: &mut [u8]) {
+        C::unchecked_write_keystream(self, buf)
     }
 }
 
@@ -249,8 +341,8 @@ macro_rules! impl_seek_num {
                 }
 
                 fn into_block_byte<T: StreamCipherCounter>(self, block_size: u8) -> Result<(T, u8), OverflowError> {
-                    let bs: Self = block_size.into();
-                    let byte = (self % bs) as u8;
+                    let bs = Self::from(block_size);
+                    let byte = u8::try_from(self % bs).expect("bs fits into u8");
                     let block = T::try_from(self / bs).map_err(|_| OverflowError)?;
                     Ok((block, byte))
                 }

cipher/src/stream/core_api.rs

@@ -37,11 +37,10 @@ pub trait StreamCipherClosure: BlockSizeUser {
 
 /// Block-level synchronous stream ciphers.
 pub trait StreamCipherCore: BlockSizeUser + Sized {
-    /// Return number of remaining blocks before cipher wraps around.
+    /// Return number of remaining blocks before the cipher wraps around.
     ///
     /// Returns `None` if number of remaining blocks can not be computed
-    /// (e.g. in ciphers based on the sponge construction) or it's too big
-    /// to fit into `usize`.
+    /// (e.g. in the case of sponge-based stream ciphers) or it’s too big to fit into `usize`.
     fn remaining_blocks(&self) -> Option<usize>;
 
     /// Process data using backend provided to the rank-2 closure.
@@ -91,23 +90,23 @@ pub trait StreamCipherCore: BlockSizeUser + Sized {
 
     /// Try to apply keystream to data not divided into blocks.
     ///
-    /// Consumes cipher since it may consume final keystream block only
-    /// partially.
+    /// Consumes cipher since it may consume the final keystream block only partially.
     ///
-    /// Returns an error if number of remaining blocks is not sufficient
-    /// for processing the input data.
+    /// Returns an error if the number of remaining blocks is not sufficient
+    /// for processing of the input data.
     #[inline]
     fn try_apply_keystream_partial(
         mut self,
         mut buf: InOutBuf<'_, '_, u8>,
     ) -> Result<(), StreamCipherError> {
-        if let Some(rem) = self.remaining_blocks() {
-            let blocks = if buf.len() % Self::BlockSize::USIZE == 0 {
-                buf.len() % Self::BlockSize::USIZE
-            } else {
-                buf.len() % Self::BlockSize::USIZE + 1
-            };
-            if blocks > rem {
+        if let Some(rem_blocks) = self.remaining_blocks() {
+            // Note that if `rem_blocks` is equal to zero, it means that
+            // the next generated block will be the last in the keystream and
+            // the cipher core will wrap to its initial state.
+            // Since we consume `self`, it's fine to generate the last keystream block,
+            // so we can use division instead of `div_ceil` to compute `req_blocks`.
+            let req_blocks = buf.len() / Self::BlockSize::USIZE;
+            if req_blocks > rem_blocks {
                 return Err(StreamCipherError);
             }
         }
@@ -164,7 +163,10 @@ pub trait StreamCipherCounter:
     + TryInto<u64>
     + TryInto<u128>
     + TryInto<usize>
+    + Copy
 {
+    /// Returns `true` if `self` is equal to the max counter value.
+    fn is_max(&self) -> bool;
 }
 
 /// Block-level seeking trait for stream ciphers.
@@ -181,7 +183,13 @@ pub trait StreamCipherSeekCore: StreamCipherCore {
 
 macro_rules! impl_counter {
     {$($t:ty )*} => {
-        $( impl StreamCipherCounter for $t { } )*
+        $(
+            impl StreamCipherCounter for $t {
+                fn is_max(&self) -> bool {
+                    *self == <$t>::MAX
+                }
+            }
+        )*
     };
 }