From fd6a0208275cf49e01d20247f682780abf824096 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 30 Jan 2023 21:24:45 +0100 Subject: [PATCH 1/2] Replace &PathBuf with &Path Fixes: https://github.com/trussed-dev/trussed/issues/81 --- CHANGELOG.md | 1 + src/service.rs | 35 ++++++++++--------- src/store/certstore.rs | 7 ++-- src/store/counterstore.rs | 7 ++-- src/store/filestore.rs | 71 ++++++++++++++++++++------------------- src/store/keystore.rs | 13 ++++--- 6 files changed, 74 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724978871f0..172bef8e03e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `ui::Status` non-exhaustive. - Made `postcard_deserialize`, `postcard_serialize` and `postcard_serialize_bytes` private. +- Changed `&PathBuf` to `&Path` where possible. ### Fixed diff --git a/src/service.rs b/src/service.rs index 16831f1f8e1..1a923b3f357 100644 --- a/src/service.rs +++ b/src/service.rs @@ -1,4 +1,7 @@ -use littlefs2::path::PathBuf; +use littlefs2::{ + path, + path::{Path, PathBuf}, +}; use rand_chacha::ChaCha8Rng; pub use rand_core::{RngCore, SeedableRng}; @@ -296,7 +299,7 @@ impl ServiceResources

{ }, Request::LocateFile(request) => { - let path = filestore.locate_file(request.location, request.dir.clone(), request.filename.clone())?; + let path = filestore.locate_file(request.location, request.dir.as_deref(), &request.filename)?; Ok(Reply::LocateFile(reply::LocateFile { path }) ) } @@ -306,14 +309,14 @@ impl ServiceResources

