netdev - Re: [PATCH 1/2] rust: add synchronous message digest support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 16 May 2023 08:25:19 +0000
From: FUJITA Tomonori <tomo@...bit.dev>
To: ebiggers@...nel.org
Cc: tomo@...bit.dev, rust-for-linux@...r.kernel.org,
 netdev@...r.kernel.org, linux-crypto@...r.kernel.org,
 fujita.tomonori@...il.com
Subject: Re: [PATCH 1/2] rust: add synchronous message digest support

Hi,

On Mon, 15 May 2023 22:52:19 -0700
Eric Biggers <ebiggers@...nel.org> wrote:

>> +#include <crypto/hash.h>
>>  #include <linux/bug.h>
>>  #include <linux/build_bug.h>
>>  #include <linux/err.h>
>> @@ -27,6 +28,29 @@
>>  #include <linux/sched/signal.h>
>>  #include <linux/wait.h>
>>  
>> +void rust_helper_crypto_free_shash(struct crypto_shash *tfm)
>> +{
>> +	crypto_free_shash(tfm);
>> +}
>> +EXPORT_SYMBOL_GPL(rust_helper_crypto_free_shash);
> 
> Shouldn't this code be compiled only when the crypto API is available?

Oops, I'll add #ifdef CONFIG_CRYPTO


>> +impl<'a> ShashDesc<'a> {
>> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
>> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        let size = core::mem::size_of::<bindings::shash_desc>()
>> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
>> +        let layout = Layout::from_size_align(size, 2)?;
>> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
>> +        let mut desc = ShashDesc { ptr, tfm, size };
>> +        // SAFETY: The `desc.tfm` is non-null and valid for the lifetime of this object.
>> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
>> +        Ok(desc)
>> +    }
>> +
>> +    /// (Re)initializes message digest.
>> +    pub fn init(&mut self) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
>> +    }
>> +
>> +    /// 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)
>> +        })
>> +    }
>> +
>> +    /// Calculates message digest.
>> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })
>> +    }
> 
> This doesn't enforce that init() is called before update() or finalize().  I
> think that needs to be checked in the Rust code, since the C code doesn't have
> defined behavior in that case.

Surely, Rust side should handle the case.

If the new() function internally calls init() before returning, it
works? The new() returns an initialized ShaDesc object.


Thanks for reviewing!