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

Put clients behind feature flags #185

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Put clients behind feature flags #185

merged 4 commits into from
Dec 19, 2024

Conversation

robin-nitrokey
Copy link
Member

We already have feature flags for some clients and syscalls that are not used in the Nitrokey 3 firmware. This PR introduces feature flags for all client traits and also puts the request and reply types behind these feature flags. In detail:

  • Introduce client features both for trussed-core and trussed as well as an all-clients feature for trussed. Applications are encouraged to depend on trussed-core and only activate the features they need.
  • Also put the request and reply structs behind these features. Otherwise there could be confusion as I could send a syscall directly using PollClient. The types used in these structs are always available to keep things simple and as they can be easily optimized out by the compiler or linker.
  • Move CryptoClient::attest into a separate AttestClient (or maybe AttestationClient?). Putting trait methods behind a feature flag is problematic because it means that the feature is non-additive. As we already have the client trait mechanism to separate feature sets and make them optional, we can also use it for attestation.
  • Introduce a Client enum with an ALL constant. This makes it possible for backends to use const assertions to ensure that all required features are activated. I don’t think we will use this much, but I added it for completeness. This will be more important for mechanisms and is a much nicer solution than what we discussed before (Improve mechanism features #159).

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@robin-nitrokey
Copy link
Member Author

robin-nitrokey commented Dec 16, 2024

Updated:

  • Use a for loop to iterate the features in the workflow definition.
  • Rename Client::ALL to Client::ENABLED.

Any opinions on AttestClient vs AttestationClient? I would now prefer AttestationClient.

Putting trait methods behind feature flags is not ideal because it means
that the feature is not additive.  As we already have the different
client traits, the easiest way to implement an optional feature is to
introduce a separate trait.
This enum can be used in const assertions to ensure that all required
crate features have been selected, for example in backends.
@robin-nitrokey robin-nitrokey merged commit e229e33 into main Dec 19, 2024
2 checks passed
@robin-nitrokey robin-nitrokey deleted the client-features branch December 19, 2024 09:58
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 this pull request may close these issues.

2 participants