From 3c774f573651630ac12b78bfeb68cebff6495f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 5 Jun 2024 16:03:18 +0200 Subject: [PATCH] Simplify delete_all_items --- src/core_api.rs | 118 +++++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 57 deletions(-) diff --git a/src/core_api.rs b/src/core_api.rs index 404a278..505d267 100644 --- a/src/core_api.rs +++ b/src/core_api.rs @@ -233,7 +233,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:?}"); @@ -2020,7 +2020,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 })?; @@ -2059,7 +2059,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 })?; @@ -2247,7 +2247,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 })?; @@ -2315,6 +2315,16 @@ 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, kinds: ItemsToDelete, @@ -2325,8 +2335,18 @@ impl> Se050Backend { let buf2 = &mut [0; 128]; let mut count = 0u16; let mut offset = 0; + let mut cont = true; + + let should_delete_internal = locations.contains(&Location::Internal); + let should_delete_external = locations.contains(&Location::External); + let should_delete_volatile = locations.contains(&Location::Volatile); + // Persistent keys do not have a "location" + let should_delete_persistent = should_delete_internal || should_delete_external; - loop { + let should_delete_keys = kinds.contains(ItemsToDelete::KEYS); + let should_delete_pins = kinds.contains(ItemsToDelete::PINS); + + while cont { let to_delete = self .se .run_command( @@ -2340,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; @@ -2357,81 +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) - && kinds.contains(ItemsToDelete::KEYS) => - { + 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) - && kinds.contains(ItemsToDelete::KEYS) => - { + 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) - && kinds.contains(ItemsToDelete::KEYS) => - { + 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(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); - } + ParsedObjectId::Pin(se_id) => { + if !should_delete_pins { + continue; } + + debug!("Deleting simple pin"); + self.delete_object(se_id.0, buf2)?; count += 1; offset = offset.saturating_sub(1); } - ParsedObjectId::PinWithDerived(se_id) - if kinds.contains(ItemsToDelete::PINS) => - { + 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); } - ParsedObjectId::Pin(_) - | ParsedObjectId::PinWithDerived(_) - | ParsedObjectId::SaltValue(_) => {} - ParsedObjectId::VolatileKey(_) | ParsedObjectId::VolatileRsaKey(_) => {} - ParsedObjectId::PersistentKey(_) => {} + // Salt value is not part of any namespace + // It is only deteled through the full device factory reset + ParsedObjectId::SaltValue(_) => {} } } - - if !to_delete.more.is_more() { - break; - } } + debug_now!("Deleted {count} items"); Ok(count.into()) }