From df2867dc906c5e1f4c38265ab06d4932834c8b5c Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 16 Dec 2024 12:53:01 +0100 Subject: [PATCH] Remove serde trait implementations for requests and replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementing Serialize and Deserialize for the request and reply structs leaks implementation details, especially when using serde_indexed. This patch removes these implementations for the request and reply structs and also for some other types that presumably only had them because they were used in these structs and that are not serialized or deserialized anywhere in the Trussed ecosystem. To make it easier to store encrypted data – where previously reply::Encrypt could be used direcly – this patch adds an EncryptedData struct that implemente Serialize and Deserialize. Fixes: https://github.com/trussed-dev/trussed/issues/183 --- CHANGELOG.md | 3 ++ core/Cargo.toml | 1 - core/src/api/macros.rs | 4 +- core/src/types.rs | 66 ++++++++++++++++++++++++++----- src/mechanisms/chacha8poly1305.rs | 26 ++++++------ 5 files changed, 72 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8b6fb86053..848696e67ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `nonce` argument to `wrap_key` and `unwrap_key` syscalls. - Use nonce as IV for Aes256Cbc mechanism. - Updated `cbor-smol` to 0.5.0. +- Removed `serde::{Deserialize, Serialize}` implementations for the API request + and reply structs, `types::{consent::{Error, Level}, reboot::To, StorageAttributes, + KeySerialization, SignatureSerialization}`. ### Fixed diff --git a/core/Cargo.toml b/core/Cargo.toml index a6bbeccd2a0..0aba4f08608 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -14,7 +14,6 @@ littlefs2-core.workspace = true postcard.workspace = true rand_core.workspace = true serde.workspace = true - serde-indexed = "0.1" [features] diff --git a/core/src/api/macros.rs b/core/src/api/macros.rs index f6772fdae84..ec3160e9952 100644 --- a/core/src/api/macros.rs +++ b/core/src/api/macros.rs @@ -70,7 +70,7 @@ macro_rules! impl_request { )*) => {$( $(#[$attr])? - #[derive(Clone, Eq, PartialEq, Debug, serde_indexed::DeserializeIndexed, serde_indexed::SerializeIndexed)] + #[derive(Clone, Eq, PartialEq, Debug)] pub struct $request { $( pub $name: $type, @@ -109,7 +109,7 @@ macro_rules! impl_reply { => {$( $(#[$attr])? - #[derive(Clone, Eq, PartialEq, Debug, serde_indexed::DeserializeIndexed, serde_indexed::SerializeIndexed)] + #[derive(Clone, Eq, PartialEq, Debug)] pub struct $reply { $( pub $name: $type, diff --git a/core/src/types.rs b/core/src/types.rs index fa197a8c8df..cfec02a95c4 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -7,15 +7,14 @@ pub use heapless::String; pub use heapless_bytes::Bytes; pub use littlefs2_core::{DirEntry, Metadata, PathBuf}; +use crate::api::{reply, request}; use crate::config::{ MAX_KEY_MATERIAL_LENGTH, MAX_MEDIUM_DATA_LENGTH, MAX_MESSAGE_LENGTH, MAX_SHORT_DATA_LENGTH, MAX_SIGNATURE_LENGTH, MAX_USER_ATTRIBUTE_LENGTH, }; pub mod consent { - use serde::{Deserialize, Serialize}; - - #[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] + #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum Level { /// There is no user present None, @@ -27,7 +26,7 @@ pub mod consent { Strong, } - #[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] + #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum Error { FailedToInterrupt, Interrupted, @@ -39,9 +38,7 @@ pub mod consent { } pub mod reboot { - use serde::{Deserialize, Serialize}; - - #[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] + #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum To { Application, ApplicationUpdate, @@ -240,7 +237,7 @@ pub enum Location { External, } -#[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Clone, Eq, PartialEq, Debug)] #[non_exhaustive] pub struct StorageAttributes { // each object must have a unique ID @@ -350,7 +347,7 @@ pub enum Mechanism { Rsa4096Pkcs1v15, } -#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum KeySerialization { // Asn1Der, Cose, @@ -366,10 +363,59 @@ pub enum KeySerialization { Pkcs8Der, } -#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum SignatureSerialization { Asn1Der, // Cose, Raw, // Sec1, } + +/// Serializable version of [`reply::Encrypt`][]. +/// +/// Sometimes it is necessary the result of an encryption together with the metadata required for +/// decryption, for example when wrapping keys. This struct stores the data that is returned by +/// the [`request::Encrypt`][] syscall, see [`reply::Encrypt`][], in a serializable format. +#[derive( + Clone, Debug, Eq, PartialEq, serde_indexed::DeserializeIndexed, serde_indexed::SerializeIndexed, +)] +#[non_exhaustive] +pub struct EncryptedData { + pub ciphertext: Message, + pub nonce: ShortData, + pub tag: ShortData, +} + +impl EncryptedData { + /// Creates a decryption request to decrypt the stored data. + pub fn decrypt( + self, + mechanism: Mechanism, + key: KeyId, + associated_data: Message, + ) -> request::Decrypt { + request::Decrypt { + mechanism, + key, + message: self.ciphertext, + associated_data, + nonce: self.nonce, + tag: self.tag, + } + } +} + +impl From for EncryptedData { + fn from(reply: reply::Encrypt) -> Self { + let reply::Encrypt { + ciphertext, + nonce, + tag, + } = reply; + Self { + ciphertext, + nonce, + tag, + } + } +} diff --git a/src/mechanisms/chacha8poly1305.rs b/src/mechanisms/chacha8poly1305.rs index c19460decf2..07a7f305af2 100644 --- a/src/mechanisms/chacha8poly1305.rs +++ b/src/mechanisms/chacha8poly1305.rs @@ -1,5 +1,6 @@ use generic_array::GenericArray; use rand_core::RngCore; +use trussed_core::types::EncryptedData; use crate::api::{reply, request}; use crate::error::Error; @@ -190,8 +191,9 @@ impl WrapKey for super::Chacha8Poly1305 { }; let encryption_reply = ::encrypt(keystore, &encryption_request)?; + let wrapped_key = EncryptedData::from(encryption_reply); let wrapped_key = - crate::postcard_serialize_bytes(&encryption_reply).map_err(|_| Error::CborError)?; + crate::postcard_serialize_bytes(&wrapped_key).map_err(|_| Error::CborError)?; Ok(reply::WrapKey { wrapped_key }) } @@ -204,20 +206,14 @@ impl UnwrapKey for super::Chacha8Poly1305 { keystore: &mut impl Keystore, request: &request::UnwrapKey, ) -> Result { - let reply::Encrypt { - ciphertext, - nonce, - tag, - } = crate::postcard_deserialize(&request.wrapped_key).map_err(|_| Error::CborError)?; - - let decryption_request = request::Decrypt { - mechanism: Mechanism::Chacha8Poly1305, - key: request.wrapping_key, - message: ciphertext, - associated_data: request.associated_data.clone(), - nonce, - tag, - }; + let encrypted_data: EncryptedData = + crate::postcard_deserialize(&request.wrapped_key).map_err(|_| Error::CborError)?; + + let decryption_request = encrypted_data.decrypt( + Mechanism::Chacha8Poly1305, + request.wrapping_key, + request.associated_data.clone(), + ); let serialized_key = if let Some(serialized_key) = ::decrypt(keystore, &decryption_request)?.plaintext