Improve mechanism features
robin-nitrokey opened this issue · 2 comments
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:
- Mechanisms from the
default-mechanisms
feature set cannot be disabled as soon as one crate in the dependency tree pulls in trussed without disabling the default features. - Failure to enable a required mechanism leads to a potentially hard to debug runtime error instead of a compiler error.
- It is hard to synchronize the mechanism set between different crates. How can a custom backend detect whether it uses the same mechanism set as the
trussed
crate?
I’m not sure what the best solution would look like. I want to suggest the following as a basis for discussion:
- 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.
- The
Mechanism
enum only contains the enabled mechanisms. This ensures that missing mechanism features lead to a compiler error in the client. - 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:
#[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:
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());
};
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.
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.