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 d3ee366..12adc7d 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}, @@ -82,6 +83,13 @@ struct WrappedKeyData { ty: WrappedKeyType, } +bitflags! { + pub(crate) 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 { @@ -224,7 +232,7 @@ impl> Se050Backend { se050_keystore: &mut impl Keystore, ) -> Result { let (parsed_key, _parsed_ty) = parse_key_id(*key, ns).ok_or_else(|| { - debug!("Failed to parse key id"); + debug!("Failed to parse key id to delete: {key:?}"); Error::RequestNotAvailable })?; debug!("Deleting {key:?}, {parsed_key:?}, {_parsed_ty:?}"); @@ -2011,7 +2019,7 @@ impl> Se050Backend { ) -> Result { debug!("Clearing: {:?}", req.key); let (parsed_key, _parsed_ty) = parse_key_id(req.key, ns).ok_or_else(|| { - debug!("Failed to parse key id"); + debug!("Failed to parse key id to clear: {:?}", req.key); Error::RequestNotAvailable })?; @@ -2050,7 +2058,7 @@ impl> Se050Backend { return Err(Error::RequestNotAvailable); } let (parsed_key, _parsed_ty) = parse_key_id(req.key, ns).ok_or_else(|| { - debug!("Failed to parse key id"); + debug!("Failed to parse key id to wrap: {:?}", req.key); // Let non-se050 keys be wrapped by the core backend Error::RequestNotAvailable })?; @@ -2238,7 +2246,7 @@ impl> Se050Backend { ns: NamespaceValue, ) -> Result { let (parsed_key, parsed_ty) = parse_key_id(req.key, ns).ok_or_else(|| { - debug!("Failed to parse key id"); + debug!("Failed to parse key id to check if exists: {:?}", req.key); // Let non-se050 keys be checked by the core backend Error::RequestNotAvailable })?; @@ -2306,17 +2314,39 @@ impl> Se050Backend { Ok(reply::Exists { exists }) } + fn delete_object(&mut self, object_id: ObjectId, buf: &mut [u8]) -> Result<(), Error> { + self.se + .run_command(&DeleteSecureObject { object_id }, buf) + .map_err(|_err| { + error!("Failed to delete object: {object_id:02x?} {_err:?}"); + Error::FunctionFailed + })?; + Ok(()) + } + pub(crate) fn delete_all_items( &mut self, - locations: &[Location], + kinds: ItemsToDelete, + // Locations for deleted keys + key_locations: &[Location], ns: NamespaceValue, ) -> Result { let buf = &mut [0; 1024]; let buf2 = &mut [0; 128]; let mut count = 0u16; let mut offset = 0; + let mut cont = true; + + let should_delete_internal = key_locations.contains(&Location::Internal); + let should_delete_external = key_locations.contains(&Location::External); + let should_delete_volatile = key_locations.contains(&Location::Volatile); + // Persistent keys do not have a "location" + let should_delete_persistent = should_delete_internal || should_delete_external; + + let should_delete_keys = kinds.contains(ItemsToDelete::KEYS); + let should_delete_pins = kinds.contains(ItemsToDelete::PINS); - loop { + while cont { let to_delete = self .se .run_command( @@ -2330,16 +2360,17 @@ impl> Se050Backend { error!("Failed to read id_list: {_err:?}"); Error::FunctionFailed })?; + cont = to_delete.more.is_more(); assert_eq!(to_delete.ids.len() % 4, 0); offset += (to_delete.ids.len() / 4) as u16; for obj_slice in to_delete.ids.chunks_exact(4) { let obj = ObjectId(obj_slice.try_into().unwrap()); - debug!("Dealing with obj: {obj:02x?}"); + debug_now!("Dealing with obj: {obj_slice:02x?}"); if !object_in_range(obj) { continue; } - debug!("In range"); + debug!("In range for a non-native object"); let Some((ns_to_check, parsed_obj_id)) = ParsedObjectId::parse(obj) else { debug!("Failed to parse object id"); continue; @@ -2347,52 +2378,64 @@ impl> Se050Backend { if ns_to_check != ns { continue; } - debug!("In ns"); + debug!("In namespace"); debug!("Got parsed: {parsed_obj_id:02x?}"); match parsed_obj_id { - ParsedObjectId::PersistentKey(obj) - if locations.contains(&Location::Internal) - || locations.contains(&Location::External) => - { + ParsedObjectId::PersistentKey(obj) => { + if !should_delete_persistent || !should_delete_keys { + continue; + } + + self.delete_object(obj.0, buf2)?; count += 1; offset = offset.saturating_sub(1); - self.se - .run_command(&DeleteSecureObject { object_id: obj.0 }, buf2) - .map_err(|_err| { - error!("Failed to delete object: {obj:02x?} {_err:?}"); - Error::FunctionFailed - })?; } - ParsedObjectId::VolatileKey(obj) if locations.contains(&Location::Volatile) => { + ParsedObjectId::VolatileKey(obj) => { + if !should_delete_volatile || !should_delete_keys { + continue; + } + + self.delete_object(obj.0, buf2)?; count += 1; offset = offset.saturating_sub(1); - self.se - .run_command(&DeleteSecureObject { object_id: obj.0 }, buf2) - .map_err(|_err| { - error!("Failed to delete object: {obj:02x?} {_err:?}"); - Error::FunctionFailed - })?; } - ParsedObjectId::VolatileRsaKey(obj) - if locations.contains(&Location::Volatile) => - { + ParsedObjectId::VolatileRsaKey(obj) => { + if !should_delete_volatile || !should_delete_keys { + continue; + } + 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(_) - | ParsedObjectId::PinWithDerived(_) - | ParsedObjectId::SaltValue(_) => {} - ParsedObjectId::VolatileKey(_) | ParsedObjectId::VolatileRsaKey(_) => {} - ParsedObjectId::PersistentKey(_) => {} - } - } + ParsedObjectId::Pin(se_id) => { + if !should_delete_pins { + continue; + } - if !to_delete.more.is_more() { - break; + debug!("Deleting simple pin"); + self.delete_object(se_id.0, buf2)?; + count += 1; + offset = offset.saturating_sub(1); + } + ParsedObjectId::PinWithDerived(se_id) => { + if !should_delete_pins { + continue; + } + + 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); + } + // Salt value is not part of any namespace + // It is only deteled through the full device factory reset + ParsedObjectId::SaltValue(_) => {} + } } } + debug_now!("Deleted {count} items"); Ok(count.into()) } @@ -2408,7 +2451,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/staging.rs b/src/staging.rs index a1d9432..40725a9 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 @@ -154,6 +155,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..42ef550 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}, @@ -436,9 +436,16 @@ impl> ExtensionImpl Ok(reply::DeletePin {}.into()) } AuthRequest::DeleteAllPins(request::DeleteAllPins) => { + use crate::core_api::ItemsToDelete; let fs = &mut fs(resources, core_ctx); + // Satisfy the borrow checker + // The `once` trick makes it loose the information that drop is a noop :/ + drop(global_fs); delete_all_pins(fs, self.metadata_location, &mut self.se)?; + + // Ensure that any remaining PIN for the application is also deleted + self.delete_all_items(ItemsToDelete::PINS, &[], ns)?; Ok(reply::DeleteAllPins.into()) } AuthRequest::PinRetries(request) => { 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)