{ Request::DebugDumpStore(_request) => { info_now!(":: PERSISTENT"); - recursively_list(self.platform.store().ifs(), PathBuf::from("/")); + recursively_list(self.platform.store().ifs(), path!("/")); info_now!(":: VOLATILE"); - recursively_list(self.platform.store().vfs(), PathBuf::from("/")); + recursively_list(self.platform.store().vfs(), path!("/")); - fn recursively_list(fs: &'static crate::store::Fs, path: PathBuf) { + fn recursively_list(fs: &'static crate::store::Fs, path: &Path) { // let fs = store.vfs(); - fs.read_dir_and_then(&path, |dir| { + fs.read_dir_and_then(path, |dir| { for (i, entry) in dir.enumerate() { let entry = entry.unwrap(); if i < 2 { @@ -322,7 +325,7 @@ impl ServiceResources

{ } info_now!("{:?} p({:?})", entry.path(), &path); if entry.file_type().is_dir() { - recursively_list(fs, PathBuf::from(entry.path())); + recursively_list(fs, entry.path()); } if entry.file_type().is_file() { let _contents: Vec = fs.read(entry.path()).unwrap(); @@ -339,7 +342,7 @@ impl ServiceResources

{ } Request::ReadDirFirst(request) => { - let maybe_entry = match filestore.read_dir_first(&request.dir, request.location, request.not_before_filename.as_ref())? { + let maybe_entry = match filestore.read_dir_first(&request.dir, request.location, request.not_before_filename.as_deref())? { Some((entry, read_dir_state)) => { ctx.read_dir_state = Some(read_dir_state); Some(entry) @@ -614,15 +617,15 @@ impl ServiceResources

{ None => { let mut filestore = self.trussed_filestore(); - let path = PathBuf::from("rng-state.bin"); + let path = path!("rng-state.bin"); // Load previous seed, e.g., externally injected entropy on first run. // Else, default to zeros - will mix in new HW RNG entropy next - let mixin_seed = if !filestore.exists(&path, Location::Internal) { + let mixin_seed = if !filestore.exists(path, Location::Internal) { [0u8; 32] } else { // Use the last saved state. - let mixin_bytes: Bytes<32> = filestore.read(&path, Location::Internal)?; + let mixin_bytes: Bytes<32> = filestore.read(path, Location::Internal)?; let mut mixin_seed = [0u8; 32]; mixin_seed.clone_from_slice(&mixin_bytes); mixin_seed @@ -662,7 +665,7 @@ impl ServiceResources

{ let mut seed_to_store = [0u8; 32]; rng.fill_bytes(&mut seed_to_store); filestore - .write(&path, Location::Internal, seed_to_store.as_ref()) + .write(path, Location::Internal, seed_to_store.as_ref()) .unwrap(); // 5. Finish @@ -744,7 +747,7 @@ impl Service { backends: &'static [BackendId], ) -> Result<(), Error> { let core_ctx = core_ctx.into(); - if core_ctx.path == PathBuf::from("trussed") { + if &*core_ctx.path == path!("trussed") { panic!("trussed is a reserved client ID"); } self.eps @@ -758,10 +761,10 @@ impl Service { pub fn set_seed_if_uninitialized(&mut self, seed: &[u8; 32]) { let mut filestore = self.resources.trussed_filestore(); - let path = PathBuf::from("rng-state.bin"); - if !filestore.exists(&path, Location::Internal) { + let path = path!("rng-state.bin"); + if !filestore.exists(path, Location::Internal) { filestore - .write(&path, Location::Internal, seed.as_ref()) + .write(path, Location::Internal, seed.as_ref()) .unwrap(); } } diff --git a/src/store/certstore.rs b/src/store/certstore.rs index dcb991f2a20..63b0ba187ac 100644 --- a/src/store/certstore.rs +++ b/src/store/certstore.rs @@ -1,4 +1,7 @@ -use littlefs2::path::PathBuf; +use littlefs2::{ + path, + path::{Path, PathBuf}, +}; use rand_chacha::ChaCha8Rng; use crate::{ @@ -65,7 +68,7 @@ impl ClientCertstore { fn cert_path(&self, id: CertId) -> PathBuf { let mut path = PathBuf::new(); path.push(&self.client_id); - path.push(&PathBuf::from("x5c")); + path.push(path!("x5c")); path.push(&PathBuf::from(id.hex().as_slice())); path } diff --git a/src/store/counterstore.rs b/src/store/counterstore.rs index e967c50b7ab..da3d5bdc783 100644 --- a/src/store/counterstore.rs +++ b/src/store/counterstore.rs @@ -1,4 +1,7 @@ -use littlefs2::path::PathBuf; +use littlefs2::{ + path, + path::{Path, PathBuf}, +}; use rand_chacha::ChaCha8Rng; use crate::{ @@ -30,7 +33,7 @@ impl ClientCounterstore { fn counter_path(&self, id: CounterId) -> PathBuf { let mut path = PathBuf::new(); path.push(&self.client_id); - path.push(&PathBuf::from("ctr")); + path.push(path!("ctr")); path.push(&PathBuf::from(id.hex().as_slice())); path } diff --git a/src/store/filestore.rs b/src/store/filestore.rs index 9be46711fc5..912ced5612e 100644 --- a/src/store/filestore.rs +++ b/src/store/filestore.rs @@ -5,6 +5,7 @@ use crate::{ types::{LfsStorage, Location, Message, UserAttribute}, Bytes, }; +use littlefs2::path; #[derive(Clone)] pub struct ReadDirState { @@ -43,7 +44,7 @@ impl ClientFilestore { } /// Client files are store below `//dat/`. - pub fn actual_path(&self, client_path: &PathBuf) -> Result { + pub fn actual_path(&self, client_path: &Path) -> Result { // Clients must not escape their namespace if client_path.as_ref().contains("..") { return Err(Error::InvalidPath); @@ -51,7 +52,7 @@ impl ClientFilestore { let mut path = PathBuf::new(); path.push(&self.client_id); - path.push(&PathBuf::from("dat")); + path.push(path!("dat")); path.push(client_path); Ok(path) } @@ -75,18 +76,18 @@ impl ClientFilestore { } pub trait Filestore { - fn read(&mut self, path: &PathBuf, location: Location) -> Result>; - fn write(&mut self, path: &PathBuf, location: Location, data: &[u8]) -> Result<()>; - fn exists(&mut self, path: &PathBuf, location: Location) -> bool; - fn metadata(&mut self, path: &PathBuf, location: Location) -> Result>; - fn remove_file(&mut self, path: &PathBuf, location: Location) -> Result<()>; - fn remove_dir(&mut self, path: &PathBuf, location: Location) -> Result<()>; - fn remove_dir_all(&mut self, path: &PathBuf, location: Location) -> Result; + fn read(&mut self, path: &Path, location: Location) -> Result>; + fn write(&mut self, path: &Path, location: Location, data: &[u8]) -> Result<()>; + fn exists(&mut self, path: &Path, location: Location) -> bool; + fn metadata(&mut self, path: &Path, location: Location) -> Result>; + fn remove_file(&mut self, path: &Path, location: Location) -> Result<()>; + fn remove_dir(&mut self, path: &Path, location: Location) -> Result<()>; + fn remove_dir_all(&mut self, path: &Path, location: Location) -> Result; fn locate_file( &mut self, location: Location, - underneath: Option, - filename: PathBuf, + underneath: Option<&Path>, + filename: &Path, ) -> Result>; /// Iterate over entries of a directory (both file and directory entries). @@ -100,9 +101,9 @@ pub trait Filestore { /// call to `read_dir_next` can resume operation. fn read_dir_first( &mut self, - dir: &PathBuf, + dir: &Path, location: Location, - not_before: Option<&PathBuf>, + not_before: Option<&Path>, ) -> Result>; /// Continue iterating over entries of a directory. @@ -120,7 +121,7 @@ pub trait Filestore { /// Additionally, files may optionally be filtered via attributes. fn read_dir_files_first( &mut self, - clients_dir: &PathBuf, + clients_dir: &Path, location: Location, user_attribute: Option, ) -> Result, ReadDirFilesState)>>; @@ -136,9 +137,9 @@ pub trait Filestore { impl ClientFilestore { fn read_dir_first_impl( &mut self, - clients_dir: &PathBuf, + clients_dir: &Path, location: Location, - not_before: Option<&PathBuf>, + not_before: Option<&Path>, fs: &'static Fs, ) -> Result> { let dir = self.actual_path(clients_dir)?; @@ -228,7 +229,7 @@ impl ClientFilestore { } fn read_dir_files_first_impl( &mut self, - clients_dir: &PathBuf, + clients_dir: &Path, location: Location, user_attribute: Option, fs: &'static Fs, @@ -346,30 +347,30 @@ impl ClientFilestore { } impl Filestore for ClientFilestore { - fn read(&mut self, path: &PathBuf, location: Location) -> Result> { + fn read(&mut self, path: &Path, location: Location) -> Result> { let path = self.actual_path(path)?; store::read(self.store, location, &path) } - fn write(&mut self, path: &PathBuf, location: Location, data: &[u8]) -> Result<()> { + fn write(&mut self, path: &Path, location: Location, data: &[u8]) -> Result<()> { let path = self.actual_path(path)?; store::store(self.store, location, &path, data) } - fn exists(&mut self, path: &PathBuf, location: Location) -> bool { + fn exists(&mut self, path: &Path, location: Location) -> bool { if let Ok(path) = self.actual_path(path) { store::exists(self.store, location, &path) } else { false } } - fn metadata(&mut self, path: &PathBuf, location: Location) -> Result> { + fn metadata(&mut self, path: &Path, location: Location) -> Result> { let path = self.actual_path(path)?; store::metadata(self.store, location, &path) } - fn remove_file(&mut self, path: &PathBuf, location: Location) -> Result<()> { + fn remove_file(&mut self, path: &Path, location: Location) -> Result<()> { let path = self.actual_path(path)?; match store::delete(self.store, location, &path) { @@ -378,7 +379,7 @@ impl Filestore for ClientFilestore { } } - fn remove_dir(&mut self, path: &PathBuf, location: Location) -> Result<()> { + fn remove_dir(&mut self, path: &Path, location: Location) -> Result<()> { let path = self.actual_path(path)?; match store::delete(self.store, location, &path) { @@ -387,7 +388,7 @@ impl Filestore for ClientFilestore { } } - fn remove_dir_all(&mut self, path: &PathBuf, location: Location) -> Result { + fn remove_dir_all(&mut self, path: &Path, location: Location) -> Result { let path = self.actual_path(path)?; store::remove_dir_all_where(self.store, location, &path, |_| true) @@ -396,9 +397,9 @@ impl Filestore for ClientFilestore { fn read_dir_first( &mut self, - clients_dir: &PathBuf, + clients_dir: &Path, location: Location, - not_before: Option<&PathBuf>, + not_before: Option<&Path>, ) -> Result> { match location { Location::Internal => { @@ -423,7 +424,7 @@ impl Filestore for ClientFilestore { fn read_dir_files_first( &mut self, - clients_dir: &PathBuf, + clients_dir: &Path, location: Location, user_attribute: Option, ) -> Result, ReadDirFilesState)>> { @@ -463,25 +464,25 @@ impl Filestore for ClientFilestore { fn locate_file( &mut self, location: Location, - underneath: Option, - filename: PathBuf, + underneath: Option<&Path>, + filename: &Path, ) -> Result> { if location != Location::Internal { return Err(Error::RequestNotAvailable); } - let clients_dir = underneath.unwrap_or_else(|| PathBuf::from("/")); - let dir = self.actual_path(&clients_dir)?; + let clients_dir = underneath.unwrap_or_else(|| path!("/")); + let dir = self.actual_path(clients_dir)?; let fs = self.store.ifs(); info_now!("base dir {:?}", &dir); fn recursively_locate( fs: &'static crate::store::Fs, - dir: PathBuf, + dir: &Path, filename: &Path, ) -> Option { - fs.read_dir_and_then(&dir, |it| { + fs.read_dir_and_then(dir, |it| { it.map(|entry| entry.unwrap()) .skip(2) .filter_map(|entry| { @@ -493,7 +494,7 @@ impl Filestore for ClientFilestore { None } } else { - recursively_locate(fs, PathBuf::from(entry.path()), filename) + recursively_locate(fs, entry.path(), filename) } }) .next() @@ -502,7 +503,7 @@ impl Filestore for ClientFilestore { .ok() } - let path = recursively_locate(fs, dir, &filename).map(|path| self.client_path(&path)); + let path = recursively_locate(fs, &dir, filename).map(|path| self.client_path(&path)); Ok(path) } diff --git a/src/store/keystore.rs b/src/store/keystore.rs index 32393b3b6f8..4241ec5337a 100644 --- a/src/store/keystore.rs +++ b/src/store/keystore.rs @@ -1,4 +1,7 @@ -use littlefs2::path::PathBuf; +use littlefs2::{ + path, + path::{Path, PathBuf}, +}; use rand_chacha::ChaCha8Rng; use crate::{ @@ -10,7 +13,7 @@ use crate::{ Bytes, }; -pub type ClientId = littlefs2::path::PathBuf; +pub type ClientId = PathBuf; pub struct ClientKeystore where @@ -76,9 +79,9 @@ impl ClientKeystore { pub fn key_directory(&self, secrecy: key::Secrecy) -> PathBuf { let mut path = PathBuf::new(); path.push(&self.client_id); - path.push(&match secrecy { - key::Secrecy::Secret => PathBuf::from("sec"), - key::Secrecy::Public => PathBuf::from("pub"), + path.push(match secrecy { + key::Secrecy::Secret => path!("sec"), + key::Secrecy::Public => path!("pub"), }); path } From 676ed8021e8fbd8efafe291c57466708f3d56fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 9 May 2023 10:17:26 +0200 Subject: [PATCH 2/2] filestore: Add remove_dir_all_where method --- src/store/filestore.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/store/filestore.rs b/src/store/filestore.rs index 912ced5612e..c093f3b5086 100644 --- a/src/store/filestore.rs +++ b/src/store/filestore.rs @@ -83,6 +83,12 @@ pub trait Filestore { fn remove_file(&mut self, path: &Path, location: Location) -> Result<()>; fn remove_dir(&mut self, path: &Path, location: Location) -> Result<()>; fn remove_dir_all(&mut self, path: &Path, location: Location) -> Result; + fn remove_dir_all_where( + &mut self, + path: &Path, + location: Location, + predicate: impl Fn(&DirEntry) -> bool, + ) -> Result; fn locate_file( &mut self, location: Location, @@ -394,6 +400,17 @@ impl Filestore for ClientFilestore { store::remove_dir_all_where(self.store, location, &path, |_| true) .map_err(|_| Error::InternalError) } + fn remove_dir_all_where( + &mut self, + path: &Path, + location: Location, + predicate: impl Fn(&DirEntry) -> bool, + ) -> Result { + let path = self.actual_path(path)?; + + store::remove_dir_all_where(self.store, location, &path, predicate) + .map_err(|_| Error::InternalError) + } fn read_dir_first( &mut self,