Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve mechanism features #159

Closed
Tracked by #177
robin-nitrokey opened this issue Apr 2, 2024 · 3 comments · Fixed by #187
Closed
Tracked by #177

Improve mechanism features #159

robin-nitrokey opened this issue Apr 2, 2024 · 3 comments · Fixed by #187

Comments

@robin-nitrokey
Copy link
Member

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:

  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:

#[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());
};
@sosthene-nitrokey
Copy link
Contributor

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
Copy link
Member Author

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.

@robin-nitrokey
Copy link
Member Author

robin-nitrokey commented Dec 16, 2024

Actually, there is a much simpler approach: have a Mechanism::ALL constant with the activated mechanisms. You can then iterate and match over the elements of this constant in a const assertion and panic if there is an unexpected entry.

Edit: I guess Mechanism::ENABLED is a better name than Mechanism::ALL.

robin-nitrokey added a commit that referenced this issue Dec 19, 2024
This makes sure that applications explicitly enable those mechanism
that they need and unused mechanisms can easily be disabled.

Fixes: #159
robin-nitrokey added a commit that referenced this issue Dec 19, 2024
This makes sure that applications explicitly enable those mechanism
that they need and unused mechanisms can easily be disabled.

Fixes: #159
robin-nitrokey added a commit that referenced this issue Dec 19, 2024
This makes sure that applications explicitly enable those mechanism
that they need and unused mechanisms can easily be disabled.

Fixes: #159
robin-nitrokey added a commit that referenced this issue Dec 19, 2024
This makes sure that applications explicitly enable those mechanism
that they need and unused mechanisms can easily be disabled.

Fixes: #159
robin-nitrokey added a commit that referenced this issue Dec 19, 2024
This makes sure that applications explicitly enable those mechanism
that they need and unused mechanisms can easily be disabled.

Fixes: #159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants