From 6247098069db2db5f4fe5f928bc8d945d5b40f73 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Apr 2020 12:52:31 +0200 Subject: [PATCH 01/27] Do not use writeable flash regions for persistent storage They don't play well with DFU. --- nrf52840dk_layout.ld | 6 ++- patches/tock/01-persistent-storage.patch | 54 +++--------------------- src/ctap/storage.rs | 15 ++++--- 3 files changed, 19 insertions(+), 56 deletions(-) diff --git a/nrf52840dk_layout.ld b/nrf52840dk_layout.ld index 218b6bb1..82927380 100644 --- a/nrf52840dk_layout.ld +++ b/nrf52840dk_layout.ld @@ -3,8 +3,10 @@ */ MEMORY { - /* The application region is 64 bytes (0x40) */ - FLASH (rx) : ORIGIN = 0x00040040, LENGTH = 0x000BFFC0 + /* The application region is 64 bytes (0x40) and we reserve 0x40000 at the end + * of the flash for the persistent storage. + */ + FLASH (rx) : ORIGIN = 0x00040040, LENGTH = 0x0007FFC0 SRAM (rwx) : ORIGIN = 0x20020000, LENGTH = 128K } diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index 57880c60..942e13df 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -106,7 +106,7 @@ index 5abd2d84..5a726fdb 100644 let word: u32 = (data[i + 0] as u32) << 0 | (data[i + 1] as u32) << 8 | (data[i + 2] as u32) << 16 -@@ -374,3 +386,178 @@ impl hil::flash::Flash for Nvmc { +@@ -374,3 +386,170 @@ impl hil::flash::Flash for Nvmc { self.erase_page(page_number) } } @@ -189,11 +189,7 @@ index 5abd2d84..5a726fdb 100644 + /// Fails with `EINVAL` if any of the following conditions does not hold: + /// - `ptr` must be word-aligned. + /// - `slice.len()` must be word-aligned. -+ /// - The slice starting at `ptr` of length `slice.len()` must fit in a writeable flash region. -+ fn write_slice(&self, appid: AppId, ptr: usize, slice: &[u8]) -> ReturnCode { -+ if !appid.in_writeable_flash_region(ptr, slice.len()) { -+ return ReturnCode::EINVAL; -+ } ++ fn write_slice(&self, ptr: usize, slice: &[u8]) -> ReturnCode { + if ptr & WORD_MASK != 0 || slice.len() & WORD_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -217,11 +213,7 @@ index 5abd2d84..5a726fdb 100644 + /// + /// Fails with `EINVAL` if any of the following conditions does not hold: + /// - `ptr` must be page-aligned. -+ /// - The slice starting at `ptr` of length `PAGE_SIZE` must fit in a writeable flash region. -+ fn erase_page(&self, appid: AppId, ptr: usize) -> ReturnCode { -+ if !appid.in_writeable_flash_region(ptr, PAGE_SIZE) { -+ return ReturnCode::EINVAL; -+ } ++ fn erase_page(&self, ptr: usize) -> ReturnCode { + if ptr & PAGE_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -257,11 +249,11 @@ index 5abd2d84..5a726fdb 100644 + None => return ReturnCode::EINVAL, + Some(slice) => slice, + }; -+ self.write_slice(appid, ptr, slice.as_ref()) ++ self.write_slice(ptr, slice.as_ref()) + }) + .unwrap_or_else(|err| err.into()), + -+ (3, ptr) => self.erase_page(appid, ptr), ++ (3, ptr) => self.erase_page(ptr), + + _ => ReturnCode::ENOSUPPORT, + } @@ -285,39 +277,3 @@ index 5abd2d84..5a726fdb 100644 + } + } +} -diff --git a/kernel/src/callback.rs b/kernel/src/callback.rs -index c812e0bf..bd1613b3 100644 ---- a/kernel/src/callback.rs -+++ b/kernel/src/callback.rs -@@ -130,6 +130,31 @@ impl AppId { - (start, end) - }) - } -+ -+ pub fn in_writeable_flash_region(&self, ptr: usize, len: usize) -> bool { -+ 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, -+ }; -+ (0..process.number_writeable_flash_regions()).any(|i| { -+ let (region_ptr, region_len) = process.get_writeable_flash_region(i); -+ let region_ptr = region_ptr as usize; -+ let region_len = region_len as usize; -+ // We want to check the 2 following inequalities: -+ // (1) `region_ptr <= ptr` -+ // (2) `ptr + len <= region_ptr + region_len` -+ // However, the second one may overflow written as is. We introduce a third -+ // inequality to solve this issue: -+ // (3) `len <= region_len` -+ // Using this third inequality, we can rewrite the second one as: -+ // (4) `ptr - region_ptr <= region_len - len` -+ // This fourth inequality is equivalent to the second one but doesn't overflow when -+ // the first and third inequalities hold. -+ region_ptr <= ptr && len <= region_len && ptr - region_ptr <= region_len - len -+ }) -+ }) -+ } - } - - /// Type to uniquely identify a callback subscription across all drivers. diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index f0a2ca4f..3d0c4d2f 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -138,12 +138,15 @@ const PAGE_SIZE: usize = 0x100; #[cfg(not(feature = "ram_storage"))] const PAGE_SIZE: usize = 0x1000; +// We have the following layout: +// 0x00000-0x2ffff: Tock +// 0x30000-0x3ffff: Padding +// 0x40000-0xbffff: App +// 0xc0000-0xfffff: Store +const STORE_ADDR: usize = 0xC0000; +const STORE_SIZE_LIMIT: usize = 0x40000; const STORE_SIZE: usize = NUM_PAGES * PAGE_SIZE; -#[cfg(not(any(test, feature = "ram_storage")))] -#[link_section = ".app_state"] -static STORE: [u8; STORE_SIZE] = [0xff; STORE_SIZE]; - impl PersistentStore { /// Gives access to the persistent store. /// @@ -164,9 +167,11 @@ impl PersistentStore { #[cfg(not(any(test, feature = "ram_storage")))] fn new_prod_storage() -> Storage { + // This should ideally be a compile-time assert, but Rust doesn't have native support. + assert!(STORE_SIZE <= STORE_SIZE_LIMIT); let store = unsafe { // Safety: The store cannot alias because this function is called only once. - core::slice::from_raw_parts_mut(STORE.as_ptr() as *mut u8, STORE_SIZE) + core::slice::from_raw_parts_mut(STORE_ADDR as *mut u8, STORE_SIZE) }; unsafe { // Safety: The store is in a writeable flash region. From 892f950cc13247a6b772f304dedc49b45c12e310 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Apr 2020 12:59:22 +0200 Subject: [PATCH 02/27] Add missing cfg --- src/ctap/storage.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 3d0c4d2f..879c1e73 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -143,6 +143,7 @@ const PAGE_SIZE: usize = 0x1000; // 0x30000-0x3ffff: Padding // 0x40000-0xbffff: App // 0xc0000-0xfffff: Store +#[cfg(not(any(test, feature = "ram_storage")))] const STORE_ADDR: usize = 0xC0000; const STORE_SIZE_LIMIT: usize = 0x40000; const STORE_SIZE: usize = NUM_PAGES * PAGE_SIZE; @@ -154,6 +155,8 @@ impl PersistentStore { /// /// This should be at most one instance of persistent store per program lifetime. pub fn new(rng: &mut impl Rng256) -> PersistentStore { + // This should ideally be a compile-time assert, but Rust doesn't have native support. + assert!(STORE_SIZE <= STORE_SIZE_LIMIT); #[cfg(not(any(test, feature = "ram_storage")))] let storage = PersistentStore::new_prod_storage(); #[cfg(any(test, feature = "ram_storage"))] @@ -167,8 +170,6 @@ impl PersistentStore { #[cfg(not(any(test, feature = "ram_storage")))] fn new_prod_storage() -> Storage { - // This should ideally be a compile-time assert, but Rust doesn't have native support. - assert!(STORE_SIZE <= STORE_SIZE_LIMIT); let store = unsafe { // Safety: The store cannot alias because this function is called only once. core::slice::from_raw_parts_mut(STORE_ADDR as *mut u8, STORE_SIZE) From 88920035fa9b69106fcb26cc729206d8462bc96d Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Apr 2020 13:50:42 +0200 Subject: [PATCH 03/27] Permit the app to read the storage --- patches/tock/01-persistent-storage.patch | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index 942e13df..fd293069 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -277,3 +277,39 @@ index 5abd2d84..5a726fdb 100644 + } + } +} +diff --git a/kernel/src/process.rs b/kernel/src/process.rs +index eb00f274..35c19d15 100644 +--- a/kernel/src/process.rs ++++ b/kernel/src/process.rs +@@ -1604,6 +1604,31 @@ impl Process<'a, C> { + return Ok((None, 0)); + } + ++ // Allocate MPU region for storage. ++ const STORAGE_PTR: usize = 0xc0000; ++ const STORAGE_LEN: usize = 0x40000; ++ if chip ++ .mpu() ++ .allocate_region( ++ STORAGE_PTR as *const u8, ++ STORAGE_LEN, ++ STORAGE_LEN, ++ mpu::Permissions::ReadOnly, ++ &mut mpu_config, ++ ) ++ .is_none() ++ { ++ if config::CONFIG.debug_load_processes { ++ debug!( ++ "[!] flash=[{:#010X}:{:#010X}] process={:?} - couldn't allocate flash region", ++ STORAGE_PTR, ++ STORAGE_PTR + STORAGE_LEN, ++ process_name ++ ); ++ } ++ return Ok((None, 0)); ++ } ++ + // Determine how much space we need in the application's + // memory space just for kernel and grant state. We need to make + // sure we allocate enough memory just for that. From 5c2b72ce83d7a23db486b5c7a082c22077f6ff10 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 29 Apr 2020 15:09:50 +0200 Subject: [PATCH 04/27] Move storage bound checking to driver --- patches/tock/01-persistent-storage.patch | 48 +++++++++++++++++++----- src/ctap/storage.rs | 3 -- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index fd293069..73f8ef35 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -42,7 +42,7 @@ index fe493727..105f7120 100644 platform.pconsole.start(); diff --git a/chips/nrf52/src/nvmc.rs b/chips/nrf52/src/nvmc.rs -index 5abd2d84..5a726fdb 100644 +index 60fc2da8..77e7423d 100644 --- a/chips/nrf52/src/nvmc.rs +++ b/chips/nrf52/src/nvmc.rs @@ -3,6 +3,7 @@ @@ -62,7 +62,7 @@ index 5abd2d84..5a726fdb 100644 use crate::deferred_call_tasks::DeferredCallTask; -@@ -141,7 +142,13 @@ register_bitfields! [u32, +@@ -141,7 +142,33 @@ register_bitfields! [u32, static DEFERRED_CALL: DeferredCall = unsafe { DeferredCall::new(DeferredCallTask::Nvmc) }; @@ -73,10 +73,30 @@ index 5abd2d84..5a726fdb 100644 +const MAX_PAGE_ERASES: usize = 10000; +const WORD_MASK: usize = WORD_SIZE - 1; +const PAGE_MASK: usize = PAGE_SIZE - 1; ++ ++// The storage is currently static and readable by all processes. This will be fixed once Tock ++// supports persistent storage outside the application flash (i.e. not the current writeable flash ++// regions support). ++const STORAGE_PTR: usize = 0xc0000; ++const STORAGE_LEN: usize = 0x40000; ++ ++fn in_storage(ptr: usize, len: usize) -> bool { ++ // We want to check the 2 following inequalities: ++ // (1) `STORAGE_PTR <= ptr` ++ // (2) `ptr + len <= STORAGE_PTR + STORAGE_LEN` ++ // However, the second one may overflow written as is. We introduce a third ++ // inequality to solve this issue: ++ // (3) `len <= STORAGE_LEN` ++ // Using this third inequality, we can rewrite the second one as: ++ // (4) `ptr - STORAGE_PTR <= STORAGE_LEN - len` ++ // This fourth inequality is equivalent to the second one but doesn't overflow when ++ // the first and third inequalities hold. ++ STORAGE_PTR <= ptr && len <= STORAGE_LEN && ptr - STORAGE_PTR <= STORAGE_LEN - len ++} /// This is a wrapper around a u8 array that is sized to a single page for the /// nrf. Users of this module must pass an object of this type to use the -@@ -215,6 +222,11 @@ impl Nvmc { +@@ -215,6 +242,11 @@ impl Nvmc { } } @@ -88,7 +108,7 @@ index 5abd2d84..5a726fdb 100644 /// Configure the NVMC to allow writes to flash. pub fn configure_writeable(&self) { let regs = &*self.registers; -@@ -230,7 +242,7 @@ impl Nvmc { +@@ -230,7 +262,7 @@ impl Nvmc { let regs = &*self.registers; regs.config.write(Configuration::WEN::Een); while !self.is_ready() {} @@ -97,7 +117,7 @@ index 5abd2d84..5a726fdb 100644 while !self.is_ready() {} } -@@ -314,7 +326,7 @@ impl Nvmc { +@@ -322,7 +354,7 @@ impl Nvmc { // Put the NVMC in write mode. regs.config.write(Configuration::WEN::Wen); @@ -106,7 +126,7 @@ index 5abd2d84..5a726fdb 100644 let word: u32 = (data[i + 0] as u32) << 0 | (data[i + 1] as u32) << 8 | (data[i + 2] as u32) << 16 -@@ -374,3 +386,170 @@ impl hil::flash::Flash for Nvmc { +@@ -390,3 +422,178 @@ impl hil::flash::Flash for Nvmc { self.erase_page(page_number) } } @@ -189,7 +209,11 @@ index 5abd2d84..5a726fdb 100644 + /// Fails with `EINVAL` if any of the following conditions does not hold: + /// - `ptr` must be word-aligned. + /// - `slice.len()` must be word-aligned. ++ /// - The slice starting at `ptr` of length `slice.len()` must fit in the storage. + fn write_slice(&self, ptr: usize, slice: &[u8]) -> ReturnCode { ++ if !in_storage(ptr, slice.len()) { ++ return ReturnCode::EINVAL; ++ } + if ptr & WORD_MASK != 0 || slice.len() & WORD_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -213,7 +237,11 @@ index 5abd2d84..5a726fdb 100644 + /// + /// Fails with `EINVAL` if any of the following conditions does not hold: + /// - `ptr` must be page-aligned. ++ /// - The slice starting at `ptr` of length `PAGE_SIZE` must fit in the storage. + fn erase_page(&self, ptr: usize) -> ReturnCode { ++ if !in_storage(ptr, PAGE_SIZE) { ++ return ReturnCode::EINVAL; ++ } + if ptr & PAGE_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -278,14 +306,16 @@ index 5abd2d84..5a726fdb 100644 + } +} diff --git a/kernel/src/process.rs b/kernel/src/process.rs -index eb00f274..35c19d15 100644 +index eb00f274..663c2422 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs -@@ -1604,6 +1604,31 @@ impl Process<'a, C> { +@@ -1604,6 +1604,33 @@ impl Process<'a, C> { return Ok((None, 0)); } -+ // Allocate MPU region for storage. ++ // Allocate MPU region for storage. The storage is currently static and readable by all ++ // processes. This will be fixed once Tock supports persistent storage outside the ++ // application flash (i.e. not the current writeable flash regions support). + const STORAGE_PTR: usize = 0xc0000; + const STORAGE_LEN: usize = 0x40000; + if chip diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 879c1e73..2a6e52eb 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -145,7 +145,6 @@ const PAGE_SIZE: usize = 0x1000; // 0xc0000-0xfffff: Store #[cfg(not(any(test, feature = "ram_storage")))] const STORE_ADDR: usize = 0xC0000; -const STORE_SIZE_LIMIT: usize = 0x40000; const STORE_SIZE: usize = NUM_PAGES * PAGE_SIZE; impl PersistentStore { @@ -155,8 +154,6 @@ impl PersistentStore { /// /// This should be at most one instance of persistent store per program lifetime. pub fn new(rng: &mut impl Rng256) -> PersistentStore { - // This should ideally be a compile-time assert, but Rust doesn't have native support. - assert!(STORE_SIZE <= STORE_SIZE_LIMIT); #[cfg(not(any(test, feature = "ram_storage")))] let storage = PersistentStore::new_prod_storage(); #[cfg(any(test, feature = "ram_storage"))] From 3edb387615acdadb5ea3c89f9c3afaccbc3d7e65 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 6 May 2020 15:18:27 +0200 Subject: [PATCH 05/27] Remove writable flash regions support Removing support for writable flash regions permits to save 1 page in the binary due to alignment constraints. It also permits to reduce the diff with libtock-rs which doesn't support writable flash regions. This commit also updates the `SyscallStorage` documentation. --- layout.ld | 12 ------------ src/embedded_flash/syscall.rs | 17 +++++++++-------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/layout.ld b/layout.ld index 5bc9d0cc..fde1c400 100644 --- a/layout.ld +++ b/layout.ld @@ -71,18 +71,6 @@ SECTIONS { . = ALIGN(32); } > FLASH =0xFF - /* App state section. Used for persistent app data. - * We put this first because this is what libtock-c does. They provide the - * following explanation: if the app code changes but the persistent data - * doesn't, the app_state can be preserved. - */ - .wfr.app_state : - { - . = ALIGN(4K); - KEEP (*(.app_state)) - . = ALIGN(4K); - } > FLASH =0xFFFFFFFF - /* Text section, Code! */ .text : { diff --git a/src/embedded_flash/syscall.rs b/src/embedded_flash/syscall.rs index 7fca17af..b67a7c4a 100644 --- a/src/embedded_flash/syscall.rs +++ b/src/embedded_flash/syscall.rs @@ -55,7 +55,7 @@ impl SyscallStorage { /// /// # Safety /// - /// The `storage` must be in a writeable flash region. + /// The `storage` must be readable. /// /// # Errors /// @@ -74,14 +74,15 @@ impl SyscallStorage { /// # extern crate ctap2; /// # use ctap2::embedded_flash::SyscallStorage; /// # use ctap2::embedded_flash::StorageResult; - /// # const NUM_PAGES: usize = 1; - /// # const PAGE_SIZE: usize = 1; - /// #[link_section = ".app_state"] - /// static mut STORAGE: [u8; NUM_PAGES * PAGE_SIZE] = [0xff; NUM_PAGES * PAGE_SIZE]; + /// # const STORAGE_ADDR: usize = 0x1000; + /// # const STORAGE_SIZE: usize = 0x1000; /// # fn foo() -> StorageResult { - /// // This is safe because this is the only use of `STORAGE` in the whole program and this is - /// // called only once. - /// unsafe { SyscallStorage::new(&mut STORAGE) } + /// // This is safe because we create and use `storage` only once in the whole program. + /// let storage = unsafe { + /// core::slice::from_raw_parts_mut(STORAGE_ADDR as *mut u8, STORAGE_SIZE) + /// }; + /// // This is safe because `storage` is readable. + /// unsafe { SyscallStorage::new(storage) } /// # } /// ``` pub unsafe fn new(storage: &'static mut [u8]) -> StorageResult { From ecf02eb6ce6406eb9e6dd00b013f3c048a10d3db Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 8 May 2020 17:00:59 +0200 Subject: [PATCH 06/27] Only store the storage location in the Kernel --- patches/tock/01-persistent-storage.patch | 12 +++++- src/ctap/storage.rs | 29 +++---------- src/embedded_flash/syscall.rs | 55 ++++++++---------------- 3 files changed, 34 insertions(+), 62 deletions(-) diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index 73f8ef35..1df57ae1 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -42,7 +42,7 @@ index fe493727..105f7120 100644 platform.pconsole.start(); diff --git a/chips/nrf52/src/nvmc.rs b/chips/nrf52/src/nvmc.rs -index 60fc2da8..77e7423d 100644 +index 60fc2da8..45c75f89 100644 --- a/chips/nrf52/src/nvmc.rs +++ b/chips/nrf52/src/nvmc.rs @@ -3,6 +3,7 @@ @@ -126,7 +126,7 @@ index 60fc2da8..77e7423d 100644 let word: u32 = (data[i + 0] as u32) << 0 | (data[i + 1] as u32) << 8 | (data[i + 2] as u32) << 16 -@@ -390,3 +422,178 @@ impl hil::flash::Flash for Nvmc { +@@ -390,3 +422,186 @@ impl hil::flash::Flash for Nvmc { self.erase_page(page_number) } } @@ -158,6 +158,8 @@ index 60fc2da8..77e7423d 100644 +/// - COMMAND(1, 2): Get the maximum number of word writes between page erasures (always 2). +/// - COMMAND(1, 3): Get the maximum number page erasures in the lifetime of the flash (always +/// 10000). ++/// - COMMAND(1, 4): Get the storage address (page-aligned). ++/// - COMMAND(1, 5): Get the storage length (page-aligned). +/// - COMMAND(2, ptr): Write the allow slice to the flash region starting at `ptr`. +/// - `ptr` must be word-aligned. +/// - The allow slice length must be word aligned. @@ -268,6 +270,12 @@ index 60fc2da8..77e7423d 100644 + (1, 3) => ReturnCode::SuccessWithValue { + value: MAX_PAGE_ERASES, + }, ++ (1, 4) => ReturnCode::SuccessWithValue { ++ value: STORAGE_PTR, ++ }, ++ (1, 5) => ReturnCode::SuccessWithValue { ++ value: STORAGE_LEN, ++ }, + (1, _) => ReturnCode::EINVAL, + + (2, ptr) => self diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 2a6e52eb..6fc398ee 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -133,20 +133,6 @@ 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; - -// We have the following layout: -// 0x00000-0x2ffff: Tock -// 0x30000-0x3ffff: Padding -// 0x40000-0xbffff: App -// 0xc0000-0xfffff: Store -#[cfg(not(any(test, feature = "ram_storage")))] -const STORE_ADDR: usize = 0xC0000; -const STORE_SIZE: usize = NUM_PAGES * PAGE_SIZE; - impl PersistentStore { /// Gives access to the persistent store. /// @@ -167,19 +153,16 @@ impl PersistentStore { #[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. - core::slice::from_raw_parts_mut(STORE_ADDR as *mut u8, STORE_SIZE) - }; - unsafe { - // Safety: The store is in a writeable flash region. - Storage::new(store).unwrap() - } + Storage::new(NUM_PAGES).unwrap() } #[cfg(any(test, feature = "ram_storage"))] fn new_test_storage() -> Storage { - let store = vec![0xff; STORE_SIZE].into_boxed_slice(); + #[cfg(not(test))] + const PAGE_SIZE: usize = 0x100; + #[cfg(test)] + const PAGE_SIZE: usize = 0x1000; + let store = vec![0xff; NUM_PAGES * PAGE_SIZE].into_boxed_slice(); let options = embedded_flash::BufferOptions { word_size: 4, page_size: PAGE_SIZE, diff --git a/src/embedded_flash/syscall.rs b/src/embedded_flash/syscall.rs index b67a7c4a..09a7cddf 100644 --- a/src/embedded_flash/syscall.rs +++ b/src/embedded_flash/syscall.rs @@ -24,6 +24,8 @@ mod command_nr { pub const PAGE_SIZE: usize = 1; pub const MAX_WORD_WRITES: usize = 2; pub const MAX_PAGE_ERASES: usize = 3; + pub const STORAGE_PTR: usize = 4; + pub const STORAGE_LEN: usize = 5; } pub const WRITE_SLICE: usize = 2; pub const ERASE_PAGE: usize = 3; @@ -53,46 +55,31 @@ pub struct SyscallStorage { impl SyscallStorage { /// Provides access to the embedded flash if available. /// - /// # Safety - /// - /// The `storage` must be readable. - /// /// # Errors /// /// Returns `BadFlash` if any of the following conditions do not hold: - /// - The word size is not a power of two. - /// - The page size is not a power of two. - /// - The page size is not a multiple of the word size. - /// - /// Returns `NotAligned` if any of the following conditions do not hold: - /// - `storage` is page-aligned. - /// - `storage.len()` is a multiple of the page size. + /// - 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. + /// - The storage is page-aligned. /// - /// # Examples - /// - /// ```rust - /// # extern crate ctap2; - /// # use ctap2::embedded_flash::SyscallStorage; - /// # use ctap2::embedded_flash::StorageResult; - /// # const STORAGE_ADDR: usize = 0x1000; - /// # const STORAGE_SIZE: usize = 0x1000; - /// # fn foo() -> StorageResult { - /// // This is safe because we create and use `storage` only once in the whole program. - /// let storage = unsafe { - /// core::slice::from_raw_parts_mut(STORAGE_ADDR as *mut u8, STORAGE_SIZE) - /// }; - /// // This is safe because `storage` is readable. - /// unsafe { SyscallStorage::new(storage) } - /// # } - /// ``` - pub unsafe fn new(storage: &'static mut [u8]) -> StorageResult { + /// Returns `OutOfBounds` the number of pages does not fit in the storage. + pub fn new(num_pages: usize) -> StorageResult { let word_size = get_info(command_nr::get_info_nr::WORD_SIZE)?; let page_size = get_info(command_nr::get_info_nr::PAGE_SIZE)?; let max_word_writes = get_info(command_nr::get_info_nr::MAX_WORD_WRITES)?; let max_page_erases = get_info(command_nr::get_info_nr::MAX_PAGE_ERASES)?; + let storage_ptr = get_info(command_nr::get_info_nr::STORAGE_PTR)?; + let max_storage_len = get_info(command_nr::get_info_nr::STORAGE_LEN)?; if !word_size.is_power_of_two() || !page_size.is_power_of_two() { return Err(StorageError::BadFlash); } + let storage_len = num_pages * page_size; + if storage_len > max_storage_len { + return Err(StorageError::OutOfBounds); + } + let storage = + unsafe { core::slice::from_raw_parts_mut(storage_ptr as *mut u8, storage_len) }; let syscall = SyscallStorage { word_size, page_size, @@ -100,16 +87,10 @@ impl SyscallStorage { max_page_erases, storage, }; - if !syscall.is_word_aligned(page_size) { + if !syscall.is_word_aligned(page_size) || !syscall.is_page_aligned(storage_ptr) { return Err(StorageError::BadFlash); } - if syscall.is_page_aligned(syscall.storage.as_ptr() as usize) - && syscall.is_page_aligned(syscall.storage.len()) - { - Ok(syscall) - } else { - Err(StorageError::NotAligned) - } + Ok(syscall) } fn is_word_aligned(&self, x: usize) -> bool { From f4b791ed9149b25e0489bb2d5a392af46f1dbac8 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Sat, 9 May 2020 15:55:55 +0200 Subject: [PATCH 07/27] Encode credentials as a protocol buffer message This permits to decode a credential of a different version without failing. --- libraries/cbor/src/macros.rs | 22 ++++-- libraries/cbor/src/writer.rs | 2 +- src/ctap/data_formats.rs | 126 +++++++++++++++++++++-------------- src/ctap/mod.rs | 4 ++ src/ctap/storage.rs | 7 +- 5 files changed, 102 insertions(+), 59 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index 5a3d8f52..e5f3781e 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -41,18 +41,30 @@ macro_rules! cbor_map_options { }; ( $( $key:expr => $value:expr ),* ) => { + cbor_extend_map_options! ( + ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(), + $( $key => $value, )* + ) + }; +} + +#[macro_export] +macro_rules! cbor_extend_map_options { + // Add trailing comma if missing. + ( $initial:expr, $( $key:expr => $value:expr ),+ ) => { + cbor_extend_map_options! ( $initial, $($key => $value,)+ ) + }; + + ( $initial:expr, $( $key:expr => $value:expr, )* ) => { { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValueOption}; - let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(); + let mut _map = $initial; $( - { - let opt: Option<$crate::values::Value> = $value.into_cbor_value_option(); - if let Some(val) = opt { + if let Some(val) = $value.into_cbor_value_option() { _map.insert($key.into_cbor_key(), val); } - } )* $crate::values::Value::Map(_map) } diff --git a/libraries/cbor/src/writer.rs b/libraries/cbor/src/writer.rs index 12731609..0764851d 100644 --- a/libraries/cbor/src/writer.rs +++ b/libraries/cbor/src/writer.rs @@ -36,7 +36,7 @@ impl<'a> Writer<'a> { return false; } match value { - Value::KeyValue(KeyType::Unsigned(unsigned)) => self.start_item(0, unsigned as u64), + Value::KeyValue(KeyType::Unsigned(unsigned)) => self.start_item(0, unsigned), Value::KeyValue(KeyType::Negative(negative)) => { self.start_item(1, -(negative + 1) as u64) } diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 8e6c7d9c..6873d8a9 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -445,59 +445,83 @@ pub struct PublicKeyCredentialSource { pub user_handle: Vec, // not optional, but nullable pub other_ui: Option, pub cred_random: Option>, + + /// Contains the unknown fields when parsing a CBOR value. + /// + /// Those fields could either be deleted fields from older versions (they should have reserved + /// tags) or fields from newer versions (the tags should not be reserved). If this is empty, + /// then the parsed credential is probably from the same version (but not necessarily). + pub unknown_fields: BTreeMap, +} + +// We simulate protocol buffers in CBOR with maps. Each field of a message is associated with a +// unique tag, implemented with a CBOR unsigned key. +#[repr(u64)] +enum PublicKeyCredentialSourceField { + CredentialId = 0, + PrivateKey = 1, + RpId = 2, + UserHandle = 3, + OtherUi = 4, + CredRandom = 5, + // 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. +} + +impl From for cbor::KeyType { + fn from(field: PublicKeyCredentialSourceField) -> cbor::KeyType { + (field as u64).into() + } } impl From for cbor::Value { fn from(credential: PublicKeyCredentialSource) -> cbor::Value { - let mut private_key = [0u8; 32]; + use PublicKeyCredentialSourceField::*; + let mut private_key = [0; 32]; credential.private_key.to_bytes(&mut private_key); - let other_ui = match credential.other_ui { - None => cbor_null!(), - Some(other_ui) => cbor_text!(other_ui), - }; - let cred_random = match credential.cred_random { - None => cbor_null!(), - Some(cred_random) => cbor_bytes!(cred_random), - }; - cbor_array! { - credential.credential_id, - private_key, - credential.rp_id, - credential.user_handle, - other_ui, - cred_random, + cbor_extend_map_options! { + credential.unknown_fields, + CredentialId => Some(credential.credential_id), + PrivateKey => Some(private_key.to_vec()), + RpId => Some(credential.rp_id), + UserHandle => Some(credential.user_handle), + OtherUi => credential.other_ui, + CredRandom => credential.cred_random } } } -impl TryFrom for PublicKeyCredentialSource { - type Error = Ctap2StatusCode; - - fn try_from(cbor_value: cbor::Value) -> Result { - use cbor::{SimpleValue, Value}; +impl PublicKeyCredentialSource { + pub fn parse_cbor(cbor_value: cbor::Value) -> Option { + use PublicKeyCredentialSourceField::*; + let mut map = match cbor_value { + cbor::Value::Map(x) => x, + _ => return None, + }; - let fields = read_array(&cbor_value)?; - if fields.len() != 6 { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); - } - let credential_id = read_byte_string(&fields[0])?; - let private_key = read_byte_string(&fields[1])?; + let credential_id = read_byte_string(&map.remove(&CredentialId.into())?).ok()?; + let private_key = read_byte_string(&map.remove(&PrivateKey.into())?).ok()?; if private_key.len() != 32 { - return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); + return None; } - let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32)) - .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?; - let rp_id = read_text_string(&fields[2])?; - let user_handle = read_byte_string(&fields[3])?; - let other_ui = match &fields[4] { - Value::Simple(SimpleValue::NullValue) => None, - cbor_value => Some(read_text_string(cbor_value)?), - }; - let cred_random = match &fields[5] { - Value::Simple(SimpleValue::NullValue) => None, - cbor_value => Some(read_byte_string(cbor_value)?), - }; - Ok(PublicKeyCredentialSource { + let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32))?; + let rp_id = read_text_string(&map.remove(&RpId.into())?).ok()?; + let user_handle = read_byte_string(&map.remove(&UserHandle.into())?).ok()?; + let other_ui = map + .remove(&OtherUi.into()) + .as_ref() + .map(read_text_string) + .transpose() + .ok()?; + let cred_random = map + .remove(&CredRandom.into()) + .as_ref() + .map(read_byte_string) + .transpose() + .ok()?; + let unknown_fields = map; + Some(PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, private_key, @@ -505,6 +529,7 @@ impl TryFrom for PublicKeyCredentialSource { user_handle, other_ui, cred_random, + unknown_fields, }) } } @@ -1218,11 +1243,12 @@ mod test { user_handle: b"foo".to_vec(), other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), }; assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), - Ok(credential.clone()) + PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), + Some(credential.clone()) ); let credential = PublicKeyCredentialSource { @@ -1231,8 +1257,8 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), - Ok(credential.clone()) + PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), + Some(credential.clone()) ); let credential = PublicKeyCredentialSource { @@ -1241,15 +1267,15 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), - Ok(credential) + PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), + Some(credential) ); } #[test] fn test_credential_source_invalid_cbor() { - assert!(PublicKeyCredentialSource::try_from(cbor_false!()).is_err()); - assert!(PublicKeyCredentialSource::try_from(cbor_array!(false)).is_err()); - assert!(PublicKeyCredentialSource::try_from(cbor_array!(b"foo".to_vec())).is_err()); + assert!(PublicKeyCredentialSource::parse_cbor(cbor_false!()).is_none()); + assert!(PublicKeyCredentialSource::parse_cbor(cbor_array!(false)).is_none()); + assert!(PublicKeyCredentialSource::parse_cbor(cbor_array!(b"foo".to_vec())).is_none()); } } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index a39a983f..55a1e406 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -335,6 +335,7 @@ where user_handle: vec![], other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), }) } @@ -501,6 +502,7 @@ where .user_display_name .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random, + unknown_fields: BTreeMap::new(), }; self.persistent_store.store_credential(credential_source)?; random_id @@ -1279,6 +1281,7 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), }; assert!(ctap_state .persistent_store @@ -1476,6 +1479,7 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), }; assert!(ctap_state .persistent_store diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index f0a2ca4f..41f4ebee 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -16,9 +16,9 @@ use crate::crypto::rng256::Rng256; use crate::ctap::data_formats::PublicKeyCredentialSource; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::PIN_AUTH_LENGTH; +use alloc::collections::BTreeMap; use alloc::string::String; use alloc::vec::Vec; -use core::convert::TryInto; use ctap2::embedded_flash::{self, StoreConfig, StoreEntry, StoreError, StoreIndex}; #[cfg(any(test, feature = "ram_storage"))] @@ -420,8 +420,7 @@ impl From for Ctap2StatusCode { } fn deserialize_credential(data: &[u8]) -> Option { - let cbor = cbor::read(data).ok()?; - cbor.try_into().ok() + PublicKeyCredentialSource::parse_cbor(cbor::read(data).ok()?) } fn serialize_credential(credential: PublicKeyCredentialSource) -> Result, Ctap2StatusCode> { @@ -454,6 +453,7 @@ mod test { user_handle, other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), } } @@ -623,6 +623,7 @@ mod test { user_handle: vec![0x00], other_ui: None, cred_random: None, + unknown_fields: BTreeMap::new(), }; assert_eq!(found_credential, Some(expected_credential)); } From e6fdcacd329628b2df739f68d5d3ed6e5bc7d683 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 11 May 2020 15:18:27 +0200 Subject: [PATCH 08/27] Remove mention to protobuf --- src/ctap/data_formats.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 6873d8a9..f84720cd 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -454,8 +454,8 @@ pub struct PublicKeyCredentialSource { pub unknown_fields: BTreeMap, } -// We simulate protocol buffers in CBOR with maps. Each field of a message is associated with a -// unique tag, implemented with a CBOR unsigned key. +// We serialize credentials for the persistent storage using CBOR maps. Each field of a credential +// is associated with a unique tag, implemented with a CBOR unsigned key. #[repr(u64)] enum PublicKeyCredentialSourceField { CredentialId = 0, From 491721b421ca07ae3b88f1cd7c7657714376ad14 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 11 May 2020 16:07:59 +0200 Subject: [PATCH 09/27] Rename extend_cbor_map_options --- libraries/cbor/src/macros.rs | 21 ++++++++++----------- src/ctap/data_formats.rs | 8 +++++--- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index e5f3781e..51bfc836 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -41,32 +41,31 @@ macro_rules! cbor_map_options { }; ( $( $key:expr => $value:expr ),* ) => { - cbor_extend_map_options! ( - ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(), - $( $key => $value, )* - ) + { + let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(); + extend_cbor_map_options! ( &mut _map, $( $key => $value, )* ); + $crate::values::Value::Map(_map) + } }; } #[macro_export] -macro_rules! cbor_extend_map_options { +macro_rules! extend_cbor_map_options { // Add trailing comma if missing. - ( $initial:expr, $( $key:expr => $value:expr ),+ ) => { - cbor_extend_map_options! ( $initial, $($key => $value,)+ ) + ( &mut $initial:expr, $( $key:expr => $value:expr ),+ ) => { + extend_cbor_map_options! ( &mut $initial, $($key => $value,)+ ) }; - ( $initial:expr, $( $key:expr => $value:expr, )* ) => { + ( &mut $initial:expr, $( $key:expr => $value:expr, )* ) => { { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValueOption}; - let mut _map = $initial; $( if let Some(val) = $value.into_cbor_value_option() { - _map.insert($key.into_cbor_key(), val); + $initial.insert($key.into_cbor_key(), val); } )* - $crate::values::Value::Map(_map) } }; } diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index f84720cd..e227d87e 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -480,15 +480,17 @@ impl From for cbor::Value { use PublicKeyCredentialSourceField::*; let mut private_key = [0; 32]; credential.private_key.to_bytes(&mut private_key); - cbor_extend_map_options! { - credential.unknown_fields, + let mut result = credential.unknown_fields; + extend_cbor_map_options! { + &mut result, CredentialId => Some(credential.credential_id), PrivateKey => Some(private_key.to_vec()), RpId => Some(credential.rp_id), UserHandle => Some(credential.user_handle), OtherUi => credential.other_ui, CredRandom => credential.cred_random - } + }; + cbor::Value::Map(result) } } From 479de32a9544e15badb23c575dcfbfa083e6266d Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Mon, 11 May 2020 16:40:11 +0200 Subject: [PATCH 10/27] Rename $initial to $map --- libraries/cbor/src/macros.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index 51bfc836..81d303e0 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -52,18 +52,18 @@ macro_rules! cbor_map_options { #[macro_export] macro_rules! extend_cbor_map_options { // Add trailing comma if missing. - ( &mut $initial:expr, $( $key:expr => $value:expr ),+ ) => { - extend_cbor_map_options! ( &mut $initial, $($key => $value,)+ ) + ( &mut $map:expr, $( $key:expr => $value:expr ),+ ) => { + extend_cbor_map_options! ( &mut $map, $($key => $value,)+ ) }; - ( &mut $initial:expr, $( $key:expr => $value:expr, )* ) => { + ( &mut $map:expr, $( $key:expr => $value:expr, )* ) => { { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValueOption}; $( if let Some(val) = $value.into_cbor_value_option() { - $initial.insert($key.into_cbor_key(), val); + $map.insert($key.into_cbor_key(), val); } )* } From 20f65f9dd713c00b3e196895eaad1cd24bc81f2e Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Tue, 14 Apr 2020 16:42:25 +0200 Subject: [PATCH 11/27] Add GitHub workflow to check that binaries are reproducible. --- .github/workflows/reproducible.yml | 39 +++++++++++++++++++ deploy.py | 14 +++---- reproduce_board.sh | 25 ++++++++++++ reproduce_hashes.sh | 36 +++++++++++++++++ .../reference_binaries_macos-10.15.sha256sum | 9 +++++ .../reference_binaries_ubuntu-18.04.sha256sum | 9 +++++ reproducible/sample_crypto_data/opensk.key | 8 ++++ .../sample_crypto_data/opensk_cert.pem | 9 +++++ 8 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/reproducible.yml create mode 100755 reproduce_board.sh create mode 100755 reproduce_hashes.sh create mode 100644 reproducible/reference_binaries_macos-10.15.sha256sum create mode 100644 reproducible/reference_binaries_ubuntu-18.04.sha256sum create mode 100644 reproducible/sample_crypto_data/opensk.key create mode 100644 reproducible/sample_crypto_data/opensk_cert.pem diff --git a/.github/workflows/reproducible.yml b/.github/workflows/reproducible.yml new file mode 100644 index 00000000..5a1e61bd --- /dev/null +++ b/.github/workflows/reproducible.yml @@ -0,0 +1,39 @@ +--- +name: Check that binaries are reproducible +on: + push: + pull_request: + types: [opened, synchronize, reopened] + +jobs: + check_hashes: + 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: Use sample cryptographic material + run: rm -R crypto_data/ && cp -r reproducible/sample_crypto_data crypto_data + - name: Computing cryptographic hashes + run: ./reproduce_hashes.sh + + - name: Upload reproduced binaries + uses: actions/upload-artifact@v1 + with: + name: reproduced-${{ matrix.os }} + path: reproducible/reproduced.tar + + - name: Comparing cryptographic hashes + run: git diff --no-index reproducible/reference_binaries_${{ matrix.os }}.sha256sum reproducible/binaries.sha256sum diff --git a/deploy.py b/deploy.py index 57b7b867..3b7e89ff 100755 --- a/deploy.py +++ b/deploy.py @@ -392,19 +392,17 @@ def create_tab_file(self, binaries): assert self.args.application info("Generating Tock TAB file for application/example {}".format( self.args.application)) - package_parameter = "-n" elf2tab_ver = self.checked_command_output(["elf2tab", "--version"]).split( - " ", maxsplit=1)[1] - # Starting from v0.5.0-dev the parameter changed. - # Current pyblished crate is 0.4.0 but we don't want developers - # running the HEAD from github to be stuck - if "0.5.0-dev" in elf2tab_ver: - package_parameter = "--package-name" + "\n", maxsplit=1)[0] + if elf2tab_ver != "elf2tab 0.5.0": + fatal("Unsupported elf2tab version {!a}. Please use 0.5.0.".format( + elf2tab_ver)) os.makedirs(self.tab_folder, exist_ok=True) tab_filename = os.path.join(self.tab_folder, "{}.tab".format(self.args.application)) elf2tab_args = [ - "elf2tab", package_parameter, self.args.application, "-o", tab_filename + "elf2tab", "--deterministic", "--package-name", self.args.application, + "-o", tab_filename ] if self.args.verbose_build: elf2tab_args.append("--verbose") diff --git a/reproduce_board.sh b/reproduce_board.sh new file mode 100755 index 00000000..19730e8f --- /dev/null +++ b/reproduce_board.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# 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. + +set -ex + +echo "Board: $BOARD" +./deploy.py --verbose-build --board=$BOARD --no-app --programmer=none +./third_party/tock/tools/sha256sum/target/debug/sha256sum third_party/tock/target/thumbv7em-none-eabi/release/$BOARD.bin >> reproducible/binaries.sha256sum +tar -rvf reproducible/reproduced.tar third_party/tock/target/thumbv7em-none-eabi/release/$BOARD.bin + +./deploy.py --verbose-build --board=$BOARD --opensk --programmer=none +./third_party/tock/tools/sha256sum/target/debug/sha256sum target/${BOARD}_merged.hex >> reproducible/binaries.sha256sum +tar -rvf reproducible/reproduced.tar target/${BOARD}_merged.hex diff --git a/reproduce_hashes.sh b/reproduce_hashes.sh new file mode 100755 index 00000000..9480472b --- /dev/null +++ b/reproduce_hashes.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# 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. + +set -ex + +rm -f reproducible/binaries.sha256sum + +echo "Creating reproducible/reproduced.tar" +touch empty_file +tar -cvf reproducible/reproduced.tar empty_file +rm empty_file + +echo "Building sha256sum tool..." +cargo build --manifest-path third_party/tock/tools/sha256sum/Cargo.toml + +echo "Computing SHA-256 sums of the boards..." +for board in nrf52840dk nrf52840_dongle nrf52840_dongle_dfu nrf52840_mdk_dfu +do + BOARD=$board ./reproduce_board.sh +done + +echo "Computing SHA-256 sum of the TAB file..." +./third_party/tock/tools/sha256sum/target/debug/sha256sum target/tab/ctap2.tab >> reproducible/binaries.sha256sum +tar -rvf reproducible/reproduced.tar target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum new file mode 100644 index 00000000..935573c0 --- /dev/null +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -0,0 +1,9 @@ +b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +b16a815411e4dfdd8ceb8588b47861e33f9282b0ffa10660692783b4e2cd6179 target/nrf52840dk_merged.hex +346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +5ba6bdd42d1036df6347119020925404029999bcba51409a56327070bca1ff62 target/nrf52840_dongle_merged.hex +adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +bf6aeafe25197ca500e7c39abf713b85d748722a6a65edefe8a9093d4d1f8100 target/nrf52840_dongle_dfu_merged.hex +97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +45631c7fb72d56446a966a3b58a6ff5513f94a6052641938cd49e9a9498b959f target/nrf52840_mdk_dfu_merged.hex +5ae401ae89f6155820527d5099948f337f8e3fc93da18ccdd150c645b6a53ea9 target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum new file mode 100644 index 00000000..408b7d4b --- /dev/null +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -0,0 +1,9 @@ +921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +450c3775cc16e812519b9a65aaaa21c9cd8cc89881735f2c2c5f540793f54fe1 target/nrf52840dk_merged.hex +aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +bdc557bedaedb39e74d234a243a86fbf15623498a47a8941ec588cfc83fb4f56 target/nrf52840_dongle_merged.hex +26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +4d5dee48187600023bbac889d61ca0f626891590cafff45f73dcbcf4ef9875e5 target/nrf52840_dongle_dfu_merged.hex +7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +34adf76fec8502d86a298607bf74d559710e4e8c644dc19d2e41e2f830cf9203 target/nrf52840_mdk_dfu_merged.hex +89b80eccf75175f9a8c9be2c3adccbe7d2ee9b7cbca896bf0605c3a6b8a09cf6 target/tab/ctap2.tab diff --git a/reproducible/sample_crypto_data/opensk.key b/reproducible/sample_crypto_data/opensk.key new file mode 100644 index 00000000..5b68558d --- /dev/null +++ b/reproducible/sample_crypto_data/opensk.key @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEICMfnFy7L3y5p2MOGezavAeS+noKYtT21mDcWllN7Y1zoAoGCCqGSM49 +AwEHoUQDQgAEhZflF2Fq4xmAofKOxG/0sx8bucdpJPRLR4HXArAFXJzdLF9ofkpn +gzsVWzTYFr+nNiyxySyJsdkH/qQv4rCV0A== +-----END EC PRIVATE KEY----- diff --git a/reproducible/sample_crypto_data/opensk_cert.pem b/reproducible/sample_crypto_data/opensk_cert.pem new file mode 100644 index 00000000..169321ea --- /dev/null +++ b/reproducible/sample_crypto_data/opensk_cert.pem @@ -0,0 +1,9 @@ +-----BEGIN CERTIFICATE----- +MIIBPDCB4wIUTEbgPPL3tr2rLkI83EyzyQJQmYEwCgYIKoZIzj0EAwIwGzEZMBcG +A1UEAwwQR29vZ2xlIE9wZW5TSyBDQTAeFw0yMDA0MTQxNTM5MDRaFw0zMDA0MTQx +NTM5MDRaMCcxJTAjBgNVBAMMHEdvb2dsZSBPcGVuU0sgSGFja2VyIEVkaXRpb24w +WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASFl+UXYWrjGYCh8o7Eb/SzHxu5x2kk +9EtHgdcCsAVcnN0sX2h+SmeDOxVbNNgWv6c2LLHJLImx2Qf+pC/isJXQMAoGCCqG +SM49BAMCA0gAMEUCIBKkHijpTbjlPDv3oFw/nW/ta8jEMhY8iNCBp9N0+NNYAiEA +ywzrGpmc0reEUFCGHBBdvC2E2SxIlvaefz7umT8ajy4= +-----END CERTIFICATE----- From ca6f910c26bf567b95133c9175f07ed8408d29b2 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 13 May 2020 11:09:32 +0200 Subject: [PATCH 12/27] Remove unknown fields --- libraries/cbor/src/macros.rs | 25 +++------- src/ctap/data_formats.rs | 89 ++++++++++++++++-------------------- src/ctap/mod.rs | 4 -- src/ctap/storage.rs | 6 +-- 4 files changed, 49 insertions(+), 75 deletions(-) diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs index 81d303e0..5a3d8f52 100644 --- a/libraries/cbor/src/macros.rs +++ b/libraries/cbor/src/macros.rs @@ -41,31 +41,20 @@ macro_rules! cbor_map_options { }; ( $( $key:expr => $value:expr ),* ) => { - { - let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(); - extend_cbor_map_options! ( &mut _map, $( $key => $value, )* ); - $crate::values::Value::Map(_map) - } - }; -} - -#[macro_export] -macro_rules! extend_cbor_map_options { - // Add trailing comma if missing. - ( &mut $map:expr, $( $key:expr => $value:expr ),+ ) => { - extend_cbor_map_options! ( &mut $map, $($key => $value,)+ ) - }; - - ( &mut $map:expr, $( $key:expr => $value:expr, )* ) => { { // The import is unused if the list is empty. #[allow(unused_imports)] use $crate::values::{IntoCborKey, IntoCborValueOption}; + let mut _map = ::alloc::collections::BTreeMap::<_, $crate::values::Value>::new(); $( - if let Some(val) = $value.into_cbor_value_option() { - $map.insert($key.into_cbor_key(), val); + { + let opt: Option<$crate::values::Value> = $value.into_cbor_value_option(); + if let Some(val) = opt { + _map.insert($key.into_cbor_key(), val); } + } )* + $crate::values::Value::Map(_map) } }; } diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index e227d87e..98a05247 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -433,6 +433,9 @@ impl TryFrom<&cbor::Value> for SignatureAlgorithm { } // https://www.w3.org/TR/webauthn/#public-key-credential-source +// +// Note that we only use the WebAuthn definition as an example. This data-structure is not specified +// by FIDO. In particular we may choose how we serialize and deserialize it. #[derive(Clone)] #[cfg_attr(test, derive(PartialEq))] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] @@ -445,18 +448,10 @@ pub struct PublicKeyCredentialSource { pub user_handle: Vec, // not optional, but nullable pub other_ui: Option, pub cred_random: Option>, - - /// Contains the unknown fields when parsing a CBOR value. - /// - /// Those fields could either be deleted fields from older versions (they should have reserved - /// tags) or fields from newer versions (the tags should not be reserved). If this is empty, - /// then the parsed credential is probably from the same version (but not necessarily). - pub unknown_fields: BTreeMap, } // We serialize credentials for the persistent storage using CBOR maps. Each field of a credential // is associated with a unique tag, implemented with a CBOR unsigned key. -#[repr(u64)] enum PublicKeyCredentialSourceField { CredentialId = 0, PrivateKey = 1, @@ -480,50 +475,48 @@ impl From for cbor::Value { use PublicKeyCredentialSourceField::*; let mut private_key = [0; 32]; credential.private_key.to_bytes(&mut private_key); - let mut result = credential.unknown_fields; - extend_cbor_map_options! { - &mut result, + cbor_map_options! { CredentialId => Some(credential.credential_id), PrivateKey => Some(private_key.to_vec()), RpId => Some(credential.rp_id), UserHandle => Some(credential.user_handle), OtherUi => credential.other_ui, CredRandom => credential.cred_random - }; - cbor::Value::Map(result) + } } } -impl PublicKeyCredentialSource { - pub fn parse_cbor(cbor_value: cbor::Value) -> Option { - use PublicKeyCredentialSourceField::*; - let mut map = match cbor_value { - cbor::Value::Map(x) => x, - _ => return None, - }; +impl TryFrom<&cbor::Value> for PublicKeyCredentialSource { + type Error = Ctap2StatusCode; - let credential_id = read_byte_string(&map.remove(&CredentialId.into())?).ok()?; - let private_key = read_byte_string(&map.remove(&PrivateKey.into())?).ok()?; + fn try_from(cbor_value: &cbor::Value) -> Result { + use PublicKeyCredentialSourceField::*; + let map = read_map(cbor_value)?; + let credential_id = read_byte_string(ok_or_missing(map.get(&CredentialId.into()))?)?; + let private_key = read_byte_string(ok_or_missing(map.get(&PrivateKey.into()))?)?; if private_key.len() != 32 { - return None; + return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); } - let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32))?; - let rp_id = read_text_string(&map.remove(&RpId.into())?).ok()?; - let user_handle = read_byte_string(&map.remove(&UserHandle.into())?).ok()?; - let other_ui = map - .remove(&OtherUi.into()) - .as_ref() - .map(read_text_string) - .transpose() - .ok()?; + let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32)) + .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?; + let rp_id = read_text_string(ok_or_missing(map.get(&RpId.into()))?)?; + let user_handle = read_byte_string(ok_or_missing(map.get(&UserHandle.into()))?)?; + let other_ui = map.get(&OtherUi.into()).map(read_text_string).transpose()?; let cred_random = map - .remove(&CredRandom.into()) - .as_ref() + .get(&CredRandom.into()) .map(read_byte_string) - .transpose() - .ok()?; - let unknown_fields = map; - Some(PublicKeyCredentialSource { + .transpose()?; + // We don't return whether there were unknown fields in the CBOR value. This means that + // deserialization is not injective. In particular deserialization is only an inverse of + // serialization at a given version of OpenSK. This is not a problem because: + // 1. When a field is deprecated, its tag is reserved and never reused in future versions, + // including to be reintroduced with the same semantics. In other words, removing a field + // is permanent. + // 2. OpenSK is never used with a more recent version of the storage. In particular, OpenSK + // is never rolled-back. + // As a consequence, the unknown fields are only reserved fields and don't need to be + // preserved. + Ok(PublicKeyCredentialSource { key_type: PublicKeyCredentialType::PublicKey, credential_id, private_key, @@ -531,7 +524,6 @@ impl PublicKeyCredentialSource { user_handle, other_ui, cred_random, - unknown_fields, }) } } @@ -1245,12 +1237,11 @@ mod test { user_handle: b"foo".to_vec(), other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), }; assert_eq!( - PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), - Some(credential.clone()) + PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + Ok(credential.clone()) ); let credential = PublicKeyCredentialSource { @@ -1259,8 +1250,8 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), - Some(credential.clone()) + PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + Ok(credential.clone()) ); let credential = PublicKeyCredentialSource { @@ -1269,15 +1260,15 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::parse_cbor(cbor::Value::from(credential.clone())), - Some(credential) + PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + Ok(credential) ); } #[test] fn test_credential_source_invalid_cbor() { - assert!(PublicKeyCredentialSource::parse_cbor(cbor_false!()).is_none()); - assert!(PublicKeyCredentialSource::parse_cbor(cbor_array!(false)).is_none()); - assert!(PublicKeyCredentialSource::parse_cbor(cbor_array!(b"foo".to_vec())).is_none()); + assert!(PublicKeyCredentialSource::try_from(&cbor_false!()).is_err()); + assert!(PublicKeyCredentialSource::try_from(&cbor_array!(false)).is_err()); + assert!(PublicKeyCredentialSource::try_from(&cbor_array!(b"foo".to_vec())).is_err()); } } diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 55a1e406..a39a983f 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -335,7 +335,6 @@ where user_handle: vec![], other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), }) } @@ -502,7 +501,6 @@ where .user_display_name .map(|s| truncate_to_char_boundary(&s, 64).to_string()), cred_random, - unknown_fields: BTreeMap::new(), }; self.persistent_store.store_credential(credential_source)?; random_id @@ -1281,7 +1279,6 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), }; assert!(ctap_state .persistent_store @@ -1479,7 +1476,6 @@ mod test { user_handle: vec![], other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), }; assert!(ctap_state .persistent_store diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 41f4ebee..2a1a1829 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -16,9 +16,9 @@ use crate::crypto::rng256::Rng256; use crate::ctap::data_formats::PublicKeyCredentialSource; use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::PIN_AUTH_LENGTH; -use alloc::collections::BTreeMap; use alloc::string::String; use alloc::vec::Vec; +use core::convert::TryFrom; use ctap2::embedded_flash::{self, StoreConfig, StoreEntry, StoreError, StoreIndex}; #[cfg(any(test, feature = "ram_storage"))] @@ -420,7 +420,7 @@ impl From for Ctap2StatusCode { } fn deserialize_credential(data: &[u8]) -> Option { - PublicKeyCredentialSource::parse_cbor(cbor::read(data).ok()?) + PublicKeyCredentialSource::try_from(&cbor::read(data).ok()?).ok() } fn serialize_credential(credential: PublicKeyCredentialSource) -> Result, Ctap2StatusCode> { @@ -453,7 +453,6 @@ mod test { user_handle, other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), } } @@ -623,7 +622,6 @@ mod test { user_handle: vec![0x00], other_ui: None, cred_random: None, - unknown_fields: BTreeMap::new(), }; assert_eq!(found_credential, Some(expected_credential)); } From 39a3becffb86a6765f6e5c2aeb4a84ad62a2226c Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Wed, 13 May 2020 12:35:10 +0200 Subject: [PATCH 13/27] Address review comments. --- deploy.py | 5 +++-- setup.sh | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/deploy.py b/deploy.py index 3b7e89ff..aae8e8fb 100755 --- a/deploy.py +++ b/deploy.py @@ -395,8 +395,9 @@ def create_tab_file(self, binaries): elf2tab_ver = self.checked_command_output(["elf2tab", "--version"]).split( "\n", maxsplit=1)[0] if elf2tab_ver != "elf2tab 0.5.0": - fatal("Unsupported elf2tab version {!a}. Please use 0.5.0.".format( - elf2tab_ver)) + error( + ("Detected unsupported elf2tab version {!a}. The following " + "commands may fail. Please use 0.5.0 instead.").format(elf2tab_ver)) os.makedirs(self.tab_folder, exist_ok=True) tab_filename = os.path.join(self.tab_folder, "{}.tab".format(self.args.application)) diff --git a/setup.sh b/setup.sh index 439003b0..493297f9 100755 --- a/setup.sh +++ b/setup.sh @@ -90,4 +90,4 @@ pip3 install --user --upgrade 'tockloader~=1.4' six intelhex rustup target add thumbv7em-none-eabi # Install dependency to create applications. -cargo install elf2tab +cargo install elf2tab --version 0.5.0 From 0604be61a6da1b14562b43db65bdd9b91d91da83 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Wed, 13 May 2020 13:12:44 +0200 Subject: [PATCH 14/27] Don't fail fast on reproducible workflow and update reference binaries. --- .github/workflows/reproducible.yml | 1 + reproducible/reference_binaries_macos-10.15.sha256sum | 10 +++++----- reproducible/reference_binaries_ubuntu-18.04.sha256sum | 10 +++++----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/reproducible.yml b/.github/workflows/reproducible.yml index 5a1e61bd..44f816c2 100644 --- a/.github/workflows/reproducible.yml +++ b/.github/workflows/reproducible.yml @@ -10,6 +10,7 @@ jobs: strategy: matrix: os: [ubuntu-18.04, macos-10.15] + fail-fast: false runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v2 diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index 935573c0..9d3264a0 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -b16a815411e4dfdd8ceb8588b47861e33f9282b0ffa10660692783b4e2cd6179 target/nrf52840dk_merged.hex +d81264ffbce075a78067b666db1988cd0e44e25c0e3f708ada28e90ff29f7339 target/nrf52840dk_merged.hex 346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -5ba6bdd42d1036df6347119020925404029999bcba51409a56327070bca1ff62 target/nrf52840_dongle_merged.hex +0c50decccd94e93612f3342ab833a73959d96a754aa6845b755a779a2240c484 target/nrf52840_dongle_merged.hex adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -bf6aeafe25197ca500e7c39abf713b85d748722a6a65edefe8a9093d4d1f8100 target/nrf52840_dongle_dfu_merged.hex +bbcbd0e72e40257cea94a2fee829d6b2c365e8473f2fb8b6ae685cda4d925f7d target/nrf52840_dongle_dfu_merged.hex 97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -45631c7fb72d56446a966a3b58a6ff5513f94a6052641938cd49e9a9498b959f target/nrf52840_mdk_dfu_merged.hex -5ae401ae89f6155820527d5099948f337f8e3fc93da18ccdd150c645b6a53ea9 target/tab/ctap2.tab +d42f1ec737bd945ae2ddc74a5c179573b36fa720f9a7149b2af67a0e3667fac7 target/nrf52840_mdk_dfu_merged.hex +166523347144bcf9b936f5c265dfb2d7de9ab28569881276df344a6a42423f1b target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index 408b7d4b..3aca0952 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ 921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -450c3775cc16e812519b9a65aaaa21c9cd8cc89881735f2c2c5f540793f54fe1 target/nrf52840dk_merged.hex +776dab253e0903bffc54a9aeb09323717a4d69e6f1db8a4d88641378a89e47fe target/nrf52840dk_merged.hex aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -bdc557bedaedb39e74d234a243a86fbf15623498a47a8941ec588cfc83fb4f56 target/nrf52840_dongle_merged.hex +945bfb77e7852d5d6425436208517c478af4ef66a675eded14f768de81512ab4 target/nrf52840_dongle_merged.hex 26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -4d5dee48187600023bbac889d61ca0f626891590cafff45f73dcbcf4ef9875e5 target/nrf52840_dongle_dfu_merged.hex +55efa672a5fce6f8757efe16a2c3444d11f8f5a575fc0fedbc2892b587c6ecac target/nrf52840_dongle_dfu_merged.hex 7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -34adf76fec8502d86a298607bf74d559710e4e8c644dc19d2e41e2f830cf9203 target/nrf52840_mdk_dfu_merged.hex -89b80eccf75175f9a8c9be2c3adccbe7d2ee9b7cbca896bf0605c3a6b8a09cf6 target/tab/ctap2.tab +ed2beb9efd3bab6c91f7d0c6e3c1622d4920f5476844bd7551d51ba524ad4a71 target/nrf52840_mdk_dfu_merged.hex +ee12b35c75402db7fe191f2793209c51657c98662be2b6ff41ae09bccdc7e62a target/tab/ctap2.tab From cf31110922789e13dbdb515045137bc45bef6488 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 13 May 2020 15:17:35 +0200 Subject: [PATCH 15/27] Define the storage locations in the board --- patches/tock/01-persistent-storage.patch | 229 +++++++++++++++++------ patches/tock/02-usb.patch | 6 +- src/embedded_flash/syscall.rs | 137 +++++++++----- 3 files changed, 261 insertions(+), 111 deletions(-) diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index 1df57ae1..e7d9eb1f 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -1,5 +1,33 @@ +diff --git a/boards/nordic/nrf52840dk/src/main.rs b/boards/nordic/nrf52840dk/src/main.rs +index 44a6c1cc..2ebc2868 100644 +--- a/boards/nordic/nrf52840dk/src/main.rs ++++ b/boards/nordic/nrf52840dk/src/main.rs +@@ -117,6 +117,11 @@ static mut APP_MEMORY: [u8; 0x3C000] = [0; 0x3C000]; + static mut PROCESSES: [Option<&'static dyn kernel::procs::ProcessType>; NUM_PROCS] = + [None, None, None, None, None, None, None, None]; + ++static mut STORAGE_LOCATIONS: [kernel::StorageLocation; 1] = [kernel::StorageLocation { ++ address: 0xC0000, ++ size: 0x40000, ++}]; ++ + static mut CHIP: Option<&'static nrf52840::chip::Chip> = None; + + /// Dummy buffer that causes the linker to reserve enough space for the stack. +@@ -146,7 +151,10 @@ pub unsafe fn reset_handler() { + UartChannel::Pins(UartPins::new(UART_RTS, UART_TXD, UART_CTS, UART_RXD)) + }; + +- let board_kernel = static_init!(kernel::Kernel, kernel::Kernel::new(&PROCESSES)); ++ let board_kernel = static_init!( ++ kernel::Kernel, ++ kernel::Kernel::new_with_storage(&PROCESSES, &STORAGE_LOCATIONS) ++ ); + let gpio = components::gpio::GpioComponent::new(board_kernel).finalize( + components::gpio_component_helper!( + &nrf52840::gpio::PORT[Pin::P1_01], diff --git a/boards/nordic/nrf52dk_base/src/lib.rs b/boards/nordic/nrf52dk_base/src/lib.rs -index fe493727..105f7120 100644 +index 5dd4328e..b8867461 100644 --- a/boards/nordic/nrf52dk_base/src/lib.rs +++ b/boards/nordic/nrf52dk_base/src/lib.rs @@ -104,6 +104,7 @@ pub struct Platform { @@ -18,7 +46,7 @@ index fe493727..105f7120 100644 kernel::ipc::DRIVER_NUM => f(Some(&self.ipc)), _ => f(None), } -@@ -405,6 +407,14 @@ pub unsafe fn setup_board( +@@ -405,6 +407,15 @@ pub unsafe fn setup_board( ); nrf52::acomp::ACOMP.set_client(analog_comparator); @@ -26,14 +54,15 @@ index fe493727..105f7120 100644 + nrf52::nvmc::SyscallDriver, + nrf52::nvmc::SyscallDriver::new( + &nrf52::nvmc::NVMC, -+ board_kernel.create_grant(&memory_allocation_capability) ++ board_kernel.create_grant(&memory_allocation_capability), ++ board_kernel.storage_locations(), + ) + ); + // Start all of the clocks. Low power operation will require a better // approach than this. nrf52::clock::CLOCK.low_stop(); -@@ -439,6 +449,7 @@ pub unsafe fn setup_board( +@@ -431,6 +442,7 @@ pub unsafe fn setup_board( analog_comparator: analog_comparator, nonvolatile_storage: nonvolatile_storage, ipc: kernel::ipc::IPC::new(board_kernel, &memory_allocation_capability), @@ -42,7 +71,7 @@ index fe493727..105f7120 100644 platform.pconsole.start(); diff --git a/chips/nrf52/src/nvmc.rs b/chips/nrf52/src/nvmc.rs -index 60fc2da8..45c75f89 100644 +index 60fc2da8..6e59d197 100644 --- a/chips/nrf52/src/nvmc.rs +++ b/chips/nrf52/src/nvmc.rs @@ -3,6 +3,7 @@ @@ -58,11 +87,11 @@ index 60fc2da8..45c75f89 100644 use kernel::common::StaticRef; use kernel::hil; -use kernel::ReturnCode; -+use kernel::{AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared}; ++use kernel::{AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared, StorageLocation}; use crate::deferred_call_tasks::DeferredCallTask; -@@ -141,7 +142,33 @@ register_bitfields! [u32, +@@ -141,7 +142,13 @@ register_bitfields! [u32, static DEFERRED_CALL: DeferredCall = unsafe { DeferredCall::new(DeferredCallTask::Nvmc) }; @@ -73,30 +102,10 @@ index 60fc2da8..45c75f89 100644 +const MAX_PAGE_ERASES: usize = 10000; +const WORD_MASK: usize = WORD_SIZE - 1; +const PAGE_MASK: usize = PAGE_SIZE - 1; -+ -+// The storage is currently static and readable by all processes. This will be fixed once Tock -+// supports persistent storage outside the application flash (i.e. not the current writeable flash -+// regions support). -+const STORAGE_PTR: usize = 0xc0000; -+const STORAGE_LEN: usize = 0x40000; -+ -+fn in_storage(ptr: usize, len: usize) -> bool { -+ // We want to check the 2 following inequalities: -+ // (1) `STORAGE_PTR <= ptr` -+ // (2) `ptr + len <= STORAGE_PTR + STORAGE_LEN` -+ // However, the second one may overflow written as is. We introduce a third -+ // inequality to solve this issue: -+ // (3) `len <= STORAGE_LEN` -+ // Using this third inequality, we can rewrite the second one as: -+ // (4) `ptr - STORAGE_PTR <= STORAGE_LEN - len` -+ // This fourth inequality is equivalent to the second one but doesn't overflow when -+ // the first and third inequalities hold. -+ STORAGE_PTR <= ptr && len <= STORAGE_LEN && ptr - STORAGE_PTR <= STORAGE_LEN - len -+} /// This is a wrapper around a u8 array that is sized to a single page for the /// nrf. Users of this module must pass an object of this type to use the -@@ -215,6 +242,11 @@ impl Nvmc { +@@ -215,6 +222,11 @@ impl Nvmc { } } @@ -108,7 +117,7 @@ index 60fc2da8..45c75f89 100644 /// Configure the NVMC to allow writes to flash. pub fn configure_writeable(&self) { let regs = &*self.registers; -@@ -230,7 +262,7 @@ impl Nvmc { +@@ -230,7 +242,7 @@ impl Nvmc { let regs = &*self.registers; regs.config.write(Configuration::WEN::Een); while !self.is_ready() {} @@ -117,7 +126,7 @@ index 60fc2da8..45c75f89 100644 while !self.is_ready() {} } -@@ -322,7 +354,7 @@ impl Nvmc { +@@ -322,7 +334,7 @@ impl Nvmc { // Put the NVMC in write mode. regs.config.write(Configuration::WEN::Wen); @@ -126,7 +135,7 @@ index 60fc2da8..45c75f89 100644 let word: u32 = (data[i + 0] as u32) << 0 | (data[i + 1] as u32) << 8 | (data[i + 2] as u32) << 16 -@@ -390,3 +422,186 @@ impl hil::flash::Flash for Nvmc { +@@ -390,3 +402,217 @@ impl hil::flash::Flash for Nvmc { self.erase_page(page_number) } } @@ -158,8 +167,9 @@ index 60fc2da8..45c75f89 100644 +/// - COMMAND(1, 2): Get the maximum number of word writes between page erasures (always 2). +/// - COMMAND(1, 3): Get the maximum number page erasures in the lifetime of the flash (always +/// 10000). -+/// - COMMAND(1, 4): Get the storage address (page-aligned). -+/// - COMMAND(1, 5): Get the storage length (page-aligned). ++/// - COMMAND(1, 4): The number of storage locations. ++/// - COMMAND(1, 5, i): Get the address of the i-th storage location (page-aligned). ++/// - COMMAND(1, 6, i): Get the size of the i-th storage location (page-aligned). +/// - COMMAND(2, ptr): Write the allow slice to the flash region starting at `ptr`. +/// - `ptr` must be word-aligned. +/// - The allow slice length must be word aligned. @@ -172,6 +182,7 @@ index 60fc2da8..45c75f89 100644 +pub struct SyscallDriver { + nvmc: &'static Nvmc, + apps: Grant, ++ storage_locations: &'static [StorageLocation], +} + +pub const DRIVER_NUM: usize = 0x50003; @@ -183,8 +194,16 @@ index 60fc2da8..45c75f89 100644 +} + +impl SyscallDriver { -+ pub fn new(nvmc: &'static Nvmc, apps: Grant) -> SyscallDriver { -+ SyscallDriver { nvmc, apps } ++ pub fn new( ++ nvmc: &'static Nvmc, ++ apps: Grant, ++ storage_locations: &'static [StorageLocation], ++ ) -> SyscallDriver { ++ SyscallDriver { ++ nvmc, ++ apps, ++ storage_locations, ++ } + } +} + @@ -194,6 +213,24 @@ index 60fc2da8..45c75f89 100644 +} + +impl SyscallDriver { ++ fn in_storage_locations(&self, ptr: usize, len: usize) -> bool { ++ self.storage_locations.iter().any(|storage_location| { ++ let storage_ptr = storage_location.address; ++ let storage_len = storage_location.size; ++ // We want to check the 2 following inequalities: ++ // (1) `storage_ptr <= ptr` ++ // (2) `ptr + len <= storage_ptr + storage_len` ++ // However, the second one may overflow written as is. We introduce a third ++ // inequality to solve this issue: ++ // (3) `len <= storage_len` ++ // Using this third inequality, we can rewrite the second one as: ++ // (4) `ptr - storage_ptr <= storage_len - len` ++ // This fourth inequality is equivalent to the second one but doesn't overflow when ++ // the first and third inequalities hold. ++ storage_ptr <= ptr && len <= storage_len && ptr - storage_ptr <= storage_len - len ++ }) ++ } ++ + /// Writes a word-aligned slice at a word-aligned address. + /// + /// Words are written only if necessary, i.e. if writing the new value would change the current @@ -213,7 +250,7 @@ index 60fc2da8..45c75f89 100644 + /// - `slice.len()` must be word-aligned. + /// - The slice starting at `ptr` of length `slice.len()` must fit in the storage. + fn write_slice(&self, ptr: usize, slice: &[u8]) -> ReturnCode { -+ if !in_storage(ptr, slice.len()) { ++ if !self.in_storage_locations(ptr, slice.len()) { + return ReturnCode::EINVAL; + } + if ptr & WORD_MASK != 0 || slice.len() & WORD_MASK != 0 { @@ -241,7 +278,7 @@ index 60fc2da8..45c75f89 100644 + /// - `ptr` must be page-aligned. + /// - The slice starting at `ptr` of length `PAGE_SIZE` must fit in the storage. + fn erase_page(&self, ptr: usize) -> ReturnCode { -+ if !in_storage(ptr, PAGE_SIZE) { ++ if !self.in_storage_locations(ptr, PAGE_SIZE) { + return ReturnCode::EINVAL; + } + if ptr & PAGE_MASK != 0 { @@ -258,8 +295,8 @@ index 60fc2da8..45c75f89 100644 + ReturnCode::ENOSUPPORT + } + -+ fn command(&self, cmd: usize, arg: usize, _: usize, appid: AppId) -> ReturnCode { -+ match (cmd, arg) { ++ fn command(&self, cmd: usize, arg0: usize, arg1: usize, appid: AppId) -> ReturnCode { ++ match (cmd, arg0) { + (0, _) => ReturnCode::SUCCESS, + + (1, 0) => ReturnCode::SuccessWithValue { value: WORD_SIZE }, @@ -271,10 +308,13 @@ index 60fc2da8..45c75f89 100644 + value: MAX_PAGE_ERASES, + }, + (1, 4) => ReturnCode::SuccessWithValue { -+ value: STORAGE_PTR, ++ value: self.storage_locations.len(), ++ }, ++ (1, 5) if arg1 < self.storage_locations.len() => ReturnCode::SuccessWithValue { ++ value: self.storage_locations[arg1].address, + }, -+ (1, 5) => ReturnCode::SuccessWithValue { -+ value: STORAGE_LEN, ++ (1, 6) if arg1 < self.storage_locations.len() => ReturnCode::SuccessWithValue { ++ value: self.storage_locations[arg1].size, + }, + (1, _) => ReturnCode::EINVAL, + @@ -313,35 +353,48 @@ index 60fc2da8..45c75f89 100644 + } + } +} +diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs +index ebe8052a..a6dcd278 100644 +--- a/kernel/src/lib.rs ++++ b/kernel/src/lib.rs +@@ -47,7 +47,7 @@ pub use crate::platform::systick::SysTick; + pub use crate::platform::{mpu, Chip, Platform}; + pub use crate::platform::{ClockInterface, NoClockControl, NO_CLOCK_CONTROL}; + pub use crate::returncode::ReturnCode; +-pub use crate::sched::Kernel; ++pub use crate::sched::{Kernel, StorageLocation}; + + // Export only select items from the process module. To remove the name conflict + // this cannot be called `process`, so we use a shortened version. These diff --git a/kernel/src/process.rs b/kernel/src/process.rs -index eb00f274..663c2422 100644 +index eb00f274..268a270d 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -1604,6 +1604,33 @@ impl Process<'a, C> { return Ok((None, 0)); } -+ // Allocate MPU region for storage. The storage is currently static and readable by all -+ // processes. This will be fixed once Tock supports persistent storage outside the -+ // application flash (i.e. not the current writeable flash regions support). -+ const STORAGE_PTR: usize = 0xc0000; -+ const STORAGE_LEN: usize = 0x40000; -+ if chip -+ .mpu() -+ .allocate_region( -+ STORAGE_PTR as *const u8, -+ STORAGE_LEN, -+ STORAGE_LEN, -+ mpu::Permissions::ReadOnly, -+ &mut mpu_config, -+ ) -+ .is_none() -+ { ++ // Allocate MPU region for the storage locations. The storage locations are currently ++ // readable by all processes due to lack of stable app id. ++ for storage_location in kernel.storage_locations() { ++ if chip ++ .mpu() ++ .allocate_region( ++ storage_location.address as *const u8, ++ storage_location.size, ++ storage_location.size, ++ mpu::Permissions::ReadOnly, ++ &mut mpu_config, ++ ) ++ .is_some() ++ { ++ continue; ++ } + if config::CONFIG.debug_load_processes { + debug!( + "[!] flash=[{:#010X}:{:#010X}] process={:?} - couldn't allocate flash region", -+ STORAGE_PTR, -+ STORAGE_PTR + STORAGE_LEN, ++ storage_location.address, ++ storage_location.address + storage_location.size, + process_name + ); + } @@ -351,3 +404,57 @@ index eb00f274..663c2422 100644 // Determine how much space we need in the application's // memory space just for kernel and grant state. We need to make // sure we allocate enough memory just for that. +diff --git a/kernel/src/sched.rs b/kernel/src/sched.rs +index fbd68319..43cce76f 100644 +--- a/kernel/src/sched.rs ++++ b/kernel/src/sched.rs +@@ -24,6 +24,12 @@ const KERNEL_TICK_DURATION_US: u32 = 10000; + /// Skip re-scheduling a process if its quanta is nearly exhausted + const MIN_QUANTA_THRESHOLD_US: u32 = 500; + ++/// Represents a storage location in flash. ++pub struct StorageLocation { ++ pub address: usize, ++ pub size: usize, ++} ++ + /// Main object for the kernel. Each board will need to create one. + pub struct Kernel { + /// How many "to-do" items exist at any given time. These include +@@ -33,6 +39,9 @@ pub struct Kernel { + /// This holds a pointer to the static array of Process pointers. + processes: &'static [Option<&'static dyn process::ProcessType>], + ++ /// List of storage locations. ++ storage_locations: &'static [StorageLocation], ++ + /// A counter which keeps track of how many process identifiers have been + /// created. This is used to create new unique identifiers for processes. + process_identifier_max: Cell, +@@ -51,9 +60,17 @@ pub struct Kernel { + + impl Kernel { + pub fn new(processes: &'static [Option<&'static dyn process::ProcessType>]) -> Kernel { ++ Kernel::new_with_storage(processes, &[]) ++ } ++ ++ pub fn new_with_storage( ++ processes: &'static [Option<&'static dyn process::ProcessType>], ++ storage_locations: &'static [StorageLocation], ++ ) -> Kernel { + Kernel { + work: Cell::new(0), + processes: processes, ++ storage_locations: storage_locations, + process_identifier_max: Cell::new(0), + grant_counter: Cell::new(0), + grants_finalized: Cell::new(false), +@@ -599,4 +616,8 @@ impl Kernel { + } + systick.reset(); + } ++ ++ pub fn storage_locations(&self) -> &'static [StorageLocation] { ++ self.storage_locations ++ } + } diff --git a/patches/tock/02-usb.patch b/patches/tock/02-usb.patch index 55f807bb..076b3d99 100644 --- a/patches/tock/02-usb.patch +++ b/patches/tock/02-usb.patch @@ -14,7 +14,7 @@ diff --git a/boards/nordic/nrf52840dk/src/main.rs b/boards/nordic/nrf52840dk/src index 127c4f2f..a5847805 100644 --- a/boards/nordic/nrf52840dk/src/main.rs +++ b/boards/nordic/nrf52840dk/src/main.rs -@@ -236,6 +236,7 @@ pub unsafe fn reset_handler() { +@@ -244,6 +244,7 @@ pub unsafe fn reset_handler() { FAULT_RESPONSE, nrf52840::uicr::Regulator0Output::DEFAULT, false, @@ -70,7 +70,7 @@ index 105f7120..535e5cd8 100644 chip: &'static nrf52::chip::NRF52, ) { // Make non-volatile memory writable and activate the reset button -@@ -415,6 +426,44 @@ pub unsafe fn setup_board( +@@ -416,6 +427,44 @@ pub unsafe fn setup_board( ) ); @@ -115,7 +115,7 @@ index 105f7120..535e5cd8 100644 // Start all of the clocks. Low power operation will require a better // approach than this. nrf52::clock::CLOCK.low_stop(); -@@ -447,6 +496,7 @@ pub unsafe fn setup_board( +@@ -440,6 +489,7 @@ pub unsafe fn setup_board( temp: temp, alarm: alarm, analog_comparator: analog_comparator, diff --git a/src/embedded_flash/syscall.rs b/src/embedded_flash/syscall.rs index 09a7cddf..64f3aa5d 100644 --- a/src/embedded_flash/syscall.rs +++ b/src/embedded_flash/syscall.rs @@ -13,6 +13,7 @@ // limitations under the License. use super::{Index, Storage, StorageError, StorageResult}; +use alloc::vec::Vec; use libtock::syscalls; const DRIVER_NUMBER: usize = 0x50003; @@ -24,8 +25,9 @@ mod command_nr { pub const PAGE_SIZE: usize = 1; pub const MAX_WORD_WRITES: usize = 2; pub const MAX_PAGE_ERASES: usize = 3; - pub const STORAGE_PTR: usize = 4; - pub const STORAGE_LEN: usize = 5; + pub const STORAGE_CNT: usize = 4; + pub const STORAGE_PTR: usize = 5; + pub const STORAGE_LEN: usize = 6; } pub const WRITE_SLICE: usize = 2; pub const ERASE_PAGE: usize = 3; @@ -35,8 +37,8 @@ mod allow_nr { pub const WRITE_SLICE: usize = 0; } -fn get_info(nr: usize) -> StorageResult { - let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::GET_INFO, nr, 0) }; +fn get_info(nr: usize, arg: usize) -> StorageResult { + let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::GET_INFO, nr, arg) }; if code < 0 { Err(StorageError::KernelError { code }) } else { @@ -47,9 +49,10 @@ fn get_info(nr: usize) -> StorageResult { pub struct SyscallStorage { word_size: usize, page_size: usize, + num_pages: usize, max_word_writes: usize, max_page_erases: usize, - storage: &'static mut [u8], + storage_locations: Vec<&'static [u8]>, } impl SyscallStorage { @@ -64,31 +67,36 @@ impl SyscallStorage { /// - The storage is page-aligned. /// /// Returns `OutOfBounds` the number of pages does not fit in the storage. - pub fn new(num_pages: usize) -> StorageResult { - let word_size = get_info(command_nr::get_info_nr::WORD_SIZE)?; - let page_size = get_info(command_nr::get_info_nr::PAGE_SIZE)?; - let max_word_writes = get_info(command_nr::get_info_nr::MAX_WORD_WRITES)?; - let max_page_erases = get_info(command_nr::get_info_nr::MAX_PAGE_ERASES)?; - let storage_ptr = get_info(command_nr::get_info_nr::STORAGE_PTR)?; - let max_storage_len = get_info(command_nr::get_info_nr::STORAGE_LEN)?; - if !word_size.is_power_of_two() || !page_size.is_power_of_two() { + pub fn new(mut num_pages: usize) -> StorageResult { + let mut syscall = SyscallStorage { + word_size: get_info(command_nr::get_info_nr::WORD_SIZE, 0)?, + page_size: get_info(command_nr::get_info_nr::PAGE_SIZE, 0)?, + num_pages, + max_word_writes: get_info(command_nr::get_info_nr::MAX_WORD_WRITES, 0)?, + max_page_erases: get_info(command_nr::get_info_nr::MAX_PAGE_ERASES, 0)?, + storage_locations: Vec::new(), + }; + if !syscall.word_size.is_power_of_two() + || !syscall.page_size.is_power_of_two() + || !syscall.is_word_aligned(syscall.page_size) + { return Err(StorageError::BadFlash); } - let storage_len = num_pages * page_size; - if storage_len > max_storage_len { - return Err(StorageError::OutOfBounds); + for i in 0..get_info(command_nr::get_info_nr::STORAGE_CNT, 0)? { + let storage_ptr = get_info(command_nr::get_info_nr::STORAGE_PTR, i)?; + let max_storage_len = get_info(command_nr::get_info_nr::STORAGE_LEN, i)?; + if !syscall.is_page_aligned(storage_ptr) || !syscall.is_page_aligned(max_storage_len) { + return Err(StorageError::BadFlash); + } + let storage_len = core::cmp::min(num_pages * syscall.page_size, max_storage_len); + num_pages -= storage_len / syscall.page_size; + syscall + .storage_locations + .push(unsafe { core::slice::from_raw_parts(storage_ptr as *mut u8, storage_len) }); } - let storage = - unsafe { core::slice::from_raw_parts_mut(storage_ptr as *mut u8, storage_len) }; - let syscall = SyscallStorage { - word_size, - page_size, - max_word_writes, - max_page_erases, - storage, - }; - if !syscall.is_word_aligned(page_size) || !syscall.is_page_aligned(storage_ptr) { - return Err(StorageError::BadFlash); + if num_pages > 0 { + // The storage locations don't have enough pages. + return Err(StorageError::OutOfBounds); } Ok(syscall) } @@ -112,7 +120,7 @@ impl Storage for SyscallStorage { } fn num_pages(&self) -> usize { - self.storage.len() / self.page_size + self.num_pages } fn max_word_writes(&self) -> usize { @@ -124,14 +132,15 @@ impl Storage for SyscallStorage { } fn read_slice(&self, index: Index, length: usize) -> StorageResult<&[u8]> { - Ok(&self.storage[index.range(length, self)?]) + let start = index.range(length, self)?.start; + find_slice(&self.storage_locations, start, length) } 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); } - let range = index.range(value.len(), self)?; + let ptr = self.read_slice(index, value.len())?.as_ptr() as usize; let code = unsafe { syscalls::allow_ptr( DRIVER_NUMBER, @@ -145,14 +154,7 @@ impl Storage for SyscallStorage { if code < 0 { return Err(StorageError::KernelError { code }); } - let code = unsafe { - syscalls::command( - DRIVER_NUMBER, - command_nr::WRITE_SLICE, - self.storage[range].as_ptr() as usize, - 0, - ) - }; + let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::WRITE_SLICE, ptr, 0) }; if code < 0 { return Err(StorageError::KernelError { code }); } @@ -160,18 +162,59 @@ impl Storage for SyscallStorage { } fn erase_page(&mut self, page: usize) -> StorageResult<()> { - let range = Index { page, byte: 0 }.range(self.page_size(), self)?; - let code = unsafe { - syscalls::command( - DRIVER_NUMBER, - command_nr::ERASE_PAGE, - self.storage[range].as_ptr() as usize, - 0, - ) - }; + let index = Index { page, byte: 0 }; + let length = self.page_size(); + let ptr = self.read_slice(index, length)?.as_ptr() as usize; + let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::ERASE_PAGE, ptr, 0) }; if code < 0 { return Err(StorageError::KernelError { code }); } Ok(()) } } + +fn find_slice<'a>( + slices: &'a [&'a [u8]], + mut start: usize, + length: usize, +) -> StorageResult<&'a [u8]> { + for slice in slices { + if start >= slice.len() { + start -= slice.len(); + continue; + } + if start + length > slice.len() { + break; + } + return Ok(&slice[start..][..length]); + } + Err(StorageError::OutOfBounds) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn find_slice_ok() { + assert_eq!( + find_slice(&[&[1, 2, 3, 4]], 0, 4).ok(), + Some(&[1u8, 2, 3, 4] as &[u8]) + ); + assert_eq!( + find_slice(&[&[1, 2, 3, 4], &[5, 6]], 1, 2).ok(), + Some(&[2u8, 3] as &[u8]) + ); + assert_eq!( + find_slice(&[&[1, 2, 3, 4], &[5, 6]], 4, 2).ok(), + Some(&[5u8, 6] as &[u8]) + ); + assert_eq!( + find_slice(&[&[1, 2, 3, 4], &[5, 6]], 4, 0).ok(), + Some(&[] as &[u8]) + ); + assert!(find_slice(&[], 0, 1).is_err()); + assert!(find_slice(&[&[1, 2, 3, 4], &[5, 6]], 6, 0).is_err()); + assert!(find_slice(&[&[1, 2, 3, 4], &[5, 6]], 3, 2).is_err()); + } +} From ba5caf06917f5fa52852d8797dc0f666a0fe4689 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 14 May 2020 20:19:21 +0200 Subject: [PATCH 16/27] Filter syscall at platform-level instead of driver-level --- patches/tock/01-persistent-storage.patch | 200 +++++++++++++++-------- patches/tock/02-usb.patch | 6 +- src/embedded_flash/syscall.rs | 29 +++- 3 files changed, 156 insertions(+), 79 deletions(-) diff --git a/patches/tock/01-persistent-storage.patch b/patches/tock/01-persistent-storage.patch index e7d9eb1f..2584a2a1 100644 --- a/patches/tock/01-persistent-storage.patch +++ b/patches/tock/01-persistent-storage.patch @@ -27,7 +27,7 @@ index 44a6c1cc..2ebc2868 100644 components::gpio_component_helper!( &nrf52840::gpio::PORT[Pin::P1_01], diff --git a/boards/nordic/nrf52dk_base/src/lib.rs b/boards/nordic/nrf52dk_base/src/lib.rs -index 5dd4328e..b8867461 100644 +index 5dd4328e..a117d35f 100644 --- a/boards/nordic/nrf52dk_base/src/lib.rs +++ b/boards/nordic/nrf52dk_base/src/lib.rs @@ -104,6 +104,7 @@ pub struct Platform { @@ -38,7 +38,7 @@ index 5dd4328e..b8867461 100644 } impl kernel::Platform for Platform { -@@ -128,6 +129,7 @@ impl kernel::Platform for Platform { +@@ -128,10 +129,30 @@ impl kernel::Platform for Platform { capsules::nonvolatile_storage_driver::DRIVER_NUM => { f(self.nonvolatile_storage.map_or(None, |nv| Some(nv))) } @@ -46,7 +46,30 @@ index 5dd4328e..b8867461 100644 kernel::ipc::DRIVER_NUM => f(Some(&self.ipc)), _ => f(None), } -@@ -405,6 +407,15 @@ pub unsafe fn setup_board( + } ++ ++ fn filter_syscall( ++ &self, ++ process: &dyn kernel::procs::ProcessType, ++ syscall: &kernel::syscall::Syscall, ++ ) -> Result<(), kernel::ReturnCode> { ++ use kernel::syscall::Syscall; ++ match *syscall { ++ Syscall::COMMAND { ++ driver_number: nrf52::nvmc::DRIVER_NUM, ++ subdriver_number: cmd, ++ arg0: ptr, ++ arg1: len, ++ } if (cmd == 2 || cmd == 3) && !process.fits_in_storage_location(ptr, len) => { ++ Err(kernel::ReturnCode::EINVAL) ++ } ++ _ => Ok(()), ++ } ++ } + } + + /// Generic function for starting an nrf52dk board. +@@ -405,6 +426,14 @@ pub unsafe fn setup_board( ); nrf52::acomp::ACOMP.set_client(analog_comparator); @@ -55,14 +78,13 @@ index 5dd4328e..b8867461 100644 + nrf52::nvmc::SyscallDriver::new( + &nrf52::nvmc::NVMC, + board_kernel.create_grant(&memory_allocation_capability), -+ board_kernel.storage_locations(), + ) + ); + // Start all of the clocks. Low power operation will require a better // approach than this. nrf52::clock::CLOCK.low_stop(); -@@ -431,6 +442,7 @@ pub unsafe fn setup_board( +@@ -431,6 +460,7 @@ pub unsafe fn setup_board( analog_comparator: analog_comparator, nonvolatile_storage: nonvolatile_storage, ipc: kernel::ipc::IPC::new(board_kernel, &memory_allocation_capability), @@ -71,7 +93,7 @@ index 5dd4328e..b8867461 100644 platform.pconsole.start(); diff --git a/chips/nrf52/src/nvmc.rs b/chips/nrf52/src/nvmc.rs -index 60fc2da8..6e59d197 100644 +index 60fc2da8..ca41b899 100644 --- a/chips/nrf52/src/nvmc.rs +++ b/chips/nrf52/src/nvmc.rs @@ -3,6 +3,7 @@ @@ -87,7 +109,7 @@ index 60fc2da8..6e59d197 100644 use kernel::common::StaticRef; use kernel::hil; -use kernel::ReturnCode; -+use kernel::{AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared, StorageLocation}; ++use kernel::{AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared}; use crate::deferred_call_tasks::DeferredCallTask; @@ -135,7 +157,7 @@ index 60fc2da8..6e59d197 100644 let word: u32 = (data[i + 0] as u32) << 0 | (data[i + 1] as u32) << 8 | (data[i + 2] as u32) << 16 -@@ -390,3 +402,217 @@ impl hil::flash::Flash for Nvmc { +@@ -390,3 +402,180 @@ impl hil::flash::Flash for Nvmc { self.erase_page(page_number) } } @@ -167,22 +189,18 @@ index 60fc2da8..6e59d197 100644 +/// - COMMAND(1, 2): Get the maximum number of word writes between page erasures (always 2). +/// - COMMAND(1, 3): Get the maximum number page erasures in the lifetime of the flash (always +/// 10000). -+/// - COMMAND(1, 4): The number of storage locations. -+/// - COMMAND(1, 5, i): Get the address of the i-th storage location (page-aligned). -+/// - COMMAND(1, 6, i): Get the size of the i-th storage location (page-aligned). -+/// - COMMAND(2, ptr): Write the allow slice to the flash region starting at `ptr`. ++/// - COMMAND(2, ptr, len): Write the allow slice to the flash region starting at `ptr`. +/// - `ptr` must be word-aligned. +/// - The allow slice length must be word aligned. +/// - The region starting at `ptr` of the same length as the allow slice must be in a writeable +/// flash region. -+/// - COMMAND(3, ptr): Erase a page. ++/// - COMMAND(3, ptr, len): Erase a page. +/// - `ptr` must be page-aligned. +/// - The page starting at `ptr` must be in a writeable flash region. +/// - ALLOW(0): The allow slice for COMMAND(2). +pub struct SyscallDriver { + nvmc: &'static Nvmc, + apps: Grant, -+ storage_locations: &'static [StorageLocation], +} + +pub const DRIVER_NUM: usize = 0x50003; @@ -194,16 +212,8 @@ index 60fc2da8..6e59d197 100644 +} + +impl SyscallDriver { -+ pub fn new( -+ nvmc: &'static Nvmc, -+ apps: Grant, -+ storage_locations: &'static [StorageLocation], -+ ) -> SyscallDriver { -+ SyscallDriver { -+ nvmc, -+ apps, -+ storage_locations, -+ } ++ pub fn new(nvmc: &'static Nvmc, apps: Grant) -> SyscallDriver { ++ SyscallDriver { nvmc, apps } + } +} + @@ -213,24 +223,6 @@ index 60fc2da8..6e59d197 100644 +} + +impl SyscallDriver { -+ fn in_storage_locations(&self, ptr: usize, len: usize) -> bool { -+ self.storage_locations.iter().any(|storage_location| { -+ let storage_ptr = storage_location.address; -+ let storage_len = storage_location.size; -+ // We want to check the 2 following inequalities: -+ // (1) `storage_ptr <= ptr` -+ // (2) `ptr + len <= storage_ptr + storage_len` -+ // However, the second one may overflow written as is. We introduce a third -+ // inequality to solve this issue: -+ // (3) `len <= storage_len` -+ // Using this third inequality, we can rewrite the second one as: -+ // (4) `ptr - storage_ptr <= storage_len - len` -+ // This fourth inequality is equivalent to the second one but doesn't overflow when -+ // the first and third inequalities hold. -+ storage_ptr <= ptr && len <= storage_len && ptr - storage_ptr <= storage_len - len -+ }) -+ } -+ + /// Writes a word-aligned slice at a word-aligned address. + /// + /// Words are written only if necessary, i.e. if writing the new value would change the current @@ -250,9 +242,6 @@ index 60fc2da8..6e59d197 100644 + /// - `slice.len()` must be word-aligned. + /// - The slice starting at `ptr` of length `slice.len()` must fit in the storage. + fn write_slice(&self, ptr: usize, slice: &[u8]) -> ReturnCode { -+ if !self.in_storage_locations(ptr, slice.len()) { -+ return ReturnCode::EINVAL; -+ } + if ptr & WORD_MASK != 0 || slice.len() & WORD_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -278,9 +267,6 @@ index 60fc2da8..6e59d197 100644 + /// - `ptr` must be page-aligned. + /// - The slice starting at `ptr` of length `PAGE_SIZE` must fit in the storage. + fn erase_page(&self, ptr: usize) -> ReturnCode { -+ if !self.in_storage_locations(ptr, PAGE_SIZE) { -+ return ReturnCode::EINVAL; -+ } + if ptr & PAGE_MASK != 0 { + return ReturnCode::EINVAL; + } @@ -296,40 +282,39 @@ index 60fc2da8..6e59d197 100644 + } + + fn command(&self, cmd: usize, arg0: usize, arg1: usize, appid: AppId) -> ReturnCode { -+ match (cmd, arg0) { -+ (0, _) => ReturnCode::SUCCESS, ++ match (cmd, arg0, arg1) { ++ (0, _, _) => ReturnCode::SUCCESS, + -+ (1, 0) => ReturnCode::SuccessWithValue { value: WORD_SIZE }, -+ (1, 1) => ReturnCode::SuccessWithValue { value: PAGE_SIZE }, -+ (1, 2) => ReturnCode::SuccessWithValue { ++ (1, 0, _) => ReturnCode::SuccessWithValue { value: WORD_SIZE }, ++ (1, 1, _) => ReturnCode::SuccessWithValue { value: PAGE_SIZE }, ++ (1, 2, _) => ReturnCode::SuccessWithValue { + value: MAX_WORD_WRITES, + }, -+ (1, 3) => ReturnCode::SuccessWithValue { ++ (1, 3, _) => ReturnCode::SuccessWithValue { + value: MAX_PAGE_ERASES, + }, -+ (1, 4) => ReturnCode::SuccessWithValue { -+ value: self.storage_locations.len(), -+ }, -+ (1, 5) if arg1 < self.storage_locations.len() => ReturnCode::SuccessWithValue { -+ value: self.storage_locations[arg1].address, -+ }, -+ (1, 6) if arg1 < self.storage_locations.len() => ReturnCode::SuccessWithValue { -+ value: self.storage_locations[arg1].size, -+ }, -+ (1, _) => ReturnCode::EINVAL, ++ (1, _, _) => ReturnCode::EINVAL, + -+ (2, ptr) => self ++ (2, ptr, len) => self + .apps + .enter(appid, |app, _| { + let slice = match app.slice.take() { + None => return ReturnCode::EINVAL, + Some(slice) => slice, + }; ++ if len != slice.len() { ++ return ReturnCode::EINVAL; ++ } + self.write_slice(ptr, slice.as_ref()) + }) + .unwrap_or_else(|err| err.into()), + -+ (3, ptr) => self.erase_page(ptr), ++ (3, ptr, len) => { ++ if len != PAGE_SIZE { ++ return ReturnCode::EINVAL; ++ } ++ self.erase_page(ptr) ++ } + + _ => ReturnCode::ENOSUPPORT, + } @@ -366,11 +351,90 @@ index ebe8052a..a6dcd278 100644 // Export only select items from the process module. To remove the name conflict // this cannot be called `process`, so we use a shortened version. These +diff --git a/kernel/src/memop.rs b/kernel/src/memop.rs +index 7537d2b4..61870ccd 100644 +--- a/kernel/src/memop.rs ++++ b/kernel/src/memop.rs +@@ -108,6 +108,25 @@ crate fn memop(process: &dyn ProcessType, op_type: usize, r1: usize) -> ReturnCo + ReturnCode::SUCCESS + } + ++ // Op Type 12: Number of storage locations. ++ 12 => ReturnCode::SuccessWithValue { value: process.number_storage_locations() }, ++ ++ // Op Type 13: The start address of the storage location indexed by r1. ++ 13 => { ++ match process.get_storage_location(r1) { ++ None => ReturnCode::FAIL, ++ Some(x) => ReturnCode::SuccessWithValue { value: x.address } ++ } ++ } ++ ++ // Op Type 14: The size of the storage location indexed by r1. ++ 14 => { ++ match process.get_storage_location(r1) { ++ None => ReturnCode::FAIL, ++ Some(x) => ReturnCode::SuccessWithValue { value: x.size } ++ } ++ } ++ + _ => ReturnCode::ENOSUPPORT, + } + } diff --git a/kernel/src/process.rs b/kernel/src/process.rs -index eb00f274..268a270d 100644 +index eb00f274..41243c8e 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs -@@ -1604,6 +1604,33 @@ impl Process<'a, C> { +@@ -281,6 +281,15 @@ pub trait ProcessType { + /// writeable flash region. + fn get_writeable_flash_region(&self, region_index: usize) -> (u32, u32); + ++ /// How many storage locations are defined for this process. ++ fn number_storage_locations(&self) -> usize; ++ ++ /// Get the i-th storage location. ++ fn get_storage_location(&self, index: usize) -> Option<&crate::StorageLocation>; ++ ++ /// Whether a slice fits in a storage location. ++ fn fits_in_storage_location(&self, ptr: usize, len: usize) -> bool; ++ + /// Debug function to update the kernel on where the stack starts for this + /// process. Processes are not required to call this through the memop + /// system call, but it aids in debugging the process. +@@ -999,6 +1008,32 @@ impl ProcessType for Process<'a, C> { + self.header.get_writeable_flash_region(region_index) + } + ++ fn number_storage_locations(&self) -> usize { ++ self.kernel.storage_locations().len() ++ } ++ ++ fn get_storage_location(&self, index: usize) -> Option<&crate::StorageLocation> { ++ self.kernel.storage_locations().get(index) ++ } ++ ++ fn fits_in_storage_location(&self, ptr: usize, len: usize) -> bool { ++ self.kernel.storage_locations().iter().any(|storage_location| { ++ let storage_ptr = storage_location.address; ++ let storage_len = storage_location.size; ++ // We want to check the 2 following inequalities: ++ // (1) `storage_ptr <= ptr` ++ // (2) `ptr + len <= storage_ptr + storage_len` ++ // However, the second one may overflow written as is. We introduce a third ++ // inequality to solve this issue: ++ // (3) `len <= storage_len` ++ // Using this third inequality, we can rewrite the second one as: ++ // (4) `ptr - storage_ptr <= storage_len - len` ++ // This fourth inequality is equivalent to the second one but doesn't overflow when ++ // the first and third inequalities hold. ++ storage_ptr <= ptr && len <= storage_len && ptr - storage_ptr <= storage_len - len ++ }) ++ } ++ + fn update_stack_start_pointer(&self, stack_pointer: *const u8) { + if stack_pointer >= self.mem_start() && stack_pointer < self.mem_end() { + self.debug.map(|debug| { +@@ -1604,6 +1639,33 @@ impl Process<'a, C> { return Ok((None, 0)); } diff --git a/patches/tock/02-usb.patch b/patches/tock/02-usb.patch index 076b3d99..d062c282 100644 --- a/patches/tock/02-usb.patch +++ b/patches/tock/02-usb.patch @@ -62,7 +62,7 @@ index 105f7120..535e5cd8 100644 kernel::ipc::DRIVER_NUM => f(Some(&self.ipc)), _ => f(None), } -@@ -157,6 +167,7 @@ pub unsafe fn setup_board( +@@ -176,6 +186,7 @@ pub unsafe fn setup_board( app_fault_response: kernel::procs::FaultResponse, reg_vout: Regulator0Output, nfc_as_gpios: bool, @@ -70,7 +70,7 @@ index 105f7120..535e5cd8 100644 chip: &'static nrf52::chip::NRF52, ) { // Make non-volatile memory writable and activate the reset button -@@ -416,6 +427,44 @@ pub unsafe fn setup_board( +@@ -434,6 +445,44 @@ pub unsafe fn setup_board( ) ); @@ -115,7 +115,7 @@ index 105f7120..535e5cd8 100644 // Start all of the clocks. Low power operation will require a better // approach than this. nrf52::clock::CLOCK.low_stop(); -@@ -440,6 +489,7 @@ pub unsafe fn setup_board( +@@ -458,6 +507,7 @@ pub unsafe fn setup_board( temp: temp, alarm: alarm, analog_comparator: analog_comparator, diff --git a/src/embedded_flash/syscall.rs b/src/embedded_flash/syscall.rs index 64f3aa5d..fd5e7387 100644 --- a/src/embedded_flash/syscall.rs +++ b/src/embedded_flash/syscall.rs @@ -25,9 +25,6 @@ mod command_nr { pub const PAGE_SIZE: usize = 1; pub const MAX_WORD_WRITES: usize = 2; pub const MAX_PAGE_ERASES: usize = 3; - pub const STORAGE_CNT: usize = 4; - pub const STORAGE_PTR: usize = 5; - pub const STORAGE_LEN: usize = 6; } pub const WRITE_SLICE: usize = 2; pub const ERASE_PAGE: usize = 3; @@ -37,6 +34,12 @@ mod allow_nr { pub const WRITE_SLICE: usize = 0; } +mod memop_nr { + pub const STORAGE_CNT: u32 = 12; + pub const STORAGE_PTR: u32 = 13; + pub const STORAGE_LEN: u32 = 14; +} + fn get_info(nr: usize, arg: usize) -> StorageResult { let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::GET_INFO, nr, arg) }; if code < 0 { @@ -46,6 +49,15 @@ fn get_info(nr: usize, arg: usize) -> StorageResult { } } +fn memop(nr: u32, arg: usize) -> StorageResult { + let code = unsafe { syscalls::memop(nr, arg) }; + if code < 0 { + Err(StorageError::KernelError { code }) + } else { + Ok(code as usize) + } +} + pub struct SyscallStorage { word_size: usize, page_size: usize, @@ -82,9 +94,9 @@ impl SyscallStorage { { return Err(StorageError::BadFlash); } - for i in 0..get_info(command_nr::get_info_nr::STORAGE_CNT, 0)? { - let storage_ptr = get_info(command_nr::get_info_nr::STORAGE_PTR, i)?; - let max_storage_len = get_info(command_nr::get_info_nr::STORAGE_LEN, i)?; + 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); } @@ -154,7 +166,8 @@ impl Storage for SyscallStorage { if code < 0 { return Err(StorageError::KernelError { code }); } - let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::WRITE_SLICE, ptr, 0) }; + let code = + unsafe { syscalls::command(DRIVER_NUMBER, command_nr::WRITE_SLICE, ptr, value.len()) }; if code < 0 { return Err(StorageError::KernelError { code }); } @@ -165,7 +178,7 @@ impl Storage for SyscallStorage { let index = Index { page, byte: 0 }; let length = self.page_size(); let ptr = self.read_slice(index, length)?.as_ptr() as usize; - let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::ERASE_PAGE, ptr, 0) }; + let code = unsafe { syscalls::command(DRIVER_NUMBER, command_nr::ERASE_PAGE, ptr, length) }; if code < 0 { return Err(StorageError::KernelError { code }); } From e51e214f2c52a2918511cc483210ec062bf05ea9 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 14 May 2020 20:36:47 +0200 Subject: [PATCH 17/27] Update reproducible hashes --- .../reference_binaries_macos-10.15.sha256sum | 18 +++++++++--------- .../reference_binaries_ubuntu-18.04.sha256sum | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index 9d3264a0..8bd92778 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ -b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -d81264ffbce075a78067b666db1988cd0e44e25c0e3f708ada28e90ff29f7339 target/nrf52840dk_merged.hex -346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -0c50decccd94e93612f3342ab833a73959d96a754aa6845b755a779a2240c484 target/nrf52840_dongle_merged.hex -adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -bbcbd0e72e40257cea94a2fee829d6b2c365e8473f2fb8b6ae685cda4d925f7d target/nrf52840_dongle_dfu_merged.hex -97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -d42f1ec737bd945ae2ddc74a5c179573b36fa720f9a7149b2af67a0e3667fac7 target/nrf52840_mdk_dfu_merged.hex -166523347144bcf9b936f5c265dfb2d7de9ab28569881276df344a6a42423f1b target/tab/ctap2.tab +1003863864e06553e730eec6df4bf8d30c99f697ef9380efdc35eba679b4db78 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +b4d3b099ee30687cfa8f8705da9d70aa55a4c39f6ce97b7b16317bc18bbf5a96 target/nrf52840dk_merged.hex +88f00a5e1dae6ab3f7571c254ac75f5f3e29ebea7f3ca46c16cfdc3708e804fc third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +a36027b1720ed5d451690c4bd3e5df2ab257618a8d94829a02cbceed316c3f02 target/nrf52840_dongle_merged.hex +1bc69b48a2c48da55db8b322902e1fe3f2e095c0dd8517db28837d86e0addc85 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +82abeb8001247dabbc864501ad1544c9ea859e891df04116ff0e9364a3af5f7e target/nrf52840_dongle_dfu_merged.hex +f38ee31d3a09e7e11848e78b5318f95517b6dcd076afcb37e6e3d3e5e9995cc7 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +e9effd0350a80c53b6b5422cadd5d8120e1d5924b94c3086dfea09aef89d6466 target/nrf52840_mdk_dfu_merged.hex +5d081b56927e33c6369228e78a9c126ac55a27bf02397d42f29e2ecf9afa69f1 target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index 3aca0952..4ccd51f5 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ -921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -776dab253e0903bffc54a9aeb09323717a4d69e6f1db8a4d88641378a89e47fe target/nrf52840dk_merged.hex -aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -945bfb77e7852d5d6425436208517c478af4ef66a675eded14f768de81512ab4 target/nrf52840_dongle_merged.hex -26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -55efa672a5fce6f8757efe16a2c3444d11f8f5a575fc0fedbc2892b587c6ecac target/nrf52840_dongle_dfu_merged.hex -7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -ed2beb9efd3bab6c91f7d0c6e3c1622d4920f5476844bd7551d51ba524ad4a71 target/nrf52840_mdk_dfu_merged.hex -ee12b35c75402db7fe191f2793209c51657c98662be2b6ff41ae09bccdc7e62a target/tab/ctap2.tab +c182bb4902fff51b2f56810fc2a27df3646cd66ba21359162354d53445623ab8 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +94cce0a02b2b44a1e4fd47a927455609890ff03e0574e2de99f355b732463af7 target/nrf52840dk_merged.hex +0a9929ba8fa57e8a502a49fc7c53177397202e1b11f4c7c3cb6ed68b2b99dd46 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +85464721e36e846262ce9e2147889ecadfb7e90ba2af4d791c7e4c920ad9809f target/nrf52840_dongle_merged.hex +cca9086c9149c607589b23ffa599a5e4c26db7c20bd3700b79528bd3a5df991d third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +be1e4f43dbb676e6963bb4d60a0a467d8213b5ebf764834a5dde0ff018bd2efc target/nrf52840_dongle_dfu_merged.hex +8857488ba6a69e366f0da229bbfc012a2ad291d3a88d9494247d600c10bb19b7 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +e7736d57434121cf6c198417af0751bfbca100070042f0a28c659c1343f2669f target/nrf52840_mdk_dfu_merged.hex +ee1e1cd305b902631001fe977b7d692d54a8c2718de36406cf16d1b067ef0567 target/tab/ctap2.tab From 146e6f083b9ceb7b45260ff1d6fa82f5a24772aa Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 14 May 2020 21:32:16 +0200 Subject: [PATCH 18/27] Don't rely on unification for array element type --- src/ctap/data_formats.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 98a05247..63f74f19 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -473,7 +473,7 @@ impl From for cbor::KeyType { impl From for cbor::Value { fn from(credential: PublicKeyCredentialSource) -> cbor::Value { use PublicKeyCredentialSourceField::*; - let mut private_key = [0; 32]; + let mut private_key = [0u8; 32]; credential.private_key.to_bytes(&mut private_key); cbor_map_options! { CredentialId => Some(credential.credential_id), From 721a5ba08b4926a38deaa2dca05f873d8d82a076 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 14 May 2020 21:48:55 +0200 Subject: [PATCH 19/27] Update reproducible hashes --- reproducible/reference_binaries_macos-10.15.sha256sum | 10 +++++----- reproducible/reference_binaries_ubuntu-18.04.sha256sum | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index 9d3264a0..700519f4 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -d81264ffbce075a78067b666db1988cd0e44e25c0e3f708ada28e90ff29f7339 target/nrf52840dk_merged.hex +9dfb62c7826994bf6e2bae972ea4dc03f1c62f4b49cea379c1f0aff9bbcfe208 target/nrf52840dk_merged.hex 346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -0c50decccd94e93612f3342ab833a73959d96a754aa6845b755a779a2240c484 target/nrf52840_dongle_merged.hex +c0467efe4a0536b5d835da7e000baccc06849c144907550a4ce6a6142fa60031 target/nrf52840_dongle_merged.hex adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -bbcbd0e72e40257cea94a2fee829d6b2c365e8473f2fb8b6ae685cda4d925f7d target/nrf52840_dongle_dfu_merged.hex +c74ff231e13e518652fae9f2dee13cd55492dd9348eadb42453a4f688bb399fd target/nrf52840_dongle_dfu_merged.hex 97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -d42f1ec737bd945ae2ddc74a5c179573b36fa720f9a7149b2af67a0e3667fac7 target/nrf52840_mdk_dfu_merged.hex -166523347144bcf9b936f5c265dfb2d7de9ab28569881276df344a6a42423f1b target/tab/ctap2.tab +b13a33e67a9858c829bdcb38bfd895e1960bdabf2ed365adeaf80a068e773f96 target/nrf52840_mdk_dfu_merged.hex +bcd1a523dc6d236ae8ac3924ae7887efa4c990da24fd445f2e03cad7522f0a1f target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index 3aca0952..cc3241a5 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ 921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -776dab253e0903bffc54a9aeb09323717a4d69e6f1db8a4d88641378a89e47fe target/nrf52840dk_merged.hex +9f1fdb14c3f84a91be7c22ce59f3842f87e5154379832de7cd48a09bc6df9797 target/nrf52840dk_merged.hex aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -945bfb77e7852d5d6425436208517c478af4ef66a675eded14f768de81512ab4 target/nrf52840_dongle_merged.hex +a607473c599a867ecbce8ed748c36c4b3731d69ad6c369765c14675057817149 target/nrf52840_dongle_merged.hex 26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -55efa672a5fce6f8757efe16a2c3444d11f8f5a575fc0fedbc2892b587c6ecac target/nrf52840_dongle_dfu_merged.hex +36fd56412e1dc72c0b61eea3a6de60343ccf8eeeb4bda664994f02e046ba375d target/nrf52840_dongle_dfu_merged.hex 7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -ed2beb9efd3bab6c91f7d0c6e3c1622d4920f5476844bd7551d51ba524ad4a71 target/nrf52840_mdk_dfu_merged.hex -ee12b35c75402db7fe191f2793209c51657c98662be2b6ff41ae09bccdc7e62a target/tab/ctap2.tab +5592d1617f6a647cd63ed19f398caa150b1f471c606f5b1afac1cffe73e3029a target/nrf52840_mdk_dfu_merged.hex +2c663f6c90f57af1c751085cb868ff905cdd1b71773cea10c31a898f201012b0 target/tab/ctap2.tab From 4e3162c475e5c889a6bf69f8ecc5193f1a896dfe Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 15 May 2020 19:43:37 +0200 Subject: [PATCH 20/27] Parse credentials by value --- src/ctap/data_formats.rs | 62 ++++++++++++++++++++++++++++------------ src/ctap/storage.rs | 5 ++-- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 63f74f19..d55121e3 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -486,25 +486,28 @@ impl From for cbor::Value { } } -impl TryFrom<&cbor::Value> for PublicKeyCredentialSource { +impl TryFrom for PublicKeyCredentialSource { type Error = Ctap2StatusCode; - fn try_from(cbor_value: &cbor::Value) -> Result { + fn try_from(cbor_value: cbor::Value) -> Result { use PublicKeyCredentialSourceField::*; - let map = read_map(cbor_value)?; - let credential_id = read_byte_string(ok_or_missing(map.get(&CredentialId.into()))?)?; - let private_key = read_byte_string(ok_or_missing(map.get(&PrivateKey.into()))?)?; + let mut map = extract_map(cbor_value)?; + let credential_id = extract_byte_string(ok_or_missing(map.remove(&CredentialId.into()))?)?; + let private_key = extract_byte_string(ok_or_missing(map.remove(&PrivateKey.into()))?)?; if private_key.len() != 32 { return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR); } let private_key = ecdsa::SecKey::from_bytes(array_ref!(private_key, 0, 32)) .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR)?; - let rp_id = read_text_string(ok_or_missing(map.get(&RpId.into()))?)?; - let user_handle = read_byte_string(ok_or_missing(map.get(&UserHandle.into()))?)?; - let other_ui = map.get(&OtherUi.into()).map(read_text_string).transpose()?; + let rp_id = extract_text_string(ok_or_missing(map.remove(&RpId.into()))?)?; + let user_handle = extract_byte_string(ok_or_missing(map.remove(&UserHandle.into()))?)?; + let other_ui = map + .remove(&OtherUi.into()) + .map(extract_text_string) + .transpose()?; let cred_random = map - .get(&CredRandom.into()) - .map(read_byte_string) + .remove(&CredRandom.into()) + .map(extract_byte_string) .transpose()?; // We don't return whether there were unknown fields in the CBOR value. This means that // deserialization is not injective. In particular deserialization is only an inverse of @@ -671,6 +674,13 @@ pub fn read_byte_string(cbor_value: &cbor::Value) -> Result, Ctap2Status } } +fn extract_byte_string(cbor_value: cbor::Value) -> Result, Ctap2StatusCode> { + match cbor_value { + cbor::Value::KeyValue(cbor::KeyType::ByteString(byte_string)) => Ok(byte_string), + _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE), + } +} + pub(super) fn read_text_string(cbor_value: &cbor::Value) -> Result { match cbor_value { cbor::Value::KeyValue(cbor::KeyType::TextString(text_string)) => { @@ -680,6 +690,13 @@ pub(super) fn read_text_string(cbor_value: &cbor::Value) -> Result Result { + match cbor_value { + cbor::Value::KeyValue(cbor::KeyType::TextString(text_string)) => Ok(text_string), + _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE), + } +} + pub(super) fn read_array(cbor_value: &cbor::Value) -> Result<&Vec, Ctap2StatusCode> { match cbor_value { cbor::Value::Array(array) => Ok(array), @@ -696,6 +713,15 @@ pub(super) fn read_map( } } +fn extract_map( + cbor_value: cbor::Value, +) -> Result, Ctap2StatusCode> { + match cbor_value { + cbor::Value::Map(map) => Ok(map), + _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE), + } +} + pub(super) fn read_bool(cbor_value: &cbor::Value) -> Result { match cbor_value { cbor::Value::Simple(cbor::SimpleValue::FalseValue) => Ok(false), @@ -704,9 +730,7 @@ pub(super) fn read_bool(cbor_value: &cbor::Value) -> Result, -) -> Result<&cbor::Value, Ctap2StatusCode> { +pub(super) fn ok_or_missing(value_option: Option) -> Result { value_option.ok_or(Ctap2StatusCode::CTAP2_ERR_MISSING_PARAMETER) } @@ -1240,7 +1264,7 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), Ok(credential.clone()) ); @@ -1250,7 +1274,7 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), Ok(credential.clone()) ); @@ -1260,15 +1284,15 @@ mod test { }; assert_eq!( - PublicKeyCredentialSource::try_from(&cbor::Value::from(credential.clone())), + PublicKeyCredentialSource::try_from(cbor::Value::from(credential.clone())), Ok(credential) ); } #[test] fn test_credential_source_invalid_cbor() { - assert!(PublicKeyCredentialSource::try_from(&cbor_false!()).is_err()); - assert!(PublicKeyCredentialSource::try_from(&cbor_array!(false)).is_err()); - assert!(PublicKeyCredentialSource::try_from(&cbor_array!(b"foo".to_vec())).is_err()); + assert!(PublicKeyCredentialSource::try_from(cbor_false!()).is_err()); + assert!(PublicKeyCredentialSource::try_from(cbor_array!(false)).is_err()); + assert!(PublicKeyCredentialSource::try_from(cbor_array!(b"foo".to_vec())).is_err()); } } diff --git a/src/ctap/storage.rs b/src/ctap/storage.rs index 2a1a1829..f0a2ca4f 100644 --- a/src/ctap/storage.rs +++ b/src/ctap/storage.rs @@ -18,7 +18,7 @@ use crate::ctap::status_code::Ctap2StatusCode; use crate::ctap::PIN_AUTH_LENGTH; use alloc::string::String; use alloc::vec::Vec; -use core::convert::TryFrom; +use core::convert::TryInto; use ctap2::embedded_flash::{self, StoreConfig, StoreEntry, StoreError, StoreIndex}; #[cfg(any(test, feature = "ram_storage"))] @@ -420,7 +420,8 @@ impl From for Ctap2StatusCode { } fn deserialize_credential(data: &[u8]) -> Option { - PublicKeyCredentialSource::try_from(&cbor::read(data).ok()?).ok() + let cbor = cbor::read(data).ok()?; + cbor.try_into().ok() } fn serialize_credential(credential: PublicKeyCredentialSource) -> Result, Ctap2StatusCode> { From 0a5c5eafda44ad4551b4aacfbbe3c4dc3172c117 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Fri, 15 May 2020 19:55:51 +0200 Subject: [PATCH 21/27] Update reproducible hashes --- reproducible/reference_binaries_macos-10.15.sha256sum | 10 +++++----- reproducible/reference_binaries_ubuntu-18.04.sha256sum | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index 700519f4..b4211e92 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -9dfb62c7826994bf6e2bae972ea4dc03f1c62f4b49cea379c1f0aff9bbcfe208 target/nrf52840dk_merged.hex +11a43fdafe73f59a5c715d1c850b28b1e0572cedd6acc93c860a3a36f6a3d4ac target/nrf52840dk_merged.hex 346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -c0467efe4a0536b5d835da7e000baccc06849c144907550a4ce6a6142fa60031 target/nrf52840_dongle_merged.hex +6a9ecb26886e266e3f92bc57ba8d0f6441ecfbfffdfe60ab981113b92524d3a3 target/nrf52840_dongle_merged.hex adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -c74ff231e13e518652fae9f2dee13cd55492dd9348eadb42453a4f688bb399fd target/nrf52840_dongle_dfu_merged.hex +55baecf731cf87966e1674fe6198aba2b955d33228192fb5b958e09b828aa3ea target/nrf52840_dongle_dfu_merged.hex 97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -b13a33e67a9858c829bdcb38bfd895e1960bdabf2ed365adeaf80a068e773f96 target/nrf52840_mdk_dfu_merged.hex -bcd1a523dc6d236ae8ac3924ae7887efa4c990da24fd445f2e03cad7522f0a1f target/tab/ctap2.tab +407b4bbc970901ce788ab3759571cf7e671f54a8c6a8546b7aff21ef96411df8 target/nrf52840_mdk_dfu_merged.hex +5947ae9da7436dc41fd9e89cb1faacb197282d03d0e755e2ac0099dc87484b4a target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index cc3241a5..5e45b7f2 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ 921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -9f1fdb14c3f84a91be7c22ce59f3842f87e5154379832de7cd48a09bc6df9797 target/nrf52840dk_merged.hex +223072356135c3d1f2114c34a29dd498da4a958722167b425c844333b48f82ef target/nrf52840dk_merged.hex aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -a607473c599a867ecbce8ed748c36c4b3731d69ad6c369765c14675057817149 target/nrf52840_dongle_merged.hex +98afad474b45a9ec8bddfcac6e61b149a25d5ed241e98bf1fa1174fa2efc91d2 target/nrf52840_dongle_merged.hex 26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -36fd56412e1dc72c0b61eea3a6de60343ccf8eeeb4bda664994f02e046ba375d target/nrf52840_dongle_dfu_merged.hex +d164e7670a6a6e9b59e158d0f5357547cc205a8d34cd9e73b0232b57d4f9538f target/nrf52840_dongle_dfu_merged.hex 7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -5592d1617f6a647cd63ed19f398caa150b1f471c606f5b1afac1cffe73e3029a target/nrf52840_mdk_dfu_merged.hex -2c663f6c90f57af1c751085cb868ff905cdd1b71773cea10c31a898f201012b0 target/tab/ctap2.tab +b6bd82285985ddbe4201ee2e37cef960d7c663a41ca5f382c5770d8c675e1f6f target/nrf52840_mdk_dfu_merged.hex +e338e3b091ae73f41d4663f49f539e93d5a9b3585a8bf1a791577e9486828cc1 target/tab/ctap2.tab From 23e564a9dedd0707a710fc8ca4ed5781118e776d Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 27 May 2020 11:58:41 +0200 Subject: [PATCH 22/27] Update reproducible hashes --- reproducible/reference_binaries_macos-10.15.sha256sum | 10 +++++----- reproducible/reference_binaries_ubuntu-18.04.sha256sum | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index b4211e92..cf4fea06 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -11a43fdafe73f59a5c715d1c850b28b1e0572cedd6acc93c860a3a36f6a3d4ac target/nrf52840dk_merged.hex +13d923f9efad028c9fa5d14920f940ad0108b290088a84ca82f08a3e09a4e569 target/nrf52840dk_merged.hex 346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -6a9ecb26886e266e3f92bc57ba8d0f6441ecfbfffdfe60ab981113b92524d3a3 target/nrf52840_dongle_merged.hex +f1f0de2e54c0dc88fd503271322a985f9c769fc26a72e651f84b834cd623c09c target/nrf52840_dongle_merged.hex adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -55baecf731cf87966e1674fe6198aba2b955d33228192fb5b958e09b828aa3ea target/nrf52840_dongle_dfu_merged.hex +d080c9267370b4a69e64e6098391cded7d3532feaa123c6331b12608aacd0112 target/nrf52840_dongle_dfu_merged.hex 97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -407b4bbc970901ce788ab3759571cf7e671f54a8c6a8546b7aff21ef96411df8 target/nrf52840_mdk_dfu_merged.hex -5947ae9da7436dc41fd9e89cb1faacb197282d03d0e755e2ac0099dc87484b4a target/tab/ctap2.tab +5bdcfa69ff5e86cd8175e0d78912e54e803c284502e035416333a8da2eaa1616 target/nrf52840_mdk_dfu_merged.hex +c68c723d21264b54a7167d999ad18294d10b05d48412595ccbe596865e166e5d target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index 5e45b7f2..8e87cacc 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ 921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -223072356135c3d1f2114c34a29dd498da4a958722167b425c844333b48f82ef target/nrf52840dk_merged.hex +e8e6b802722bf41eb5054ec94e87332fa996e954aad7d02db09beb798d7e381c target/nrf52840dk_merged.hex aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -98afad474b45a9ec8bddfcac6e61b149a25d5ed241e98bf1fa1174fa2efc91d2 target/nrf52840_dongle_merged.hex +aab3193acc08bc38c4219968da5e9d99f6f73b78fe91d74e8cbc3b94166c7dd3 target/nrf52840_dongle_merged.hex 26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -d164e7670a6a6e9b59e158d0f5357547cc205a8d34cd9e73b0232b57d4f9538f target/nrf52840_dongle_dfu_merged.hex +99a71cc0622f7cc0abdce686ff6581ec8b9fee6075b7cd771a68c08717e4198f target/nrf52840_dongle_dfu_merged.hex 7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -b6bd82285985ddbe4201ee2e37cef960d7c663a41ca5f382c5770d8c675e1f6f target/nrf52840_mdk_dfu_merged.hex -e338e3b091ae73f41d4663f49f539e93d5a9b3585a8bf1a791577e9486828cc1 target/tab/ctap2.tab +810f3ef6353177742bba06618bdc865efd819e97cbed605769f12f9d66831671 target/nrf52840_mdk_dfu_merged.hex +54e05927ab650c2ccf10cde165e992e48869888609cc6e8b4429a5c7a420f145 target/tab/ctap2.tab From 80510e27701aaf1ddcdd30b85d574a4b0033399f Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Sat, 30 May 2020 12:05:50 +0200 Subject: [PATCH 23/27] Add --clear-storage flag --- deploy.py | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/deploy.py b/deploy.py index aae8e8fb..7136dcc4 100755 --- a/deploy.py +++ b/deploy.py @@ -60,6 +60,10 @@ "app_ldscript", # Flash address at which the app should be written "app_address", + # Flash address of the storage + "storage_address", + # Size of the storage + "storage_size", # Target name for flashing the board using pyOCD "pyocd_target", # The cfg file in OpenOCD board folder @@ -89,6 +93,8 @@ padding_address=0x30000, app_ldscript="nrf52840_layout.ld", app_address=0x40000, + storage_address=0xC0000, + storage_size=0x40000, pyocd_target="nrf52840", openocd_board="nordic_nrf52840_dongle.cfg", openocd_options=[], @@ -106,6 +112,8 @@ padding_address=0x30000, app_ldscript="nrf52840_layout.ld", app_address=0x40000, + storage_address=0xC0000, + storage_size=0x40000, pyocd_target="nrf52840", openocd_board="nordic_nrf52840_dongle.cfg", openocd_options=[], @@ -123,6 +131,8 @@ padding_address=0x30000, app_ldscript="nrf52840_layout.ld", app_address=0x40000, + storage_address=0xC0000, + storage_size=0x40000, pyocd_target="nrf52840", openocd_board="nordic_nrf52840_dongle.cfg", openocd_options=[], @@ -140,6 +150,8 @@ padding_address=0x30000, app_ldscript="nrf52840_layout.ld", app_address=0x40000, + storage_address=0xC0000, + storage_size=0x40000, pyocd_target="nrf52840", openocd_board="nordic_nrf52840_dongle.cfg", openocd_options=[], @@ -493,6 +505,40 @@ def clear_apps(self): info(("A non-critical error occurred while erasing " "apps: {}".format(str(e)))) + def clear_storage(self): + if self.args.programmer == "none": + return 0 + board_props = SUPPORTED_BOARDS[self.args.board] + storage = bytes([0xFF] * board_props.storage_size) + # Use tockloader if possible + if self.args.programmer in ("jlink", "openocd"): + info("Erasing the persistent storage") + tock = loader.TockLoader(self.tockloader_default_args) + tock.open() + try: + tock.flash_binary(storage, board_props.storage_address) + except TockLoaderException as e: + fatal("Couldn't erase the persistent storage: {}".format(str(e))) + return 0 + # Create an intelhex file otherwise + info("Creating the persistent storage HEX file") + # pylint: disable=g-import-not-at-top,import-outside-toplevel + import intelhex + storage_hex = intelhex.IntelHex() + storage_hex.frombytes(storage, offset=board_props.storage_address) + os.makedirs("target", exist_ok=True) + storage_file = "target/{}_storage.hex".format(self.args.board) + storage_hex.tofile(storage_file, format="hex") + # Flash the intelhex file + info("Flashing the persistent storage HEX file") + if self.args.programmer == "pyocd": + self.checked_command([ + "pyocd", "flash", "--target={}".format(board_props.pyocd_target), + "--format=hex", "--erase=auto", storage_file + ]) + return 0 + fatal("Programmer {} is not supported.".format(self.args.programmer)) + # pylint: disable=protected-access def verify_flashed_app(self, expected_app): if self.args.programmer not in ("jlink", "openocd"): @@ -594,7 +640,8 @@ def run(self): self.check_prerequisites() self.update_rustc_if_needed() - if not self.args.tockos and not self.args.application: + if not (self.args.tockos or self.args.application or + self.args.clear_storage): info("Nothing to do.") return 0 @@ -610,6 +657,10 @@ def run(self): else: self.build_example() + # Erase persistent storage + if self.args.clear_storage: + self.clear_storage() + # Flashing board_props = SUPPORTED_BOARDS[self.args.board] if self.args.programmer in ("jlink", "openocd"): @@ -717,6 +768,14 @@ def main(args): help=("When installing an application, previously installed " "applications won't be erased from the board."), ) + main_parser.add_argument( + "--clear-storage", + action="store_true", + default=False, + dest="clear_storage", + help=("Erases the persistent storage when installing an application. " + "All stored data will be permanently lost."), + ) main_parser.add_argument( "--programmer", metavar="METHOD", From 31365f6d8d5b035971612f27ec3c3659e6a44e70 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Sat, 30 May 2020 12:53:40 +0200 Subject: [PATCH 24/27] Update reproducible hashes --- .../reference_binaries_macos-10.15.sha256sum | 18 +++++++++--------- .../reference_binaries_ubuntu-18.04.sha256sum | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/reproducible/reference_binaries_macos-10.15.sha256sum b/reproducible/reference_binaries_macos-10.15.sha256sum index cf4fea06..98b20af2 100644 --- a/reproducible/reference_binaries_macos-10.15.sha256sum +++ b/reproducible/reference_binaries_macos-10.15.sha256sum @@ -1,9 +1,9 @@ -b113945b033eb229e3821542f5889769e5fd2e2ae3cb85c6d13a4e05a44a9866 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -13d923f9efad028c9fa5d14920f940ad0108b290088a84ca82f08a3e09a4e569 target/nrf52840dk_merged.hex -346016903ddf244a239162b7c703aafe7ec70a115175e2204892e874f930f6be third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -f1f0de2e54c0dc88fd503271322a985f9c769fc26a72e651f84b834cd623c09c target/nrf52840_dongle_merged.hex -adcc4caaea86f7b0d54111d3080802e7389a4e69a8f17945d026ee732ea8daa4 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -d080c9267370b4a69e64e6098391cded7d3532feaa123c6331b12608aacd0112 target/nrf52840_dongle_dfu_merged.hex -97a7dbdb7c3caa345307d5ff7f7607dad5c2cdc523b43c68d3b741ddce318e92 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -5bdcfa69ff5e86cd8175e0d78912e54e803c284502e035416333a8da2eaa1616 target/nrf52840_mdk_dfu_merged.hex -c68c723d21264b54a7167d999ad18294d10b05d48412595ccbe596865e166e5d target/tab/ctap2.tab +1003863864e06553e730eec6df4bf8d30c99f697ef9380efdc35eba679b4db78 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +84d97929d7592d89c7f321ffccafa4148263607e28918e53d9286be8ca55c209 target/nrf52840dk_merged.hex +88f00a5e1dae6ab3f7571c254ac75f5f3e29ebea7f3ca46c16cfdc3708e804fc third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +02009688b6ef8583f78f9b94ba8af65dfa3749b20516972cdb0d8ea7ac409268 target/nrf52840_dongle_merged.hex +1bc69b48a2c48da55db8b322902e1fe3f2e095c0dd8517db28837d86e0addc85 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +a24e7459b93eea1fc7557ecd9e4271a2ed729425990d6198be6791f364b1384b target/nrf52840_dongle_dfu_merged.hex +f38ee31d3a09e7e11848e78b5318f95517b6dcd076afcb37e6e3d3e5e9995cc7 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +5315b77b997952de10e239289e54b44c24105646f2411074332bb46f4b686ae6 target/nrf52840_mdk_dfu_merged.hex +9012744b93f8bac122fa18916cf8c22d1b8f729a284366802ed222bbc985e3f0 target/tab/ctap2.tab diff --git a/reproducible/reference_binaries_ubuntu-18.04.sha256sum b/reproducible/reference_binaries_ubuntu-18.04.sha256sum index 8e87cacc..3171ce7d 100644 --- a/reproducible/reference_binaries_ubuntu-18.04.sha256sum +++ b/reproducible/reference_binaries_ubuntu-18.04.sha256sum @@ -1,9 +1,9 @@ -921d6fc31f7235456dd41abc7e634a37ee87b5016b80c979d20ac5d3fcfc6b6b third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin -e8e6b802722bf41eb5054ec94e87332fa996e954aad7d02db09beb798d7e381c target/nrf52840dk_merged.hex -aab5bdc406b1e874b83872c9358d310070b3ce948ec0e20c054fb923ec879249 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin -aab3193acc08bc38c4219968da5e9d99f6f73b78fe91d74e8cbc3b94166c7dd3 target/nrf52840_dongle_merged.hex -26b8513e76058e86a01a4b408411ce429834eb2843993eb1671f2487b160bc9a third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin -99a71cc0622f7cc0abdce686ff6581ec8b9fee6075b7cd771a68c08717e4198f target/nrf52840_dongle_dfu_merged.hex -7cc558a66505e8cf8170aab50e6ddcb28f349fd7ced35ce841ccec33a533bea1 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin -810f3ef6353177742bba06618bdc865efd819e97cbed605769f12f9d66831671 target/nrf52840_mdk_dfu_merged.hex -54e05927ab650c2ccf10cde165e992e48869888609cc6e8b4429a5c7a420f145 target/tab/ctap2.tab +c182bb4902fff51b2f56810fc2a27df3646cd66ba21359162354d53445623ab8 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840dk.bin +805ca30b898b41a091cc136ab9b78b4e566e10c5469902d18c326c132ed4193e target/nrf52840dk_merged.hex +0a9929ba8fa57e8a502a49fc7c53177397202e1b11f4c7c3cb6ed68b2b99dd46 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle.bin +960dce1eb78f34a3c4cfdb543314da8ce211dced41f34da053669c8773926e1d target/nrf52840_dongle_merged.hex +cca9086c9149c607589b23ffa599a5e4c26db7c20bd3700b79528bd3a5df991d third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_dongle_dfu.bin +1755746cb3a28162a0bbd0b994332fa9ffaedca684803dfd9ef584040cea73ca target/nrf52840_dongle_dfu_merged.hex +8857488ba6a69e366f0da229bbfc012a2ad291d3a88d9494247d600c10bb19b7 third_party/tock/target/thumbv7em-none-eabi/release/nrf52840_mdk_dfu.bin +04b94cd65bf83fa12030c4deaa831e0251f5f8b9684ec972d03a46e8f32e98b4 target/nrf52840_mdk_dfu_merged.hex +69dd51b947013b77e3577784384c935ed76930d1fb3ba46e9a5b6b5d71941057 target/tab/ctap2.tab From 2edbc567efe06de4415c67d8bc6edd893e691267 Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Tue, 2 Jun 2020 15:53:42 +0200 Subject: [PATCH 25/27] Use erase command for pyocd --- deploy.py | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/deploy.py b/deploy.py index 7136dcc4..c6982644 100755 --- a/deploy.py +++ b/deploy.py @@ -508,11 +508,11 @@ def clear_apps(self): def clear_storage(self): if self.args.programmer == "none": return 0 + info("Erasing the persistent storage") board_props = SUPPORTED_BOARDS[self.args.board] - storage = bytes([0xFF] * board_props.storage_size) # Use tockloader if possible if self.args.programmer in ("jlink", "openocd"): - info("Erasing the persistent storage") + storage = bytes([0xFF] * board_props.storage_size) tock = loader.TockLoader(self.tockloader_default_args) tock.open() try: @@ -520,21 +520,11 @@ def clear_storage(self): except TockLoaderException as e: fatal("Couldn't erase the persistent storage: {}".format(str(e))) return 0 - # Create an intelhex file otherwise - info("Creating the persistent storage HEX file") - # pylint: disable=g-import-not-at-top,import-outside-toplevel - import intelhex - storage_hex = intelhex.IntelHex() - storage_hex.frombytes(storage, offset=board_props.storage_address) - os.makedirs("target", exist_ok=True) - storage_file = "target/{}_storage.hex".format(self.args.board) - storage_hex.tofile(storage_file, format="hex") - # Flash the intelhex file - info("Flashing the persistent storage HEX file") if self.args.programmer == "pyocd": self.checked_command([ - "pyocd", "flash", "--target={}".format(board_props.pyocd_target), - "--format=hex", "--erase=auto", storage_file + "pyocd", "erase", "--target={}".format(board_props.pyocd_target), + "--sector {}+{}".format(board_props.storage_address, + board_props.storage_size) ]) return 0 fatal("Programmer {} is not supported.".format(self.args.programmer)) From 90911fb32a1a8b0c2e703419538a128a168a693f Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Tue, 2 Jun 2020 17:01:31 +0200 Subject: [PATCH 26/27] Fix missing = in pyocd arguments --- deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy.py b/deploy.py index c6982644..cb0bcfbd 100755 --- a/deploy.py +++ b/deploy.py @@ -523,7 +523,7 @@ def clear_storage(self): if self.args.programmer == "pyocd": self.checked_command([ "pyocd", "erase", "--target={}".format(board_props.pyocd_target), - "--sector {}+{}".format(board_props.storage_address, + "--sector={}+{}".format(board_props.storage_address, board_props.storage_size) ]) return 0 From 2a304c58cba2084bac096cd02b8228b413a8296c Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Tue, 2 Jun 2020 17:42:40 +0200 Subject: [PATCH 27/27] Fix --sector flag and X+Y positional argument --- deploy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy.py b/deploy.py index cb0bcfbd..38be4977 100755 --- a/deploy.py +++ b/deploy.py @@ -523,8 +523,8 @@ def clear_storage(self): if self.args.programmer == "pyocd": self.checked_command([ "pyocd", "erase", "--target={}".format(board_props.pyocd_target), - "--sector={}+{}".format(board_props.storage_address, - board_props.storage_size) + "--sector", "{}+{}".format(board_props.storage_address, + board_props.storage_size) ]) return 0 fatal("Programmer {} is not supported.".format(self.args.programmer))