CipherCtx::cipher_update is too pessimistic

[wiktor-k]

Hi,

I'm using the symmetric cipher encryption API and it seems the constraints in CipherCtx::cipher_update are too strict for block ciphers compared to what OpenSSL requires.

Currently the code checks if the output size has sufficient size and for streaming ciphers it's OK but for block ciphers it always assumes pessimistic case of partial block updates - that is feeding inputs that are smaller than the block size. Then, and only then the output needs to be big enough to contain the extra block.

I'm using block ciphers in a very strict case of always feeding data one entire block at a time and I'd like the output buffer to also be one block of length (this would make it consistent with other cryptographical backends that we use and also consistent with OpenSSL itself).

I see the following solutions:

1. introduce an unsafe function that would not require the output buffer to be big enough for additional block. I've written a sample code and tested that against our data:

    pub unsafe fn unchecked_cipher_update(
        &mut self,
        input: &[u8],
        output: &mut [u8],
    ) -> Result<usize, ErrorStack> {
        let inlen = LenType::try_from(input.len()).unwrap();

        let block_size = self.block_size();
        if block_size > 1 {
            // block cipher
            assert_eq!(block_size, input.len());
            assert!(output.len() >= block_size);
        } else {
            // stream cipher
            assert_eq!(input.len(), output.len());
        }

        let mut outlen = 0;

        cvt(ffi::EVP_CipherUpdate(
            self.as_ptr(),
            output.as_mut_ptr(),
            &mut outlen,
            input.as_ptr(),
            inlen,
        ))?;

        Ok(outlen as usize)
    }

(Full code at wiktor-k@7d0fff8). Note that this function is unsafe only when mixed with partial block updates with CipherCtx::cipher_update. The disadvantage of this approach is exposing unsafe functions.

2. Create other struct that has almost identical API but only allows stream ciphers or block ciphers where input.len() == block_size == output.len(). This has the advantage of offering safe API but the disadvantage of duplicating code (although for my use case only parts are needed).

3. In CipherCtx keep track of how many bytes were fed into update and thus always require the output buffer to be of correct size. The advantage is that no public API would change and the cipher_update would just become more flexible. The disadvantage: additional complexity of tracking bytes modulo block size.

4. Do not change anything and just make clients use raw ffi API. I'd like to avoid it but if there's no better solution I'd have to do that.

Maybe there are some other better ideas, if so I'm all ears!

Thank you for your time and sorry that this issue is so long 🙇

---

For reference I got the exact algorithm description from Matt from OpenSSL (source):

EVP_EncryptUpdate() can be called repeatedly, incrementally feeding in
the data to be encrypted. The ECB mode (when used with AES-128) will
encrypt input data 16 bytes at a time, and the output size will also be
16 bytes per input block. If the data that you feed in to
EVP_EncryptUpdate() is not a multiple of 16 bytes then the amount of
data that is over a multiple of 16 bytes will be cached until a
subsequent call where it does have 16 bytes.

Let's say you call EVP_EncryptUpdate() with 15 bytes of data. In that
case all 15 bytes will be cached and 0 bytes will be output.

If you then call it again with 17 bytes of data, then added to the 15
bytes already cached we have a total of 32 bytes. This is a multiple of
16, so 2 blocks (32 bytes) will be output, so:

(inl + cipher_block_size - 1) = (17 + 16 - 1) = 32

[sfackler]

I'd like to avoid having CipherCtx track any extra state internally. If there's a way to access the internal accounting of how much sub-block data is currently buffered then we could hopefully use that to have a more accurate output buffer check. Otherwise, adding an unchecked update method like you suggested would probably be the way to go.