Merge remote-tracking branch 'upstream/master' into ctap1-new-apdu-parser

.github/workflows/cargo_check.yml

@@ -66,12 +66,6 @@ jobs:
           command: check
           args: --target thumbv7em-none-eabi --release --features debug_allocations
 
-      - name: Check OpenSK ram_storage
-        uses: actions-rs/cargo@v1
-        with:
-          command: check
-          args: --target thumbv7em-none-eabi --release --features ram_storage
-
       - name: Check OpenSK verbose
         uses: actions-rs/cargo@v1
         with:

.vscode/extensions.json

@@ -1,7 +0,0 @@
-{
-    "recommendations": [
-        "davidanson.vscode-markdownlint",
-        "rust-lang.rust",
-        "ms-python.python"
-    ]
-}

.vscode/settings.json

@@ -1,27 +0,0 @@
-{
-  "clang-format.fallbackStyle": "google",
-  "editor.detectIndentation": true,
-  "editor.formatOnPaste": false,
-  "editor.formatOnSave": true,
-  "editor.formatOnType": true,
-  "editor.insertSpaces": true,
-  "editor.tabSize": 4,
-  "files.insertFinalNewline": true,
-  "files.trimTrailingWhitespace": true,
-  "rust-client.channel": "nightly",
-  // The toolchain is updated from time to time so let's make sure that RLS is updated too
-  "rust-client.updateOnStartup": true,
-  "rust.clippy_preference": "on",
-  // Try to make VSCode formating as close as possible to the Google style.
-  "python.formatting.provider": "yapf",
-  "python.formatting.yapfArgs": [
-    "--style=yapf"
-  ],
-  "python.linting.enabled": true,
-  "python.linting.lintOnSave": true,
-  "python.linting.pylintEnabled": true,
-  "python.linting.pylintPath": "pylint",
-  "[python]": {
-    "editor.tabSize": 2
-  },
-}

Cargo.toml

@@ -15,6 +15,7 @@ libtock_drivers = { path = "third_party/libtock-drivers" }
 lang_items = { path = "third_party/lang-items" }
 cbor = { path = "libraries/cbor" }
 crypto = { path = "libraries/crypto" }
+persistent_store = { path = "libraries/persistent_store" }
 byteorder = { version = "1", default-features = false }
 arrayref = "0.3.6"
 subtle = { version = "2.2", default-features = false, features = ["nightly"] }
@@ -23,8 +24,7 @@ subtle = { version = "2.2", default-features = false, features = ["nightly"] }
 debug_allocations = ["lang_items/debug_allocations"]
 debug_ctap = ["crypto/derive_debug", "libtock_drivers/debug_ctap"]
 panic_console = ["lang_items/panic_console"]
-std = ["cbor/std", "crypto/std", "crypto/derive_debug", "lang_items/std"]
+std = ["cbor/std", "crypto/std", "crypto/derive_debug", "lang_items/std", "persistent_store/std"]
-ram_storage = []
 verbose = ["debug_ctap", "libtock_drivers/verbose_usb"]
 with_ctap1 = ["crypto/with_ctap1"]
 with_ctap2_1 = []

OpenSK.code-workspace

@@ -0,0 +1,57 @@
+{
+        "folders": [
+                {
+                        "name": "OpenSK",
+                        "path": "."
+                },
+                {
+                        "name": "tock",
+                        "path": "third_party/tock"
+                },
+                {
+                        "name": "libtock-rs",
+                        "path": "third_party/libtock-rs"
+                },
+                {
+                        "name": "libtock-drivers",
+                        "path": "third_party/libtock-drivers"
+                }
+        ],
+        "settings": {
+                "clang-format.fallbackStyle": "google",
+                "editor.detectIndentation": true,
+                "editor.formatOnPaste": false,
+                "editor.formatOnSave": true,
+                "editor.formatOnType": true,
+                "editor.insertSpaces": true,
+                "editor.tabSize": 4,
+                "files.insertFinalNewline": true,
+                "files.trimTrailingWhitespace": true,
+                // Ensure we use the toolchain we set in rust-toolchain file
+                "rust-client.channel": "default",
+                // The toolchain is updated from time to time so let's make sure that RLS is updated too
+                "rust-client.updateOnStartup": true,
+                "rust.clippy_preference": "on",
+                "rust.target": "thumbv7em-none-eabi",
+                "rust.all_targets": false,
+                // Try to make VSCode formating as close as possible to the Google style.
+                "python.formatting.provider": "yapf",
+                "python.formatting.yapfArgs": [
+                        "--style=yapf"
+                ],
+                "python.linting.enabled": true,
+                "python.linting.lintOnSave": true,
+                "python.linting.pylintEnabled": true,
+                "python.linting.pylintPath": "pylint",
+                "[python]": {
+                        "editor.tabSize": 2
+                },
+        },
+        "extensions": {
+                "recommendations": [
+                        "davidanson.vscode-markdownlint",
+                        "rust-lang.rust",
+                        "ms-python.python"
+                ]
+        }
+}

deploy.py

@@ -863,14 +863,6 @@ if __name__ == "__main__":
             "This is useful to allow flashing multiple OpenSK authenticators "
             "in a row without them being considered clones."),
   )
-  main_parser.add_argument(
-      "--no-persistent-storage",
-      action="append_const",
-      const="ram_storage",
-      dest="features",
-      help=("Compiles and installs the OpenSK application without persistent "
-            "storage (i.e. unplugging the key will reset the key)."),
-  )
 
   main_parser.add_argument(
       "--elf2tab-output",
@@ -907,6 +899,12 @@ if __name__ == "__main__":
       const="crypto_bench",
       help=("Compiles and installs the crypto_bench example that benchmarks "
             "the performance of the cryptographic algorithms on the board."))
