Add multiple (Service) Backends abstraction

[daringer]

This adds the structure to allow multiple service backends. Essentially the client now has the possibility to choose a order-of-dispatch in its ClientContext. This is an elementary step towards transparent hardware crypto devices like the se050.

• Extend ClientId to be a proper struct and call it make it a ClientContext (as the name suggests)
• Fix several concurrency issues during directory traversal using the ClientContext instead of a quasi-global
• extend all necessary components and reply_to dispatching based on client-chosen service backends

[sosthene-nitrokey]

I've rebased it on top of 8e347abf99dba81fca11c5779a473d263a7b0565 in sosthene-nitrokey:client-context-rebased

[robin-nitrokey]

I think we could further modularize this PR by moving the backends out of Trussed. The backend enum could be defined together with the platform by the runner. The backend implementations (other than the software backend) could reside in separate crates. This would mean that we would have to make Request and the types that use it generic over the additional backends, but I think it would be a cleaner approach and would cause less overhead.

I’m currently polishing a draft implementation and will share it here in the next days for you to review.

[nickray]

Agreed that any and all non-default backends should be independent crates, and the integration designed as such. Honestly, even the software backend shouldn't be hardcoded into Trussed, but likely generated based on "constraints" of what is needed and which low-level implementations to use.

The generic approach has a danger of leading to a deluge of generics; this could be improved with more code generation (essentially the hardcoded/upstreamed approach, but on-demand and dynamic).

On the other hand, code generation can get arbitrarily complex. So maybe we can contain what varies in a few types that then actually get generated.

[robin-nitrokey]

For reference, here is my current implementation:

• https://github.com/Nitrokey/trussed/pull/9
• https://github.com/Nitrokey/trussed/pull/10

I’ll update this PR once we’ve finished testing the integration into the firmware. While this adds some additional type parameters, I think the approach using the const-generics interchange implementation is a good trade-off between type complexity and flexibility.

[robin-nitrokey]

this could be improved with more code generation

I’d rather reduce the amount of code generation in Trussed because I feel that the code generation macros often are not flexible enough and create additional restrictions (e. g. platform! does not support generics). My suggestion would be to provide simple defaults for the trivial cases (e. g. no custom backends) while allowing users to manually implement more complex cases.

[nickray]

Specific to your alternative PR, we had a lot more generic parameters in the past and got rid of them. E.g., code like

impl<B: 'static, I: TrussedInterchange<B>, S: Syscall> HmacBlake2s
    for ClientImplementation<B, I, S>

is repetitive and quite annoying to maintain. It will just get worse once we wish to add more flexibility. It also has no actual value in flexibility, as things are set (I think?) once and for all in setup of an actual build.

Understood that the current "macros by example" are limited. I'm not thinking of even more such macros, but rather a formal step that could go two ways (that I can think of):

a) in a build.rs that "somehow" is configured (it isn't really idiomatic to do a lot of work in build.rs though) a') annoying alternative pre-build step, that is more flexible than build.rs b) the "config crate idea", which I first heard of from James Munns

What is the config crate? Say we have a type like

    if #[cfg(feature = "clients-12")] {
        interchange::interchange! {
            TrussedInterchange: (Request, Result<Reply, Error>, 12)
        }

Here the clients-* features are already a hack, as Cargo features are only booleans.

The idea is this: Let trussed depend on trussed-config, which has some kind of default type definitions (e.g. TrussedInterchange as an interchange for 3 clients). Then inside trussed, we somewhere have

pub mod config {
  pub use trussed_config::*;
}

and everything else depends on trussed::config.

In firmware code (or PC runners, or examples, etc.), one defines a local crate (defining e.g. TrussedInterchange with 12 clients), and has a [patch] section in the main Cargo.toml to use this instead.

This approach localizes all the things that must vary in one place; the code depending on it in trussed has no generics anymore.

As a first step, these config crates would be "written by hand", but that gets painful too.

So then ideally, we have an actual code generator just for this (i.e. not Rust macros, but an actual dedicated software in Rust or Python), which itself can be driven from a TOML or other config file.

Or maybe even the users don't need to supply these crates, and just supply a trussed.toml config file, which trussed's build.rs uses to run the code generator on.

An alternative is to tie all these generic parameters here, so we'd still model them properly in your sense, but trussed only sees the generic-free specific instantiation.

This build step could/should also be able to run the "dependency resolver" that analyzes apps for their cryptographic and other needs, and generates a specific software backend that has just that.

Understood that this is just a sketch and at most a basis for a deeper discussion. I do think that the question "how do we configure Trussed" is core and central to Trussed's success, and applicability in a variety of use cases (even just SoloKeys vs Nitrokey, but even more so beyond these to desktops + servers).

[robin-nitrokey]

I would separate three different cases:

• Firstly, we have types like Client that are the main Trussed API and are used in all components related to Trussed. I agree that they should be as simple as possible.
• Secondly, we have types like Platform and Service. They are mostly used in the runners. While we should aim to keep them simple, I think adding some complexity is okay here.
• Finally, we have low-level types like ClientImplementation. They are mostly only used in Trussed itself so I think adding complexity to these types should not be an issue.

It also has no actual value in flexibility, as things are set (I think?) once and for all in setup of an actual build.

This might be true for the runners themselves. But in the last weeks, I’ve been working with virtual Trussed devices for unit tests and integration tests. Here the added flexibility really helps.

the "config crate idea", which I first heard of from James Munns

Interesting idea! I’ll have to think about this some more time. Currently I fear this would make the build process a lot harder to manage. It is already quite painful to keep track of patched dependencies in different repositories, and adding a dependency that always has to be patched does not sound fun.

I’d prefer to tackle this problem with a configuration trait instead. Parameters like the backend types or the request size could be associated types or constants of a Config trait. There could be a default implementation for () or, if that is not possible, a macro that generates simple implementations. For more complex cases, users could manually implement that trait. We use a similar approach to manage our runner for multiple platforms, see e. g. https://github.com/Nitrokey/nitrokey-3-firmware/blob/a7518fab60ebf5f95802f80c08e960f63ffd1f9d/runners/embedded/src/types.rs#L34-L57.

Regarding the specific code you mentioned, it gets a bit easier to read by using the const-generics interchange implementation:

impl<B: 'static, S: Syscall> HmacBlake2s for ClientImplementation<'_, B, S> {}

https://github.com/robin-nitrokey/trussed/blob/client-context-interchange/src/client/mechanisms.rs

Furthermore, it allows us to drop the clients-* features and just calculate the client count instead:

pub const CLIENT_COUNT: usize =
    cfg!(feature = "admin-app") as usize +
    cfg!(feature = "fido-authenticator") as usize +
    cfg!(feature = "oauth-authenticator") as usize +
    cfg!(feature = "opcard") as usize +
    cfg!(feature = "provisioner-app") as usize;
pub type Service<'a, P> = Service<'a, P, (), CLIENT_COUNT>;

[nickray]

Yes, a Config trait might work for some use cases too. The overarching message I'm trying to send is to keep the overall generic count in any given part of the code as low as possible.

I feel a little sad when I see ClientImplementation<S> turn into ClientImplementation<'a, B: 'static, S>, and all impl blocks turn from impl<S: Syscall> HmacSha256 for ClientImplementation<S> to have the verbose impl<B: 'static, S: Syscall> HmacSha256 for ClientImplementation<'_, B, S>.

--

I think the config crate patching is different from the patches you mention, it's an "always-on" configuration of the specific build, not some externality.

--

That's a cute trick to count the clients :)

[robin-nitrokey]

Closing in favor of https://github.com/trussed-dev/trussed/pull/68