Dear crypto maintainers,
Fujita Tomonori has created some Rust bindings for the crypto API seen in
this thread. Here is a fragment of my review of said code:
On 25.06.23 12:08, Benno Lossin wrote:
>>>> + /// Adds data to message digest for processing.
>>>> + pub fn update(&mut self, data: &[u8]) -> Result {
>>>> + // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> + to_result(unsafe {
>>>> + bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>>>> + })
>>>
>>> What if `data.len() > u32::MAX`?
>>
>> The buffer might not be updated properly, I guess. Should check the case?
>
> Not sure what we should do in that case, will bring it up at the next
> team meeting. In Rust, `write` and `read` functions often output the
> number of bytes that were actually read/written. So maybe we should also
> do that here? Then you could just return `u32::MAX` and the user would
> have to call again. We could also call the C side multiple times until
> the entire buffer has been processed. But as the C side only supports
> u32 anyway, I think it would be a rare occurrence for `data` to be large.
I noted that in the code segment above that the length of the data
that is to be hashed is cast from a `usize` to a `u32`. Since
`usize = uintptr_t` this might be a problem for very large arguments.
Since the C side only accepts an `unsigned int`, it seems as if large inputs
are never the case. On the Rust side we are forced to use `usize`, since that
is the length of slices (the input type `&[u8]`).
We came up with the following solutions, but could not come to a consensus on any
particular one, could you please assist us in making this decision?
1. create a loop that calls the C API multiple times if the input is large
2. panic
3. truncate
4. return an error
Thanks a lot!
--
Cheers,
Benno
