Skip to content

Commit

Permalink
Fix factory-reset-app not deleting pins
Browse files Browse the repository at this point in the history
  • Loading branch information
sosthene-nitrokey committed May 24, 2024
1 parent 91619be commit 51d55de
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 11 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
47 changes: 43 additions & 4 deletions src/core_api.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
//! 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},
Checked, CtChoice, Encoding, U4096,
};
use embedded_hal::blocking::delay::DelayUs;
use hex_literal::hex;
use iso7816::Status;
use littlefs2::path::PathBuf;
use rand::{CryptoRng, RngCore};
use se05x::{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -2308,6 +2317,7 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> Se050Backend<Twi, D> {

pub(crate) fn delete_all_items(
&mut self,
kinds: ItemsToDelete,
locations: &[Location],
ns: NamespaceValue,
) -> Result<usize, Error> {
Expand Down Expand Up @@ -2352,7 +2362,8 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> Se050Backend<Twi, D> {
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);
Expand All @@ -2363,7 +2374,10 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> Se050Backend<Twi, D> {
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
Expand All @@ -2374,13 +2388,38 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> Se050Backend<Twi, D> {
})?;
}
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(_) => {}
Expand Down Expand Up @@ -2408,7 +2447,7 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> Se050Backend<Twi, D> {
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,
Expand Down
28 changes: 28 additions & 0 deletions src/manage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,34 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> ExtensionImpl<Se050ManageExtension> 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())
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/staging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Twi: I2CForT1, D: DelayUs<u32>> ExtensionImpl<WrapKeyToFileExtension>
Expand Down Expand Up @@ -159,6 +160,7 @@ impl<Twi: I2CForT1, D: DelayUs<u32>> ExtensionImpl<ManageExtension> for Se050Bac
Error::RequestNotAvailable
})?;
self.delete_all_items(
ItemsToDelete::KEYS | ItemsToDelete::PINS,
&[Location::Volatile, Location::External, Location::Internal],
ns,
)?;
Expand Down
2 changes: 1 addition & 1 deletion src/trussed_auth_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
33 changes: 27 additions & 6 deletions src/trussed_auth_impl/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,8 @@ impl PinData {
pub fn delete_with_derived<Twi: I2CForT1, D: DelayUs<u32>>(
se050: &mut Se05X<Twi, D>,
se_id: PinObjectIdWithDerived,
) -> Result<(), Error> {
) -> Result<u16, Error> {
let mut count = 0;
let buf = &mut [0; 1024];
debug!("checking existence");
let exists = se050
Expand Down Expand Up @@ -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 ");
Expand Down Expand Up @@ -525,31 +544,33 @@ impl PinData {
error!("Failed to delete user id: {err:?}");
err
})?;
Ok(())
count += 1;
Ok(count)
}

pub fn delete<Twi: I2CForT1, D: DelayUs<u32>>(
self,
fs: &mut impl Filestore,
location: Location,
se050: &mut Se05X<Twi, D>,
) -> Result<(), Error> {
) -> Result<u16, Error> {
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)
Expand Down

0 comments on commit 51d55de

Please sign in to comment.