Provide eps in Service::process

This patch decouples the Service from the ServiceEndpoint instances.
This gives us more flexibility when constructing the clients and removes
the need for the hard-coded client count in Service.

CHANGELOG.md

@@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Put all client traits, requests, replies and implementations behind feature flags.
   - Put all mechanisms behind feature flags.
   - Move `CryptoClient::attest` into new `AttestationClient`.
+- Pass endpoints to `Service::process` instead of storing them in the service.
 
 ### Fixed
 

src/pipe.rs

@@ -8,7 +8,7 @@ use crate::api::{Reply, Request};
 use crate::backend::BackendId;
 use crate::config;
 use crate::error::Error;
-use crate::types::Context;
+use crate::types::{Context, CoreContext};
 
 type TrussedInterchangeInner =
     Interchange<Request, Result<Reply, Error>, { config::MAX_SERVICE_CLIENTS }>;
@@ -31,11 +31,25 @@ pub type TrussedRequester = Requester<'static, Request, Result<Reply, Error>>;
 // https://doc.micrium.com/display/osiiidoc/Using+Message+Queues
 
 pub struct ServiceEndpoint<I: 'static, C> {
-    pub interchange: TrussedResponder,
+    pub(crate) interchange: TrussedResponder,
     // service (trusted) has this, not client (untrusted)
     // used among other things to namespace cryptographic material
-    pub ctx: Context<C>,
-    pub backends: &'static [BackendId<I>],
+    pub(crate) ctx: Context<C>,
+    pub(crate) backends: &'static [BackendId<I>],
+}
+
+impl<I: 'static, C: Default> ServiceEndpoint<I, C> {
+    pub fn new(
+        interchange: TrussedResponder,
+        context: CoreContext,
+        backends: &'static [BackendId<I>],
+    ) -> Self {
+        Self {
+            interchange,
+            ctx: context.into(),
+            backends,
+        }
+    }
 }
 
 // pub type ClientEndpoint = Requester<TrussedInterchange>;

src/service.rs

@@ -2,14 +2,14 @@ use littlefs2_core::{path, DynFilesystem, Path, PathBuf};
 use rand_chacha::ChaCha8Rng;
 pub use rand_core::{RngCore, SeedableRng};
 
+use crate::api::{reply, request, Reply, Request};
 use crate::backend::{BackendId, CoreOnly, Dispatch};
-use crate::config::{MAX_MESSAGE_LENGTH, MAX_SERVICE_CLIENTS};
+use crate::config::MAX_MESSAGE_LENGTH;
 use crate::error::{Error, Result};
 pub use crate::key;
 #[cfg(feature = "crypto-client")]
 use crate::mechanisms;
 pub use crate::pipe::ServiceEndpoint;
