Skip to content

Commit

Permalink
Remove serde trait implementations for requests and replies
Browse files Browse the repository at this point in the history
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: #183
  • Loading branch information
robin-nitrokey committed Dec 17, 2024
1 parent 1b62220 commit df2867d
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 28 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ littlefs2-core.workspace = true
postcard.workspace = true
rand_core.workspace = true
serde.workspace = true

serde-indexed = "0.1"

[features]
Expand Down
4 changes: 2 additions & 2 deletions core/src/api/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
66 changes: 56 additions & 10 deletions core/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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<reply::Encrypt> for EncryptedData {
fn from(reply: reply::Encrypt) -> Self {
let reply::Encrypt {
ciphertext,
nonce,
tag,
} = reply;
Self {
ciphertext,
nonce,
tag,
}
}
}
26 changes: 11 additions & 15 deletions src/mechanisms/chacha8poly1305.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -190,8 +191,9 @@ impl WrapKey for super::Chacha8Poly1305 {
};
let encryption_reply = <super::Chacha8Poly1305>::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 })
}
Expand All @@ -204,20 +206,14 @@ impl UnwrapKey for super::Chacha8Poly1305 {
keystore: &mut impl Keystore,
request: &request::UnwrapKey,
) -> Result<reply::UnwrapKey, Error> {
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) =
<super::Chacha8Poly1305>::decrypt(keystore, &decryption_request)?.plaintext
Expand Down

0 comments on commit df2867d

Please sign in to comment.