trussed-dev / trussed

Modern Cryptographic Firmware
https://trussed.dev
Apache License 2.0
402 stars 26 forks source link

Improve mechanism features #159

Open robin-nitrokey opened 4 months ago

robin-nitrokey commented 4 months ago

Currently, the mechanisms are behind feature flags, but most of them are enabled by default via the default-mechanisms feature. The Mechanism enum always contains all mechanisms, not depending on the activated features.

This approach has some problems:

I’m not sure what the best solution would look like. I want to suggest the following as a basis for discussion:

  1. All mechanism features are disabled by default. Clients need to explicitly enable the mechanisms they require. This ensures that only the required mechanisms are actually enabled, improving binary size and stack usage.
  2. The Mechanism enum only contains the enabled mechanisms. This ensures that missing mechanism features lead to a compiler error in the client.
  3. We introduce a Mechanisms helper struct (see below) that can be used to trigger a compiler error if a backend or the firmware does not have the same mechanism set as Trussed.
Mechanisms In Trussed: ```rust #[non_exhaustive] pub struct Mechanisms { #[cfg(feature = "aes256-cbc")] pub aes256_cbc: bool, #[cfg(feature = "ed255")] pub ed255: bool, // ... } impl Mechanisms { pub const fn new() -> Self { Self { #[cfg(feature = "aes256-cbc")] aes256_cbc: false, #[cfg(feature = "ed255")] ed255: false, } } pub const fn all_supported(&self) -> bool { let mut all_supported = true; #[cfg(feature = "aes256-cbc")] all_supported = all_supported && self.aes256_cbc; #[cfg(feature = "ed255")] all_supported = all_supported && self.ed255; all_supported } } ``` In a backend or runner: ```rust const _ENSURE_MECHANISMS_SUPPORTED: () = { let mut mechanisms = trussed::Mechanisms::new(); #[cfg(feature = "aes256-cbc")] mechanisms.aes256_cbc = true; #[cfg(feature = "ed255")] mechanisms.ed255 = true; assert!(mechanisms.all_supported()); }; ```
sosthene-nitrokey commented 4 months ago

This would be a significant breaking change but I like it.

I also like that the required mechanisms are not one more trait bound. Maybe the const assertion in the backend or runner can be created through a macro.

robin-nitrokey commented 2 months ago

On second thought, a simpler solution instead of (3) would be to just add an ExhaustiveMechanism enum that implements From<Mechanism> and is documented to not follow semver.