From 51d55de9b365f96b85e7ef7aa4dda4b85d4c31d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Fri, 24 May 2024 15:10:21 +0200 Subject: [PATCH] Fix factory-reset-app not deleting pins --- Cargo.toml | 1 + src/core_api.rs | 47 ++++++++++++++++++++++++++++++++--- src/manage.rs | 28 +++++++++++++++++++++ src/staging.rs | 2 ++ src/trussed_auth_impl.rs | 2 +- src/trussed_auth_impl/data.rs | 33 +++++++++++++++++++----- 6 files changed, 102 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 993e1d0..841b95f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ p256 = { version = "0.13.2", default-features = false, features = ["ecdsa-core"] salty = "0.3.0" p256-cortex-m4 = { version = "0.1.0-alpha.6", features = ["prehash", "sec1-signatures"] } admin-app = "0.1.0" +bitflags = "2.5.0" [dev-dependencies] admin-app = { version = "0.1.0", features = ["migration-tests"] } diff --git a/src/core_api.rs b/src/core_api.rs index e2918cb..873b192 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -1,5 +1,6 @@ //! Implementations of the core APIs for the SE050 backend +use bitflags::bitflags; use cbor_smol::{cbor_deserialize, cbor_serialize}; use crypto_bigint::{ subtle::{ConstantTimeEq, ConstantTimeGreater, CtOption}, @@ -7,6 +8,7 @@ use crypto_bigint::{ }; use embedded_hal::blocking::delay::DelayUs; use hex_literal::hex; +use iso7816::Status; use littlefs2::path::PathBuf; use rand::{CryptoRng, RngCore}; use se05x::{ @@ -82,6 +84,13 @@ struct WrappedKeyData { ty: WrappedKeyType, } +bitflags! { + pub struct ItemsToDelete: u8 { + const KEYS = 0b00000001; + const PINS = 0b00000010; + } +} + /// The bool returned points at wether the mechanism is raw RSA fn bits_and_kind_from_mechanism(mechanism: Mechanism) -> Result<(usize, key::Kind, bool), Error> { match mechanism { @@ -2308,6 +2317,7 @@ impl> Se050Backend { pub(crate) fn delete_all_items( &mut self, + kinds: ItemsToDelete, locations: &[Location], ns: NamespaceValue, ) -> Result { @@ -2352,7 +2362,8 @@ impl> Se050Backend { match parsed_obj_id { ParsedObjectId::PersistentKey(obj) if locations.contains(&Location::Internal) - || locations.contains(&Location::External) => + || locations.contains(&Location::External) + && kinds.contains(ItemsToDelete::KEYS) => { count += 1; offset = offset.saturating_sub(1); @@ -2363,7 +2374,10 @@ impl> Se050Backend { Error::FunctionFailed })?; } - ParsedObjectId::VolatileKey(obj) if locations.contains(&Location::Volatile) => { + ParsedObjectId::VolatileKey(obj) + if locations.contains(&Location::Volatile) + && kinds.contains(ItemsToDelete::KEYS) => + { count += 1; offset = offset.saturating_sub(1); self.se @@ -2374,13 +2388,38 @@ impl> Se050Backend { })?; } ParsedObjectId::VolatileRsaKey(obj) - if locations.contains(&Location::Volatile) => + if locations.contains(&Location::Volatile) + && kinds.contains(ItemsToDelete::KEYS) => { let deleted_count = self.delete_volatile_rsa_key_on_se050(obj)?; // VolatileRsaKeys can appear twice (intermediary and real key. They should be counted as one) count += if deleted_count == 0 { 0 } else { 1 }; offset = offset.saturating_sub(deleted_count); } + ParsedObjectId::Pin(se_id) if kinds.contains(ItemsToDelete::PINS) => { + debug!("Deleting simple pin"); + let res = self + .se + .run_command(&DeleteSecureObject { object_id: se_id.0 }, buf2); + match res { + Err(se05x::se05x::Error::Status(Status::KeyReferenceNotFound)) + | Ok(_) => {} + Err(_err) => { + error_now!("Failed to delete pin: {_err:?}"); + return Err(Error::FunctionFailed); + } + } + count += 1; + offset = offset.saturating_sub(1); + } + ParsedObjectId::PinWithDerived(se_id) + if kinds.contains(ItemsToDelete::PINS) => + { + use crate::trussed_auth_impl::data::PinData; + let deleted_count = PinData::delete_with_derived(&mut self.se, se_id)?; + count += if deleted_count == 0 { 0 } else { 1 }; + offset = offset.saturating_sub(deleted_count); + } ParsedObjectId::Pin(_) | ParsedObjectId::PinWithDerived(_) | ParsedObjectId::SaltValue(_) => {} @@ -2408,7 +2447,7 @@ impl> Se050Backend { debug!("Deleted core: {core_count}"); debug!("Deleted fs: {_fs_count}"); - let count = self.delete_all_items(&[req.location], ns)?; + let count = self.delete_all_items(ItemsToDelete::KEYS, &[req.location], ns)?; Ok(reply::DeleteAllKeys { count: core_count + count, diff --git a/src/manage.rs b/src/manage.rs index 3c6b3fa..b182e78 100644 --- a/src/manage.rs +++ b/src/manage.rs @@ -149,6 +149,34 @@ impl> ExtensionImpl for Se0 reply.push(i).ok(); } + let mut more = true; + let mut offset = 0; + while more { + let objects = self + .se + .run_command( + &se05x::se05x::commands::ReadIdList::builder() + .offset(offset.into()) + .filter(se05x::se05x::SecureObjectFilter::All) + .build(), + &mut buf, + ) + .map_err(|_err| { + error!("Failed to get id list: {_err:?}"); + Error::FunctionFailed + })?; + more = objects.more == se05x::se05x::MoreIndicator::More; + offset += (objects.ids.len() / 4) as u16; + + for obj in objects.ids.chunks(4) { + if let Ok(arr) = obj.try_into() { + debug_now!("Object: {:?}", se05x::se05x::ObjectId(arr)); + } else { + debug_now!("Bad chunk: {obj:02x?}"); + } + } + } + Ok(TestSe050Reply { reply }.into()) } } diff --git a/src/staging.rs b/src/staging.rs index 60764f9..a92b437 100644 --- a/src/staging.rs +++ b/src/staging.rs @@ -19,6 +19,7 @@ use trussed_wrap_key_to_file::{ reply as ext_reply, WrapKeyToFileExtension, WrapKeyToFileReply, WrapKeyToFileRequest, }; +use crate::core_api::ItemsToDelete; use crate::{core_api::CORE_DIR, Se050Backend, BACKEND_DIR}; impl> ExtensionImpl @@ -159,6 +160,7 @@ impl> ExtensionImpl for Se050Bac Error::RequestNotAvailable })?; self.delete_all_items( + ItemsToDelete::KEYS | ItemsToDelete::PINS, &[Location::Volatile, Location::External, Location::Internal], ns, )?; diff --git a/src/trussed_auth_impl.rs b/src/trussed_auth_impl.rs index bca002b..693c0c7 100644 --- a/src/trussed_auth_impl.rs +++ b/src/trussed_auth_impl.rs @@ -22,7 +22,7 @@ use trussed::{ }; use trussed_auth::MAX_HW_KEY_LEN; -mod data; +pub(crate) mod data; use crate::{ namespacing::{namespace, NamespaceValue, ObjectKind}, diff --git a/src/trussed_auth_impl/data.rs b/src/trussed_auth_impl/data.rs index 84e261d..7924d4c 100644 --- a/src/trussed_auth_impl/data.rs +++ b/src/trussed_auth_impl/data.rs @@ -439,7 +439,8 @@ impl PinData { pub fn delete_with_derived>( se050: &mut Se05X, se_id: PinObjectIdWithDerived, - ) -> Result<(), Error> { + ) -> Result { + let mut count = 0; let buf = &mut [0; 1024]; debug!("checking existence"); let exists = se050 @@ -467,6 +468,24 @@ impl PinData { debug!("Failed deletion: {_err:?}"); _err })?; + count += 1; + } + + let exists = se050 + .run_command( + &CheckObjectExists { + object_id: se_id.pin_id(), + }, + buf, + ) + .map_err(|_err| { + error!("Failed existence check: {_err:?}"); + Error::ReadFailed + })? + .result + .is_success(); + if !exists { + return Ok(count); } debug!("Writing userid "); @@ -525,7 +544,8 @@ impl PinData { error!("Failed to delete user id: {err:?}"); err })?; - Ok(()) + count += 1; + Ok(count) } pub fn delete>( @@ -533,23 +553,24 @@ impl PinData { fs: &mut impl Filestore, location: Location, se050: &mut Se05X, - ) -> Result<(), Error> { + ) -> Result { debug!("Deleting {self:02x?}"); - match self.se_id { + let deleted_count = match self.se_id { PinSeId::WithDerived(se_id) => Self::delete_with_derived(se050, se_id)?, PinSeId::Raw(se_id) => { let buf = &mut [0; 1024]; debug!("Deleting simple"); se050.run_command(&DeleteSecureObject { object_id: se_id.0 }, buf)?; + 1 } - } + }; debug!("Removing file {}", self.id.path()); fs.remove_file(&self.id.path(), location).map_err(|_err| { error!("Removing file failed: {_err:?}"); Error::WriteFailed })?; - Ok(()) + Ok(deleted_count) } /// Returns (tried, max)