From e52a671810cc2b64eb6769311ff8bbb6264ad3df Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Tue, 3 Mar 2020 23:47:32 +0100 Subject: [PATCH 01/10] Support storing in RAM instead of flash This permits to run without persistent storage. The benefit is that the board doesn't implement a the syscall API in Tock. The disadvantage is that rebooting the key will reset the storage. --- .github/workflows/cargo_check.yml | 6 ++++++ Cargo.toml | 1 + deploy.py | 8 ++++++++ run_desktop_tests.sh | 1 + src/ctap/storage.rs | 21 ++++++++++++++------- src/embedded_flash/buffer.rs | 3 ++- src/embedded_flash/mod.rs | 2 -- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/.github/workflows/cargo_check.yml b/.github/workflows/cargo_check.yml index 60b96d1..a6c4c46 100644 --- a/.github/workflows/cargo_check.yml +++ b/.github/workflows/cargo_check.yml @@ -58,6 +58,12 @@ 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 debug_ctap,with_ctap1 uses: actions-rs/cargo@v1 with: diff --git a/Cargo.toml b/Cargo.toml index 03dc034..3155106 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ debug_ctap = ["crypto/derive_debug"] with_ctap1 = ["crypto/with_ctap1"] panic_console = ["libtock/panic_console"] debug_allocations = ["libtock/debug_allocations"] +ram_storage = [] [dev-dependencies] elf2tab = "0.4.0" diff --git a/deploy.py b/deploy.py index ee7f54f..dc69c24 100755 --- a/deploy.py +++ b/deploy.py @@ -405,6 +405,14 @@ if __name__ == "__main__": "(i.e. more debug messages will be sent over the console port " "such as hexdumps of packets)."), ) + app_commands.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)."), + ) apps_group = app_commands.add_mutually_exclusive_group() apps_group.add_argument( "--opensk", diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index e3f2b1e..b8e8896 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -30,6 +30,7 @@ cargo check --release --target=thumbv7em-none-eabi --features with_ctap1 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 debug_ctap,with_ctap1 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1,panic_console,debug_allocations diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index e693a13..7569c1b 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -21,9 +21,9 @@ use alloc::vec::Vec; use core::convert::TryInto; use ctap2::embedded_flash::{self, StoreConfig, StoreEntry, StoreError, StoreIndex}; -#[cfg(test)] +#[cfg(any(test, feature = "ram_storage"))] type Storage = embedded_flash::BufferStorage; -#[cfg(not(test))] +#[cfg(not(any(test, feature = "ram_storage")))] type Storage = embedded_flash::SyscallStorage; // Those constants may be modified before compilation to tune the behavior of the key. @@ -44,6 +44,9 @@ type Storage = embedded_flash::SyscallStorage; // We have: I = ((P - 1) * 4092 - K * S) / 12 * C // // With P=20 and K=150, we have I > 2M which is enough for 500 increments per day for 10 years. +#[cfg(feature = "ram_storage")] +const NUM_PAGES: usize = 2; +#[cfg(not(feature = "ram_storage"))] const NUM_PAGES: usize = 20; const MAX_SUPPORTED_RESIDENTIAL_KEYS: usize = 150; @@ -130,10 +133,14 @@ pub struct PersistentStore { store: embedded_flash::Store, } +#[cfg(feature = "ram_storage")] +const PAGE_SIZE: usize = 0x100; +#[cfg(not(feature = "ram_storage"))] const PAGE_SIZE: usize = 0x1000; + const STORE_SIZE: usize = NUM_PAGES * PAGE_SIZE; -#[cfg(not(test))] +#[cfg(not(any(test, feature = "ram_storage")))] #[link_section = ".app_state"] static STORE: [u8; STORE_SIZE] = [0xff; STORE_SIZE]; @@ -144,9 +151,9 @@ impl PersistentStore { /// /// This should be at most one instance of persistent store per program lifetime. pub fn new(rng: &mut impl Rng256) -> PersistentStore { - #[cfg(not(test))] + #[cfg(not(any(test, feature = "ram_storage")))] let storage = PersistentStore::new_prod_storage(); - #[cfg(test)] + #[cfg(any(test, feature = "ram_storage"))] let storage = PersistentStore::new_test_storage(); let mut store = PersistentStore { store: embedded_flash::Store::new(storage, Config).unwrap(), @@ -155,7 +162,7 @@ impl PersistentStore { store } - #[cfg(not(test))] + #[cfg(not(any(test, feature = "ram_storage")))] fn new_prod_storage() -> Storage { let store = unsafe { // Safety: The store cannot alias because this function is called only once. @@ -167,7 +174,7 @@ impl PersistentStore { } } - #[cfg(test)] + #[cfg(any(test, feature = "ram_storage"))] fn new_test_storage() -> Storage { let store = vec![0xff; STORE_SIZE].into_boxed_slice(); let options = embedded_flash::BufferOptions { diff --git a/src/embedded_flash/buffer.rs b/src/embedded_flash/buffer.rs index b7cf3f1..4fcd80f 100644 --- a/src/embedded_flash/buffer.rs +++ b/src/embedded_flash/buffer.rs @@ -13,6 +13,7 @@ // limitations under the License. use super::{Index, Storage, StorageError, StorageResult}; +use alloc::boxed::Box; pub struct BufferStorage { storage: Box<[u8]>, @@ -230,7 +231,7 @@ impl Snapshot { fn get(&mut self) -> Result, usize> { let mut snapshot = Snapshot::Ready; - std::mem::swap(self, &mut snapshot); + core::mem::swap(self, &mut snapshot); match snapshot { Snapshot::Armed { delay } => Err(delay), Snapshot::Taken { storage } => Ok(storage), diff --git a/src/embedded_flash/mod.rs b/src/embedded_flash/mod.rs index 5e54059..8551a52 100644 --- a/src/embedded_flash/mod.rs +++ b/src/embedded_flash/mod.rs @@ -12,13 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[cfg(feature = "std")] mod buffer; mod storage; mod store; mod syscall; -#[cfg(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}; From bf9e3620ec65a3f63a17941031a0c9114e0f3797 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 11:24:07 +0100 Subject: [PATCH 02/10] Sync with upstream Tock to remove the latest patch. --- patches/tock/01-persistent-storage.patch | 2 +- .../tock/04-fix-dynamic-deferred-call.patch | 46 ------------------- third_party/tock | 2 +- 3 files changed, 2 insertions(+), 48 deletions(-) delete mode 100644 patches/tock/04-fix-dynamic-deferred-call.patch diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index 5aaec06..ecfb0b0 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -295,7 +295,7 @@ index ece4a443..9a1afc84 100644 } + + pub fn in_writeable_flash_region(&self, ptr: usize, len: usize) -> bool { -+ self.kernel.process_map_or(false, self.idx, |process| { ++ self.kernel.process_map_or(false, *self, |process| { + let ptr = match ptr.checked_sub(process.flash_start() as usize) { + None => return false, + Some(ptr) => ptr, diff --git a/patches/tock/04-fix-dynamic-deferred-call.patch b/patches/tock/04-fix-dynamic-deferred-call.patch deleted file mode 100644 index 4e29004..0000000 --- a/patches/tock/04-fix-dynamic-deferred-call.patch +++ /dev/null @@ -1,46 +0,0 @@ -diff --git a/kernel/src/common/dynamic_deferred_call.rs b/kernel/src/common/dynamic_deferred_call.rs -index 53f5143d..ca349972 100644 ---- a/kernel/src/common/dynamic_deferred_call.rs -+++ b/kernel/src/common/dynamic_deferred_call.rs -@@ -226,23 +226,25 @@ impl DynamicDeferredCall { - /// `call_global_instance_while`. - pub(self) fn call_while bool>(&self, f: F) { - if self.call_pending.get() { -- // Reset call_pending here, as it may be set again in the deferred calls -- self.call_pending.set(false); -+ for (i, client_state) in self.client_states.iter().enumerate() { -+ if !f() { -+ break; -+ } -+ if client_state.scheduled.get() { -+ client_state.client.map(|client| { -+ client_state.scheduled.set(false); -+ client.call(DeferredCallHandle(i)); -+ }); -+ } -+ } - -- self.client_states -- .iter() -- .enumerate() -- .filter(|(_i, client_state)| client_state.scheduled.get()) -- .filter_map(|(i, client_state)| { -- client_state -- .client -- .map(|c| (i, &client_state.scheduled, *c)) -- }) -- .take_while(|_| f()) -- .for_each(|(i, call_reqd, client)| { -- call_reqd.set(false); -- client.call(DeferredCallHandle(i)); -- }); -+ // Recompute call_pending here, as some deferred calls may have been skipped due to the -+ // `f` predicate becoming false. -+ self.call_pending.set( -+ self.client_states -+ .iter() -+ .any(|client_state| client_state.scheduled.get()), -+ ); - } - } - } diff --git a/third_party/tock b/third_party/tock index 3a7d6b7..fbc863f 160000 --- a/third_party/tock +++ b/third_party/tock @@ -1 +1 @@ -Subproject commit 3a7d6b775d972798bfd731cba8365b58fab27175 +Subproject commit fbc863faf0c9615537ee52dcdccdfcb9204d2467 From 68ec75593632d9271924d63e0d899fff009c225d Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 11:34:45 +0100 Subject: [PATCH 03/10] Add tests that supported boards build properly. --- run_desktop_tests.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index b8e8896..6e5a27d 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -40,6 +40,16 @@ cargo check --release --target=thumbv7em-none-eabi --examples echo "Checking that CTAP2 builds and links properly (1 set of features)..." cargo build --release --target=thumbv7em-none-eabi --features with_ctap1 +echo "Checking that supported boards build properly..." +cd third_party/tock/boards +cd nordic/nrf52840dk +make +cd ../.. +cd nordic/nrf52840_dongle +make +cd ../.. +cd ../../.. + if [ -z "${TRAVIS_OS_NAME}" -o "${TRAVIS_OS_NAME}" = "linux" ] then echo "Running unit tests on the desktop (release mode)..." From 0fe488962470c7534616c16e55b1836daacfea34 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 11:50:26 +0100 Subject: [PATCH 04/10] Add GitHub workflow to build the boards. --- .github/workflows/boards_build.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/boards_build.yml diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml new file mode 100644 index 0000000..477657f --- /dev/null +++ b/.github/workflows/boards_build.yml @@ -0,0 +1,30 @@ +--- +name: Build supported boards +on: + push: + pull_request: + types: [opened, synchronize, reopened] + +jobs: + build_boards: + strategy: + matrix: + os: [ubuntu-18.04, macos-10.15] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + with: + target: thumbv7em-none-eabi + - uses: actions/setup-python@v1 + with: + python-version: 3.7 + - name: Install Python dependencies + run: python -m pip install --upgrade pip setuptools wheel + - name: Set up OpenSK + run: ./setup.sh + + - name: Building board nrf52840dk + run: cd third_party/tock/boards/nordic/nrf52840dk && make + - name: Building board nrf52840_dongle + run: cd third_party/tock/boards/nordic/nrf52840_dongle && make From 6d323f3c5ae9c108c45edd68a35a812612010201 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 12:03:35 +0100 Subject: [PATCH 05/10] Apply make suggestions. --- .github/workflows/boards_build.yml | 4 ++-- run_desktop_tests.sh | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml index 477657f..61d1b5a 100644 --- a/.github/workflows/boards_build.yml +++ b/.github/workflows/boards_build.yml @@ -25,6 +25,6 @@ jobs: run: ./setup.sh - name: Building board nrf52840dk - run: cd third_party/tock/boards/nordic/nrf52840dk && make + run: make -C third_party/tock/boards/nordic/nrf52840dk - name: Building board nrf52840_dongle - run: cd third_party/tock/boards/nordic/nrf52840_dongle && make + run: make -C third_party/tock/boards/nordic/nrf52840_dongle diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index 6e5a27d..33b2090 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -41,14 +41,8 @@ echo "Checking that CTAP2 builds and links properly (1 set of features)..." cargo build --release --target=thumbv7em-none-eabi --features with_ctap1 echo "Checking that supported boards build properly..." -cd third_party/tock/boards -cd nordic/nrf52840dk -make -cd ../.. -cd nordic/nrf52840_dongle -make -cd ../.. -cd ../../.. +make -C third_party/tock/boards/nordic/nrf52840dk +make -C third_party/tock/boards/nordic/nrf52840_dongle if [ -z "${TRAVIS_OS_NAME}" -o "${TRAVIS_OS_NAME}" = "linux" ] then From eac6f1d0bdd0fa0371e79f3c9778286df6f38fbf Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 12:08:28 +0100 Subject: [PATCH 06/10] Build boards only when relevant paths are affected. --- .github/workflows/boards_build.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml index 61d1b5a..5231b92 100644 --- a/.github/workflows/boards_build.yml +++ b/.github/workflows/boards_build.yml @@ -2,8 +2,14 @@ name: Build supported boards on: push: + paths: + - 'patches/tock/**' + - 'third_party/**' pull_request: types: [opened, synchronize, reopened] + paths: + - 'patches/tock/**' + - 'third_party/**' jobs: build_boards: From ed350192c9f7c5c181b9b44c41ef13dd7234dc98 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 12:28:01 +0100 Subject: [PATCH 07/10] Move third_party/tock to a long build directory to catch limits in the linker scripts. --- .github/workflows/boards_build.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml index 5231b92..5c19e93 100644 --- a/.github/workflows/boards_build.yml +++ b/.github/workflows/boards_build.yml @@ -29,8 +29,10 @@ jobs: run: python -m pip install --upgrade pip setuptools wheel - name: Set up OpenSK run: ./setup.sh + - name: Create a long build directory + run: mkdir this-is-a-long-build-directory-0123456789abcdefghijklmnopqrstuvwxyz && mv third_party this-is-a-long-build-directory-0123456789abcdefghijklmnopqrstuvwxyz/ - name: Building board nrf52840dk - run: make -C third_party/tock/boards/nordic/nrf52840dk + run: make -C this-is-a-long-build-directory-0123456789abcdefghijklmnopqrstuvwxyz/third_party/tock/boards/nordic/nrf52840dk - name: Building board nrf52840_dongle - run: make -C third_party/tock/boards/nordic/nrf52840_dongle + run: make -C this-is-a-long-build-directory-0123456789abcdefghijklmnopqrstuvwxyz/third_party/tock/boards/nordic/nrf52840_dongle From 0036858db91853cfa0373a3d6bbd3e0ff6b0c276 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 12:33:12 +0100 Subject: [PATCH 08/10] Increase ROM size in layout.ld so that boards build on long build directories. --- patches/tock/04-increase-rom-nordic.patch | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 patches/tock/04-increase-rom-nordic.patch diff --git a/patches/tock/04-increase-rom-nordic.patch b/patches/tock/04-increase-rom-nordic.patch new file mode 100644 index 0000000..83948f8 --- /dev/null +++ b/patches/tock/04-increase-rom-nordic.patch @@ -0,0 +1,24 @@ +diff --git a/boards/nordic/nrf52840_dongle/layout.ld b/boards/nordic/nrf52840_dongle/layout.ld +index 657b0d26..f86b2321 100644 +--- a/boards/nordic/nrf52840_dongle/layout.ld ++++ b/boards/nordic/nrf52840_dongle/layout.ld +@@ -1,6 +1,6 @@ + MEMORY + { +- rom (rx) : ORIGIN = 0x00000000, LENGTH = 128K ++ rom (rx) : ORIGIN = 0x00000000, LENGTH = 192K + prog (rx) : ORIGIN = 0x00030000, LENGTH = 832K + ram (rwx) : ORIGIN = 0x20000000, LENGTH = 256K + } +diff --git a/boards/nordic/nrf52840dk/layout.ld b/boards/nordic/nrf52840dk/layout.ld +index 657b0d26..f86b2321 100644 +--- a/boards/nordic/nrf52840dk/layout.ld ++++ b/boards/nordic/nrf52840dk/layout.ld +@@ -1,6 +1,6 @@ + MEMORY + { +- rom (rx) : ORIGIN = 0x00000000, LENGTH = 128K ++ rom (rx) : ORIGIN = 0x00000000, LENGTH = 192K + prog (rx) : ORIGIN = 0x00030000, LENGTH = 832K + ram (rwx) : ORIGIN = 0x20000000, LENGTH = 256K + } From 1fca16316e66a7da94eb180f88ffb766cdc7b175 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 15:31:32 +0100 Subject: [PATCH 09/10] Remove paths rules from workflows/boards_build. --- .github/workflows/boards_build.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml index 5c19e93..185c4f0 100644 --- a/.github/workflows/boards_build.yml +++ b/.github/workflows/boards_build.yml @@ -2,14 +2,8 @@ name: Build supported boards on: push: - paths: - - 'patches/tock/**' - - 'third_party/**' pull_request: types: [opened, synchronize, reopened] - paths: - - 'patches/tock/**' - - 'third_party/**' jobs: build_boards: From 3af13f1957ce6655cf064b989e96849455b0b334 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Thu, 5 Mar 2020 17:12:03 +0100 Subject: [PATCH 10/10] Add path protection on push for workflows/boards_build. According to the rules at https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#patterns-to-match-file-paths --- .github/workflows/boards_build.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/boards_build.yml b/.github/workflows/boards_build.yml index 185c4f0..ef71f42 100644 --- a/.github/workflows/boards_build.yml +++ b/.github/workflows/boards_build.yml @@ -2,6 +2,9 @@ name: Build supported boards on: push: + paths: + - 'patches/tock/*' + - 'third_party/tock/**' pull_request: types: [opened, synchronize, reopened]