From 216664ae0b7c341bc740756a6fb2cc42246ff7dd Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 24 Jan 2025 11:23:45 +0100 Subject: [PATCH 1/2] Simplify Store implementation This patch updates trussed to use the simplified Store trait. --- Cargo.lock | 6 +-- Cargo.toml | 6 +-- components/apps/src/lib.rs | 12 ++--- components/boards/src/store.rs | 74 +++++++++++++-------------- components/provisioner-app/src/lib.rs | 16 +++--- runners/embedded/src/nk3xn/init.rs | 2 +- runners/usbip/src/main.rs | 2 +- runners/usbip/src/store.rs | 3 +- 8 files changed, 59 insertions(+), 62 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73ebef8d..f5851ed4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5,7 +5,7 @@ version = 3 [[package]] name = "admin-app" version = "0.1.0" -source = "git+https://github.com/Nitrokey/admin-app.git?tag=v0.1.0-nitrokey.19#d5f1c6df405e4edeb6524f908c1c713139173e81" +source = "git+https://github.com/Nitrokey/admin-app.git?tag=v0.1.0-nitrokey.20#ff574d810f718c7ce0912512f6c551ca2ea7c88a" dependencies = [ "apdu-app", "cbor-smol", @@ -3188,7 +3188,7 @@ dependencies = [ [[package]] name = "trussed" version = "0.1.0" -source = "git+https://github.com/trussed-dev/trussed.git?rev=ede9fc02dc69eba7b35536b2bf4fd189a82e50b7#ede9fc02dc69eba7b35536b2bf4fd189a82e50b7" +source = "git+https://github.com/trussed-dev/trussed.git?rev=5003249c3187dca841f83551ba625921611a5ace#5003249c3187dca841f83551ba625921611a5ace" dependencies = [ "aes", "bitflags 2.6.0", @@ -3385,7 +3385,7 @@ dependencies = [ [[package]] name = "trussed-staging" version = "0.3.2" -source = "git+https://github.com/trussed-dev/trussed-staging.git?rev=1e1ca03a3a62ea9b802f4070ea4bce002eeb4bec#1e1ca03a3a62ea9b802f4070ea4bce002eeb4bec" +source = "git+https://github.com/trussed-dev/trussed-staging.git?rev=7f305b8db3cc9dc0cd2c1bfb8470e9f531e34abe#7f305b8db3cc9dc0cd2c1bfb8470e9f531e34abe" dependencies = [ "aead", "chacha20poly1305", diff --git a/Cargo.toml b/Cargo.toml index a8364548..eea50e33 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,11 +22,11 @@ memory-regions = { path = "components/memory-regions" } # unreleased libraries p256-cortex-m4 = { git = "https://github.com/ycrypto/p256-cortex-m4.git", rev = "cdb31e12594b4dc1f045b860a885fdc94d96aee2" } lpc55-hal = { git = "https://github.com/lpc55/lpc55-hal.git", rev = "1a25fc366013503b46af938646c88aed4e36d74c" } -trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "ede9fc02dc69eba7b35536b2bf4fd189a82e50b7" } +trussed = { git = "https://github.com/trussed-dev/trussed.git", rev = "5003249c3187dca841f83551ba625921611a5ace" } trussed-usbip = { git = "https://github.com/trussed-dev/pc-usbip-runner.git", rev = "a0e9b855809577f0067a93e08c716aa285b03700" } # applications -admin-app = { git = "https://github.com/Nitrokey/admin-app.git", tag = "v0.1.0-nitrokey.19" } +admin-app = { git = "https://github.com/Nitrokey/admin-app.git", tag = "v0.1.0-nitrokey.20" } fido-authenticator = { git = "https://github.com/Nitrokey/fido-authenticator.git",tag = "v0.1.1-nitrokey.25" } opcard = { git = "https://github.com/Nitrokey/opcard-rs", rev = "39ec4c37f808c0cfeb84e0a8493bbee06f02c8e2" } piv-authenticator = { git = "https://github.com/Nitrokey/piv-authenticator.git", rev = "65552820b4f931c21e1c7675b1bd6072cb872531" } @@ -37,7 +37,7 @@ webcrypt = { git = "https://github.com/nitrokey/nitrokey-websmartcard-rust", tag trussed-auth-backend = { git = "https://github.com/trussed-dev/trussed-auth", tag = "v0.4.0" } trussed-rsa-alloc = { git = "https://github.com/trussed-dev/trussed-rsa-backend.git", rev = "743d9aaa3d8a17d7dbf492bd54dc18ab8fca3dc0" } trussed-se050-backend = { git = "https://github.com/Nitrokey/trussed-se050-backend.git", rev = "131c973fbe74d677fb8c8df97c210f78608994f0" } -trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "1e1ca03a3a62ea9b802f4070ea4bce002eeb4bec" } +trussed-staging = { git = "https://github.com/trussed-dev/trussed-staging.git", rev = "7f305b8db3cc9dc0cd2c1bfb8470e9f531e34abe" } [profile.release] codegen-units = 1 diff --git a/components/apps/src/lib.rs b/components/apps/src/lib.rs index 9598588d..96722599 100644 --- a/components/apps/src/lib.rs +++ b/components/apps/src/lib.rs @@ -319,7 +319,7 @@ pub trait Runner { type Syscall: Syscall + Clone + 'static; type Reboot: Reboot; - type Store: trussed::store::Store; + type Store: trussed::store::Store + Clone; #[cfg(feature = "provisioner-app")] type Filesystem: trussed::types::LfsStorage + 'static; #[cfg(feature = "se050")] @@ -612,7 +612,7 @@ impl Apps { let trussed = client_builder.client::>(runner, &()); // TODO: use CLIENT_ID directly - let mut filestore = ClientFilestore::new(ADMIN_APP_CLIENT_ID.into(), data.store); + let mut filestore = ClientFilestore::new(ADMIN_APP_CLIENT_ID.into(), data.store.clone()); let version = data.version.encode(); let valid_migrators = migrations::MIGRATORS; @@ -653,10 +653,10 @@ impl Apps { trussed_auth_backend::FilesystemLayout::V0, dispatch::AUTH_LOCATION, path!("opcard"), - data.store, + data.store.clone(), ) .unwrap_or_default(); - let mut fs = ClientFilestore::new(path!("opcard").into(), data.store); + let mut fs = ClientFilestore::new(path!("opcard").into(), data.store.clone()); let opcard_used = fs .read_dir_first(path!(""), Location::External, &NotBefore::None) .unwrap_or_default() @@ -711,7 +711,7 @@ impl Apps { .unwrap_or_default(); let migration_success = app - .migrate(migration_version, data.store, &mut filestore) + .migrate(migration_version, data.store.clone(), &mut filestore) .is_ok(); if !migration_success { data.init_status.insert(InitStatus::MIGRATION_ERROR); @@ -1262,7 +1262,7 @@ impl App for ProvisionerApp { let uuid = runner.uuid(); Self::new( trussed, - data.store, + data.store.clone(), data.stolen_filesystem, data.nfc_powered, uuid, diff --git a/components/boards/src/store.rs b/components/boards/src/store.rs index 79f3a14d..44981807 100644 --- a/components/boards/src/store.rs +++ b/components/boards/src/store.rs @@ -6,8 +6,9 @@ use littlefs2::{ driver::Storage, fs::{Allocation, Filesystem}, io::Result, + object_safe::DynFilesystem, }; -use trussed::store::{Fs, Store}; +use trussed::store::Store; use crate::Board; @@ -66,9 +67,6 @@ pub trait StoragePointers: 'static { type ExternalStorage: Storage; unsafe fn ifs_storage() -> &'static mut MaybeUninit; - unsafe fn ifs_ptr() -> *mut Fs; - - unsafe fn efs_ptr() -> *mut Fs; } #[cfg_attr( @@ -87,20 +85,6 @@ macro_rules! impl_storage_pointers { ::core::mem::MaybeUninit::uninit(); (&mut *&raw mut IFS_STORAGE) } - - unsafe fn ifs_ptr() -> *mut ::trussed::store::Fs { - static mut IFS: ::core::mem::MaybeUninit<::trussed::store::Fs<$I>> = - ::core::mem::MaybeUninit::uninit(); - let ifs_ptr: *mut ::core::mem::MaybeUninit<::trussed::store::Fs<$I>> = &raw mut IFS; - ifs_ptr as _ - } - - unsafe fn efs_ptr() -> *mut ::trussed::store::Fs { - static mut EFS: ::core::mem::MaybeUninit<::trussed::store::Fs<$E>> = - ::core::mem::MaybeUninit::uninit(); - let efs_ptr: *mut ::core::mem::MaybeUninit<::trussed::store::Fs<$E>> = &raw mut EFS; - efs_ptr as _ - } } }; } @@ -111,20 +95,37 @@ macro_rules! impl_storage_pointers { )] pub(crate) use impl_storage_pointers; +struct StorePointers { + ifs: MaybeUninit<&'static dyn DynFilesystem>, + efs: MaybeUninit<&'static dyn DynFilesystem>, + vfs: MaybeUninit<&'static dyn DynFilesystem>, +} + +impl StorePointers { + const fn new() -> Self { + Self { + ifs: MaybeUninit::uninit(), + efs: MaybeUninit::uninit(), + vfs: MaybeUninit::uninit(), + } + } +} + pub struct RunnerStore { _marker: PhantomData<*mut S>, } impl RunnerStore { fn new( - ifs: &'static Filesystem<'static, S::InternalStorage>, - efs: &'static Filesystem<'static, S::ExternalStorage>, - vfs: &'static Filesystem<'static, VolatileStorage>, + ifs: &'static dyn DynFilesystem, + efs: &'static dyn DynFilesystem, + vfs: &'static dyn DynFilesystem, ) -> Self { unsafe { - S::ifs_ptr().write(Fs::new(ifs)); - S::efs_ptr().write(Fs::new(efs)); - Self::vfs_ptr().write(Fs::new(vfs)); + let pointers = Self::pointers(); + pointers.ifs.write(ifs); + pointers.efs.write(efs); + pointers.vfs.write(vfs); } Self { @@ -132,10 +133,9 @@ impl RunnerStore { } } - unsafe fn vfs_ptr() -> *mut Fs { - static mut VFS: MaybeUninit> = MaybeUninit::uninit(); - let vfs_ptr: *mut MaybeUninit> = &raw mut VFS; - vfs_ptr as _ + unsafe fn pointers() -> &'static mut StorePointers { + static mut POINTERS: StorePointers = StorePointers::new(); + (&raw mut POINTERS).as_mut().unwrap() } } @@ -147,21 +147,17 @@ impl Clone for RunnerStore { impl Copy for RunnerStore {} -unsafe impl Store for RunnerStore { - type I = S::InternalStorage; - type E = S::ExternalStorage; - type V = VolatileStorage; - - fn ifs(self) -> &'static Fs { - unsafe { &*S::ifs_ptr() } +impl Store for RunnerStore { + fn ifs(&self) -> &dyn DynFilesystem { + unsafe { Self::pointers().ifs.assume_init() } } - fn efs(self) -> &'static Fs { - unsafe { &*S::efs_ptr() } + fn efs(&self) -> &dyn DynFilesystem { + unsafe { Self::pointers().efs.assume_init() } } - fn vfs(self) -> &'static Fs { - unsafe { &*Self::vfs_ptr() } + fn vfs(&self) -> &dyn DynFilesystem { + unsafe { Self::pointers().vfs.assume_init() } } } diff --git a/components/provisioner-app/src/lib.rs b/components/provisioner-app/src/lib.rs index 3f93f1ad..a22ab28d 100644 --- a/components/provisioner-app/src/lib.rs +++ b/components/provisioner-app/src/lib.rs @@ -194,7 +194,7 @@ where // logging::dump_hex(&self.buffer_file_contents, self.buffer_file_contents.len()); let res = store::store( - self.store, + &self.store, trussed::types::Location::Internal, &buffer_path, &self.buffer_file_contents, @@ -241,7 +241,7 @@ where let serialized_bytes = serialized_key.serialize(); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_P256_SECRET, &serialized_bytes, @@ -271,7 +271,7 @@ where let serialized_bytes = serialized_key.serialize(); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_ED255_SECRET, &serialized_bytes, @@ -300,7 +300,7 @@ where let serialized_bytes = serialized_key.serialize(); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_X255_SECRET, &serialized_bytes, @@ -320,7 +320,7 @@ where } else { info!("saving P256 CERT, {} bytes", data.len()); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_P256_CERT, data, @@ -336,7 +336,7 @@ where } else { info!("saving ED25519 CERT, {} bytes", data.len()); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_ED255_CERT, data, @@ -352,7 +352,7 @@ where } else { info!("saving X25519 CERT, {} bytes", data.len()); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_X255_CERT, data, @@ -375,7 +375,7 @@ where let serialized_key = serialized_key.serialize(); store::store( - self.store, + &self.store, trussed::types::Location::Internal, FILENAME_T1_PUBLIC, &serialized_key, diff --git a/runners/embedded/src/nk3xn/init.rs b/runners/embedded/src/nk3xn/init.rs index f5a82655..f54fc704 100644 --- a/runners/embedded/src/nk3xn/init.rs +++ b/runners/embedded/src/nk3xn/init.rs @@ -791,7 +791,7 @@ impl Stage6 { if self.basic.old_firmware_version <= 4194306 { debug!("data migration: updating FIDO2 attestation cert"); let res = trussed::store::store( - self.store, + &self.store, Location::Internal, path!("fido/x5c/00"), include_bytes!("../../data/fido-cert.der"), diff --git a/runners/usbip/src/main.rs b/runners/usbip/src/main.rs index e2632350..669c8ab6 100644 --- a/runners/usbip/src/main.rs +++ b/runners/usbip/src/main.rs @@ -115,7 +115,7 @@ impl apps::Runner for Runner { type Store = store::Store; #[cfg(feature = "provisioner")] - type Filesystem = ::I; + type Filesystem = store::InternalStorage; type Twi = (); type Se050Timer = (); diff --git a/runners/usbip/src/store.rs b/runners/usbip/src/store.rs index e54cd3cc..4a1d4d99 100644 --- a/runners/usbip/src/store.rs +++ b/runners/usbip/src/store.rs @@ -50,7 +50,7 @@ const_ram_storage!( const_ram_storage!(VolatileStorage, IFS_STORAGE_SIZE); // TODO: use 256 -- would cause a panic because formatting fails -type InternalStorage = FilesystemOrRamStorage; +pub type InternalStorage = FilesystemOrRamStorage; type ExternalStorage = FilesystemOrRamStorage; pub struct FilesystemStorage { @@ -207,6 +207,7 @@ impl FilesystemOrRam { impl StoreProvider for FilesystemOrRam { type Store = Store; + type Ifs = InternalStorage; unsafe fn ifs() -> &'static mut InternalStorage { #[allow(clippy::deref_addrof)] From 098b281ea12238fd9cc5748fa2fc57842a2ab7ef Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 3 Mar 2025 19:10:29 +0100 Subject: [PATCH 2/2] ci: Temporarily ignore errors related to the metrics comment This temporarily reverts https://github.com/Nitrokey/nitrokey-3-firmware/pull/594 as the current implementation does not work with merge commits. --- .gitlab-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a3b1ba32..3d0ec5a6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -197,6 +197,8 @@ metrics: stage: metrics script: - repometrics generate --cache > metrics.toml + after_script: + - !reference [notify_github, script] # use notify_github from include - repometrics run --base origin/main --output-format markdown | tee --append metrics-comment.md - if [ -n "$CI_COMMIT_BRANCH" ] && [ "$CI_COMMIT_BRANCH" != "main" ] ; then nitrokey-ci write-comment --owner Nitrokey --repo nitrokey-3-firmware --id repometrics --commit $(git rev-parse HEAD) metrics-comment.md ;