-use crate::pipe::TrussedResponder;
 use crate::platform::{consent, ui, Platform, Store, UserInterface};
 pub use crate::store::{
     self,
@@ -19,12 +19,8 @@ pub use crate::store::{
     keystore::{ClientKeystore, Keystore},
 };
 use crate::types::ui::Status;
-use crate::types::{Context, CoreContext, Location, Mechanism, MediumData, Message, Vec};
+use crate::types::{Context, CoreContext, Location, Mechanism, MediumData, Message};
 use crate::Bytes;
-use crate::{
-    api::{reply, request, Reply, Request},
-    interrupt::InterruptFlag,
-};
 
 #[cfg(feature = "attestation-client")]
 pub mod attest;
@@ -90,7 +86,6 @@ where
     P: Platform,
     D: Dispatch,
 {
-    eps: Vec<ServiceEndpoint<D::BackendId, D::Context>, { MAX_SERVICE_CLIENTS }>,
     resources: ServiceResources<P>,
     dispatch: D,
 }
@@ -943,34 +938,13 @@ impl<P: Platform, D: Dispatch> Service<P, D> {
     pub fn with_dispatch(platform: P, dispatch: D) -> Self {
         let resources = ServiceResources::new(platform);
         Self {
-            eps: Vec::new(),
             resources,
             dispatch,
         }
     }
 }
 
 impl<P: Platform, D: Dispatch> Service<P, D> {
-    pub fn add_endpoint(
-        &mut self,
-        interchange: TrussedResponder,
-        client: impl Into<PathBuf>,
-        backends: &'static [BackendId<D::BackendId>],
-        interrupt: Option<&'static InterruptFlag>,
-    ) -> Result<(), Error> {
-        let core_ctx = CoreContext::with_interrupt(client.into(), interrupt);
-        if &*core_ctx.path == path!("trussed") {
-            panic!("trussed is a reserved client ID");
-        }
-        self.eps
-            .push(ServiceEndpoint {
-                interchange,
-                ctx: core_ctx.into(),
-                backends,
-            })
-            .map_err(|_| Error::ClientCountExceeded)
-    }
-
     pub fn set_seed_if_uninitialized(&mut self, seed: &[u8; 32]) {
         let mut filestore = self.resources.trussed_filestore();
         let path = path!("rng-state.bin");
@@ -993,35 +967,35 @@ impl<P: Platform, D: Dispatch> Service<P, D> {
     }
 
     // process one request per client which has any
-    pub fn process(&mut self) {
-        // split self since we iter-mut over eps and need &mut of the other resources
-        let eps = &mut self.eps;
-        let resources = &mut self.resources;
-
-        for ep in eps.iter_mut() {
+    pub fn process(&mut self, eps: &mut [ServiceEndpoint<D::BackendId, D::Context>]) {
+        for ep in eps {
             if let Ok(request) = ep.interchange.request() {
-                resources
+                self.resources
                     .platform
                     .user_interface()
                     .set_status(ui::Status::Processing);
                 // #[cfg(test)] println!("service got request: {:?}", &request);
 
                 // resources.currently_serving = ep.client_id.clone();
                 let reply_result = if ep.backends.is_empty() {
-                    resources.reply_to(&mut ep.ctx.core, request)
+                    self.resources.reply_to(&mut ep.ctx.core, request)
                 } else {
                     let mut reply_result = Err(Error::RequestNotAvailable);
                     for backend in ep.backends {
-                        reply_result =
-                            resources.dispatch(&mut self.dispatch, backend, &mut ep.ctx, request);
+                        reply_result = self.resources.dispatch(
+                            &mut self.dispatch,
+                            backend,
+                            &mut ep.ctx,
+                            request,
+                        );
                         if reply_result != Err(Error::RequestNotAvailable) {
                             break;
                         }
                     }
                     reply_result
                 };
 
-                resources
+                self.resources
                     .platform
                     .user_interface()
                     .set_status(ui::Status::Idle);

src/tests.rs

@@ -126,11 +126,14 @@ type Memory = (
     &'static mut VolatileStorage,
 );
 
-struct ServiceSyscall<'a, P: platform::Platform>(&'a mut crate::Service<P>);
+struct ServiceSyscall<P: platform::Platform> {
+    service: crate::Service<P>,
+    ep: crate::pipe::ServiceEndpoint<crate::backend::NoId, crate::types::NoData>,
+}
 
-impl<P: platform::Platform> platform::Syscall for ServiceSyscall<'_, P> {
+impl<P: platform::Platform> platform::Syscall for ServiceSyscall<P> {
     fn syscall(&mut self) {
-        self.0.process();
+        self.service.process(core::slice::from_mut(&mut self.ep));
     }
 }
 
@@ -182,17 +185,15 @@ macro_rules! setup {
             .claim()
             .expect("could not setup TEST TrussedInterchange");
         let test_client_id = path!("TEST");
-
-        assert!(trussed
-            .add_endpoint(test_trussed_responder, test_client_id, &[], None)
-            .is_ok());
+        let context = crate::types::CoreContext::new(test_client_id.into());
+        let ep = crate::pipe::ServiceEndpoint::new(test_trussed_responder, context, &[]);
 
         trussed.set_seed_if_uninitialized(&$seed);
-        let mut $client = {
-            pub type TestClient<'a> =
-                crate::ClientImplementation<ServiceSyscall<'a, $platform>>;
-            TestClient::new(test_trussed_requester, ServiceSyscall(&mut trussed), None)
+        let syscall = ServiceSyscall {
+            service: trussed,
+            ep,
         };
+        let mut $client = crate::ClientImplementation::<_>::new(test_trussed_requester, syscall, None);
     };
 }
 

src/types.rs

@@ -126,15 +126,13 @@ pub struct CoreContext {
 
 impl CoreContext {
     pub fn new(path: PathBuf) -> Self {
-        Self {
-            path,
-            read_dir_state: None,
-            read_dir_files_state: None,
-            interrupt: None,
-        }
+        Self::with_interrupt(path, None)
     }
 
     pub fn with_interrupt(path: PathBuf, interrupt: Option<&'static InterruptFlag>) -> Self {
+        if path.as_str() == "trussed" {
+            panic!("trussed is a reserved client ID");
+        }
         Self {
             path,
             read_dir_state: None,
@@ -144,18 +142,6 @@ impl CoreContext {
     }
 }
 
-impl From<PathBuf> for CoreContext {
-    fn from(path: PathBuf) -> Self {
-        Self::new(path)
-    }
-}
-
-impl From<&str> for CoreContext {
-    fn from(s: &str) -> Self {
-        Self::new(s.try_into().unwrap())
-    }
-}
-
 // Object Hierarchy according to Cryptoki
 // - Storage
 //   - Domain parameters

src/virt.rs

@@ -20,9 +20,10 @@ use rand_core::SeedableRng as _;
 
 use crate::{
     backend::{BackendId, CoreOnly, Dispatch},
-    pipe::TRUSSED_INTERCHANGE,
+    pipe::{ServiceEndpoint, TrussedResponder, TRUSSED_INTERCHANGE},
     platform,
     service::Service,
+    types::CoreContext,
     ClientImplementation,
 };
 
@@ -125,36 +126,53 @@ impl platform::Syscall for Syscall {
     }
 }
 
-struct Runner {
+struct Runner<I: 'static, C> {
     syscall_tx: Sender<()>,
     syscall_rx: Receiver<()>,
+    eps: Vec<ServiceEndpoint<I, C>>,
 }
 
-impl Runner {
+impl<I: 'static, C: Default> Runner<I, C> {
     fn new() -> Self {
         let (syscall_tx, syscall_rx) = mpsc::channel();
         Self {
             syscall_tx,
             syscall_rx,
+            eps: Vec::new(),
         }
     }
 
     fn syscall(&self) -> Syscall {
         Syscall(self.syscall_tx.clone())
     }
 
-    fn run<S, D, F, R>(self, mut service: Service<Platform<S>, D>, f: F) -> R
+    fn add_endpoint(
+        &mut self,
+        responder: TrussedResponder,
+        client_id: &str,
+        backends: &'static [BackendId<I>],
+    ) {
+        let context = CoreContext::new(client_id.try_into().unwrap());
+        self.eps
+            .push(ServiceEndpoint::new(responder, context, backends));
+    }
+
+    fn run<P, D, F, R>(self, platform: P, dispatch: D, f: F) -> R
     where
-        S: StoreProvider,
-        D: Dispatch,
+        P: platform::Platform,
+        D: Dispatch<Context = C, BackendId = I>,
+        C: Send + Sync,
+        I: Send + Sync,
         F: FnOnce() -> R,
     {
         let (stop_tx, stop_rx) = mpsc::channel();
+        let mut service = Service::with_dispatch(platform, dispatch);
         thread::scope(|s| {
             s.spawn(move || {
+                let mut eps = self.eps;
                 while stop_rx.try_recv().is_err() {
                     if self.syscall_rx.try_recv().is_ok() {
-                        service.process();
+                        service.process(&mut eps);
                     }
                 }
             });
@@ -182,34 +200,30 @@ impl<S: StoreProvider> Platform<S> {
         dispatch: D,
         backends: &'static [BackendId<D::BackendId>],
         test: impl FnOnce(Client<D>) -> R,
-    ) -> R {
-        let runner = Runner::new();
-        let mut service = Service::with_dispatch(self, dispatch);
-        let client_id = littlefs2::path::PathBuf::try_from(client_id).unwrap();
+    ) -> R
+    where
+        D::Context: Send + Sync,
+        D::BackendId: Send + Sync,
+    {
+        let mut runner = Runner::new();
         let (requester, responder) = TRUSSED_INTERCHANGE.claim().unwrap();
-        service
-            .add_endpoint(responder, client_id, backends, None)
-            .unwrap();
+        runner.add_endpoint(responder, client_id, backends);
         let client = Client::new(requester, runner.syscall(), None);
-        runner.run(service, || test(client))
+        runner.run(self, dispatch, || test(client))
     }
 
     pub fn run_clients<R, const N: usize>(
         self,
         client_ids: [&str; N],
         test: impl FnOnce([Client; N]) -> R,
     ) -> R {
-        let runner = Runner::new();
-        let mut service = Service::new(self);
+        let mut runner = Runner::new();
         let clients = client_ids.map(|id| {
-            let client_id = littlefs2::path::PathBuf::try_from(id).unwrap();
             let (requester, responder) = TRUSSED_INTERCHANGE.claim().unwrap();
-            service
-                .add_endpoint(responder, client_id, &[], None)
-                .unwrap();
+            runner.add_endpoint(responder, id, &[]);
             Client::new(requester, runner.syscall(), None)
         });
-        runner.run(service, || test(clients))
+        runner.run(self, CoreOnly, || test(clients))
     }
 
     /// Using const generics rather than a `Vec` to allow destructuring in the method
@@ -218,18 +232,18 @@ impl<S: StoreProvider> Platform<S> {
         client_ids: [(&str, &'static [BackendId<D::BackendId>]); N],
         dispatch: D,
         test: impl FnOnce([Client<D>; N]) -> R,
-    ) -> R {
-        let runner = Runner::new();
-        let mut service = Service::with_dispatch(self, dispatch);
+    ) -> R
+    where
+        D::Context: Send + Sync,
+        D::BackendId: Send + Sync,
+    {
+        let mut runner = Runner::new();
         let clients = client_ids.map(|(id, backends)| {
-            let client_id = littlefs2::path::PathBuf::try_from(id).unwrap();
             let (requester, responder) = TRUSSED_INTERCHANGE.claim().unwrap();
-            service
-                .add_endpoint(responder, client_id, backends, None)
-                .unwrap();
+            runner.add_endpoint(responder, id, backends);
             Client::new(requester, runner.syscall(), None)
         });
-        runner.run(service, || test(clients))
+        runner.run(self, dispatch, || test(clients))
     }
 }