Re: [PATCH 1/3] rust: add untrusted data abstraction

[Finn Behrens]

On 13 Sep 2024, at 13:26, Benno Lossin wrote:

> When reading data from userspace, hardware or other external untrusted
> sources, the data must be validated before it is used for logic within
> the kernel. This abstraction provides a generic newtype wrapper that
> prevents direct access to the inner type. It does allow access through
> the `untrusted()` method, but that should be a telltale sign to
> reviewers that untrusted data is being used at that point.
>
> Any API that reads data from an untrusted source should return
> `Untrusted<T>` where `T` is the type of the underlying untrusted data.
> This generic allows other abstractions to return their custom type
> wrapped by `Untrusted`, signaling to the caller that the data must be
> validated before use. This allows those abstractions to be used both in
> a trusted and untrusted manner, increasing their generality.
> Additionally, using the arbitrary self types feature, APIs can be
> designed to explicitly read untrusted data:
>
>     impl MyCustomDataSource {
>         pub fn read(self: &Untrusted<Self>) -> &Untrusted<[u8]>;
>     }
>
> To validate data, the `Validator` trait is introduced, readers of
> untrusted data should implement it for their type and move all of their
> validation and parsing logic into its `validate` function. That way
> reviewers and later contributors have a central location to consult for
> data validation.
>
> Signed-off-by: Benno Lossin <benno.lossin@xxxxxxxxx>
> ---
>  rust/kernel/types.rs | 248 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 247 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9e7ca066355c..20ef04b1b417 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
…
> +
> +/// Validates untrusted data.
> +///
> +/// # Examples
> +///
> +/// ## Using an API returning untrusted data
> +///
> +/// Create the type of the data that you want to parse:
> +///
> +/// ```
> +/// pub struct FooData {
> +///     data: [u8; 4],
> +/// }
> +/// ```
> +///
> +/// Then implement this trait:
> +///
> +/// ```
> +/// use kernel::types::{Untrusted, Validator};
> +/// # pub struct FooData {
> +/// #     data: [u8; 4],
> +/// # }
> +/// impl Validator for FooData {
> +///     type Input = [u8];
> +///     type Output = FooData;
> +///     type Err = Error;
> +///
> +///     fn validate(untrusted: &Untrusted<Self::Input>) -> Result<Self::Output, Self::Err> {
> +///         let untrusted = untrusted.untrusted();
> +///         let untrusted = <[u8; 4]>::try_from(untrusted);
> +///         for byte in &untrusted {
> +///             if byte & 0xf0 != 0 {
> +///                 return Err(());
> +///             }
> +///         }
> +///         Ok(FooData { data: untrusted })
> +///     }
> +/// }
> +/// ```
> +///
> +/// And then use the API that returns untrusted data:
> +///
> +/// ```ignore
> +/// let result = get_untrusted_data().validate::<FooData>();
> +/// ```
> +///
> +/// ## Creating an API returning untrusted data
> +///
> +/// In your API instead of just returning the untrusted data, wrap it in [`Untrusted<T>`]:
> +///
> +/// ```
> +/// pub fn get_untrusted_data(&self) -> &Untrusted<[u8]> {
> +///     todo!()
> +/// }
> +/// ```
> +pub trait Validator {
> +    /// Type of the input data that is untrusted.
> +    type Input: ?Sized;

I would like to explore this trait with being generic over Input, instead of having Input as an associated type. Might be nicer to have Validators for different input types if the valid data is always the same?

> +    /// Type of the validated data.
> +    type Output;
> +    /// Validation error.
> +    type Err;
> +
> +    /// Validate the given untrusted data and parse it into the output type.
> +    ///
> +    /// When implementing this function, you can use [`Untrusted::untrusted()`] to get access to
> +    /// the raw untrusted data.
> +    fn validate(untrusted: &Untrusted<Self::Input>) -> Result<Self::Output, Self::Err>;
> +}
> -- 
> 2.46.0