From db26f6125b69f7dd77a7d213e2ca06be6cac8c6d Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Tue, 31 May 2022 21:46:02 +0300 Subject: [PATCH 1/9] Implement file-backed persistent storage This allows running ctap2 authenticator application on non-embedded host OS to implement virtual FIDO2 authenticator for QEMU --- libraries/persistent_store/Cargo.toml | 4 + libraries/persistent_store/src/file.rs | 183 ++++++++++++++++++++++ libraries/persistent_store/src/lib.rs | 4 + libraries/persistent_store/src/storage.rs | 12 ++ libraries/persistent_store/src/store.rs | 2 + run_desktop_tests.sh | 2 +- 6 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 libraries/persistent_store/src/file.rs diff --git a/libraries/persistent_store/Cargo.toml b/libraries/persistent_store/Cargo.toml index 5fe7c9f..7c778f8 100644 --- a/libraries/persistent_store/Cargo.toml +++ b/libraries/persistent_store/Cargo.toml @@ -7,5 +7,9 @@ edition = "2018" [dependencies] +[dev-dependencies] +tempfile = "3" + [features] std = [] +hostenv = ["std"] diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs new file mode 100644 index 0000000..bb40e67 --- /dev/null +++ b/libraries/persistent_store/src/file.rs @@ -0,0 +1,183 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! File-backed persistent flash storage for virtual authenticator. +//! +//! [`FileStorage`] implements the flash [`Storage`] interface but doesn't interface with an +//! actual flash storage. Instead it uses a host-based file to persist the storage state. + +use crate::{BufferOptions, BufferStorage, Storage, StorageIndex, StorageResult}; +use std::fs::{File, OpenOptions}; +use std::io::{Read, Seek, SeekFrom, Write}; +use std::path::Path; + +/// Simulates a flash storage using a host-based file. +/// +/// It provides same functions as BufferStorage for testing, but also saves stored +/// data between application restarts. +/// +/// Metadata, such as word write and page erase counters are not saved between restarts. +/// +pub struct FileStorage { + /// Content of the storage. + storage: BufferStorage, + /// File to persist contents of the storage. + backing_file: File, +} + +const PAGE_SIZE: usize = 0x1000; +const NUM_PAGES: usize = 20; + +impl FileStorage { + pub fn new(path: &Path) -> StorageResult { + let options = BufferOptions { + word_size: 4, + page_size: PAGE_SIZE, + max_word_writes: 2, + max_page_erases: 10000, + strict_mode: true, + }; + let store = vec![0xff; NUM_PAGES * PAGE_SIZE].into_boxed_slice(); + let store_size = (&store).len() as u64; + let mut storage = BufferStorage::new(store, options); + + let mut backing_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(path)?; + let file_len = backing_file.metadata()?.len(); + + if file_len == 0 { + backing_file.set_len(store_size)?; + backing_file.seek(SeekFrom::Start(0))?; + for i in 0..storage.num_pages() { + let buf = storage.read_slice(StorageIndex { page: i, byte: 0 }, PAGE_SIZE)?; + backing_file.write(&buf)?; + } + } else if file_len == store_size { + backing_file.seek(SeekFrom::Start(0))?; + let mut buf = [0u8; PAGE_SIZE]; + for i in 0..storage.num_pages() { + backing_file.read(&mut buf)?; + storage.write_slice(StorageIndex { page: i, byte: 0 }, &buf)?; + } + } else { + // FileStorage buffer should be of fixed size, opening previously saved file + // from storage of different size is not supported + panic!("Invalid file size {}, should be {}", file_len, store_size); + } + Ok(FileStorage { + backing_file, + storage, + }) + } +} + +impl Storage for FileStorage { + fn word_size(&self) -> usize { + self.storage.word_size() + } + + fn page_size(&self) -> usize { + self.storage.page_size() + } + + fn num_pages(&self) -> usize { + self.storage.num_pages() + } + + fn max_word_writes(&self) -> usize { + self.storage.max_word_writes() + } + + fn max_page_erases(&self) -> usize { + self.storage.max_page_erases() + } + + fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult<&[u8]> { + self.storage.read_slice(index, length) + } + + fn write_slice(&mut self, index: StorageIndex, value: &[u8]) -> StorageResult<()> { + self.backing_file.seek(SeekFrom::Start( + (index.page * self.page_size() + index.byte) as u64, + ))?; + self.backing_file.write_all(value)?; + self.storage.write_slice(index, value) + } + + fn erase_page(&mut self, page: usize) -> StorageResult<()> { + self.backing_file + .seek(SeekFrom::Start((page * self.page_size()) as u64))?; + self.backing_file + .write_all(&vec![0xffu8; self.page_size()][..])?; + self.storage.erase_page(page) + } +} + +impl core::fmt::Display for FileStorage { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + self.storage.fmt(f) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + use tempfile::TempDir; + + const BLANK_WORD: &[u8] = &[0xff, 0xff, 0xff, 0xff]; + const DATA_WORD: &[u8] = &[0xee, 0xdd, 0xbb, 0x77]; + + const FILE_NAME: &str = "opensk_storage.bin"; + + fn make_tmp_dir() -> PathBuf { + let tmp_dir = TempDir::new().unwrap(); + tmp_dir.into_path() + } + + fn remove_tmp_dir(tmp_dir: PathBuf) { + std::fs::remove_dir_all(tmp_dir).unwrap(); + } + + fn temp_storage(tmp_dir: &PathBuf) -> FileStorage { + let mut tmp_file = tmp_dir.clone(); + tmp_file.push(FILE_NAME); + FileStorage::new(&tmp_file).unwrap() + } + + #[test] + fn read_write_persist_ok() { + let index = StorageIndex { page: 0, byte: 0 }; + let next_index = StorageIndex { page: 0, byte: 4 }; + + let tmp_dir = make_tmp_dir(); + { + let mut file_storage = temp_storage(&tmp_dir); + assert_eq!(file_storage.read_slice(index, 4).unwrap(), BLANK_WORD); + file_storage.write_slice(index, DATA_WORD).unwrap(); + assert_eq!(file_storage.read_slice(index, 4).unwrap(), DATA_WORD); + assert_eq!(file_storage.read_slice(next_index, 4).unwrap(), BLANK_WORD); + } + // Reload and check the data from previously persisted storage + { + let file_storage = temp_storage(&tmp_dir); + assert_eq!(file_storage.read_slice(index, 4).unwrap(), DATA_WORD); + assert_eq!(file_storage.read_slice(next_index, 4).unwrap(), BLANK_WORD); + } + remove_tmp_dir(tmp_dir); + } +} diff --git a/libraries/persistent_store/src/lib.rs b/libraries/persistent_store/src/lib.rs index ca82a47..31332f5 100644 --- a/libraries/persistent_store/src/lib.rs +++ b/libraries/persistent_store/src/lib.rs @@ -365,6 +365,8 @@ extern crate alloc; mod buffer; #[cfg(feature = "std")] mod driver; +#[cfg(feature = "hostenv")] +mod file; mod format; pub mod fragment; #[cfg(feature = "std")] @@ -380,6 +382,8 @@ pub use self::buffer::{BufferCorruptFunction, BufferOptions, BufferStorage}; pub use self::driver::{ StoreDriver, StoreDriverOff, StoreDriverOn, StoreInterruption, StoreInvariant, }; +#[cfg(feature = "hostenv")] +pub use self::file::FileStorage; #[cfg(feature = "std")] pub use self::model::{StoreModel, StoreOperation}; pub use self::storage::{Storage, StorageError, StorageIndex, StorageResult}; diff --git a/libraries/persistent_store/src/storage.rs b/libraries/persistent_store/src/storage.rs index 696c258..d0a0659 100644 --- a/libraries/persistent_store/src/storage.rs +++ b/libraries/persistent_store/src/storage.rs @@ -34,6 +34,18 @@ pub enum StorageError { /// Implementation-specific error. CustomError, + + // I/O error + #[cfg(feature = "hostenv")] + IOError, +} + +#[cfg(feature = "hostenv")] +#[allow(unused_variables)] +impl From for StorageError { + fn from(error: std::io::Error) -> Self { + Self::IOError + } } pub type StorageResult = Result; diff --git a/libraries/persistent_store/src/store.rs b/libraries/persistent_store/src/store.rs index b946878..ebaf806 100644 --- a/libraries/persistent_store/src/store.rs +++ b/libraries/persistent_store/src/store.rs @@ -72,6 +72,8 @@ impl From for StoreError { fn from(error: StorageError) -> StoreError { match error { StorageError::CustomError => StoreError::StorageError, + #[cfg(feature = "hostenv")] + StorageError::IOError => StoreError::StorageError, // The store always calls the storage correctly. StorageError::NotAligned | StorageError::OutOfBounds => unreachable!(), } diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index a7dad81..384ba4c 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -120,7 +120,7 @@ then cargo test --features std cd ../.. cd libraries/persistent_store - cargo test --features std + cargo test --features std,hostenv cd ../.. cargo test --features std From 1cf7373bfe5763bba4dbb7e9a8ff46a2631e1785 Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Sun, 5 Jun 2022 15:14:07 +0300 Subject: [PATCH 2/9] With new Storage API there's no need to double-buffer file data read_slice(...) can return Cow::Owned buffer to the caller --- libraries/persistent_store/src/file.rs | 104 ++++++++++++------------- 1 file changed, 50 insertions(+), 54 deletions(-) diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs index bb40e67..af28733 100644 --- a/libraries/persistent_store/src/file.rs +++ b/libraries/persistent_store/src/file.rs @@ -17,23 +17,26 @@ //! [`FileStorage`] implements the flash [`Storage`] interface but doesn't interface with an //! actual flash storage. Instead it uses a host-based file to persist the storage state. -use crate::{BufferOptions, BufferStorage, Storage, StorageIndex, StorageResult}; +use crate::{BufferOptions, Storage, StorageIndex, StorageResult}; +use alloc::borrow::Cow; +use core::cell::RefCell; use std::fs::{File, OpenOptions}; use std::io::{Read, Seek, SeekFrom, Write}; use std::path::Path; /// Simulates a flash storage using a host-based file. /// -/// It provides same functions as BufferStorage for testing, but also saves stored -/// data between application restarts. -/// -/// Metadata, such as word write and page erase counters are not saved between restarts. -/// +/// This is usable for emulating authenticator hardware on VM hypervisor's host OS pub struct FileStorage { - /// Content of the storage. - storage: BufferStorage, - /// File to persist contents of the storage. - backing_file: File, + // Options of the storage + buffer_options: BufferOptions, + + /// File for persisting contents of the storage + /// Reading data from File requires mutable reference, as seeking and reading data + /// changes file's current position. + /// All operations on backing file internally always first seek to needed position, + /// so it's safe to borrow mutable reference to backing file for the time of operation. + backing_file_ref: RefCell, } const PAGE_SIZE: usize = 0x1000; @@ -41,95 +44,88 @@ const NUM_PAGES: usize = 20; impl FileStorage { pub fn new(path: &Path) -> StorageResult { - let options = BufferOptions { + let buffer_options = BufferOptions { word_size: 4, page_size: PAGE_SIZE, max_word_writes: 2, max_page_erases: 10000, strict_mode: true, }; - let store = vec![0xff; NUM_PAGES * PAGE_SIZE].into_boxed_slice(); - let store_size = (&store).len() as u64; - let mut storage = BufferStorage::new(store, options); - let mut backing_file = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .open(path)?; + let mut backing_file_ref = RefCell::new( + OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(path)?, + ); + let backing_file = backing_file_ref.get_mut(); let file_len = backing_file.metadata()?.len(); + let store_len: u64 = (PAGE_SIZE * NUM_PAGES) as u64; if file_len == 0 { - backing_file.set_len(store_size)?; backing_file.seek(SeekFrom::Start(0))?; - for i in 0..storage.num_pages() { - let buf = storage.read_slice(StorageIndex { page: i, byte: 0 }, PAGE_SIZE)?; + for _ in 0..NUM_PAGES { + let buf = [0xffu8; PAGE_SIZE]; backing_file.write(&buf)?; } - } else if file_len == store_size { - backing_file.seek(SeekFrom::Start(0))?; - let mut buf = [0u8; PAGE_SIZE]; - for i in 0..storage.num_pages() { - backing_file.read(&mut buf)?; - storage.write_slice(StorageIndex { page: i, byte: 0 }, &buf)?; - } - } else { + } else if file_len != store_len { // FileStorage buffer should be of fixed size, opening previously saved file // from storage of different size is not supported - panic!("Invalid file size {}, should be {}", file_len, store_size); + panic!("Invalid file size {}, should be {}", file_len, store_len); } Ok(FileStorage { - backing_file, - storage, + buffer_options, + backing_file_ref, }) } } impl Storage for FileStorage { fn word_size(&self) -> usize { - self.storage.word_size() + self.buffer_options.word_size } fn page_size(&self) -> usize { - self.storage.page_size() + self.buffer_options.page_size } fn num_pages(&self) -> usize { - self.storage.num_pages() + NUM_PAGES } fn max_word_writes(&self) -> usize { - self.storage.max_word_writes() + self.buffer_options.max_word_writes } fn max_page_erases(&self) -> usize { - self.storage.max_page_erases() + self.buffer_options.max_page_erases } - fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult<&[u8]> { - self.storage.read_slice(index, length) + fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult> { + let mut backing_file = self.backing_file_ref.borrow_mut(); + backing_file.seek(SeekFrom::Start( + (index.page * self.page_size() + index.byte) as u64, + ))?; + let mut buf = vec![0u8; length]; + backing_file.read_exact(&mut buf)?; + Ok(Cow::Owned(buf)) } fn write_slice(&mut self, index: StorageIndex, value: &[u8]) -> StorageResult<()> { - self.backing_file.seek(SeekFrom::Start( + let mut backing_file = self.backing_file_ref.borrow_mut(); + backing_file.seek(SeekFrom::Start( (index.page * self.page_size() + index.byte) as u64, ))?; - self.backing_file.write_all(value)?; - self.storage.write_slice(index, value) + backing_file.write_all(value)?; + Ok(()) } fn erase_page(&mut self, page: usize) -> StorageResult<()> { - self.backing_file - .seek(SeekFrom::Start((page * self.page_size()) as u64))?; - self.backing_file - .write_all(&vec![0xffu8; self.page_size()][..])?; - self.storage.erase_page(page) - } -} - -impl core::fmt::Display for FileStorage { - fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { - self.storage.fmt(f) + let mut backing_file = self.backing_file_ref.borrow_mut(); + backing_file.seek(SeekFrom::Start((page * self.page_size()) as u64))?; + backing_file.write_all(&vec![0xffu8; self.page_size()][..])?; + Ok(()) } } From f2cb2f72e70b9058560694f5a645679dd00f8320 Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Sun, 5 Jun 2022 22:39:13 +0300 Subject: [PATCH 3/9] Use StorageError::CustomError for implementations-specific (I/O) errors --- libraries/persistent_store/src/storage.rs | 11 +++-------- libraries/persistent_store/src/store.rs | 2 -- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/libraries/persistent_store/src/storage.rs b/libraries/persistent_store/src/storage.rs index d0a0659..473965b 100644 --- a/libraries/persistent_store/src/storage.rs +++ b/libraries/persistent_store/src/storage.rs @@ -34,17 +34,12 @@ pub enum StorageError { /// Implementation-specific error. CustomError, - - // I/O error - #[cfg(feature = "hostenv")] - IOError, } -#[cfg(feature = "hostenv")] -#[allow(unused_variables)] +#[cfg(feature = "std")] impl From for StorageError { - fn from(error: std::io::Error) -> Self { - Self::IOError + fn from(_: std::io::Error) -> Self { + Self::CustomError } } diff --git a/libraries/persistent_store/src/store.rs b/libraries/persistent_store/src/store.rs index ebaf806..b946878 100644 --- a/libraries/persistent_store/src/store.rs +++ b/libraries/persistent_store/src/store.rs @@ -72,8 +72,6 @@ impl From for StoreError { fn from(error: StorageError) -> StoreError { match error { StorageError::CustomError => StoreError::StorageError, - #[cfg(feature = "hostenv")] - StorageError::IOError => StoreError::StorageError, // The store always calls the storage correctly. StorageError::NotAligned | StorageError::OutOfBounds => unreachable!(), } From c0299c3225103a31dd9f9cc3fa49a928b146431a Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Sun, 5 Jun 2022 22:40:32 +0300 Subject: [PATCH 4/9] No need for specific feature, use std instead --- libraries/persistent_store/Cargo.toml | 1 - libraries/persistent_store/src/lib.rs | 4 ++-- run_desktop_tests.sh | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/persistent_store/Cargo.toml b/libraries/persistent_store/Cargo.toml index 7c778f8..b01cfdb 100644 --- a/libraries/persistent_store/Cargo.toml +++ b/libraries/persistent_store/Cargo.toml @@ -12,4 +12,3 @@ tempfile = "3" [features] std = [] -hostenv = ["std"] diff --git a/libraries/persistent_store/src/lib.rs b/libraries/persistent_store/src/lib.rs index 31332f5..a36d945 100644 --- a/libraries/persistent_store/src/lib.rs +++ b/libraries/persistent_store/src/lib.rs @@ -365,7 +365,7 @@ extern crate alloc; mod buffer; #[cfg(feature = "std")] mod driver; -#[cfg(feature = "hostenv")] +#[cfg(feature = "std")] mod file; mod format; pub mod fragment; @@ -382,7 +382,7 @@ pub use self::buffer::{BufferCorruptFunction, BufferOptions, BufferStorage}; pub use self::driver::{ StoreDriver, StoreDriverOff, StoreDriverOn, StoreInterruption, StoreInvariant, }; -#[cfg(feature = "hostenv")] +#[cfg(feature = "std")] pub use self::file::FileStorage; #[cfg(feature = "std")] pub use self::model::{StoreModel, StoreOperation}; diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index 384ba4c..a7dad81 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -120,7 +120,7 @@ then cargo test --features std cd ../.. cd libraries/persistent_store - cargo test --features std,hostenv + cargo test --features std cd ../.. cargo test --features std From 4e479682332558bdb46669577daced94ba897cef Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Sun, 5 Jun 2022 22:40:59 +0300 Subject: [PATCH 5/9] Separate options type for FileStorage, remove hardcoded constants --- libraries/persistent_store/src/file.rs | 53 +++++++++++++++----------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs index af28733..9c931a5 100644 --- a/libraries/persistent_store/src/file.rs +++ b/libraries/persistent_store/src/file.rs @@ -17,7 +17,7 @@ //! [`FileStorage`] implements the flash [`Storage`] interface but doesn't interface with an //! actual flash storage. Instead it uses a host-based file to persist the storage state. -use crate::{BufferOptions, Storage, StorageIndex, StorageResult}; +use crate::{Storage, StorageIndex, StorageResult}; use alloc::borrow::Cow; use core::cell::RefCell; use std::fs::{File, OpenOptions}; @@ -29,7 +29,7 @@ use std::path::Path; /// This is usable for emulating authenticator hardware on VM hypervisor's host OS pub struct FileStorage { // Options of the storage - buffer_options: BufferOptions, + options: FileOptions, /// File for persisting contents of the storage /// Reading data from File requires mutable reference, as seeking and reading data @@ -39,19 +39,20 @@ pub struct FileStorage { backing_file_ref: RefCell, } -const PAGE_SIZE: usize = 0x1000; -const NUM_PAGES: usize = 20; +/// Options for file-backed storage +pub struct FileOptions { + /// Size of a word in bytes. + pub word_size: usize, + + /// Size of a page in bytes. + pub page_size: usize, + + /// Number of pages in storage + pub num_pages: usize, +} impl FileStorage { - pub fn new(path: &Path) -> StorageResult { - let buffer_options = BufferOptions { - word_size: 4, - page_size: PAGE_SIZE, - max_word_writes: 2, - max_page_erases: 10000, - strict_mode: true, - }; - + pub fn new(path: &Path, options: FileOptions) -> StorageResult { let mut backing_file_ref = RefCell::new( OpenOptions::new() .read(true) @@ -61,12 +62,12 @@ impl FileStorage { ); let backing_file = backing_file_ref.get_mut(); let file_len = backing_file.metadata()?.len(); - let store_len: u64 = (PAGE_SIZE * NUM_PAGES) as u64; + let store_len: u64 = (options.page_size * options.num_pages) as u64; if file_len == 0 { backing_file.seek(SeekFrom::Start(0))?; - for _ in 0..NUM_PAGES { - let buf = [0xffu8; PAGE_SIZE]; + let buf = vec![0xffu8; options.page_size]; + for _ in 0..options.num_pages { backing_file.write(&buf)?; } } else if file_len != store_len { @@ -75,7 +76,7 @@ impl FileStorage { panic!("Invalid file size {}, should be {}", file_len, store_len); } Ok(FileStorage { - buffer_options, + options, backing_file_ref, }) } @@ -83,23 +84,23 @@ impl FileStorage { impl Storage for FileStorage { fn word_size(&self) -> usize { - self.buffer_options.word_size + self.options.word_size } fn page_size(&self) -> usize { - self.buffer_options.page_size + self.options.page_size } fn num_pages(&self) -> usize { - NUM_PAGES + self.options.num_pages } fn max_word_writes(&self) -> usize { - self.buffer_options.max_word_writes + usize::MAX } fn max_page_erases(&self) -> usize { - self.buffer_options.max_page_erases + usize::MAX } fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult> { @@ -140,6 +141,12 @@ mod tests { const FILE_NAME: &str = "opensk_storage.bin"; + const OPTIONS: FileOptions = FileOptions { + word_size: 4, + page_size: 0x1000, + num_pages: 20, + }; + fn make_tmp_dir() -> PathBuf { let tmp_dir = TempDir::new().unwrap(); tmp_dir.into_path() @@ -152,7 +159,7 @@ mod tests { fn temp_storage(tmp_dir: &PathBuf) -> FileStorage { let mut tmp_file = tmp_dir.clone(); tmp_file.push(FILE_NAME); - FileStorage::new(&tmp_file).unwrap() + FileStorage::new(&tmp_file, OPTIONS).unwrap() } #[test] From 660b6b76b2eaa789ce0839bc1251309611dd0710 Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Mon, 6 Jun 2022 15:06:29 +0300 Subject: [PATCH 6/9] Make FileOptions available to library's users --- libraries/persistent_store/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/persistent_store/src/lib.rs b/libraries/persistent_store/src/lib.rs index a36d945..75eed03 100644 --- a/libraries/persistent_store/src/lib.rs +++ b/libraries/persistent_store/src/lib.rs @@ -383,7 +383,7 @@ pub use self::driver::{ StoreDriver, StoreDriverOff, StoreDriverOn, StoreInterruption, StoreInvariant, }; #[cfg(feature = "std")] -pub use self::file::FileStorage; +pub use self::file::{FileOptions, FileStorage}; #[cfg(feature = "std")] pub use self::model::{StoreModel, StoreOperation}; pub use self::storage::{Storage, StorageError, StorageIndex, StorageResult}; From 27080749498020826292955fd2c74cc9472b6363 Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Mon, 6 Jun 2022 15:08:42 +0300 Subject: [PATCH 7/9] Improve variable names and comments readability --- libraries/persistent_store/src/file.rs | 51 ++++++++++++++------------ 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs index 9c931a5..925c86e 100644 --- a/libraries/persistent_store/src/file.rs +++ b/libraries/persistent_store/src/file.rs @@ -26,20 +26,22 @@ use std::path::Path; /// Simulates a flash storage using a host-based file. /// -/// This is usable for emulating authenticator hardware on VM hypervisor's host OS +/// This is usable for emulating authenticator hardware on VM hypervisor's host OS. pub struct FileStorage { - // Options of the storage + // Options of the storage. options: FileOptions, - /// File for persisting contents of the storage + /// File for persisting contents of the storage. + /// /// Reading data from File requires mutable reference, as seeking and reading data /// changes file's current position. + /// /// All operations on backing file internally always first seek to needed position, /// so it's safe to borrow mutable reference to backing file for the time of operation. - backing_file_ref: RefCell, + file: RefCell, } -/// Options for file-backed storage +/// Options for file-backed storage. pub struct FileOptions { /// Size of a word in bytes. pub word_size: usize, @@ -47,28 +49,28 @@ pub struct FileOptions { /// Size of a page in bytes. pub page_size: usize, - /// Number of pages in storage + /// Number of pages in storage. pub num_pages: usize, } impl FileStorage { pub fn new(path: &Path, options: FileOptions) -> StorageResult { - let mut backing_file_ref = RefCell::new( + let mut file_ref = RefCell::new( OpenOptions::new() .read(true) .write(true) .create(true) .open(path)?, ); - let backing_file = backing_file_ref.get_mut(); - let file_len = backing_file.metadata()?.len(); + let file = file_ref.get_mut(); + let file_len = file.metadata()?.len(); let store_len: u64 = (options.page_size * options.num_pages) as u64; if file_len == 0 { - backing_file.seek(SeekFrom::Start(0))?; + file.seek(SeekFrom::Start(0))?; let buf = vec![0xffu8; options.page_size]; for _ in 0..options.num_pages { - backing_file.write(&buf)?; + file.write(&buf)?; } } else if file_len != store_len { // FileStorage buffer should be of fixed size, opening previously saved file @@ -77,7 +79,7 @@ impl FileStorage { } Ok(FileStorage { options, - backing_file_ref, + file: file_ref, }) } } @@ -104,28 +106,29 @@ impl Storage for FileStorage { } fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult> { - let mut backing_file = self.backing_file_ref.borrow_mut(); - backing_file.seek(SeekFrom::Start( - (index.page * self.page_size() + index.byte) as u64, - ))?; + let mut file = self.file.borrow_mut(); + file.seek(SeekFrom::Start(index.range(length, self)?.start as u64))?; let mut buf = vec![0u8; length]; - backing_file.read_exact(&mut buf)?; + file.read_exact(&mut buf)?; Ok(Cow::Owned(buf)) } fn write_slice(&mut self, index: StorageIndex, value: &[u8]) -> StorageResult<()> { - let mut backing_file = self.backing_file_ref.borrow_mut(); - backing_file.seek(SeekFrom::Start( - (index.page * self.page_size() + index.byte) as u64, + let mut file = self.file.borrow_mut(); + file.seek(SeekFrom::Start( + index.range(value.len(), self)?.start as u64, ))?; - backing_file.write_all(value)?; + file.write_all(value)?; Ok(()) } fn erase_page(&mut self, page: usize) -> StorageResult<()> { - let mut backing_file = self.backing_file_ref.borrow_mut(); - backing_file.seek(SeekFrom::Start((page * self.page_size()) as u64))?; - backing_file.write_all(&vec![0xffu8; self.page_size()][..])?; + let mut file = self.file.borrow_mut(); + let index = StorageIndex { page, byte: 0 }; + file.seek(SeekFrom::Start( + index.range(self.page_size(), self)?.start as u64, + ))?; + file.write_all(&vec![0xffu8; self.page_size()][..])?; Ok(()) } } From 0158cc846d221d47050c3bc5ddbce8755b2ce83a Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Tue, 7 Jun 2022 15:58:08 +0300 Subject: [PATCH 8/9] Use 0xff for consistency --- libraries/persistent_store/src/file.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs index 925c86e..29fa470 100644 --- a/libraries/persistent_store/src/file.rs +++ b/libraries/persistent_store/src/file.rs @@ -68,7 +68,7 @@ impl FileStorage { if file_len == 0 { file.seek(SeekFrom::Start(0))?; - let buf = vec![0xffu8; options.page_size]; + let buf = vec![0xff; options.page_size]; for _ in 0..options.num_pages { file.write(&buf)?; } @@ -128,7 +128,7 @@ impl Storage for FileStorage { file.seek(SeekFrom::Start( index.range(self.page_size(), self)?.start as u64, ))?; - file.write_all(&vec![0xffu8; self.page_size()][..])?; + file.write_all(&vec![0xff; self.page_size()][..])?; Ok(()) } } From cc1fb2543eadd0fd9c85a7adcdd28ed019a9755f Mon Sep 17 00:00:00 2001 From: Egor Duda Date: Tue, 7 Jun 2022 17:04:18 +0300 Subject: [PATCH 9/9] Accommodate Store requirements for max_word_writes and max_page_erases --- libraries/persistent_store/src/file.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/persistent_store/src/file.rs b/libraries/persistent_store/src/file.rs index 29fa470..552cc7e 100644 --- a/libraries/persistent_store/src/file.rs +++ b/libraries/persistent_store/src/file.rs @@ -98,11 +98,15 @@ impl Storage for FileStorage { } fn max_word_writes(&self) -> usize { - usize::MAX + // We can write an unlimited amount of times in a file, but the store arithmetic + // uses `Nat` so the value should fit in a `Nat`. + u32::MAX as usize } fn max_page_erases(&self) -> usize { - usize::MAX + // We can "erase" an unlimited amount of times in a file, but the store format + // encodes the number of erase cycles on 16 bits. + u16::MAX as usize } fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult> {