From 076c6e4815cd89df9aa24c491ccf1d491985a9f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 15 May 2023 17:17:11 +0200 Subject: [PATCH 1/6] Acknowledge cancellations in the interchange --- src/service.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/service.rs b/src/service.rs index 1a923b3f357..25ce57f5124 100644 --- a/src/service.rs +++ b/src/service.rs @@ -813,7 +813,15 @@ impl Service { .platform .user_interface() .set_status(ui::Status::Idle); - ep.interchange.respond(reply_result).ok(); + match ep.interchange.respond(reply_result) { + Ok(()) => {} + Err(_) => { + info!("Cancelled request"); + if ep.interchange.is_canceled() { + ep.interchange.acknowledge_cancel().ok(); + } + } + }; } } debug_now!( From ca2964c2da3b197265d25940e1482ededbfd707c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 17 May 2023 15:34:06 +0200 Subject: [PATCH 2/6] Update filestore to only require the client id Trussed-auth needs to create filestores with a custom path for its own data. It did so by creating a "fake" CoreContext, which will be harder to do because of the interrupt. --- src/service.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/service.rs b/src/service.rs index 25ce57f5124..4089dcb49ff 100644 --- a/src/service.rs +++ b/src/service.rs @@ -106,8 +106,8 @@ impl ServiceResources

{ .map_err(|_| Error::EntropyMalfunction) } - pub fn filestore(&mut self, ctx: &CoreContext) -> ClientFilestore { - ClientFilestore::new(ctx.path.clone(), self.platform.store()) + pub fn filestore(&mut self, client_id: PathBuf) -> ClientFilestore { + ClientFilestore::new(client_id, self.platform.store()) } pub fn trussed_filestore(&mut self) -> ClientFilestore { @@ -143,7 +143,7 @@ impl ServiceResources

{ let keystore = &mut self.keystore(ctx)?; let certstore = &mut self.certstore(ctx)?; let counterstore = &mut self.counterstore(ctx)?; - let filestore = &mut self.filestore(ctx); + let filestore = &mut self.filestore(ctx.path.clone()); debug_now!("TRUSSED {:?}", request); match request { From 53e0aeb32ba6aa2963af6cfc12703bff51e990f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 22 May 2023 17:40:10 +0200 Subject: [PATCH 3/6] Add interrupt support --- src/client.rs | 43 ++++++++++++++++++++++++---- src/interrupt.rs | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/service.rs | 19 +++++++++++-- src/types.rs | 13 ++++++++- src/virt.rs | 2 +- 6 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 src/interrupt.rs diff --git a/src/client.rs b/src/client.rs index e73c32b84c4..037bed2c552 100644 --- a/src/client.rs +++ b/src/client.rs @@ -80,6 +80,7 @@ use core::{marker::PhantomData, task::Poll}; use crate::api::*; use crate::backend::{BackendId, CoreOnly, Dispatch}; use crate::error::*; +use crate::interrupt::InterruptFlag; use crate::pipe::{TrussedRequester, TRUSSED_INTERCHANGE}; use crate::service::Service; use crate::types::*; @@ -113,6 +114,9 @@ impl Client for ClientImplementation {} pub trait PollClient { fn request(&mut self, req: Rq) -> ClientResult<'_, Rq::Reply, Self>; fn poll(&mut self) -> Poll>; + fn interrupt(&self) -> Option<&'static InterruptFlag> { + None + } } pub struct FutureResult<'c, T, C: ?Sized> @@ -148,6 +152,7 @@ pub struct ClientImplementation { // RawClient: pub(crate) interchange: TrussedRequester, + pub(crate) interrupt: Option<&'static InterruptFlag>, // pending: Option>, pending: Option, _marker: PhantomData, @@ -165,11 +170,16 @@ impl ClientImplementation where S: Syscall, { - pub fn new(interchange: TrussedRequester, syscall: S) -> Self { + pub fn new( + interchange: TrussedRequester, + syscall: S, + interrupt: Option<&'static InterruptFlag>, + ) -> Self { Self { interchange, pending: None, syscall, + interrupt, _marker: Default::default(), } } @@ -205,7 +215,14 @@ where } } } - None => Poll::Pending, + None => { + debug_assert_ne!( + self.interchange.state(), + interchange::State::Idle, + "requests can't be cancelled" + ); + Poll::Pending + } } } @@ -227,6 +244,10 @@ where self.syscall.syscall(); Ok(FutureResult::new(self)) } + + fn interrupt(&self) -> Option<&'static InterruptFlag> { + self.interrupt + } } impl CertificateClient for ClientImplementation {} @@ -701,6 +722,7 @@ pub trait UiClient: PollClient { pub struct ClientBuilder { id: PathBuf, backends: &'static [BackendId], + interrupt: Option<&'static InterruptFlag>, } impl ClientBuilder { @@ -712,6 +734,7 @@ impl ClientBuilder { Self { id: id.into(), backends: &[], + interrupt: None, } } } @@ -727,9 +750,14 @@ impl ClientBuilder { ClientBuilder { id: self.id, backends, + interrupt: self.interrupt, } } + pub fn interrupt(self, interrupt: Option<&'static InterruptFlag>) -> Self { + Self { interrupt, ..self } + } + fn create_endpoint( self, service: &mut Service, @@ -737,7 +765,7 @@ impl ClientBuilder { let (requester, responder) = TRUSSED_INTERCHANGE .claim() .ok_or(Error::ClientCountExceeded)?; - service.add_endpoint(responder, self.id, self.backends)?; + service.add_endpoint(responder, self.id, self.backends, self.interrupt)?; Ok(requester) } @@ -749,8 +777,9 @@ impl ClientBuilder { self, service: &mut Service, ) -> Result, Error> { + let interrupt = self.interrupt; self.create_endpoint(service) - .map(|requester| PreparedClient::new(requester)) + .map(|requester| PreparedClient::new(requester, interrupt)) } } @@ -761,20 +790,22 @@ impl ClientBuilder { /// implementation. pub struct PreparedClient { requester: TrussedRequester, + interrupt: Option<&'static InterruptFlag>, _marker: PhantomData, } impl PreparedClient { - fn new(requester: TrussedRequester) -> Self { + fn new(requester: TrussedRequester, interrupt: Option<&'static InterruptFlag>) -> Self { Self { requester, + interrupt, _marker: Default::default(), } } /// Builds the client using the given syscall implementation. pub fn build(self, syscall: S) -> ClientImplementation { - ClientImplementation::new(self.requester, syscall) + ClientImplementation::new(self.requester, syscall, self.interrupt) } } diff --git a/src/interrupt.rs b/src/interrupt.rs new file mode 100644 index 00000000000..e2c53996169 --- /dev/null +++ b/src/interrupt.rs @@ -0,0 +1,74 @@ +use core::{ + fmt::Debug, + sync::atomic::{AtomicU8, Ordering::Relaxed}, +}; + +#[derive(Default, Debug, PartialEq, Eq)] +pub enum InterruptState { + #[default] + Idle = 0, + Working = 1, + Interrupted = 2, +} + +impl TryFrom for InterruptState { + type Error = (); + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::Idle), + 1 => Ok(Self::Working), + 2 => Ok(Self::Interrupted), + _ => Err(()), + } + } +} + +impl From for u8 { + fn from(value: InterruptState) -> Self { + value as _ + } +} + +#[derive(Default)] +pub struct InterruptFlag(AtomicU8); + +const CONV_ERROR: &str = + "Internal trussed error: InterruptState must always be set to an enum variant"; + +impl InterruptFlag { + pub const fn new() -> Self { + Self(AtomicU8::new(0)) + } + fn load(&self) -> InterruptState { + self.0.load(Relaxed).try_into().expect(CONV_ERROR) + } + + pub fn set_idle(&self) { + self.0.store(InterruptState::Idle.into(), Relaxed) + } + pub fn set_working(&self) { + self.0.store(InterruptState::Working.into(), Relaxed) + } + pub fn interrupt(&self) -> bool { + self.0 + .compare_exchange( + InterruptState::Working.into(), + InterruptState::Interrupted.into(), + Relaxed, + Relaxed, + ) + .is_ok() + } + + pub fn is_interrupted(&self) -> bool { + let res = self.load(); + info_now!("got interrupt state: {:?}", res); + res == InterruptState::Interrupted + } +} + +impl Debug for InterruptFlag { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + self.load().fmt(f) + } +} diff --git a/src/lib.rs b/src/lib.rs index 177dd376c01..1800550d70e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,6 +27,7 @@ pub mod backend; pub mod client; pub mod config; pub mod error; +pub mod interrupt; pub mod key; pub mod mechanisms; pub mod pipe; diff --git a/src/service.rs b/src/service.rs index 4089dcb49ff..06c16a64500 100644 --- a/src/service.rs +++ b/src/service.rs @@ -5,7 +5,6 @@ use littlefs2::{ use rand_chacha::ChaCha8Rng; pub use rand_core::{RngCore, SeedableRng}; -use crate::api::*; use crate::backend::{BackendId, CoreOnly, Dispatch}; use crate::client::{ClientBuilder, ClientImplementation}; use crate::config::*; @@ -23,6 +22,7 @@ pub use crate::store::{ }; use crate::types::*; use crate::Bytes; +use crate::{api::*, interrupt::InterruptFlag}; pub mod attest; @@ -523,6 +523,12 @@ impl ServiceResources

{ let previous_status = self.platform.user_interface().status(); self.platform.user_interface().set_status(ui::Status::WaitingForUserPresence); loop { + if ctx.interrupt.map(|i|i.is_interrupted()) == Some(true) { + // Error does not matter as it will be dropped anyway + info_now!("User presence request cancelled"); + return Ok(reply::RequestUserConsent{result: Err(consent::Error::Interrupted)}.into()); + } + self.platform.user_interface().refresh(); let nowtime = self.platform.user_interface().uptime(); if (nowtime - starttime) > timeout { @@ -709,8 +715,10 @@ impl Service

{ &mut self, client_id: &str, syscall: S, + interrupt: Option<&'static InterruptFlag>, ) -> Result, Error> { ClientBuilder::new(client_id) + .interrupt(interrupt) .prepare(self) .map(|p| p.build(syscall)) } @@ -721,8 +729,10 @@ impl Service

{ pub fn try_as_new_client( &mut self, client_id: &str, + interrupt: Option<&'static InterruptFlag>, ) -> Result, Error> { ClientBuilder::new(client_id) + .interrupt(interrupt) .prepare(self) .map(|p| p.build(self)) } @@ -732,8 +742,10 @@ impl Service

{ pub fn try_into_new_client( mut self, client_id: &str, + interrupt: Option<&'static InterruptFlag>, ) -> Result, Error> { ClientBuilder::new(client_id) + .interrupt(interrupt) .prepare(&mut self) .map(|p| p.build(self)) } @@ -743,10 +755,11 @@ impl Service { pub fn add_endpoint( &mut self, interchange: TrussedResponder, - core_ctx: impl Into, + client: impl Into, backends: &'static [BackendId], + interrupt: Option<&'static InterruptFlag>, ) -> Result<(), Error> { - let core_ctx = core_ctx.into(); + let core_ctx = CoreContext::with_interrupt(client.into(), interrupt); if &*core_ctx.path == path!("trussed") { panic!("trussed is a reserved client ID"); } diff --git a/src/types.rs b/src/types.rs index 81ded044bf8..46548139e64 100644 --- a/src/types.rs +++ b/src/types.rs @@ -18,8 +18,8 @@ use rand_core::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; use crate::config::*; -use crate::key::Secrecy; use crate::store::filestore::{ReadDirFilesState, ReadDirState}; +use crate::{interrupt::InterruptFlag, key::Secrecy}; pub use crate::client::FutureResult; pub use crate::platform::Platform; @@ -267,6 +267,7 @@ pub struct CoreContext { pub path: PathBuf, pub read_dir_state: Option, pub read_dir_files_state: Option, + pub interrupt: Option<&'static InterruptFlag>, } impl CoreContext { @@ -275,6 +276,16 @@ impl CoreContext { path, read_dir_state: None, read_dir_files_state: None, + interrupt: None, + } + } + + pub fn with_interrupt(path: PathBuf, interrupt: Option<&'static InterruptFlag>) -> Self { + Self { + path, + read_dir_state: None, + read_dir_files_state: None, + interrupt, } } } diff --git a/src/virt.rs b/src/virt.rs index a6ddfb7c512..f81fc1ba12a 100644 --- a/src/virt.rs +++ b/src/virt.rs @@ -84,7 +84,7 @@ impl Platform { test: impl FnOnce(ClientImplementation>) -> R, ) -> R { let service = Service::new(self); - let client = service.try_into_new_client(client_id).unwrap(); + let client = service.try_into_new_client(client_id, None).unwrap(); test(client) } From 13847d0806647f4baa8478041295859d11069ce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 23 May 2023 14:15:01 +0200 Subject: [PATCH 4/6] Fix tests --- src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index af4b9320ce2..b782685c0cf 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -184,14 +184,14 @@ macro_rules! setup { let test_client_id = "TEST"; assert!(trussed - .add_endpoint(test_trussed_responder, test_client_id, &[]) + .add_endpoint(test_trussed_responder, test_client_id, &[], None) .is_ok()); trussed.set_seed_if_uninitialized(&$seed); let mut $client = { pub type TestClient<'a> = crate::ClientImplementation<&'a mut crate::Service<$platform>>; - TestClient::new(test_trussed_requester, &mut trussed) + TestClient::new(test_trussed_requester, &mut trussed, None) }; }; } From 5e42b5dac3135ada73c40909cddae3dfb3c453a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 9 Jun 2023 12:36:55 +0200 Subject: [PATCH 5/6] Fix nits --- src/interrupt.rs | 7 +++++-- src/service.rs | 11 +++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/interrupt.rs b/src/interrupt.rs index e2c53996169..af45dc4ffe7 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -11,14 +11,17 @@ pub enum InterruptState { Interrupted = 2, } +#[derive(Default, Debug, PartialEq, Eq, Clone)] +pub struct FromU8Error; + impl TryFrom for InterruptState { - type Error = (); + type Error = FromU8Error; fn try_from(value: u8) -> Result { match value { 0 => Ok(Self::Idle), 1 => Ok(Self::Working), 2 => Ok(Self::Interrupted), - _ => Err(()), + _ => Err(FromU8Error), } } } diff --git a/src/service.rs b/src/service.rs index 06c16a64500..598fbd0c672 100644 --- a/src/service.rs +++ b/src/service.rs @@ -826,14 +826,9 @@ impl Service { .platform .user_interface() .set_status(ui::Status::Idle); - match ep.interchange.respond(reply_result) { - Ok(()) => {} - Err(_) => { - info!("Cancelled request"); - if ep.interchange.is_canceled() { - ep.interchange.acknowledge_cancel().ok(); - } - } + if ep.interchange.respond(reply_result).is_err() && ep.interchange.is_canceled() { + info!("Cancelled request"); + ep.interchange.acknowledge_cancel().ok(); }; } } From 2d0fc7b3da21195f04d3c7321ae21239ac4bd3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 9 Jun 2023 12:37:11 +0200 Subject: [PATCH 6/6] Properly format RequestUserConsent --- src/service.rs | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/service.rs b/src/service.rs index 598fbd0c672..5c94f2d6b58 100644 --- a/src/service.rs +++ b/src/service.rs @@ -515,36 +515,43 @@ impl ServiceResources

{ }, Request::RequestUserConsent(request) => { + // assert_eq!(request.level, consent::Level::Normal); let starttime = self.platform.user_interface().uptime(); - let timeout = core::time::Duration::from_millis(request.timeout_milliseconds as u64); + let timeout = + core::time::Duration::from_millis(request.timeout_milliseconds as u64); let previous_status = self.platform.user_interface().status(); - self.platform.user_interface().set_status(ui::Status::WaitingForUserPresence); + self.platform + .user_interface() + .set_status(ui::Status::WaitingForUserPresence); loop { - if ctx.interrupt.map(|i|i.is_interrupted()) == Some(true) { - // Error does not matter as it will be dropped anyway + if ctx.interrupt.map(|i| i.is_interrupted()) == Some(true) { info_now!("User presence request cancelled"); - return Ok(reply::RequestUserConsent{result: Err(consent::Error::Interrupted)}.into()); + return Ok(reply::RequestUserConsent { + result: Err(consent::Error::Interrupted), + } + .into()); } self.platform.user_interface().refresh(); let nowtime = self.platform.user_interface().uptime(); if (nowtime - starttime) > timeout { let result = Err(consent::Error::TimedOut); - return Ok(Reply::RequestUserConsent(reply::RequestUserConsent { result } )); + return Ok(Reply::RequestUserConsent(reply::RequestUserConsent { + result, + })); } let up = self.platform.user_interface().check_user_presence(); match request.level { // If Normal level consent is request, then both Strong and Normal // indications will result in success. consent::Level::Normal => { - if up == consent::Level::Normal || - up == consent::Level::Strong { - break; - } - }, + if up == consent::Level::Normal || up == consent::Level::Strong { + break; + } + } // Otherwise, only strong level indication will work. consent::Level::Strong => { if up == consent::Level::Strong { @@ -559,7 +566,9 @@ impl ServiceResources

{ self.platform.user_interface().set_status(previous_status); let result = Ok(()); - Ok(Reply::RequestUserConsent(reply::RequestUserConsent { result } )) + Ok(Reply::RequestUserConsent(reply::RequestUserConsent { + result, + })) } Request::Reboot(request) => {