+  apps_group.add_argument(
+      "--store_latency",
+      dest="application",
+      action="store_const",
+      const="store_latency",
+      help=("Compiles and installs the store_latency example."))
   apps_group.add_argument(
       "--panic_test",
       dest="application",

examples/store_latency.rs

@@ -0,0 +1,138 @@
+// Copyright 2019-2020 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.
+
+#![no_std]
+
+extern crate alloc;
+extern crate lang_items;
+
+use alloc::vec;
+use core::fmt::Write;
+use ctap2::embedded_flash::{new_storage, Storage};
+use libtock_drivers::console::Console;
+use libtock_drivers::timer::{self, Duration, Timer, Timestamp};
+use persistent_store::Store;
+
+fn timestamp(timer: &Timer) -> Timestamp<f64> {
+    Timestamp::<f64>::from_clock_value(timer.get_current_clock().ok().unwrap())
+}
+
+fn measure<T>(timer: &Timer, operation: impl FnOnce() -> T) -> (T, Duration<f64>) {
+    let before = timestamp(timer);
+    let result = operation();
+    let after = timestamp(timer);
+    (result, after - before)
+}
+
+// Only use one store at a time.
+unsafe fn boot_store(num_pages: usize, erase: bool) -> Store<Storage> {
+    let mut storage = new_storage(num_pages);
+    if erase {
+        for page in 0..num_pages {
+            use persistent_store::Storage;
+            storage.erase_page(page).unwrap();
+        }
+    }
+    Store::new(storage).ok().unwrap()
+}
+
+fn compute_latency(timer: &Timer, num_pages: usize, key_increment: usize, word_length: usize) {
+    let mut console = Console::new();
+    writeln!(
+        console,
+        "\nLatency for num_pages={} key_increment={} word_length={}.",
+        num_pages, key_increment, word_length
+    )
+    .unwrap();
+
+    let mut store = unsafe { boot_store(num_pages, true) };
+    let total_capacity = store.capacity().unwrap().total();
+    assert_eq!(store.capacity().unwrap().used(), 0);
+    assert_eq!(store.lifetime().unwrap().used(), 0);
+
+    // Burn N words to align the end of the user capacity with the virtual capacity.
+    store.insert(0, &vec![0; 4 * (num_pages - 1)]).unwrap();
+    store.remove(0).unwrap();
+    assert_eq!(store.capacity().unwrap().used(), 0);
+    assert_eq!(store.lifetime().unwrap().used(), num_pages);
+
+    // Insert entries until there is space for one more.
+    let count = total_capacity / (1 + word_length) - 1;
+    let ((), time) = measure(timer, || {
+        for i in 0..count {
+            let key = 1 + key_increment * i;
+            // For some reason the kernel sometimes fails.
+            while store.insert(key, &vec![0; 4 * word_length]).is_err() {
+                // We never enter this loop in practice, but we still need it for the kernel.
+                writeln!(console, "Retry insert.").unwrap();
+            }
+        }
+    });
+    writeln!(console, "Setup: {:.1}ms for {} entries.", time.ms(), count).unwrap();
+
+    // Measure latency of insert.
+    let key = 1 + key_increment * count;
+    let ((), time) = measure(&timer, || {
+        store.insert(key, &vec![0; 4 * word_length]).unwrap()
+    });
+    writeln!(console, "Insert: {:.1}ms.", time.ms()).unwrap();
+    assert_eq!(
+        store.lifetime().unwrap().used(),
+        num_pages + (1 + count) * (1 + word_length)
+    );
+
+    // Measure latency of boot.
+    let (mut store, time) = measure(&timer, || unsafe { boot_store(num_pages, false) });
+    writeln!(console, "Boot: {:.1}ms.", time.ms()).unwrap();
+
+    // Measure latency of remove.
+    let ((), time) = measure(&timer, || store.remove(key).unwrap());
+    writeln!(console, "Remove: {:.1}ms.", time.ms()).unwrap();
+
+    // Measure latency of compaction.
+    let length = total_capacity + num_pages - store.lifetime().unwrap().used();
+    if length > 0 {
+        // Fill the store such that compaction is needed for one word.
+        store.insert(0, &vec![0; 4 * (length - 1)]).unwrap();
+        store.remove(0).unwrap();
+    }
+    assert!(store.capacity().unwrap().remaining() > 0);
+    assert_eq!(store.lifetime().unwrap().used(), num_pages + total_capacity);
+    let ((), time) = measure(timer, || store.prepare(1).unwrap());
+    writeln!(console, "Compaction: {:.1}ms.", time.ms()).unwrap();
+    assert!(store.lifetime().unwrap().used() > total_capacity + num_pages);
+}
+
+fn main() {
+    let mut with_callback = timer::with_callback(|_, _| {});
+    let timer = with_callback.init().ok().unwrap();
+
+    writeln!(Console::new(), "\nRunning 4 tests...").unwrap();
+    // Those non-overwritten 50 words entries simulate credentials.
+    compute_latency(&timer, 3, 1, 50);
+    compute_latency(&timer, 20, 1, 50);
+    // Those overwritten 1 word entries simulate counters.
+    compute_latency(&timer, 3, 0, 1);
+    compute_latency(&timer, 6, 0, 1);
+    writeln!(Console::new(), "\nDone.").unwrap();
+
+    // Results on nrf52840dk:
+    //
+    // | Pages | Overwrite | Length    | Boot     | Compaction | Insert  | Remove  |
+    // | ----- | --------- | --------- | -------  | ---------- | ------  | ------- |
+    // | 3     | no        | 50 words  | 2.0 ms   | 132.5 ms   | 4.8 ms  | 1.2 ms  |
+    // | 20    | no        | 50 words  | 7.4 ms   | 135.5 ms   | 10.2 ms | 3.9 ms  |
+    // | 3     | yes       | 1 word    | 21.9 ms  | 94.5 ms    | 12.4 ms | 5.9 ms  |
+    // | 6     | yes       | 1 word    | 55.2 ms  | 100.8 ms   | 24.8 ms | 12.1 ms |
+}

fuzz/fuzz_helper/Cargo.toml

@@ -10,5 +10,5 @@ arrayref = "0.3.6"
 libtock_drivers = { path = "../../third_party/libtock-drivers" }
 crypto = { path = "../../libraries/crypto", features = ['std'] }
 cbor = { path = "../../libraries/cbor", features = ['std'] }
-ctap2 = { path = "../..", features = ['std', 'ram_storage'] }
+ctap2 = { path = "../..", features = ['std'] }
 lang_items = { path = "../../third_party/lang-items", features = ['std'] }

libraries/persistent_store/fuzz/src/store.rs

@@ -26,6 +26,9 @@ use std::convert::TryInto;
 // NOTE: We should be able to improve coverage by only checking the last operation. Because
 // operations before the last could be checked with a shorter entropy.
 
+// NOTE: Maybe we should split the fuzz target in smaller parts (like one per init). We should also
+// name the fuzz targets with action names.
+
 /// Checks the store against a sequence of manipulations.
 ///
 /// The entropy to generate the sequence of manipulation should be provided in `data`. Debugging
@@ -181,7 +184,7 @@ impl<'a> Fuzzer<'a> {
             println!("Power on the store.");
         }
         self.increment(StatKey::PowerOnCount);
-        let interruption = self.interruption(driver.delay_map());
+        let interruption = self.interruption(driver.count_operations());
         match driver.partial_power_on(interruption) {
             Err((storage, _)) if self.init.is_dirty() => {
                 self.entropy.consume_all();
@@ -198,7 +201,7 @@ impl<'a> Fuzzer<'a> {
         if self.debug {
             println!("{:?}", operation);
         }
-        let interruption = self.interruption(driver.delay_map(&operation));
+        let interruption = self.interruption(driver.count_operations(&operation));
         match driver.partial_apply(operation, interruption) {
             Err((store, _)) if self.init.is_dirty() => {
                 self.entropy.consume_all();
@@ -334,59 +337,48 @@ impl<'a> Fuzzer<'a> {
 
     /// Generates an interruption.
     ///
-    /// The `delay_map` describes the number of modified bits by the upcoming sequence of store
+    /// The `max_delay` describes the number of storage operations.
-    /// operations.
+    fn interruption(&mut self, max_delay: Option<usize>) -> StoreInterruption {
-    // TODO(ia0): We use too much CPU to compute the delay map. We should be able to just count the
-    // number of storage operations by checking the remaining delay. We can then use the entropy
-    // directly from the corruption function because it's called at most once.
-    fn interruption(
-        &mut self,
-        delay_map: Result<Vec<usize>, (usize, BufferStorage)>,
-    ) -> StoreInterruption {
         if self.init.is_dirty() {
             // We only test that the store can power on without crashing. If it would get
             // interrupted then it's like powering up with a different initial state, which would be
             // tested with another fuzzing input.
             return StoreInterruption::none();
         }
-        let delay_map = match delay_map {
+        let max_delay = match max_delay {
-            Ok(x) => x,
+            Some(x) => x,
-            Err((delay, storage)) => {
+            None => return StoreInterruption::none(),
-                print!("{}", storage);
-                panic!("delay={}", delay);
-            }
         };
-        let delay = self.entropy.read_range(0, delay_map.len() - 1);
+        let delay = self.entropy.read_range(0, max_delay);
-        let mut complete_bits = BitStack::default();
-        for _ in 0..delay_map[delay] {
-            complete_bits.push(self.entropy.read_bit());
-        }
         if self.debug {
-            if delay == delay_map.len() - 1 {
+            if delay == max_delay {
-                assert!(complete_bits.is_empty());
                 println!("Do not interrupt.");
             } else {
-                println!(
+                println!("Interrupt after {} operations.", delay);
-                    "Interrupt after {} operations with complete mask {}.",
-                    delay, complete_bits
-                );
             }
         }
-        if delay < delay_map.len() - 1 {
+        if delay < max_delay {
             self.increment(StatKey::InterruptionCount);
         }
         let corrupt = Box::new(move |old: &mut [u8], new: &[u8]| {
+            let mut count = 0;
+            let mut total = 0;
             for (old, new) in old.iter_mut().zip(new.iter()) {
                 for bit in 0..8 {
                     let mask = 1 << bit;
                     if *old & mask == *new & mask {
                         continue;
                     }
-                    if complete_bits.pop().unwrap() {
+                    total += 1;
+                    if self.entropy.read_bit() {
+                        count += 1;
                         *old ^= mask;
                     }
                 }
             }
+            if self.debug {
+                println!("Flip {} bits out of {}.", count, total);
+            }
         });
         StoreInterruption { delay, corrupt }
     }
@@ -432,113 +424,3 @@ impl Init {
         }
     }
 }
-
-/// Compact stack of bits.
-// NOTE: This would probably go away once the delay map is simplified.
-#[derive(Default, Clone, Debug)]
-struct BitStack {
-    /// Bits stored in little-endian (for bytes and bits).
-    ///
-    /// The last byte only contains `len` bits.
-    data: Vec<u8>,
-
-    /// Number of bits stored in the last byte.
-    ///
-    /// It is 0 if the last byte is full, not 8.
-    len: usize,
-}
-
-impl BitStack {
-    /// Returns whether the stack is empty.
-    fn is_empty(&self) -> bool {
-        self.len() == 0
-    }
-
-    /// Returns the length of the stack.
-    fn len(&self) -> usize {
-        if self.len == 0 {
-            8 * self.data.len()
-        } else {
-            8 * (self.data.len() - 1) + self.len
-        }
-    }
-
-    /// Pushes a bit to the stack.
-    fn push(&mut self, value: bool) {
-        if self.len == 0 {
-            self.data.push(0);
-        }
-        if value {
-            *self.data.last_mut().unwrap() |= 1 << self.len;
-        }
-        self.len += 1;
-        if self.len == 8 {
-            self.len = 0;
-        }
-    }
-
-    /// Pops a bit from the stack.
-    fn pop(&mut self) -> Option<bool> {
-        if self.len == 0 {
-            if self.data.is_empty() {
-                return None;
-            }
-            self.len = 8;
-        }
-        self.len -= 1;
-        let result = self.data.last().unwrap() & 1 << self.len;
-        if self.len == 0 {
-            self.data.pop().unwrap();
-        }
-        Some(result != 0)
-    }
-}
-
-impl std::fmt::Display for BitStack {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
-        let mut bits = self.clone();
-        while let Some(bit) = bits.pop() {
-            write!(f, "{}", bit as usize)?;
-        }
-        write!(f, " ({} bits)", self.len())?;
-        Ok(())
-    }
-}
-
-#[test]
-fn bit_stack_ok() {
-    let mut bits = BitStack::default();
-
-    assert_eq!(bits.pop(), None);
-
-    bits.push(true);
-    assert_eq!(bits.pop(), Some(true));
-    assert_eq!(bits.pop(), None);
-
-    bits.push(false);
-    assert_eq!(bits.pop(), Some(false));
-    assert_eq!(bits.pop(), None);
-
-    bits.push(true);
-    bits.push(false);
-    assert_eq!(bits.pop(), Some(false));
-    assert_eq!(bits.pop(), Some(true));
-    assert_eq!(bits.pop(), None);
-
-    bits.push(false);
-    bits.push(true);
-    assert_eq!(bits.pop(), Some(true));
-    assert_eq!(bits.pop(), Some(false));
-    assert_eq!(bits.pop(), None);
-
-    let n = 27;
-    for i in 0..n {
-        assert_eq!(bits.len(), i);
-        bits.push(true);
-    }
-    for i in (0..n).rev() {
-        assert_eq!(bits.pop(), Some(true));
-        assert_eq!(bits.len(), i);
-    }
-    assert_eq!(bits.pop(), None);
-}

libraries/persistent_store/src/driver.rs

@@ -311,31 +311,15 @@ impl StoreDriverOff {
         })
     }
 
-    /// Returns a mapping from delay time to number of modified bits.
+    /// Returns the number of storage operations to power on.
     ///
-    /// For example if the `i`-th value is `n`, it means that the `i`-th operation modifies `n` bits
+    /// Returns `None` if the store cannot power on successfully.
-    /// in the storage. For convenience, the vector always ends with `0` for one past the last
+    pub fn count_operations(&self) -> Option<usize> {
-    /// operation. This permits to choose a random index in the vector and then a random set of bit
+        let initial_delay = usize::MAX;
-    /// positions among the number of modified bits to simulate any possible corruption (including
+        let mut storage = self.storage.clone();
-    /// no corruption with the last index).
+        storage.arm_interruption(initial_delay);
-    pub fn delay_map(&self) -> Result<Vec<usize>, (usize, BufferStorage)> {
+        let mut store = Store::new(storage).ok()?;
-        let mut result = Vec::new();
+        Some(initial_delay - store.storage_mut().disarm_interruption())
-        loop {
-            let delay = result.len();
-            let mut storage = self.storage.clone();
-            storage.arm_interruption(delay);
-            match Store::new(storage) {
-                Err((StoreError::StorageError, x)) => storage = x,
-                Err((StoreError::InvalidStorage, mut storage)) => {
-                    storage.reset_interruption();
-                    return Err((delay, storage));
-                }
-                Ok(_) | Err(_) => break,
-            }
-            result.push(count_modified_bits(&mut storage));
-        }
-        result.push(0);
-        Ok(result)
     }
 }
 
@@ -422,29 +406,15 @@ impl StoreDriverOn {
         })
     }
 
-    /// Returns a mapping from delay time to number of modified bits.
+    /// Returns the number of storage operations to apply a store operation.
     ///
-    /// See the documentation of [`StoreDriverOff::delay_map`] for details.
+    /// Returns `None` if the store cannot apply the operation successfully.
-    ///
+    pub fn count_operations(&self, operation: &StoreOperation) -> Option<usize> {
-    /// [`StoreDriverOff::delay_map`]: struct.StoreDriverOff.html#method.delay_map
+        let initial_delay = usize::MAX;
-    pub fn delay_map(
+        let mut store = self.store.clone();
-        &self,
+        store.storage_mut().arm_interruption(initial_delay);
-        operation: &StoreOperation,
+        store.apply(operation).1.ok()?;
-    ) -> Result<Vec<usize>, (usize, BufferStorage)> {
+        Some(initial_delay - store.storage_mut().disarm_interruption())
-        let mut result = Vec::new();
-        loop {
-            let delay = result.len();
-            let mut store = self.store.clone();
-            store.storage_mut().arm_interruption(delay);
-            match store.apply(operation).1 {
-                Err(StoreError::StorageError) => (),
-                Err(StoreError::InvalidStorage) => return Err((delay, store.extract_storage())),
-                Ok(()) | Err(_) => break,
-            }
-            result.push(count_modified_bits(store.storage_mut()));
-        }
-        result.push(0);
-        Ok(result)
     }
 
     /// Powers off the store.
@@ -629,22 +599,3 @@ impl<'a> StoreInterruption<'a> {
         }
     }
 }
-
-/// Counts the number of bits modified by an interrupted operation.
-///
-/// # Panics
-///
-/// Panics if an interruption did not trigger.
-fn count_modified_bits(storage: &mut BufferStorage) -> usize {
-    let mut modified_bits = 0;
-    storage.corrupt_operation(Box::new(|before, after| {
-        modified_bits = before
-            .iter()
-            .zip(after.iter())
-            .map(|(x, y)| (x ^ y).count_ones() as usize)
-            .sum();
-    }));
-    // We should never write the same slice or erase an erased page.
-    assert!(modified_bits > 0);
-    modified_bits
-}

libraries/persistent_store/src/lib.rs

@@ -348,6 +348,7 @@
 #[macro_use]
 extern crate alloc;
 
+#[cfg(feature = "std")]
 mod buffer;
 #[cfg(feature = "std")]
 mod driver;
@@ -357,6 +358,7 @@ mod model;
 mod storage;
 mod store;
 
+#[cfg(feature = "std")]
 pub use self::buffer::{BufferCorruptFunction, BufferOptions, BufferStorage};
 #[cfg(feature = "std")]
 pub use self::driver::{

run_desktop_tests.sh

@@ -48,7 +48,6 @@ cargo check --release --target=thumbv7em-none-eabi --features with_ctap2_1
 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap
 cargo check --release --target=thumbv7em-none-eabi --features panic_console
 cargo check --release --target=thumbv7em-none-eabi --features debug_allocations
-cargo check --release --target=thumbv7em-none-eabi --features ram_storage
 cargo check --release --target=thumbv7em-none-eabi --features verbose
 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1
 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1,panic_console,debug_allocations,verbose

src/ctap/apdu.rs

@@ -1,3 +1,17 @@
+// Copyright 2020 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.
+
 use alloc::vec::Vec;
 use byteorder::{BigEndian, ByteOrder};
 use core::convert::TryFrom;

src/ctap/ctap1.rs

@@ -254,7 +254,7 @@ impl Ctap1Command {
         let sk = crypto::ecdsa::SecKey::gensk(ctap_state.rng);
         let pk = sk.genpk();
         let key_handle = ctap_state
-            .encrypt_key_handle(sk, &application, None)
+            .encrypt_key_handle(sk, &application)
             .map_err(|_| Ctap1StatusCode::SW_INTERNAL_EXCEPTION)?;
         if key_handle.len() > 0xFF {
             // This is just being defensive with unreachable code.
@@ -288,7 +288,7 @@ impl Ctap1Command {
         signature_data.extend(key_handle);
         signature_data.extend_from_slice(&user_pk);
 
-        let attestation_key = crypto::ecdsa::SecKey::from_bytes(private_key).unwrap();
+        let attestation_key = crypto::ecdsa::SecKey::from_bytes(&private_key).unwrap();
         let signature = attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);
 
         response.extend(signature.to_asn1_der());
@@ -349,7 +349,7 @@ impl Ctap1Command {
 
 #[cfg(test)]
 mod test {
-    use super::super::{key_material, CREDENTIAL_ID_BASE_SIZE, USE_SIGNATURE_COUNTER};
+    use super::super::{key_material, CREDENTIAL_ID_SIZE, USE_SIGNATURE_COUNTER};
     use super::*;
     use crypto::rng256::ThreadRng256;
     use crypto::Hash256;
@@ -389,12 +389,12 @@ mod test {
             0x00,
             0x00,
             0x00,
-            65 + CREDENTIAL_ID_BASE_SIZE as u8,
+            65 + CREDENTIAL_ID_SIZE as u8,
         ];
         let challenge = [0x0C; 32];
         message.extend(&challenge);
         message.extend(application);
-        message.push(CREDENTIAL_ID_BASE_SIZE as u8);
+        message.push(CREDENTIAL_ID_SIZE as u8);
         message.extend(key_handle);
         message
     }
@@ -434,15 +434,12 @@ mod test {
         let response =
             Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE).unwrap();
         assert_eq!(response[0], Ctap1Command::LEGACY_BYTE);
-        assert_eq!(response[66], CREDENTIAL_ID_BASE_SIZE as u8);
+        assert_eq!(response[66], CREDENTIAL_ID_SIZE as u8);
         assert!(ctap_state
-            .decrypt_credential_source(
+            .decrypt_credential_source(response[67..67 + CREDENTIAL_ID_SIZE].to_vec(), &application)
-                response[67..67 + CREDENTIAL_ID_BASE_SIZE].to_vec(),
-                &application
-            )
             .unwrap()
             .is_some());
-        const CERT_START: usize = 67 + CREDENTIAL_ID_BASE_SIZE;
+        const CERT_START: usize = 67 + CREDENTIAL_ID_SIZE;
         assert_eq!(
             &response[CERT_START..CERT_START + fake_cert.len()],
             &fake_cert[..]
@@ -491,9 +488,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
         let response = Ctap1Command::process_command(&message, &mut ctap_state, START_CLOCK_VALUE);
@@ -509,9 +504,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let application = [0x55; 32];
         let message = create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
 
@@ -528,9 +521,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let mut message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -563,9 +554,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[0] = 0xEE;
@@ -583,9 +572,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[1] = 0xEE;
@@ -603,9 +590,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let mut message =
             create_authenticate_message(&application, Ctap1Flags::CheckOnly, &key_handle);
         message[2] = 0xEE;
@@ -631,9 +616,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -660,9 +643,7 @@ mod test {
 
         let rp_id = "example.com";
         let application = crypto::sha256::Sha256::hash(rp_id.as_bytes());
-        let key_handle = ctap_state
+        let key_handle = ctap_state.encrypt_key_handle(sk, &application).unwrap();
-            .encrypt_key_handle(sk, &application, None)
-            .unwrap();
         let message = create_authenticate_message(
             &application,
             Ctap1Flags::DontEnforceUpAndSign,
@@ -684,7 +665,7 @@ mod test {
     #[test]
     fn test_process_authenticate_bad_key_handle() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; CREDENTIAL_ID_BASE_SIZE];
+        let key_handle = vec![0x00; CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 
@@ -701,7 +682,7 @@ mod test {
     #[test]
     fn test_process_authenticate_without_up() {
         let application = [0x0A; 32];
-        let key_handle = vec![0x00; CREDENTIAL_ID_BASE_SIZE];
+        let key_handle = vec![0x00; CREDENTIAL_ID_SIZE];
         let message =
             create_authenticate_message(&application, Ctap1Flags::EnforceUpAndSign, &key_handle);
 

src/ctap/data_formats.rs

@@ -498,7 +498,6 @@ pub struct PublicKeyCredentialSource {
     pub rp_id: String,
     pub user_handle: Vec<u8>, // not optional, but nullable
     pub user_display_name: Option<String>,
-    pub cred_random: Option<Vec<u8>>,
     pub cred_protect_policy: Option<CredentialProtectionPolicy>,
     pub creation_order: u64,
     pub user_name: Option<String>,
@@ -513,14 +512,14 @@ enum PublicKeyCredentialSourceField {
     RpId = 2,
     UserHandle = 3,
     UserDisplayName = 4,
-    CredRandom = 5,
     CredProtectPolicy = 6,
     CreationOrder = 7,
     UserName = 8,
     UserIcon = 9,
     // When a field is removed, its tag should be reserved and not used for new fields. We document
     // those reserved tags below.
-    // Reserved tags: none.
+    // Reserved tags:
+    // - CredRandom = 5,
 }
 
 impl From<PublicKeyCredentialSourceField> for cbor::KeyType {
@@ -539,7 +538,6 @@ impl From<PublicKeyCredentialSource> for cbor::Value {
             PublicKeyCredentialSourceField::RpId => Some(credential.rp_id),
             PublicKeyCredentialSourceField::UserHandle => Some(credential.user_handle),
             PublicKeyCredentialSourceField::UserDisplayName => credential.user_display_name,
-            PublicKeyCredentialSourceField::CredRandom => credential.cred_random,
             PublicKeyCredentialSourceField::CredProtectPolicy => credential.cred_protect_policy,
             PublicKeyCredentialSourceField::CreationOrder => credential.creation_order,
             PublicKeyCredentialSourceField::UserName => credential.user_name,
@@ -559,7 +557,6 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
                 PublicKeyCredentialSourceField::RpId => rp_id,
                 PublicKeyCredentialSourceField::UserHandle => user_handle,
                 PublicKeyCredentialSourceField::UserDisplayName => user_display_name,
-                PublicKeyCredentialSourceField::CredRandom => cred_random,
                 PublicKeyCredentialSourceField::CredProtectPolicy => cred_protect_policy,
                 PublicKeyCredentialSourceField::CreationOrder => creation_order,
                 PublicKeyCredentialSourceField::UserName => user_name,
@@ -577,7 +574,6 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
         let rp_id = extract_text_string(ok_or_missing(rp_id)?)?;
         let user_handle = extract_byte_string(ok_or_missing(user_handle)?)?;
         let user_display_name = user_display_name.map(extract_text_string).transpose()?;
-        let cred_random = cred_random.map(extract_byte_string).transpose()?;
         let cred_protect_policy = cred_protect_policy
             .map(CredentialProtectionPolicy::try_from)
             .transpose()?;
@@ -601,7 +597,6 @@ impl TryFrom<cbor::Value> for PublicKeyCredentialSource {
             rp_id,
             user_handle,
             user_display_name,
-            cred_random,
             cred_protect_policy,
             creation_order,
             user_name,
@@ -1373,7 +1368,6 @@ mod test {
             rp_id: "example.com".to_string(),
             user_handle: b"foo".to_vec(),
             user_display_name: None,
-            cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
             user_name: None,
@@ -1395,16 +1389,6 @@ mod test {
             Ok(credential.clone())
         );
 
-        let credential = PublicKeyCredentialSource {
-            cred_random: Some(vec![0x00; 32]),
-            ..credential
-        };
-
-        assert_eq!(
-            PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())),
-            Ok(credential.clone())
-        );
-
         let credential = PublicKeyCredentialSource {
             cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationOptional),
             ..credential

src/ctap/mod.rs

@@ -86,10 +86,8 @@ pub const INITIAL_SIGNATURE_COUNTER: u32 = 1;
 // - 16 byte initialization vector for AES-256,
 // - 32 byte ECDSA private key for the credential,
 // - 32 byte relying party ID hashed with SHA256,
-// - (optional) 32 byte for HMAC-secret,
 // - 32 byte HMAC-SHA256 over everything else.
-pub const CREDENTIAL_ID_BASE_SIZE: usize = 112;
+pub const CREDENTIAL_ID_SIZE: usize = 112;
-pub const CREDENTIAL_ID_MAX_SIZE: usize = CREDENTIAL_ID_BASE_SIZE + 32;
 // Set this bit when checking user presence.
 const UP_FLAG: u8 = 0x01;
 // Set this bit when checking user verification.
@@ -142,6 +140,7 @@ struct AssertionInput {
     client_data_hash: Vec<u8>,
     auth_data: Vec<u8>,
     hmac_secret_input: Option<GetAssertionHmacSecretInput>,
+    has_uv: bool,
 }
 
 struct AssertionState {
@@ -231,7 +230,6 @@ where
         &mut self,
         private_key: crypto::ecdsa::SecKey,
         application: &[u8; 32],
-        cred_random: Option<&[u8; 32]>,
     ) -> Result<Vec<u8>, Ctap2StatusCode> {
         let master_keys = self.persistent_store.master_keys()?;
         let aes_enc_key = crypto::aes256::EncryptionKey::new(&master_keys.encryption);
@@ -240,19 +238,14 @@ where
         let mut iv = [0; 16];
         iv.copy_from_slice(&self.rng.gen_uniform_u8x32()[..16]);
 
-        let block_len = if cred_random.is_some() { 6 } else { 4 };
+        let mut blocks = [[0u8; 16]; 4];
-        let mut blocks = vec![[0u8; 16]; block_len];
         blocks[0].copy_from_slice(&sk_bytes[..16]);
         blocks[1].copy_from_slice(&sk_bytes[16..]);
         blocks[2].copy_from_slice(&application[..16]);
         blocks[3].copy_from_slice(&application[16..]);
-        if let Some(cred_random) = cred_random {
-            blocks[4].copy_from_slice(&cred_random[..16]);
-            blocks[5].copy_from_slice(&cred_random[16..]);
-        }
         cbc_encrypt(&aes_enc_key, iv, &mut blocks);
 
-        let mut encrypted_id = Vec::with_capacity(16 * (block_len + 3));
+        let mut encrypted_id = Vec::with_capacity(0x70);
         encrypted_id.extend(&iv);
         for b in &blocks {
             encrypted_id.extend(b);
@@ -270,11 +263,9 @@ where
         credential_id: Vec<u8>,
         rp_id_hash: &[u8],
     ) -> Result<Option<PublicKeyCredentialSource>, Ctap2StatusCode> {
-        let has_cred_random = match credential_id.len() {
+        if credential_id.len() != CREDENTIAL_ID_SIZE {
-            CREDENTIAL_ID_BASE_SIZE => false,
+            return Ok(None);
-            CREDENTIAL_ID_MAX_SIZE => true,
+        }
-            _ => return Ok(None),
-        };
         let master_keys = self.persistent_store.master_keys()?;
         let payload_size = credential_id.len() - 32;
         if !verify_hmac_256::<Sha256>(
@@ -288,9 +279,8 @@ where
         let aes_dec_key = crypto::aes256::DecryptionKey::new(&aes_enc_key);
         let mut iv = [0; 16];
         iv.copy_from_slice(&credential_id[..16]);
-        let block_len = if has_cred_random { 6 } else { 4 };
+        let mut blocks = [[0u8; 16]; 4];
-        let mut blocks = vec![[0u8; 16]; block_len];
+        for i in 0..4 {
-        for i in 0..block_len {
             blocks[i].copy_from_slice(&credential_id[16 * (i + 1)..16 * (i + 2)]);
         }
 
@@ -301,15 +291,6 @@ where
         decrypted_sk[16..].clone_from_slice(&blocks[1]);
         decrypted_rp_id_hash[..16].clone_from_slice(&blocks[2]);
         decrypted_rp_id_hash[16..].clone_from_slice(&blocks[3]);
-        let cred_random = if has_cred_random {
-            let mut decrypted_cred_random = [0; 32];
-            decrypted_cred_random[..16].clone_from_slice(&blocks[4]);
-            decrypted_cred_random[16..].clone_from_slice(&blocks[5]);
-            Some(decrypted_cred_random.to_vec())
-        } else {
-            None
-        };
-
         if rp_id_hash != decrypted_rp_id_hash {
             return Ok(None);
         }
@@ -322,7 +303,6 @@ where
             rp_id: String::from(""),
             user_handle: vec![],
             user_display_name: None,
-            cred_random,
             cred_protect_policy: None,
             creation_order: 0,
             user_name: None,
@@ -464,11 +444,6 @@ where
             (false, DEFAULT_CRED_PROTECT)
         };
 
-        let cred_random = if use_hmac_extension {
-            Some(self.rng.gen_uniform_u8x32())
-        } else {
-            None
-        };
         let has_extension_output = use_hmac_extension || cred_protect_policy.is_some();
 
         let rp_id = rp.rp_id;
@@ -543,7 +518,6 @@ where
                 user_display_name: user
                     .user_display_name
                     .map(|s| truncate_to_char_boundary(&s, 64).to_string()),
-                cred_random: cred_random.map(|c| c.to_vec()),
                 cred_protect_policy,
                 creation_order: self.persistent_store.new_creation_order()?,
                 user_name: user
@@ -556,7 +530,7 @@ where
             self.persistent_store.store_credential(credential_source)?;
             random_id
         } else {
-            self.encrypt_key_handle(sk.clone(), &rp_id_hash, cred_random.as_ref())?
+            self.encrypt_key_handle(sk.clone(), &rp_id_hash)?
         };
 
         let mut auth_data = self.generate_auth_data(&rp_id_hash, flags)?;
@@ -592,7 +566,7 @@ where
                 .attestation_private_key()?
                 .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
             let attestation_key =
-                crypto::ecdsa::SecKey::from_bytes(attestation_private_key).unwrap();
+                crypto::ecdsa::SecKey::from_bytes(&attestation_private_key).unwrap();
             let attestation_certificate = self
                 .persistent_store
                 .attestation_certificate()?
@@ -622,10 +596,23 @@ where
         ))
     }
 
+    // Generates a different per-credential secret for each UV mode.
+    // The computation is deterministic, and private_key expected to be unique.
+    fn generate_cred_random(
+        &mut self,
+        private_key: &crypto::ecdsa::SecKey,
+        has_uv: bool,
+    ) -> Result<[u8; 32], Ctap2StatusCode> {
+        let mut private_key_bytes = [0u8; 32];
+        private_key.to_bytes(&mut private_key_bytes);
+        let key = self.persistent_store.cred_random_secret(has_uv)?;
+        Ok(hmac_256::<Sha256>(&key, &private_key_bytes))
+    }
+
     // Processes the input of a get_assertion operation for a given credential
     // and returns the correct Get(Next)Assertion response.
     fn assertion_response(
-        &self,
+        &mut self,
         credential: PublicKeyCredentialSource,
         assertion_input: AssertionInput,
         number_of_credentials: Option<usize>,
@@ -634,13 +621,15 @@ where
             client_data_hash,
             mut auth_data,
             hmac_secret_input,
+            has_uv,
         } = assertion_input;
 
         // Process extensions.
         if let Some(hmac_secret_input) = hmac_secret_input {
+            let cred_random = self.generate_cred_random(&credential.private_key, has_uv)?;
             let encrypted_output = self
                 .pin_protocol_v1
-                .process_hmac_secret(hmac_secret_input, &credential.cred_random)?;
+                .process_hmac_secret(hmac_secret_input, &cred_random)?;
             let extensions_output = cbor_map! {
                 "hmac-secret" => encrypted_output,
             };
@@ -807,6 +796,7 @@ where
             client_data_hash,
             auth_data: self.generate_auth_data(&rp_id_hash, flags)?,
             hmac_secret_input,
+            has_uv,
         };
         let number_of_credentials = if applicable_credentials.is_empty() {
             None
@@ -872,7 +862,7 @@ where
                 max_credential_count_in_list: MAX_CREDENTIAL_COUNT_IN_LIST.map(|c| c as u64),
                 // #TODO(106) update with version 2.1 of HMAC-secret
                 #[cfg(feature = "with_ctap2_1")]
-                max_credential_id_length: Some(CREDENTIAL_ID_BASE_SIZE as u64 + 32),
+                max_credential_id_length: Some(CREDENTIAL_ID_SIZE as u64),
                 #[cfg(feature = "with_ctap2_1")]
                 transports: Some(vec![AuthenticatorTransport::Usb]),
                 #[cfg(feature = "with_ctap2_1")]
@@ -1010,7 +1000,7 @@ mod test {
         #[cfg(feature = "with_ctap2_1")]
         expected_response.extend(
             [
-                0x08, 0x18, 0x90, 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61,
+                0x08, 0x18, 0x70, 0x09, 0x81, 0x63, 0x75, 0x73, 0x62, 0x0A, 0x81, 0xA2, 0x63, 0x61,
                 0x6C, 0x67, 0x26, 0x64, 0x74, 0x79, 0x70, 0x65, 0x6A, 0x70, 0x75, 0x62, 0x6C, 0x69,
                 0x63, 0x2D, 0x6B, 0x65, 0x79, 0x0D, 0x04,
             ]
@@ -1141,7 +1131,7 @@ mod test {
                 ];
                 expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8);
                 expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap());
-                expected_auth_data.extend(&[0x00, CREDENTIAL_ID_BASE_SIZE as u8]);
+                expected_auth_data.extend(&[0x00, CREDENTIAL_ID_SIZE as u8]);
                 assert_eq!(
                     auth_data[0..expected_auth_data.len()],
                     expected_auth_data[..]
@@ -1186,7 +1176,6 @@ mod test {
             rp_id: String::from("example.com"),
             user_handle: vec![],
             user_display_name: None,
-            cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
             user_name: None,
@@ -1291,7 +1280,7 @@ mod test {
                 ];
                 expected_auth_data.push(INITIAL_SIGNATURE_COUNTER as u8);
                 expected_auth_data.extend(&ctap_state.persistent_store.aaguid().unwrap());
-                expected_auth_data.extend(&[0x00, CREDENTIAL_ID_MAX_SIZE as u8]);
+                expected_auth_data.extend(&[0x00, CREDENTIAL_ID_SIZE as u8]);
                 assert_eq!(
                     auth_data[0..expected_auth_data.len()],
                     expected_auth_data[..]
@@ -1488,8 +1477,8 @@ mod test {
                 let auth_data = make_credential_response.auth_data;
                 let offset = 37 + ctap_state.persistent_store.aaguid().unwrap().len();
                 assert_eq!(auth_data[offset], 0x00);
-                assert_eq!(auth_data[offset + 1] as usize, CREDENTIAL_ID_MAX_SIZE);
+                assert_eq!(auth_data[offset + 1] as usize, CREDENTIAL_ID_SIZE);
-                auth_data[offset + 2..offset + 2 + CREDENTIAL_ID_MAX_SIZE].to_vec()
+                auth_data[offset + 2..offset + 2 + CREDENTIAL_ID_SIZE].to_vec()
             }
             _ => panic!("Invalid response type"),
         };
@@ -1604,7 +1593,6 @@ mod test {
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
-            cred_random: None,
             cred_protect_policy: Some(
                 CredentialProtectionPolicy::UserVerificationOptionalWithCredentialIdList,
             ),
@@ -1669,7 +1657,6 @@ mod test {
             rp_id: String::from("example.com"),
             user_handle: vec![0x1D],
             user_display_name: None,
-            cred_random: None,
             cred_protect_policy: Some(CredentialProtectionPolicy::UserVerificationRequired),
             creation_order: 0,
             user_name: None,
@@ -1942,7 +1929,6 @@ mod test {
             rp_id: String::from("example.com"),
             user_handle: vec![],
             user_display_name: None,
-            cred_random: None,
             cred_protect_policy: None,
             creation_order: 0,
             user_name: None,
@@ -2013,7 +1999,7 @@ mod test {
         // We are not testing the correctness of our SHA256 here, only if it is checked.
         let rp_id_hash = [0x55; 32];
         let encrypted_id = ctap_state
-            .encrypt_key_handle(private_key.clone(), &rp_id_hash, None)
+            .encrypt_key_handle(private_key.clone(), &rp_id_hash)
             .unwrap();
         let decrypted_source = ctap_state
             .decrypt_credential_source(encrypted_id, &rp_id_hash)
@@ -2023,29 +2009,6 @@ mod test {
         assert_eq!(private_key, decrypted_source.private_key);
     }
 
-    #[test]
-    fn test_encrypt_decrypt_credential_with_cred_random() {
-        let mut rng = ThreadRng256 {};
-        let user_immediately_present = |_| Ok(());
-        let private_key = crypto::ecdsa::SecKey::gensk(&mut rng);
-        let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE);
-
-        // Usually, the relying party ID or its hash is provided by the client.
-        // We are not testing the correctness of our SHA256 here, only if it is checked.
-        let rp_id_hash = [0x55; 32];
-        let cred_random = [0xC9; 32];
-        let encrypted_id = ctap_state
-            .encrypt_key_handle(private_key.clone(), &rp_id_hash, Some(&cred_random))
-            .unwrap();
-        let decrypted_source = ctap_state
-            .decrypt_credential_source(encrypted_id, &rp_id_hash)
-            .unwrap()
-            .unwrap();
-
-        assert_eq!(private_key, decrypted_source.private_key);
-        assert_eq!(Some(cred_random.to_vec()), decrypted_source.cred_random);
-    }
-
     #[test]
     fn test_encrypt_decrypt_bad_hmac() {
         let mut rng = ThreadRng256 {};
@@ -2056,30 +2019,7 @@ mod test {
         // Same as above.
         let rp_id_hash = [0x55; 32];
         let encrypted_id = ctap_state
-            .encrypt_key_handle(private_key, &rp_id_hash, None)
+            .encrypt_key_handle(private_key, &rp_id_hash)
-            .unwrap();
-        for i in 0..encrypted_id.len() {
-            let mut modified_id = encrypted_id.clone();
-            modified_id[i] ^= 0x01;
-            assert!(ctap_state
-                .decrypt_credential_source(modified_id, &rp_id_hash)
-                .unwrap()
-                .is_none());
-        }
-    }
-
-    #[test]
-    fn test_encrypt_decrypt_bad_hmac_with_cred_random() {
-        let mut rng = ThreadRng256 {};
-        let user_immediately_present = |_| Ok(());
-        let private_key = crypto::ecdsa::SecKey::gensk(&mut rng);
-        let mut ctap_state = CtapState::new(&mut rng, user_immediately_present, DUMMY_CLOCK_VALUE);
-
-        // Same as above.
-        let rp_id_hash = [0x55; 32];
-        let cred_random = [0xC9; 32];
-        let encrypted_id = ctap_state
-            .encrypt_key_handle(private_key, &rp_id_hash, Some(&cred_random))
             .unwrap();
         for i in 0..encrypted_id.len() {
             let mut modified_id = encrypted_id.clone();

src/ctap/pin_protocol_v1.rs

@@ -56,23 +56,16 @@ fn verify_pin_auth(hmac_key: &[u8], hmac_contents: &[u8], pin_auth: &[u8]) -> bo
 fn encrypt_hmac_secret_output(
     shared_secret: &[u8; 32],
     salt_enc: &[u8],
-    cred_random: &[u8],
+    cred_random: &[u8; 32],
 ) -> Result<Vec<u8>, Ctap2StatusCode> {
     if salt_enc.len() != 32 && salt_enc.len() != 64 {
         return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION);
     }
-    if cred_random.len() != 32 {
-        // We are strict here. We need at least 32 byte, but expect exactly 32.
-        return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION);
-    }
     let aes_enc_key = crypto::aes256::EncryptionKey::new(shared_secret);
     let aes_dec_key = crypto::aes256::DecryptionKey::new(&aes_enc_key);
     // The specification specifically asks for a zero IV.
     let iv = [0u8; 16];
 
-    let mut cred_random_secret = [0u8; 32];
-    cred_random_secret.copy_from_slice(cred_random);
-
     // With the if clause restriction above, block_len can only be 2 or 4.
     let block_len = salt_enc.len() / 16;
     let mut blocks = vec![[0u8; 16]; block_len];
@@ -84,7 +77,7 @@ fn encrypt_hmac_secret_output(
     let mut decrypted_salt1 = [0u8; 32];
     decrypted_salt1[..16].copy_from_slice(&blocks[0]);
     decrypted_salt1[16..].copy_from_slice(&blocks[1]);
-    let output1 = hmac_256::<Sha256>(&cred_random_secret, &decrypted_salt1[..]);
+    let output1 = hmac_256::<Sha256>(&cred_random[..], &decrypted_salt1[..]);
     for i in 0..2 {
         blocks[i].copy_from_slice(&output1[16 * i..16 * (i + 1)]);
     }
@@ -93,7 +86,7 @@ fn encrypt_hmac_secret_output(
         let mut decrypted_salt2 = [0u8; 32];
         decrypted_salt2[..16].copy_from_slice(&blocks[2]);
         decrypted_salt2[16..].copy_from_slice(&blocks[3]);
-        let output2 = hmac_256::<Sha256>(&cred_random_secret, &decrypted_salt2[..]);
+        let output2 = hmac_256::<Sha256>(&cred_random[..], &decrypted_salt2[..]);
         for i in 0..2 {
             blocks[i + 2].copy_from_slice(&output2[16 * i..16 * (i + 1)]);
         }
@@ -588,7 +581,7 @@ impl PinProtocolV1 {
     pub fn process_hmac_secret(
         &self,
         hmac_secret_input: GetAssertionHmacSecretInput,
-        cred_random: &Option<Vec<u8>>,
+        cred_random: &[u8; 32],
     ) -> Result<Vec<u8>, Ctap2StatusCode> {
         let GetAssertionHmacSecretInput {
             key_agreement,
@@ -602,12 +595,7 @@ impl PinProtocolV1 {
             // Hard to tell what the correct error code here is.
             return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION);
         }
-
+        encrypt_hmac_secret_output(&shared_secret, &salt_enc[..], cred_random)
-        match cred_random {
-            Some(cr) => encrypt_hmac_secret_output(&shared_secret, &salt_enc[..], cr),
-            // This is the case if the credential was not created with HMAC-secret.
-            None => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION),
-        }
     }
 
     #[cfg(feature = "with_ctap2_1")]
@@ -1195,14 +1183,6 @@ mod test {
         let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random);
         assert_eq!(output.unwrap().len(), 64);
 
-        let salt_enc = [0x5E; 32];
-        let cred_random = [0xC9; 33];
-        let output = encrypt_hmac_secret_output(&shared_secret, &salt_enc, &cred_random);
-        assert_eq!(
-            output,
-            Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_EXTENSION)
-        );
-
         let mut salt_enc = [0x00; 32];
         let cred_random = [0xC9; 32];
 

src/ctap/status_code.rs

@@ -81,5 +81,10 @@ pub enum Ctap2StatusCode {
     /// This type of error is unexpected and the current state is undefined.
     CTAP2_ERR_VENDOR_INTERNAL_ERROR = 0xF2,
 
+    /// The hardware is malfunctioning.
+    ///
+    /// It may be possible that some of those errors are actually internal errors.
+    CTAP2_ERR_VENDOR_HARDWARE_FAILURE = 0xF3,
+
     CTAP2_ERR_VENDOR_LAST = 0xFF,
 }

src/ctap/storage/key.rs

@@ -0,0 +1,146 @@
+// Copyright 2019-2020 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.
+
+/// Number of keys that persist the CTAP reset command.
+pub const NUM_PERSISTENT_KEYS: usize = 20;
+
+/// Defines a key given its name and value or range of values.
+macro_rules! make_key {
+    ($(#[$doc: meta])* $name: ident = $key: literal..$end: literal) => {
+        $(#[$doc])* pub const $name: core::ops::Range<usize> = $key..$end;
+    };
+    ($(#[$doc: meta])* $name: ident = $key: literal) => {
+        $(#[$doc])* pub const $name: usize = $key;
+    };
+}
+
+/// Returns the range of values of a key given its value description.
+#[cfg(test)]
+macro_rules! make_range {
+    ($key: literal..$end: literal) => {
+        $key..$end
+    };
+    ($key: literal) => {
+        $key..$key + 1
+    };
+}
+
+/// Helper to define keys as a partial partition of a range.
+macro_rules! make_partition {
+        ($range: expr,
+         $(
+             $(#[$doc: meta])*
+             $name: ident = $key: literal $(.. $end: literal)?;
+         )*) => {
+            $(
+                make_key!($(#[$doc])* $name = $key $(.. $end)?);
+            )*
+            #[cfg(test)]
+            const KEY_RANGE: core::ops::Range<usize> = $range;
+            #[cfg(test)]
+            const ALL_KEYS: &[core::ops::Range<usize>] = &[$(make_range!($key $(.. $end)?)),*];
+        };
+    }
+
+make_partition! {
+    // We reserve 0 and 2048+ for possible migration purposes. We add persistent entries starting
+    // from 1 and going up. We add non-persistent entries starting from 2047 and going down. This
+    // way, we don't commit to a fixed number of persistent keys.
+    1..2048,
+
+    // WARNING: Keys should not be deleted but prefixed with `_` to avoid accidentally reusing them.
+
+    /// The attestation private key.
+    ATTESTATION_PRIVATE_KEY = 1;
+
+    /// The attestation certificate.
+    ATTESTATION_CERTIFICATE = 2;
+
+    /// The aaguid.
+    AAGUID = 3;
+
+    // This is the persistent key limit:
+    // - When adding a (persistent) key above this message, make sure its value is smaller than
+    //   NUM_PERSISTENT_KEYS.
+    // - When adding a (non-persistent) key below this message, make sure its value is bigger or
+    //   equal than NUM_PERSISTENT_KEYS.
+
+    /// Reserved for future credential-related objects.
+    ///
+    /// In particular, additional credentials could be added there by reducing the lower bound of
+    /// the credential range below as well as the upper bound of this range in a similar manner.
+    _RESERVED_CREDENTIALS = 1000..1700;
+
+    /// The credentials.
+    ///
+    /// Depending on `MAX_SUPPORTED_RESIDENTIAL_KEYS`, only a prefix of those keys is used. Each
+    /// board may configure `MAX_SUPPORTED_RESIDENTIAL_KEYS` depending on the storage size.
+    CREDENTIALS = 1700..2000;
+
+    /// The secret of the CredRandom feature.
+    CRED_RANDOM_SECRET = 2041;
+
+    /// List of RP IDs allowed to read the minimum PIN length.
+    #[cfg(feature = "with_ctap2_1")]
+    _MIN_PIN_LENGTH_RP_IDS = 2042;
+
+    /// The minimum PIN length.
+    ///
+    /// If the entry is absent, the minimum PIN length is `DEFAULT_MIN_PIN_LENGTH`.
+    #[cfg(feature = "with_ctap2_1")]
+    MIN_PIN_LENGTH = 2043;
+
+    /// The number of PIN retries.
+    ///
+    /// If the entry is absent, the number of PIN retries is `MAX_PIN_RETRIES`.
+    PIN_RETRIES = 2044;
+
+    /// The PIN hash.
+    ///
+    /// If the entry is absent, there is no PIN set.
+    PIN_HASH = 2045;
+
+    /// The encryption and hmac keys.
+    ///
+    /// This entry is always present. It is generated at startup if absent.
+    MASTER_KEYS = 2046;
+
+    /// The global signature counter.
+    ///
+    /// If the entry is absent, the counter is 0.
+    GLOBAL_SIGNATURE_COUNTER = 2047;
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn enough_credentials() {
+        use super::super::MAX_SUPPORTED_RESIDENTIAL_KEYS;
+        assert!(MAX_SUPPORTED_RESIDENTIAL_KEYS <= CREDENTIALS.end - CREDENTIALS.start);
+    }
+
+    #[test]
+    fn keys_are_disjoint() {
+        // Check that keys are in the range.
+        for keys in ALL_KEYS {
+            assert!(KEY_RANGE.start <= keys.start && keys.end <= KEY_RANGE.end);
+        }
+        // Check that keys are assigned at most once, essentially partitioning the range.
+        for key in KEY_RANGE {
+            assert!(ALL_KEYS.iter().filter(|keys| keys.contains(&key)).count() <= 1);
+        }
+    }
+}

src/embedded_flash/buffer.rs

@@ -1,457 +0,0 @@
-// Copyright 2019 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.
-
-use super::{Index, Storage, StorageError, StorageResult};
-use alloc::boxed::Box;
-use alloc::vec;
-
-pub struct BufferStorage {
-    storage: Box<[u8]>,
-    options: BufferOptions,
-    word_writes: Box<[usize]>,
-    page_erases: Box<[usize]>,
-    snapshot: Snapshot,
-}
-
-#[derive(Copy, Clone, Debug)]
-pub struct BufferOptions {
-    /// Size of a word in bytes.
-    pub word_size: usize,
-
-    /// Size of a page in bytes.
-    pub page_size: usize,
-
-    /// How many times a word can be written between page erasures
-    pub max_word_writes: usize,
-
-    /// How many times a page can be erased.
-    pub max_page_erases: usize,
-
-    /// Bits cannot be written from 0 to 1.
-    pub strict_write: bool,
-}
-
-impl BufferStorage {
-    /// Creates a fake embedded flash using a buffer.
-    ///
-    /// This implementation checks that no words are written more than `max_word_writes` between
-    /// page erasures and than no pages are erased more than `max_page_erases`. If `strict_write` is
-    /// true, it also checks that no bits are written from 0 to 1. It also permits to take snapshots
-    /// of the storage during write and erase operations (although words would still be written or
-    /// erased completely).
-    ///
-    /// # Panics
-    ///
-    /// The following preconditions must hold:
-    /// - `options.word_size` must be a power of two.
-    /// - `options.page_size` must be a power of two.
-    /// - `options.page_size` must be word-aligned.
-    /// - `storage.len()` must be page-aligned.
-    pub fn new(storage: Box<[u8]>, options: BufferOptions) -> BufferStorage {
-        assert!(options.word_size.is_power_of_two());
-        assert!(options.page_size.is_power_of_two());
-        let num_words = storage.len() / options.word_size;
-        let num_pages = storage.len() / options.page_size;
-        let buffer = BufferStorage {
-            storage,
-            options,
-            word_writes: vec![0; num_words].into_boxed_slice(),
-            page_erases: vec![0; num_pages].into_boxed_slice(),
-            snapshot: Snapshot::Ready,
-        };
-        assert!(buffer.is_word_aligned(buffer.options.page_size));
-        assert!(buffer.is_page_aligned(buffer.storage.len()));
-        buffer
-    }
-
-    /// Takes a snapshot of the storage after a given amount of word operations.
-    ///
-    /// Each time a word is written or erased, the delay is decremented if positive. Otherwise, a
-    /// snapshot is taken before the operation is executed.
-    ///
-    /// # Panics
-    ///
-    /// Panics if a snapshot has been armed and not examined.
-    pub fn arm_snapshot(&mut self, delay: usize) {
-        self.snapshot.arm(delay);
-    }
-
-    /// Unarms and returns the snapshot or the delay remaining.
-    ///
-    /// # Panics
-    ///
-    /// Panics if a snapshot was not armed.
-    pub fn get_snapshot(&mut self) -> Result<Box<[u8]>, usize> {
-        self.snapshot.get()
-    }
-
-    /// Takes a snapshot of the storage.
-    pub fn take_snapshot(&self) -> Box<[u8]> {
-        self.storage.clone()
-    }
-
-    /// Returns the storage.
-    pub fn get_storage(self) -> Box<[u8]> {
-        self.storage
-    }
-
-    fn is_word_aligned(&self, x: usize) -> bool {
-        x & (self.options.word_size - 1) == 0
-    }
-
-    fn is_page_aligned(&self, x: usize) -> bool {
-        x & (self.options.page_size - 1) == 0
-    }
-
-    /// Writes a slice to the storage.
-    ///
-    /// The slice `value` is written to `index`. The `erase` boolean specifies whether this is an
-    /// erase operation or a write operation which matters for the checks and updating the shadow
-    /// storage. This also takes a snapshot of the storage if a snapshot was armed and the delay has
-    /// elapsed.
-    ///
-    /// The following preconditions should hold:
-    /// - `index` is word-aligned.
-    /// - `value.len()` is word-aligned.
-    ///
-    /// The following checks are performed:
-    /// - The region of length `value.len()` starting at `index` fits in a storage page.
-    /// - A word is not written more than `max_word_writes`.
-    /// - A page is not erased more than `max_page_erases`.
-    /// - The new word only switches 1s to 0s (only if `strict_write` is set).
-    fn update_storage(&mut self, index: Index, value: &[u8], erase: bool) -> StorageResult<()> {
-        debug_assert!(self.is_word_aligned(index.byte) && self.is_word_aligned(value.len()));
-        let dst = index.range(value.len(), self)?.step_by(self.word_size());
-        let src = value.chunks(self.word_size());
-        // Check and update page shadow.
-        if erase {
-            let page = index.page;
-            assert!(self.page_erases[page] < self.max_page_erases());
-            self.page_erases[page] += 1;
-        }
-        for (byte, val) in dst.zip(src) {
-            let range = byte..byte + self.word_size();
-            // The driver doesn't write identical words.
-            if &self.storage[range.clone()] == val {
-                continue;
-            }
-            // Check and update word shadow.
-            let word = byte / self.word_size();
-            if erase {
-                self.word_writes[word] = 0;
-            } else {
-                assert!(self.word_writes[word] < self.max_word_writes());
-                self.word_writes[word] += 1;
-            }
-            // Check strict write.
-            if !erase && self.options.strict_write {
-                for (byte, &val) in range.clone().zip(val) {
-                    assert_eq!(self.storage[byte] & val, val);
-                }
-            }
-            // Take snapshot if armed and delay expired.
-            self.snapshot.take(&self.storage);
-            // Write storage
-            self.storage[range].copy_from_slice(val);
-        }
-        Ok(())
-    }
-}
-
-impl Storage for BufferStorage {
-    fn word_size(&self) -> usize {
-        self.options.word_size
-    }
-
-    fn page_size(&self) -> usize {
-        self.options.page_size
-    }
-
-    fn num_pages(&self) -> usize {
-        self.storage.len() / self.options.page_size
-    }
-
-    fn max_word_writes(&self) -> usize {
-        self.options.max_word_writes
-    }
-
-    fn max_page_erases(&self) -> usize {
-        self.options.max_page_erases
-    }
-
-    fn read_slice(&self, index: Index, length: usize) -> StorageResult<&[u8]> {
-        Ok(&self.storage[index.range(length, self)?])
-    }
-
-    fn write_slice(&mut self, index: Index, value: &[u8]) -> StorageResult<()> {
-        if !self.is_word_aligned(index.byte) || !self.is_word_aligned(value.len()) {
-            return Err(StorageError::NotAligned);
-        }
-        self.update_storage(index, value, false)
-    }
-
-    fn erase_page(&mut self, page: usize) -> StorageResult<()> {
-        let index = Index { page, byte: 0 };
-        let value = vec![0xff; self.page_size()];
-        self.update_storage(index, &value, true)
-    }
-}
-
-// Controls when a snapshot of the storage is taken.
-//
-// This can be used to simulate power-offs while the device is writing to the storage or erasing a
-// page in the storage.
-enum Snapshot {
-    // Mutable word operations have normal behavior.
-    Ready,
-    // If the delay is positive, mutable word operations decrement it. If the count is zero, mutable
-    // word operations take a snapshot of the storage.
-    Armed { delay: usize },
-    // Mutable word operations have normal behavior.
-    Taken { storage: Box<[u8]> },
-}
-
-impl Snapshot {
-    fn arm(&mut self, delay: usize) {
-        match self {
-            Snapshot::Ready => *self = Snapshot::Armed { delay },
-            _ => panic!(),
-        }
-    }
-
-    fn get(&mut self) -> Result<Box<[u8]>, usize> {
-        let mut snapshot = Snapshot::Ready;
-        core::mem::swap(self, &mut snapshot);
-        match snapshot {
-            Snapshot::Armed { delay } => Err(delay),
-            Snapshot::Taken { storage } => Ok(storage),
-            _ => panic!(),
-        }
-    }
-
-    fn take(&mut self, storage: &[u8]) {
-        if let Snapshot::Armed { delay } = self {
-            if *delay == 0 {
-                let storage = storage.to_vec().into_boxed_slice();
-                *self = Snapshot::Taken { storage };
-            } else {
-                *delay -= 1;
-            }
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    const NUM_PAGES: usize = 2;
-    const OPTIONS: BufferOptions = BufferOptions {
-        word_size: 4,
-        page_size: 16,
-        max_word_writes: 2,
-        max_page_erases: 3,
-        strict_write: true,
-    };
-    // Those words are decreasing bit patterns. Bits are only changed from 1 to 0 and at last one
-    // bit is changed.
-    const BLANK_WORD: &[u8] = &[0xff, 0xff, 0xff, 0xff];
-    const FIRST_WORD: &[u8] = &[0xee, 0xdd, 0xbb, 0x77];
-    const SECOND_WORD: &[u8] = &[0xca, 0xc9, 0xa9, 0x65];
-    const THIRD_WORD: &[u8] = &[0x88, 0x88, 0x88, 0x44];
-
-    fn new_storage() -> Box<[u8]> {
-        vec![0xff; NUM_PAGES * OPTIONS.page_size].into_boxed_slice()
-    }
-
-    #[test]
-    fn words_are_decreasing() {
-        fn assert_is_decreasing(prev: &[u8], next: &[u8]) {
-            for (&prev, &next) in prev.iter().zip(next.iter()) {
-                assert_eq!(prev & next, next);
-                assert!(prev != next);
-            }
-        }
-        assert_is_decreasing(BLANK_WORD, FIRST_WORD);
-        assert_is_decreasing(FIRST_WORD, SECOND_WORD);
-        assert_is_decreasing(SECOND_WORD, THIRD_WORD);
-    }
-
-    #[test]
-    fn options_ok() {
-        let buffer = BufferStorage::new(new_storage(), OPTIONS);
-        assert_eq!(buffer.word_size(), OPTIONS.word_size);
-        assert_eq!(buffer.page_size(), OPTIONS.page_size);
-        assert_eq!(buffer.num_pages(), NUM_PAGES);
-        assert_eq!(buffer.max_word_writes(), OPTIONS.max_word_writes);
-        assert_eq!(buffer.max_page_erases(), OPTIONS.max_page_erases);
-    }
-
-    #[test]
-    fn read_write_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        let next_index = Index { page: 0, byte: 4 };
-        assert_eq!(buffer.read_slice(index, 4).unwrap(), BLANK_WORD);
-        buffer.write_slice(index, FIRST_WORD).unwrap();
-        assert_eq!(buffer.read_slice(index, 4).unwrap(), FIRST_WORD);
-        assert_eq!(buffer.read_slice(next_index, 4).unwrap(), BLANK_WORD);
-    }
-
-    #[test]
-    fn erase_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        let other_index = Index { page: 1, byte: 0 };
-        buffer.write_slice(index, FIRST_WORD).unwrap();
-        buffer.write_slice(other_index, FIRST_WORD).unwrap();
-        assert_eq!(buffer.read_slice(index, 4).unwrap(), FIRST_WORD);
-        assert_eq!(buffer.read_slice(other_index, 4).unwrap(), FIRST_WORD);
-        buffer.erase_page(0).unwrap();
-        assert_eq!(buffer.read_slice(index, 4).unwrap(), BLANK_WORD);
-        assert_eq!(buffer.read_slice(other_index, 4).unwrap(), FIRST_WORD);
-    }
-
-    #[test]
-    fn invalid_range() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 12 };
-        let half_index = Index { page: 0, byte: 14 };
-        let over_index = Index { page: 0, byte: 16 };
-        let bad_page = Index { page: 2, byte: 0 };
-
-        // Reading a word in the storage is ok.
-        assert!(buffer.read_slice(index, 4).is_ok());
-        // Reading a half-word in the storage is ok.
-        assert!(buffer.read_slice(half_index, 2).is_ok());
-        // Reading even a single byte outside a page is not ok.
-        assert!(buffer.read_slice(over_index, 1).is_err());
-        // But reading an empty slice just after a page is ok.
-        assert!(buffer.read_slice(over_index, 0).is_ok());
-        // Reading even an empty slice outside the storage is not ok.
-        assert!(buffer.read_slice(bad_page, 0).is_err());
-
-        // Writing a word in the storage is ok.
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-        // Writing an unaligned word is not ok.
-        assert!(buffer.write_slice(half_index, FIRST_WORD).is_err());
-        // Writing a word outside a page is not ok.
-        assert!(buffer.write_slice(over_index, FIRST_WORD).is_err());
-        // But writing an empty slice just after a page is ok.
-        assert!(buffer.write_slice(over_index, &[]).is_ok());
-        // Writing even an empty slice outside the storage is not ok.
-        assert!(buffer.write_slice(bad_page, &[]).is_err());
-
-        // Only pages in the storage can be erased.
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(2).is_err());
-    }
-
-    #[test]
-    fn write_twice_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 4 };
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-        assert!(buffer.write_slice(index, SECOND_WORD).is_ok());
-    }
-
-    #[test]
-    fn write_twice_and_once_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        let next_index = Index { page: 0, byte: 4 };
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-        assert!(buffer.write_slice(index, SECOND_WORD).is_ok());
-        assert!(buffer.write_slice(next_index, THIRD_WORD).is_ok());
-    }
-
-    #[test]
-    #[should_panic]
-    fn write_three_times_panics() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 4 };
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-        assert!(buffer.write_slice(index, SECOND_WORD).is_ok());
-        let _ = buffer.write_slice(index, THIRD_WORD);
-    }
-
-    #[test]
-    fn write_twice_then_once_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-        assert!(buffer.write_slice(index, SECOND_WORD).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.write_slice(index, FIRST_WORD).is_ok());
-    }
-
-    #[test]
-    fn erase_three_times_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-    }
-
-    #[test]
-    fn erase_three_times_and_once_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(1).is_ok());
-    }
-
-    #[test]
-    #[should_panic]
-    fn erase_four_times_panics() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        assert!(buffer.erase_page(0).is_ok());
-        let _ = buffer.erase_page(0).is_ok();
-    }
-
-    #[test]
-    #[should_panic]
-    fn switch_zero_to_one_panics() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        assert!(buffer.write_slice(index, SECOND_WORD).is_ok());
-        let _ = buffer.write_slice(index, FIRST_WORD);
-    }
-
-    #[test]
-    fn get_storage_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 4 };
-        buffer.write_slice(index, FIRST_WORD).unwrap();
-        let storage = buffer.get_storage();
-        assert_eq!(&storage[..4], BLANK_WORD);
-        assert_eq!(&storage[4..8], FIRST_WORD);
-    }
-
-    #[test]
-    fn snapshot_ok() {
-        let mut buffer = BufferStorage::new(new_storage(), OPTIONS);
-        let index = Index { page: 0, byte: 0 };
-        let value = [FIRST_WORD, SECOND_WORD].concat();
-        buffer.arm_snapshot(1);
-        buffer.write_slice(index, &value).unwrap();
-        let storage = buffer.get_snapshot().unwrap();
-        assert_eq!(&storage[..8], &[FIRST_WORD, BLANK_WORD].concat()[..]);
-        let storage = buffer.take_snapshot();
-        assert_eq!(&storage[..8], &value[..]);
-    }
-}

src/embedded_flash/mod.rs

@@ -1,4 +1,4 @@
-// Copyright 2019 Google LLC
+// Copyright 2019-2020 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,16 +12,41 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#[cfg(any(test, feature = "ram_storage"))]
+#[cfg(not(feature = "std"))]
-mod buffer;
-mod storage;
-mod store;
-#[cfg(not(any(test, feature = "ram_storage")))]
 mod syscall;
 
-#[cfg(any(test, feature = "ram_storage"))]
+#[cfg(not(feature = "std"))]
-pub use self::buffer::{BufferOptions, BufferStorage};
-pub use self::storage::{Index, Storage, StorageError, StorageResult};
-pub use self::store::{Store, StoreConfig, StoreEntry, StoreError, StoreIndex};
-#[cfg(not(any(test, feature = "ram_storage")))]
 pub use self::syscall::SyscallStorage;
+
+/// Storage definition for production.
+#[cfg(not(feature = "std"))]
+mod prod {
+    pub type Storage = super::SyscallStorage;
+
+    pub fn new_storage(num_pages: usize) -> Storage {
+        Storage::new(num_pages).unwrap()
+    }
+}
+#[cfg(not(feature = "std"))]
+pub use self::prod::{new_storage, Storage};
+
+/// Storage definition for testing.
+#[cfg(feature = "std")]
+mod test {
+    pub type Storage = persistent_store::BufferStorage;
+
+    pub fn new_storage(num_pages: usize) -> Storage {
+        const PAGE_SIZE: usize = 0x1000;
+        let store = vec![0xff; num_pages * PAGE_SIZE].into_boxed_slice();
+        let options = persistent_store::BufferOptions {
+            word_size: 4,
+            page_size: PAGE_SIZE,
+            max_word_writes: 2,
+            max_page_erases: 10000,
+            strict_mode: true,
+        };
+        Storage::new(store, options)
+    }
+}
+#[cfg(feature = "std")]
+pub use self::test::{new_storage, Storage};

src/embedded_flash/storage.rs

@@ -1,107 +0,0 @@
-// Copyright 2019 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.
-
-#[derive(Copy, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "std", derive(Debug))]
-pub struct Index {
-    pub page: usize,
-    pub byte: usize,
-}
-
-#[derive(Debug)]
-pub enum StorageError {
-    BadFlash,
-    NotAligned,
-    OutOfBounds,
-    KernelError { code: isize },
-}
-
-pub type StorageResult<T> = Result<T, StorageError>;
-
-/// Abstraction for embedded flash storage.
-pub trait Storage {
-    /// Returns the size of a word in bytes.
-    fn word_size(&self) -> usize;
-
-    /// Returns the size of a page in bytes.
-    fn page_size(&self) -> usize;
-
-    /// Returns the number of pages in the storage.
-    fn num_pages(&self) -> usize;
-
-    /// Returns how many times a word can be written between page erasures.
-    fn max_word_writes(&self) -> usize;
-
-    /// Returns how many times a page can be erased in the lifetime of the flash.
-    fn max_page_erases(&self) -> usize;
-
-    /// Reads a slice from the storage.
-    ///
-    /// The slice does not need to be word-aligned.
-    ///
-    /// # Errors
-    ///
-    /// The `index` must designate `length` bytes in the storage.
-    fn read_slice(&self, index: Index, length: usize) -> StorageResult<&[u8]>;
-
-    /// Writes a word-aligned slice to the storage.
-    ///
-    /// The written words should not have been written too many times since last page erasure.
-    ///
-    /// # Errors
-    ///
-    /// The following preconditions must hold:
-    /// - `index` must be word-aligned.
-    /// - `value.len()` must be a multiple of the word size.
-    /// - `index` must designate `value.len()` bytes in the storage.
-    /// - `value` must be in memory until [read-only allow][tock_1274] is resolved.
-    ///
-    /// [tock_1274]: https://github.com/tock/tock/issues/1274.
-    fn write_slice(&mut self, index: Index, value: &[u8]) -> StorageResult<()>;
-
-    /// Erases a page of the storage.
-    ///
-    /// # Errors
-    ///
-    /// The `page` must be in the storage.
-    fn erase_page(&mut self, page: usize) -> StorageResult<()>;
-}
-
-impl Index {
-    /// Returns whether a slice fits in a storage page.
-    fn is_valid(self, length: usize, storage: &impl Storage) -> bool {
-        self.page < storage.num_pages()
-            && storage
-                .page_size()
-                .checked_sub(length)
-                .map(|limit| self.byte <= limit)
-                .unwrap_or(false)
-    }
-
-    /// Returns the range of a valid slice.
-    ///
-    /// The range starts at `self` with `length` bytes.
-    pub fn range(
-        self,
-        length: usize,
-        storage: &impl Storage,
-    ) -> StorageResult<core::ops::Range<usize>> {
-        if self.is_valid(length, storage) {
-            let start = self.page * storage.page_size() + self.byte;
-            Ok(start..start + length)
-        } else {
-            Err(StorageError::OutOfBounds)
-        }
-    }
-}

src/embedded_flash/store/bitfield.rs

@@ -1,177 +0,0 @@
-// Copyright 2019 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.
-
-/// Defines a consecutive sequence of bits.
-#[derive(Copy, Clone)]
-pub struct BitRange {
-    /// The first bit of the sequence.
-    pub start: usize,
-
-    /// The length in bits of the sequence.
-    pub length: usize,
-}
-
-impl BitRange {
-    /// Returns the first bit following a bit range.
-    pub fn end(self) -> usize {
-        self.start + self.length
-    }
-}
-
-/// Defines a consecutive sequence of bytes.
-///
-/// The bits in those bytes are ignored which essentially creates a gap in a sequence of bits. The
-/// gap is necessarily at byte boundaries. This is used to ignore the user data in an entry
-/// essentially providing a view of the entry information (header and footer).
-#[derive(Copy, Clone)]
-pub struct ByteGap {
-    pub start: usize,
-    pub length: usize,
-}
-
-/// Empty gap. All bits count.
-pub const NO_GAP: ByteGap = ByteGap {
-    start: 0,
-    length: 0,
-};
-
-impl ByteGap {
-    /// Translates a bit to skip the gap.
-    fn shift(self, bit: usize) -> usize {
-        if bit < 8 * self.start {
-            bit
-        } else {
-            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.
-///
-/// The sequence of bits is little-endian (both for bytes and bits) and defined by the bits that
-/// are in `data` but not in `gap`.
-pub fn is_zero(bit: usize, data: &[u8], gap: ByteGap) -> bool {
-    let bit = gap.shift(bit);
-    debug_assert!(bit < 8 * data.len());
-    data[bit / 8] & (1 << (bit % 8)) == 0
-}
-
-/// Sets a bit to zero in a sequence of bits.
-///
-/// The sequence of bits is little-endian (both for bytes and bits) and defined by the bits that
-/// are in `data` but not in `gap`.
-pub fn set_zero(bit: usize, data: &mut [u8], gap: ByteGap) {
-    let bit = gap.shift(bit);
-    debug_assert!(bit < 8 * data.len());
-    data[bit / 8] &= !(1 << (bit % 8));
-}
-
-/// Returns a little-endian value in a sequence of bits.
-///
-/// The sequence of bits is little-endian (both for bytes and bits) and defined by the bits that
-/// are in `data` but not in `gap`. The range of bits where the value is stored in defined by
-/// `range`. The value must fit in a `usize`.
-pub fn get_range(range: BitRange, data: &[u8], gap: ByteGap) -> usize {
-    debug_assert!(range.length <= 8 * core::mem::size_of::<usize>());
-    let mut result = 0;
-    for i in 0..range.length {
-        if !is_zero(range.start + i, data, gap) {
-            result |= 1 << i;
-        }
-    }
-    result
-}
-
-/// Sets a little-endian value in a sequence of bits.
-///
-/// The sequence of bits is little-endian (both for bytes and bits) and defined by the bits that
-/// are in `data` but not in `gap`. The range of bits where the value is stored in defined by
-/// `range`. The bits set to 1 in `value` must also be set to `1` in the sequence of bits.
-pub fn set_range(range: BitRange, data: &mut [u8], gap: ByteGap, value: usize) {
-    debug_assert!(range.length == 8 * core::mem::size_of::<usize>() || value < 1 << range.length);
-    for i in 0..range.length {
-        if value & 1 << i == 0 {
-            set_zero(range.start + i, data, gap);
-        }
-    }
-}
-
-/// Tests the `is_zero` and `set_zero` pair of functions.
-#[test]
-fn zero_ok() {
-    const GAP: ByteGap = ByteGap {
-        start: 2,
-        length: 1,
-    };
-    for i in 0..24 {
-        assert!(!is_zero(i, &[0xffu8, 0xff, 0x00, 0xff] as &[u8], GAP));
-    }
-    // Tests reading and setting a bit. The result should have all bits set to 1 except for the bit
-    // to test and the gap.
-    fn test(bit: usize, result: &[u8]) {
-        assert!(is_zero(bit, result, GAP));
-        let mut data = vec![0xff; result.len()];
-        // Set the gap bits to 0.
-        for i in 0..GAP.length {
-            data[GAP.start + i] = 0x00;
-        }
-        set_zero(bit, &mut data, GAP);
-        assert_eq!(data, result);
-    }
-    test(0, &[0xfe, 0xff, 0x00, 0xff]);
-    test(1, &[0xfd, 0xff, 0x00, 0xff]);
-    test(2, &[0xfb, 0xff, 0x00, 0xff]);
-    test(7, &[0x7f, 0xff, 0x00, 0xff]);
-    test(8, &[0xff, 0xfe, 0x00, 0xff]);
-    test(15, &[0xff, 0x7f, 0x00, 0xff]);
-    test(16, &[0xff, 0xff, 0x00, 0xfe]);
-    test(17, &[0xff, 0xff, 0x00, 0xfd]);
-    test(23, &[0xff, 0xff, 0x00, 0x7f]);
-}
-
-/// Tests the `get_range` and `set_range` pair of functions.
-#[test]
-fn range_ok() {
-    // Tests reading and setting a range. The result should have all bits set to 1 except for the
-    // range to test and the gap.
-    fn test(start: usize, length: usize, value: usize, result: &[u8], gap: ByteGap) {
-        let range = BitRange { start, length };
-        assert_eq!(get_range(range, result, gap), value);
-        let mut data = vec![0xff; result.len()];
-        for i in 0..gap.length {
-            data[gap.start + i] = 0x00;
-        }
-        set_range(range, &mut data, gap, value);
-        assert_eq!(data, result);
-    }
-    test(0, 8, 42, &[42], NO_GAP);
-    test(3, 12, 0b11_0101, &[0b1010_1111, 0b1000_0001], NO_GAP);
-    test(0, 16, 0x1234, &[0x34, 0x12], NO_GAP);
-    test(4, 16, 0x1234, &[0x4f, 0x23, 0xf1], NO_GAP);
-    let mut gap = ByteGap {
-        start: 1,
-        length: 1,
-    };
-    test(3, 12, 0b11_0101, &[0b1010_1111, 0x00, 0b1000_0001], gap);
-    gap.length = 2;
-    test(0, 16, 0x1234, &[0x34, 0x00, 0x00, 0x12], gap);
-    gap.start = 2;
-    gap.length = 1;
-    test(4, 16, 0x1234, &[0x4f, 0x23, 0x00, 0xf1], gap);
-}

src/embedded_flash/store/format.rs

@@ -1,565 +0,0 @@
-// Copyright 2019 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.
-
-use super::super::{Index, Storage};
-use super::{bitfield, StoreConfig, StoreEntry, StoreError};
-use alloc::vec;
-use alloc::vec::Vec;
-
-/// Whether a user entry is a replace entry.
-pub enum IsReplace {
-    /// This is a replace entry.
-    Replace,
-
-    /// This is an insert entry.
-    Insert,
-}
-
-/// Helpers to parse the store format.
-///
-/// See the store module-level documentation for information about the format.
-pub struct Format {
-    pub word_size: usize,
-    pub page_size: usize,
-    pub num_pages: usize,
-    pub max_page_erases: usize,
-    pub num_tags: usize,
-
-    /// Whether an entry is present.
-    ///
-    /// - 0 for entries (user entries or internal entries).
-    /// - 1 for free space until the end of the page.
-    present_bit: usize,
-
-    /// Whether an entry is deleted.
-    ///
-    /// - 0 for deleted entries.
-    /// - 1 for alive entries.
-    deleted_bit: usize,
-
-    /// Whether an entry is internal.
-    ///
-    /// - 0 for internal entries.
-    /// - 1 for user entries.
-    internal_bit: usize,
-
-    /// Whether a user entry is a replace entry.
-    ///
-    /// - 0 for replace entries.
-    /// - 1 for insert entries.
-    replace_bit: usize,
-
-    /// Whether a user entry has sensitive data.
-    ///
-    /// - 0 for sensitive data.
-    /// - 1 for non-sensitive data.
-    ///
-    /// When a user entry with sensitive data is deleted, the data is overwritten with zeroes. This
-    /// feature is subject to the same guarantees as all other features of the store, in particular
-    /// deleting a sensitive entry is atomic. See the store module-level documentation for more
-    /// information.
-    sensitive_bit: usize,
-
-    /// The data length of a user entry.
-    length_range: bitfield::BitRange,
-
-    /// The tag of a user entry.
-    tag_range: bitfield::BitRange,
-
-    /// The page index of a replace entry.
-    replace_page_range: bitfield::BitRange,
-
-    /// The byte index of a replace entry.
-    replace_byte_range: bitfield::BitRange,
-
-    /// The index of the page to erase.
-    ///
-    /// This is only present for internal entries.
-    old_page_range: bitfield::BitRange,
-
-    /// The current erase count of the page to erase.
-    ///
-    /// This is only present for internal entries.
-    saved_erase_count_range: bitfield::BitRange,
-
-    /// Whether a page is initialized.
-    ///
-    /// - 0 for initialized pages.
-    /// - 1 for uninitialized pages.
-    initialized_bit: usize,
-
-    /// The erase count of a page.
-    erase_count_range: bitfield::BitRange,
-
-    /// Whether a page is being compacted.
-    ///
-    /// - 0 for pages being compacted.
-    /// - 1 otherwise.
-    compacting_bit: usize,
-
-    /// The page index to which a page is being compacted.
-    new_page_range: bitfield::BitRange,
-}
-
-impl Format {
-    /// Returns a helper to parse the store format for a given storage and config.
-    ///
-    /// # Errors
-    ///
-    /// Returns `None` if any of the following conditions does not hold:
-    /// - The word size must be a power of two.
-    /// - The page size must be a power of two.
-    /// - There should be at least 2 pages in the storage.
-    /// - It should be possible to write a word at least twice.
-    /// - It should be possible to erase a page at least once.
-    /// - There should be at least 1 tag.
-    pub fn new<S: Storage, C: StoreConfig>(storage: &S, config: &C) -> Option<Format> {
-        let word_size = storage.word_size();
-        let page_size = storage.page_size();
-        let num_pages = storage.num_pages();
-        let max_word_writes = storage.max_word_writes();
-        let max_page_erases = storage.max_page_erases();
-        let num_tags = config.num_tags();
-        if !(word_size.is_power_of_two()
-            && page_size.is_power_of_two()
-            && num_pages > 1
-            && max_word_writes >= 2
-            && max_page_erases > 0
-            && num_tags > 0)
-        {
-            return None;
-        }
-        // Compute how many bits we need to store the fields.
-        let page_bits = num_bits(num_pages);
-        let byte_bits = num_bits(page_size);
-        let tag_bits = num_bits(num_tags);
-        let erase_bits = num_bits(max_page_erases + 1);
-        // Compute the bit position of the fields.
-        let present_bit = 0;
-        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: sensitive_bit + 1,
-            length: byte_bits,
-        };
-        let tag_range = bitfield::BitRange {
-            start: length_range.end(),
-            length: tag_bits,
-        };
-        let replace_page_range = bitfield::BitRange {
-            start: tag_range.end(),
-            length: page_bits,
-        };
-        let replace_byte_range = bitfield::BitRange {
-            start: replace_page_range.end(),
-            length: byte_bits,
-        };
-        let old_page_range = bitfield::BitRange {
-            start: internal_bit + 1,
-            length: page_bits,
-        };
-        let saved_erase_count_range = bitfield::BitRange {
-            start: old_page_range.end(),
-            length: erase_bits,
-        };
-        let initialized_bit = 0;
-        let erase_count_range = bitfield::BitRange {
-            start: initialized_bit + 1,
-            length: erase_bits,
-        };
-        let compacting_bit = erase_count_range.end();
-        let new_page_range = bitfield::BitRange {
-            start: compacting_bit + 1,
-            length: page_bits,
-        };
-        let format = Format {
-            word_size,
-            page_size,
-            num_pages,
-            max_page_erases,
-            num_tags,
-            present_bit,
-            deleted_bit,
-            internal_bit,
-            replace_bit,
-            sensitive_bit,
-            length_range,
-            tag_range,
-            replace_page_range,
-            replace_byte_range,
-            old_page_range,
-            saved_erase_count_range,
-            initialized_bit,
-            erase_count_range,
-            compacting_bit,
-            new_page_range,
-        };
-        // 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 (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(true) != word_size
-        {
-            return None;
-        }
-        Some(format)
-    }
-
-    /// Ensures a user entry is valid.
-    pub fn validate_entry(&self, entry: StoreEntry) -> Result<(), StoreError> {
-        if entry.tag >= self.num_tags {
-            return Err(StoreError::InvalidTag);
-        }
-        if entry.data.len() >= self.page_size {
-            return Err(StoreError::StoreFull);
-        }
-        Ok(())
-    }
-
-    /// 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`. 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 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 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.
-        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.
-    ///
-    /// 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, 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
-        // different words.
-        if entry_size == word_size {
-            entry_size += word_size;
-        }
-        entry_size
-    }
-
-    /// Returns the length in bytes of an internal entry.
-    pub fn internal_entry_size(&self) -> usize {
-        let length = self.bits_to_bytes(self.saved_erase_count_range.end());
-        self.align_word(length)
-    }
-
-    pub fn is_present(&self, header: &[u8]) -> bool {
-        bitfield::is_zero(self.present_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_present(&self, header: &mut [u8]) {
-        bitfield::set_zero(self.present_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn is_deleted(&self, header: &[u8]) -> bool {
-        bitfield::is_zero(self.deleted_bit, header, bitfield::NO_GAP)
-    }
-
-    /// Returns whether an entry is present and not deleted.
-    pub fn is_alive(&self, header: &[u8]) -> bool {
-        self.is_present(header) && !self.is_deleted(header)
-    }
-
-    pub fn set_deleted(&self, header: &mut [u8]) {
-        bitfield::set_zero(self.deleted_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn is_internal(&self, header: &[u8]) -> bool {
-        bitfield::is_zero(self.internal_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_internal(&self, header: &mut [u8]) {
-        bitfield::set_zero(self.internal_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn is_replace(&self, header: &[u8]) -> IsReplace {
-        if bitfield::is_zero(self.replace_bit, header, bitfield::NO_GAP) {
-            IsReplace::Replace
-        } else {
-            IsReplace::Insert
-        }
-    }
-
-    fn set_replace(&self, header: &mut [u8]) {
-        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)
-    }
-
-    fn set_length(&self, header: &mut [u8], length: usize) {
-        bitfield::set_range(self.length_range, header, bitfield::NO_GAP, length)
-    }
-
-    pub fn get_data<'a>(&self, entry: &'a [u8]) -> &'a [u8] {
-        &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.
-    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 }
-    }
-
-    pub fn get_tag(&self, entry: &[u8]) -> usize {
-        bitfield::get_range(self.tag_range, entry, self.entry_gap(entry))
-    }
-
-    fn set_tag(&self, entry: &mut [u8], tag: usize) {
-        bitfield::set_range(self.tag_range, entry, self.entry_gap(entry), tag)
-    }
-
-    pub fn get_replace_index(&self, entry: &[u8]) -> Index {
-        let gap = self.entry_gap(entry);
-        let page = bitfield::get_range(self.replace_page_range, entry, gap);
-        let byte = bitfield::get_range(self.replace_byte_range, entry, gap);
-        Index { page, byte }
-    }
-
-    fn set_replace_page(&self, entry: &mut [u8], page: usize) {
-        bitfield::set_range(self.replace_page_range, entry, self.entry_gap(entry), page)
-    }
-
-    fn set_replace_byte(&self, entry: &mut [u8], byte: usize) {
-        bitfield::set_range(self.replace_byte_range, entry, self.entry_gap(entry), byte)
-    }
-
-    /// Returns the bit position of the `committed` bit.
-    ///
-    /// This cannot be precomputed like other fields since it depends on the length of the entry.
-    fn committed_bit(&self, entry: &[u8]) -> usize {
-        8 * entry.len() - 2
-    }
-
-    /// Returns the bit position of the `complete` bit.
-    ///
-    /// This cannot be precomputed like other fields since it depends on the length of the entry.
-    fn complete_bit(&self, entry: &[u8]) -> usize {
-        8 * entry.len() - 1
-    }
-
-    pub fn is_committed(&self, entry: &[u8]) -> bool {
-        bitfield::is_zero(self.committed_bit(entry), entry, bitfield::NO_GAP)
-    }
-
-    pub fn set_committed(&self, entry: &mut [u8]) {
-        bitfield::set_zero(self.committed_bit(entry), entry, bitfield::NO_GAP)
-    }
-
-    pub fn is_complete(&self, entry: &[u8]) -> bool {
-        bitfield::is_zero(self.complete_bit(entry), entry, bitfield::NO_GAP)
-    }
-
-    fn set_complete(&self, entry: &mut [u8]) {
-        bitfield::set_zero(self.complete_bit(entry), entry, bitfield::NO_GAP)
-    }
-
-    pub fn get_old_page(&self, header: &[u8]) -> usize {
-        bitfield::get_range(self.old_page_range, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_old_page(&self, header: &mut [u8], old_page: usize) {
-        bitfield::set_range(self.old_page_range, header, bitfield::NO_GAP, old_page)
-    }
-
-    pub fn get_saved_erase_count(&self, header: &[u8]) -> usize {
-        bitfield::get_range(self.saved_erase_count_range, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_saved_erase_count(&self, header: &mut [u8], erase_count: usize) {
-        bitfield::set_range(
-            self.saved_erase_count_range,
-            header,
-            bitfield::NO_GAP,
-            erase_count,
-        )
-    }
-
-    /// 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,
-            sensitive,
-        } = user_entry;
-        let is_replace = match replace {
-            None => IsReplace::Insert,
-            Some(_) => IsReplace::Replace,
-        };
-        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(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);
-        // Build the footer.
-        entry.resize(entry_len, 0xff);
-        self.set_tag(&mut entry[..], tag);
-        self.set_complete(&mut entry[..]);
-        match replace {
-            None => self.set_committed(&mut entry[..]),
-            Some(Index { page, byte }) => {
-                self.set_replace(&mut entry[..]);
-                self.set_replace_page(&mut entry[..], page);
-                self.set_replace_byte(&mut entry[..], byte);
-            }
-        }
-        entry
-    }
-
-    /// Builds an entry for replace or insert operations.
-    pub fn build_erase_entry(&self, old_page: usize, saved_erase_count: usize) -> Vec<u8> {
-        let mut entry = vec![0xff; self.internal_entry_size()];
-        self.set_present(&mut entry[..]);
-        self.set_internal(&mut entry[..]);
-        self.set_old_page(&mut entry[..], old_page);
-        self.set_saved_erase_count(&mut entry[..], saved_erase_count);
-        entry
-    }
-
-    /// Returns the length in bytes of a page header entry.
-    ///
-    /// This includes the word padding.
-    pub fn page_header_size(&self) -> usize {
-        self.align_word(self.bits_to_bytes(self.erase_count_range.end()))
-    }
-
-    pub fn is_initialized(&self, header: &[u8]) -> bool {
-        bitfield::is_zero(self.initialized_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_initialized(&self, header: &mut [u8]) {
-        bitfield::set_zero(self.initialized_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn get_erase_count(&self, header: &[u8]) -> usize {
-        bitfield::get_range(self.erase_count_range, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_erase_count(&self, header: &mut [u8], count: usize) {
-        bitfield::set_range(self.erase_count_range, header, bitfield::NO_GAP, count)
-    }
-
-    pub fn is_compacting(&self, header: &[u8]) -> bool {
-        bitfield::is_zero(self.compacting_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_compacting(&self, header: &mut [u8]) {
-        bitfield::set_zero(self.compacting_bit, header, bitfield::NO_GAP)
-    }
-
-    pub fn get_new_page(&self, header: &[u8]) -> usize {
-        bitfield::get_range(self.new_page_range, header, bitfield::NO_GAP)
-    }
-
-    pub fn set_new_page(&self, header: &mut [u8], new_page: usize) {
-        bitfield::set_range(self.new_page_range, header, bitfield::NO_GAP, new_page)
-    }
-
-    /// Returns the smallest word boundary greater or equal to a value.
-    fn align_word(&self, value: usize) -> usize {
-        let word_size = self.word_size;
-        (value + word_size - 1) / word_size * word_size
-    }
-
-    /// Returns the minimum number of bytes to represent a given number of bits.
-    fn bits_to_bytes(&self, bits: usize) -> usize {
-        (bits + 7) / 8
-    }
-}
-
-/// Returns the number of bits necessary to write numbers smaller than `x`.
-fn num_bits(x: usize) -> usize {
-    x.next_power_of_two().trailing_zeros() as usize
-}
-
-#[test]
-fn num_bits_ok() {
-    assert_eq!(num_bits(0), 0);
-    assert_eq!(num_bits(1), 0);
-    assert_eq!(num_bits(2), 1);
-    assert_eq!(num_bits(3), 2);
-    assert_eq!(num_bits(4), 2);
-    assert_eq!(num_bits(5), 3);
-    assert_eq!(num_bits(8), 3);
-    assert_eq!(num_bits(9), 4);
-    assert_eq!(num_bits(16), 4);
-}

src/embedded_flash/syscall.rs

@@ -1,4 +1,4 @@
-// Copyright 2019 Google LLC
+// Copyright 2019-2020 Google LLC
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,9 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use super::{Index, Storage, StorageError, StorageResult};
 use alloc::vec::Vec;
 use libtock_core::syscalls;
+use persistent_store::{Storage, StorageError, StorageIndex, StorageResult};
 
 const DRIVER_NUMBER: usize = 0x50003;
 
@@ -42,15 +42,13 @@ mod memop_nr {
 
 fn get_info(nr: usize, arg: usize) -> StorageResult<usize> {
     let code = syscalls::command(DRIVER_NUMBER, command_nr::GET_INFO, nr, arg);
-    code.map_err(|e| StorageError::KernelError {
+    code.map_err(|_| StorageError::CustomError)
-        code: e.return_code,
-    })
 }
 
 fn memop(nr: u32, arg: usize) -> StorageResult<usize> {
     let code = unsafe { syscalls::raw::memop(nr, arg) };
     if code < 0 {
-        Err(StorageError::KernelError { code })
+        Err(StorageError::CustomError)
     } else {
         Ok(code as usize)
     }
@@ -70,7 +68,7 @@ impl SyscallStorage {
     ///
     /// # Errors
     ///
-    /// Returns `BadFlash` if any of the following conditions do not hold:
+    /// Returns `CustomError` if any of the following conditions do not hold:
     /// - The word size is a power of two.
     /// - The page size is a power of two.
     /// - The page size is a multiple of the word size.
@@ -90,13 +88,13 @@ impl SyscallStorage {
             || !syscall.page_size.is_power_of_two()
             || !syscall.is_word_aligned(syscall.page_size)
         {
-            return Err(StorageError::BadFlash);
+            return Err(StorageError::CustomError);
         }
         for i in 0..memop(memop_nr::STORAGE_CNT, 0)? {
             let storage_ptr = memop(memop_nr::STORAGE_PTR, i)?;
             let max_storage_len = memop(memop_nr::STORAGE_LEN, i)?;
             if !syscall.is_page_aligned(storage_ptr) || !syscall.is_page_aligned(max_storage_len) {
-                return Err(StorageError::BadFlash);
+                return Err(StorageError::CustomError);
             }
             let storage_len = core::cmp::min(num_pages * syscall.page_size, max_storage_len);
             num_pages -= storage_len / syscall.page_size;
@@ -141,12 +139,12 @@ impl Storage for SyscallStorage {
         self.max_page_erases
     }
 
-    fn read_slice(&self, index: Index, length: usize) -> StorageResult<&[u8]> {
+    fn read_slice(&self, index: StorageIndex, length: usize) -> StorageResult<&[u8]> {
         let start = index.range(length, self)?.start;
         find_slice(&self.storage_locations, start, length)
     }
 
-    fn write_slice(&mut self, index: Index, value: &[u8]) -> StorageResult<()> {
+    fn write_slice(&mut self, index: StorageIndex, value: &[u8]) -> StorageResult<()> {
         if !self.is_word_aligned(index.byte) || !self.is_word_aligned(value.len()) {
             return Err(StorageError::NotAligned);
         }
@@ -163,28 +161,24 @@ impl Storage for SyscallStorage {
             )
         };
         if code < 0 {
-            return Err(StorageError::KernelError { code });
+            return Err(StorageError::CustomError);
         }
 
         let code = syscalls::command(DRIVER_NUMBER, command_nr::WRITE_SLICE, ptr, value.len());
-        if let Err(e) = code {
+        if code.is_err() {
-            return Err(StorageError::KernelError {
+            return Err(StorageError::CustomError);
-                code: e.return_code,
-            });
         }
 
         Ok(())
     }
 
     fn erase_page(&mut self, page: usize) -> StorageResult<()> {
-        let index = Index { page, byte: 0 };
+        let index = StorageIndex { page, byte: 0 };
         let length = self.page_size();
         let ptr = self.read_slice(index, length)?.as_ptr() as usize;
         let code = syscalls::command(DRIVER_NUMBER, command_nr::ERASE_PAGE, ptr, length);
-        if let Err(e) = code {
+        if code.is_err() {
-            return Err(StorageError::KernelError {
+            return Err(StorageError::CustomError);
-                code: e.return_code,
-            });
         }
         Ok(())
     }

third_party/lang-items/Cargo.toml

@@ -11,7 +11,7 @@ edition = "2018"
 [dependencies]
 libtock_core = { path = "../../third_party/libtock-rs/core", default-features = false, features = ["alloc_init", "custom_panic_handler", "custom_alloc_error_handler"] }
 libtock_drivers = { path = "../libtock-drivers" }
-linked_list_allocator = { version = "0.8.1", default-features = false }
+linked_list_allocator = { version = "0.8.7", default-features = false, features = ["const_mut_refs"] }
 
 [features]
 debug_allocations = []