From 61a4fb97846ffcd0617468d321c13eea45868c4b Mon Sep 17 00:00:00 2001
From: Julien Cretin <cretin@google.com>
Date: Wed, 4 Mar 2020 18:50:24 +0100
Subject: [PATCH] Wipe sensitive data on entry deletion
When inserting (or replacing) entries in the store, the data may be marked as
sensitive. When that entry is deleted, the data is wiped by overwritting it with
zeroes. This may cost a few bytes of overhead per entry with sensitive data to
satisfy the constraint that words may only be written twice.
---
src/ctap/storage.rs | 14 ++-
src/embedded_flash/store/bitfield.rs | 5 +
src/embedded_flash/store/format.rs | 91 ++++++++++----
src/embedded_flash/store/mod.rs | 172 +++++++++++++++++++++++----
4 files changed, 233 insertions(+), 49 deletions(-)
diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs
index 7569c1b..565b0b9 100644
--- a/src/ctap/storage.rs
+++ b/src/ctap/storage.rs
@@ -198,6 +198,7 @@ impl PersistentStore {
.insert(StoreEntry {
tag: MASTER_KEYS,
data: &master_keys,
+ sensitive: true,
})
.unwrap();
}
@@ -206,6 +207,7 @@ impl PersistentStore {
.insert(StoreEntry {
tag: PIN_RETRIES,
data: &[MAX_PIN_RETRIES],
+ sensitive: false,
})
.unwrap();
}
@@ -245,6 +247,7 @@ impl PersistentStore {
let new_entry = StoreEntry {
tag: TAG_CREDENTIAL,
data: &credential,
+ sensitive: true,
};
match old_entry {
None => self.store.insert(new_entry)?,
@@ -299,6 +302,7 @@ impl PersistentStore {
.insert(StoreEntry {
tag: GLOBAL_SIGNATURE_COUNTER,
data: &buffer,
+ sensitive: false,
})
.unwrap();
}
@@ -312,6 +316,7 @@ impl PersistentStore {
StoreEntry {
tag: GLOBAL_SIGNATURE_COUNTER,
data: &buffer,
+ sensitive: false,
},
)
.unwrap();
@@ -339,6 +344,7 @@ impl PersistentStore {
let entry = StoreEntry {
tag: PIN_HASH,
data: pin_hash,
+ sensitive: true,
};
match self.store.find_one(&Key::PinHash) {
None => self.store.insert(entry).unwrap(),
@@ -368,6 +374,7 @@ impl PersistentStore {
StoreEntry {
tag: PIN_RETRIES,
data: &[new_value],
+ sensitive: false,
},
)
.unwrap();
@@ -381,6 +388,7 @@ impl PersistentStore {
StoreEntry {
tag: PIN_RETRIES,
data: &[MAX_PIN_RETRIES],
+ sensitive: false,
},
)
.unwrap();
@@ -465,9 +473,9 @@ mod test {
let storage = Storage::new(store, options);
let store = embedded_flash::Store::new(storage, Config).unwrap();
// We can replace 3 bytes with minimal overhead.
- assert_eq!(store.replace_len(0), 2 * WORD_SIZE);
- assert_eq!(store.replace_len(3), 2 * WORD_SIZE);
- assert_eq!(store.replace_len(4), 3 * WORD_SIZE);
+ assert_eq!(store.replace_len(false, 0), 2 * WORD_SIZE);
+ assert_eq!(store.replace_len(false, 3), 3 * WORD_SIZE);
+ assert_eq!(store.replace_len(false, 4), 3 * WORD_SIZE);
}
#[test]
diff --git a/src/embedded_flash/store/bitfield.rs b/src/embedded_flash/store/bitfield.rs
index 60c6f86..797c78b 100644
--- a/src/embedded_flash/store/bitfield.rs
+++ b/src/embedded_flash/store/bitfield.rs
@@ -55,6 +55,11 @@ impl ByteGap {
bit + 8 * self.length
}
}
+
+ /// Returns the slice of `data` corresponding to the gap.
+ pub fn slice(self, data: &[u8]) -> &[u8] {
+ &data[self.start..self.start + self.length]
+ }
}
/// Returns whether a bit is set in a sequence of bits.
diff --git a/src/embedded_flash/store/format.rs b/src/embedded_flash/store/format.rs
index cbef61f..8308c4a 100644
--- a/src/embedded_flash/store/format.rs
+++ b/src/embedded_flash/store/format.rs
@@ -59,6 +59,14 @@ pub struct Format {
/// - 1 for insert entries.
replace_bit: usize,
+ /// Whether a user entry has sensitive data.
+ ///
+ /// When a user entry with sensitive data is deleted, the data is overwritten with zeroes.
+ ///
+ /// - 0 for sensitive data.
+ /// - 1 for non-sensitive data.
+ sensitive_bit: usize,
+
/// The data length of a user entry.
length_range: bitfield::BitRange,
@@ -138,8 +146,9 @@ impl Format {
let deleted_bit = present_bit + 1;
let internal_bit = deleted_bit + 1;
let replace_bit = internal_bit + 1;
+ let sensitive_bit = replace_bit + 1;
let length_range = bitfield::BitRange {
- start: replace_bit + 1,
+ start: sensitive_bit + 1,
length: byte_bits,
};
let tag_range = bitfield::BitRange {
@@ -182,6 +191,7 @@ impl Format {
deleted_bit,
internal_bit,
replace_bit,
+ sensitive_bit,
length_range,
tag_range,
replace_page_range,
@@ -196,10 +206,11 @@ impl Format {
// Make sure all the following conditions hold:
// - The page header is one word.
// - The internal entry is one word.
- // - The entry header fits in one word.
+ // - The entry header fits in one word (which is equivalent to the entry header size being
+ // exactly one word for sensitive entries).
if format.page_header_size() != word_size
|| format.internal_entry_size() != word_size
- || format.header_size() > word_size
+ || format.header_size(true) != word_size
{
return None;
}
@@ -220,28 +231,46 @@ impl Format {
/// Returns the entry header length in bytes.
///
/// This is the smallest number of bytes necessary to store all fields of the entry info up to
- /// and including `length`.
- pub fn header_size(&self) -> usize {
- self.bits_to_bytes(self.length_range.end())
+ /// and including `length`. For sensitive entries, the result is word-aligned.
+ pub fn header_size(&self, sensitive: bool) -> usize {
+ let mut size = self.bits_to_bytes(self.length_range.end());
+ if sensitive {
+ // We need to align to the next word boundary so that wiping the user data will not
+ // count as a write to the header.
+ size = self.align_word(size);
+ }
+ size
+ }
+
+ /// Returns the entry header length in bytes.
+ ///
+ /// This is a convenience function for `header_size` above.
+ fn header_offset(&self, entry: &[u8]) -> usize {
+ self.header_size(self.is_sensitive(entry))
}
/// Returns the entry info length in bytes.
///
/// This is the number of bytes necessary to store all fields of the entry info. This also
- /// includes the internal padding to protect the `committed` bit from the `deleted` bit.
- fn info_size(&self, is_replace: IsReplace) -> usize {
+ /// includes the internal padding to protect the `committed` bit from the `deleted` bit and to
+ /// protect the entry info from the user data for sensitive entries.
+ fn info_size(&self, is_replace: IsReplace, sensitive: bool) -> usize {
let suffix_bits = 2; // committed + complete
let info_bits = match is_replace {
IsReplace::Replace => self.replace_byte_range.end() + suffix_bits,
IsReplace::Insert => self.tag_range.end() + suffix_bits,
};
- let info_size = self.bits_to_bytes(info_bits);
+ let mut info_size = self.bits_to_bytes(info_bits);
// If the suffix bits would end up in the header, we need to add one byte for them.
- if info_size == self.header_size() {
- info_size + 1
- } else {
- info_size
+ let header_size = self.header_size(sensitive);
+ if info_size <= header_size {
+ info_size = header_size + 1;
+ }
+ // If the entry is sensitive, we need to align to the next word boundary.
+ if sensitive {
+ info_size = self.align_word(info_size);
}
+ info_size
}
/// Returns the length in bytes of an entry.
@@ -249,8 +278,8 @@ impl Format {
/// This depends on the length of the user data and whether the entry replaces an old entry or
/// is an insertion. This also includes the internal padding to protect the `committed` bit from
/// the `deleted` bit.
- pub fn entry_size(&self, is_replace: IsReplace, length: usize) -> usize {
- let mut entry_size = length + self.info_size(is_replace);
+ pub fn entry_size(&self, is_replace: IsReplace, sensitive: bool, length: usize) -> usize {
+ let mut entry_size = length + self.info_size(is_replace, sensitive);
let word_size = self.word_size;
entry_size = self.align_word(entry_size);
// The entry must be at least 2 words such that the `committed` and `deleted` bits are on
@@ -308,6 +337,14 @@ impl Format {
bitfield::set_zero(self.replace_bit, header, bitfield::NO_GAP)
}
+ pub fn is_sensitive(&self, header: &[u8]) -> bool {
+ bitfield::is_zero(self.sensitive_bit, header, bitfield::NO_GAP)
+ }
+
+ pub fn set_sensitive(&self, header: &mut [u8]) {
+ bitfield::set_zero(self.sensitive_bit, header, bitfield::NO_GAP)
+ }
+
pub fn get_length(&self, header: &[u8]) -> usize {
bitfield::get_range(self.length_range, header, bitfield::NO_GAP)
}
@@ -317,16 +354,19 @@ impl Format {
}
pub fn get_data<'a>(&self, entry: &'a [u8]) -> &'a [u8] {
- &entry[self.header_size()..][..self.get_length(entry)]
+ &entry[self.header_offset(entry)..][..self.get_length(entry)]
}
/// Returns the span of user data in an entry.
///
/// The complement of this gap in the entry is exactly the entry info. The header is before the
/// gap and the footer is after the gap.
- fn entry_gap(&self, entry: &[u8]) -> bitfield::ByteGap {
- let start = self.header_size();
- let length = self.get_length(entry);
+ pub fn entry_gap(&self, entry: &[u8]) -> bitfield::ByteGap {
+ let start = self.header_offset(entry);
+ let mut length = self.get_length(entry);
+ if self.is_sensitive(entry) {
+ length = self.align_word(length);
+ }
bitfield::ByteGap { start, length }
}
@@ -406,16 +446,23 @@ impl Format {
/// Builds an entry for replace or insert operations.
pub fn build_entry(&self, replace: Option<Index>, user_entry: StoreEntry) -> Vec<u8> {
- let StoreEntry { tag, data } = user_entry;
+ let StoreEntry {
+ tag,
+ data,
+ sensitive,
+ } = user_entry;
let is_replace = match replace {
None => IsReplace::Insert,
Some(_) => IsReplace::Replace,
};
- let entry_len = self.entry_size(is_replace, data.len());
+ let entry_len = self.entry_size(is_replace, sensitive, data.len());
let mut entry = Vec::with_capacity(entry_len);
// Build the header.
- entry.resize(self.header_size(), 0xff);
+ entry.resize(self.header_size(sensitive), 0xff);
self.set_present(&mut entry[..]);
+ if sensitive {
+ self.set_sensitive(&mut entry[..]);
+ }
self.set_length(&mut entry[..], data.len());
// Add the data.
entry.extend_from_slice(data);
diff --git a/src/embedded_flash/store/mod.rs b/src/embedded_flash/store/mod.rs
index 0431073..4111068 100644
--- a/src/embedded_flash/store/mod.rs
+++ b/src/embedded_flash/store/mod.rs
@@ -57,7 +57,7 @@
//! new_page:page_bits
//! Padding(word)
//! Entry := Header Data Footer
-//! // Let X be the byte following `length` in `Info`.
+//! // Let X be the byte (word-aligned for sensitive queries) following `length` in `Info`.
//! Header := Info[..X] // must fit in one word
//! Footer := Info[X..] // must fit in one word
//! Info :=
@@ -65,6 +65,7 @@
//! deleted:1
//! internal=1
//! replace:1
+//! sensitive:1
//! length:byte_bits
//! tag:tag_bits
//! [ // present if `replace` is 0
@@ -109,15 +110,16 @@
//! 0.1 deleted
//! 0.2 internal
//! 0.3 replace
-//! 0.4 length (9 bits)
-//! 1.5 tag (least significant 3 bits out of 5)
+//! 0.4 sensitive
+//! 0.5 length (9 bits)
+//! 1.6 tag (least significant 2 bits out of 5)
//! (the header ends at the first byte boundary after `length`)
//! 2.0 <user data> (2 bytes in this example)
//! (the footer starts immediately after the user data)
-//! 4.0 tag (most significant 2 bits out of 5)
-//! 4.2 replace_page (6 bits)
-//! 5.0 replace_byte (9 bits)
-//! 6.1 padding (make sure the 2 properties below hold)
+//! 4.0 tag (most significant 3 bits out of 5)
+//! 4.3 replace_page (6 bits)
+//! 5.1 replace_byte (9 bits)
+//! 6.2 padding (make sure the 2 properties below hold)
//! 7.6 committed
//! 7.7 complete (on a different word than `present`)
//! 8.0 <end> (word-aligned)
@@ -203,6 +205,11 @@ pub struct StoreEntry<'a> {
/// The data of the entry.
pub data: &'a [u8],
+
+ /// Whether the data is sensitive.
+ ///
+ /// Sensitive data is overwritten with zeroes when the entry is deleted.
+ pub sensitive: bool,
}
/// Implements a configurable multi-set on top of any storage.
@@ -262,6 +269,7 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
StoreEntry {
tag: self.format.get_tag(entry),
data: self.format.get_data(entry),
+ sensitive: self.format.is_sensitive(entry),
},
))
} else {
@@ -326,7 +334,7 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
self.format.validate_entry(new)?;
let mut old_index = old.index;
// Find a slot.
- let entry_len = self.replace_len(new.data.len());
+ let entry_len = self.replace_len(new.sensitive, new.data.len());
let index = self.find_slot_for_write(entry_len, Some(&mut old_index))?;
// Build a new entry replacing the old one.
let entry = self.format.build_entry(Some(old_index), new);
@@ -360,17 +368,20 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
/// Returns the byte cost of a replace operation.
///
/// Computes the length in bytes that would be used in the storage if a replace operation is
- /// executed provided the data of the new entry has `length` bytes.
- pub fn replace_len(&self, length: usize) -> usize {
- self.format.entry_size(IsReplace::Replace, length)
+ /// executed provided the data of the new entry has `length` bytes and whether this data is
+ /// sensitive.
+ pub fn replace_len(&self, sensitive: bool, length: usize) -> usize {
+ self.format
+ .entry_size(IsReplace::Replace, sensitive, length)
}
/// Returns the byte cost of an insert operation.
///
/// Computes the length in bytes that would be used in the storage if an insert operation is
- /// executed provided the data of the inserted entry has `length` bytes.
- pub fn insert_len(&self, length: usize) -> usize {
- self.format.entry_size(IsReplace::Insert, length)
+ /// executed provided the data of the inserted entry has `length` bytes and whether this data is
+ /// sensitive.
+ pub fn insert_len(&self, sensitive: bool, length: usize) -> usize {
+ self.format.entry_size(IsReplace::Insert, sensitive, length)
}
/// Returns the erase count of all pages.
@@ -410,8 +421,11 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
let entry_index = index;
let entry = self.read_entry(index);
index.byte += entry.len();
- if !self.format.is_alive(entry) {
- // Skip deleted entries (or the page padding).
+ if !self.format.is_present(entry) {
+ // Reached the end of the page.
+ } else if self.format.is_deleted(entry) {
+ // Wipe sensitive data if needed.
+ self.wipe_sensitive_data(entry_index);
} else if self.format.is_internal(entry) {
// Finish page compaction.
self.erase_page(entry_index);
@@ -449,6 +463,31 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
/// The provided index must point to the beginning of an entry.
fn delete_index(&mut self, index: Index) {
self.update_word(index, |format, word| format.set_deleted(word));
+ self.wipe_sensitive_data(index);
+ }
+
+ /// Wipes the data of a sensitive entry.
+ ///
+ /// If the entry at the provided index is sensitive, overwrites the data with zeroes. Otherwise,
+ /// does nothing.
+ fn wipe_sensitive_data(&mut self, mut index: Index) {
+ let entry = self.read_entry(index);
+ debug_assert!(self.format.is_present(entry));
+ debug_assert!(self.format.is_deleted(entry));
+ if self.format.is_internal(entry) || !self.format.is_sensitive(entry) {
+ // No need to wipe the data.
+ return;
+ }
+ let gap = self.format.entry_gap(entry);
+ let data = gap.slice(entry);
+ if data.iter().all(|&byte| byte == 0x00) {
+ // The data is already wiped.
+ return;
+ }
+ index.byte += gap.start;
+ self.storage
+ .write_slice(index, &vec![0; gap.length])
+ .unwrap();
}
/// Finds a page with enough free space.
@@ -555,10 +594,13 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
} else if self.format.is_internal(first_byte) {
self.format.internal_entry_size()
} else {
- let header = self.read_slice(index, self.format.header_size());
+ // We don't know if the entry is sensitive or not, but it doesn't matter here. We just
+ // need to read the replace, sensitive, and length fields.
+ let header = self.read_slice(index, self.format.header_size(false));
let replace = self.format.is_replace(header);
+ let sensitive = self.format.is_sensitive(header);
let length = self.format.get_length(header);
- self.format.entry_size(replace, length)
+ self.format.entry_size(replace, sensitive, length)
};
// Truncate the length to fit the page. This can only happen in case of corruption or
// partial writes.
@@ -673,7 +715,7 @@ impl<S: Storage, C: StoreConfig> Store<S, C> {
// Save the old page index and erase count to the new page.
let erase_index = new_index;
let erase_entry = self.format.build_erase_entry(old_page, erase_count);
- self.storage.write_slice(new_index, &erase_entry).unwrap();
+ self.write_entry(new_index, &erase_entry);
// Erase the page.
self.erase_page(erase_index);
// Increase generation.
@@ -728,6 +770,22 @@ impl<C: StoreConfig> Store<BufferStorage, C> {
pub fn set_erase_count(&mut self, page: usize, erase_count: usize) {
self.initialize_page(page, erase_count);
}
+
+ /// Checks whether all deleted sensitive entries have been wiped.
+ pub fn check_wiped(&self) {
+ for (_, entry) in Iter::new(self) {
+ if !self.format.is_present(entry)
+ || !self.format.is_deleted(entry)
+ || self.format.is_internal(entry)
+ || !self.format.is_sensitive(entry)
+ {
+ continue;
+ }
+ let gap = self.format.entry_gap(entry);
+ let data = gap.slice(entry);
+ assert!(data.iter().all(|&byte| byte == 0x00));
+ }
+ }
}
/// Maps an index from an old page to a new page if needed.
@@ -843,7 +901,27 @@ mod tests {
let tag = 0;
let key = 1;
let data = &[key, 2];
- let entry = StoreEntry { tag, data };
+ let entry = StoreEntry {
+ tag,
+ data,
+ sensitive: false,
+ };
+ store.insert(entry).unwrap();
+ assert_eq!(store.iter().count(), 1);
+ assert_eq!(store.find_one(&key).unwrap().1, entry);
+ }
+
+ #[test]
+ fn insert_sensitive_ok() {
+ let mut store = new_store();
+ let tag = 0;
+ let key = 1;
+ let data = &[key, 4];
+ let entry = StoreEntry {
+ tag,
+ data,
+ sensitive: true,
+ };
store.insert(entry).unwrap();
assert_eq!(store.iter().count(), 1);
assert_eq!(store.find_one(&key).unwrap().1, entry);
@@ -857,6 +935,7 @@ mod tests {
let entry = StoreEntry {
tag,
data: &[key, 2],
+ sensitive: false,
};
store.insert(entry).unwrap();
assert_eq!(store.find_all(&key).count(), 1);
@@ -866,6 +945,25 @@ mod tests {
assert_eq!(store.iter().count(), 0);
}
+ #[test]
+ fn delete_sensitive_ok() {
+ let mut store = new_store();
+ let tag = 0;
+ let key = 1;
+ let entry = StoreEntry {
+ tag,
+ data: &[key, 2],
+ sensitive: true,
+ };
+ store.insert(entry).unwrap();
+ assert_eq!(store.find_all(&key).count(), 1);
+ let (index, _) = store.find_one(&key).unwrap();
+ store.delete(index).unwrap();
+ assert_eq!(store.find_all(&key).count(), 0);
+ assert_eq!(store.iter().count(), 0);
+ store.check_wiped();
+ }
+
#[test]
fn insert_until_full() {
let mut store = new_store();
@@ -875,6 +973,7 @@ mod tests {
.insert(StoreEntry {
tag,
data: &[key, 0],
+ sensitive: false,
})
.is_ok()
{
@@ -892,6 +991,7 @@ mod tests {
.insert(StoreEntry {
tag,
data: &[key, 0],
+ sensitive: false,
})
.is_ok()
{
@@ -903,6 +1003,7 @@ mod tests {
.insert(StoreEntry {
tag: 0,
data: &[key, 0],
+ sensitive: false,
})
.unwrap();
for k in 1..=key {
@@ -916,7 +1017,11 @@ mod tests {
let tag = 0;
let key = 1;
let data = &[key, 2];
- let entry = StoreEntry { tag, data };
+ let entry = StoreEntry {
+ tag,
+ data,
+ sensitive: false,
+ };
store.insert(entry).unwrap();
// Reboot the store.
@@ -934,10 +1039,12 @@ mod tests {
let old_entry = StoreEntry {
tag,
data: &[key, 2, 3, 4, 5, 6],
+ sensitive: false,
};
let new_entry = StoreEntry {
tag,
data: &[key, 7, 8, 9],
+ sensitive: false,
};
let mut delay = 0;
loop {
@@ -973,6 +1080,7 @@ mod tests {
.insert(StoreEntry {
tag,
data: &[key, 0],
+ sensitive: false,
})
.is_ok()
{
@@ -983,7 +1091,14 @@ mod tests {
let (index, _) = store.find_one(&1).unwrap();
store.arm_snapshot(delay);
store
- .replace(index, StoreEntry { tag, data: &[1, 1] })
+ .replace(
+ index,
+ StoreEntry {
+ tag,
+ data: &[1, 1],
+ sensitive: false,
+ },
+ )
.unwrap();
let (complete, store) = match store.get_snapshot() {
Err(_) => (true, store.get_storage()),
@@ -995,7 +1110,11 @@ mod tests {
assert_eq!(store.find_all(&k).count(), 1);
assert_eq!(
store.find_one(&k).unwrap().1,
- StoreEntry { tag, data: &[k, 0] }
+ StoreEntry {
+ tag,
+ data: &[k, 0],
+ sensitive: false,
+ }
);
}
assert_eq!(store.find_all(&1).count(), 1);
@@ -1012,7 +1131,11 @@ mod tests {
#[test]
fn invalid_tag() {
let mut store = new_store();
- let entry = StoreEntry { tag: 1, data: &[] };
+ let entry = StoreEntry {
+ tag: 1,
+ data: &[],
+ sensitive: false,
+ };
assert_eq!(store.insert(entry), Err(StoreError::InvalidTag));
}
@@ -1022,6 +1145,7 @@ mod tests {
let entry = StoreEntry {
tag: 0,
data: &[0; PAGE_SIZE],
+ sensitive: false,
};
assert_eq!(store.insert(entry), Err(StoreError::StoreFull));
}
--
GitLab