From b0529bc943c9da0eb5f43335d06779d6064b765a Mon Sep 17 00:00:00 2001 From: huntc Date: Fri, 6 Jan 2023 22:21:39 +1100 Subject: [PATCH] Support codesigning in the firmware updater This commit provides a method to verify that firmware has been signed with a private key given its public key. The implementation uses ed25519-dalek as the signature verifier. An "ed25519" feature is required to enable the functionality. When disabled (the default), calling the firmware updater's verify method will return a failure. --- .github/workflows/rust.yml | 10 +- embassy-boot/boot/Cargo.toml | 16 +- embassy-boot/boot/src/lib.rs | 367 ++++++++++++++++++++++- examples/boot/application/nrf/Cargo.toml | 5 + examples/boot/application/nrf/README.md | 2 +- 5 files changed, 382 insertions(+), 18 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b93c8783..3bfe5ef0 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -68,5 +68,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Test - run: cd embassy-sync && cargo test + + - name: Test boot + working-directory: ./embassy-boot/boot + run: cargo test && cargo test --features "ed25519-dalek" && cargo test --features "ed25519-salty" + + - name: Test sync + working-directory: ./embassy-sync + run: cargo test diff --git a/embassy-boot/boot/Cargo.toml b/embassy-boot/boot/Cargo.toml index ae4efbd2..0b0c77b1 100644 --- a/embassy-boot/boot/Cargo.toml +++ b/embassy-boot/boot/Cargo.toml @@ -25,12 +25,26 @@ features = ["defmt"] [dependencies] defmt = { version = "0.3", optional = true } log = { version = "0.4", optional = true } +ed25519-dalek = { version = "1.0.1", default_features = false, features = ["u32_backend"], optional = true } embassy-sync = { version = "0.1.0", path = "../../embassy-sync" } embedded-storage = "0.3.0" embedded-storage-async = "0.3.0" +salty = { git = "https://github.com/ycrypto/salty.git", rev = "a9f17911a5024698406b75c0fac56ab5ccf6a8c7", optional = true } +signature = { version = "1.6.4", default-features = false } [dev-dependencies] log = "0.4" env_logger = "0.9" -rand = "0.8" +rand = "0.7" # ed25519-dalek v1.0.1 depends on this exact version futures = { version = "0.3", features = ["executor"] } + +[dev-dependencies.ed25519-dalek] +default_features = false +features = ["rand", "std", "u32_backend"] + +[features] +ed25519-dalek = ["dep:ed25519-dalek", "_verify"] +ed25519-salty = ["dep:salty", "_verify"] + +#Internal features +_verify = [] \ No newline at end of file diff --git a/embassy-boot/boot/src/lib.rs b/embassy-boot/boot/src/lib.rs index 76b14bc8..be254e9d 100644 --- a/embassy-boot/boot/src/lib.rs +++ b/embassy-boot/boot/src/lib.rs @@ -52,6 +52,16 @@ pub enum BootError { BadMagic, } +#[cfg(feature = "defmt")] +impl defmt::Format for BootError { + fn format(&self, fmt: defmt::Formatter) { + match self { + BootError::Flash(_) => defmt::write!(fmt, "BootError::Flash(_)"), + BootError::BadMagic => defmt::write!(fmt, "BootError::BadMagic"), + } + } +} + impl From for BootError where E: NorFlashError, @@ -557,6 +567,33 @@ where self.state } } +/// Errors returned by FirmwareUpdater +#[derive(Debug)] +pub enum FirmwareUpdaterError { + /// Error from flash. + Flash(NorFlashErrorKind), + /// Signature errors. + Signature(signature::Error), +} + +#[cfg(feature = "defmt")] +impl defmt::Format for FirmwareUpdaterError { + fn format(&self, fmt: defmt::Formatter) { + match self { + FirmwareUpdaterError::Flash(_) => defmt::write!(fmt, "FirmwareUpdaterError::Flash(_)"), + FirmwareUpdaterError::Signature(_) => defmt::write!(fmt, "FirmwareUpdaterError::Signature(_)"), + } + } +} + +impl From for FirmwareUpdaterError +where + E: NorFlashError, +{ + fn from(error: E) -> Self { + FirmwareUpdaterError::Flash(error.kind()) + } +} /// FirmwareUpdater is an application API for interacting with the BootLoader without the ability to /// 'mess up' the internal bootloader state @@ -609,7 +646,11 @@ impl FirmwareUpdater { /// This is useful to check if the bootloader has just done a swap, in order /// to do verifications and self-tests of the new image before calling /// `mark_booted`. - pub async fn get_state(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result { + pub async fn get_state( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result { flash.read(self.state.from as u32, aligned).await?; if !aligned.iter().any(|&b| b != SWAP_MAGIC) { @@ -619,12 +660,126 @@ impl FirmwareUpdater { } } + /// Verify the DFU given a public key. If there is an error then DO NOT + /// proceed with updating the firmware as it must be signed with a + /// corresponding private key (otherwise it could be malicious firmware). + /// + /// Mark to trigger firmware swap on next boot if verify suceeds. + /// + /// If the "ed25519-salty" feature is set (or another similar feature) then the signature is expected to have + /// been generated from a SHA-512 digest of the firmware bytes. + /// + /// If no signature feature is set then this method will always return a + /// signature error. + /// + /// # Safety + /// + /// The `_aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being read from + /// and written to. + #[cfg(feature = "_verify")] + pub async fn verify_and_mark_updated( + &mut self, + _flash: &mut F, + _public_key: &[u8], + _signature: &[u8], + _update_len: usize, + _aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { + let _end = self.dfu.from + _update_len; + let _read_size = _aligned.len(); + + assert_eq!(_aligned.len(), F::WRITE_SIZE); + assert!(_end <= self.dfu.to); + + #[cfg(feature = "ed25519-dalek")] + { + use ed25519_dalek::{Digest, PublicKey, Sha512, Signature, SignatureError, Verifier}; + + let into_signature_error = |e: SignatureError| FirmwareUpdaterError::Signature(e.into()); + + let public_key = PublicKey::from_bytes(_public_key).map_err(into_signature_error)?; + let signature = Signature::from_bytes(_signature).map_err(into_signature_error)?; + + let mut digest = Sha512::new(); + + let mut offset = self.dfu.from; + let last_offset = _end / _read_size * _read_size; + + while offset < last_offset { + _flash.read(offset as u32, _aligned).await?; + digest.update(&_aligned); + offset += _read_size; + } + + let remaining = _end % _read_size; + + if remaining > 0 { + _flash.read(last_offset as u32, _aligned).await?; + digest.update(&_aligned[0..remaining]); + } + + public_key + .verify(&digest.finalize(), &signature) + .map_err(into_signature_error)? + } + #[cfg(feature = "ed25519-salty")] + { + use salty::constants::{PUBLICKEY_SERIALIZED_LENGTH, SIGNATURE_SERIALIZED_LENGTH}; + use salty::{PublicKey, Sha512, Signature}; + + fn into_signature_error(_: E) -> FirmwareUpdaterError { + FirmwareUpdaterError::Signature(signature::Error::default()) + } + + let public_key: [u8; PUBLICKEY_SERIALIZED_LENGTH] = _public_key.try_into().map_err(into_signature_error)?; + let public_key = PublicKey::try_from(&public_key).map_err(into_signature_error)?; + let signature: [u8; SIGNATURE_SERIALIZED_LENGTH] = _signature.try_into().map_err(into_signature_error)?; + let signature = Signature::try_from(&signature).map_err(into_signature_error)?; + + let mut digest = Sha512::new(); + + let mut offset = self.dfu.from; + let last_offset = _end / _read_size * _read_size; + + while offset < last_offset { + _flash.read(offset as u32, _aligned).await?; + digest.update(&_aligned); + offset += _read_size; + } + + let remaining = _end % _read_size; + + if remaining > 0 { + _flash.read(last_offset as u32, _aligned).await?; + digest.update(&_aligned[0..remaining]); + } + + let message = digest.finalize(); + let r = public_key.verify(&message, &signature); + trace!( + "Verifying with public key {}, signature {} and message {} yields ok: {}", + public_key.to_bytes(), + signature.to_bytes(), + message, + r.is_ok() + ); + r.map_err(into_signature_error)? + } + + self.set_magic(_aligned, SWAP_MAGIC, _flash).await + } + /// Mark to trigger firmware swap on next boot. /// /// # Safety /// /// The `aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being written to. - pub async fn mark_updated(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> { + #[cfg(not(feature = "_verify"))] + pub async fn mark_updated( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { assert_eq!(aligned.len(), F::WRITE_SIZE); self.set_magic(aligned, SWAP_MAGIC, flash).await } @@ -634,7 +789,11 @@ impl FirmwareUpdater { /// # Safety /// /// The `aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being written to. - pub async fn mark_booted(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> { + pub async fn mark_booted( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { assert_eq!(aligned.len(), F::WRITE_SIZE); self.set_magic(aligned, BOOT_MAGIC, flash).await } @@ -644,7 +803,7 @@ impl FirmwareUpdater { aligned: &mut [u8], magic: u8, flash: &mut F, - ) -> Result<(), F::Error> { + ) -> Result<(), FirmwareUpdaterError> { flash.read(self.state.from as u32, aligned).await?; if aligned.iter().any(|&b| b != magic) { @@ -672,7 +831,7 @@ impl FirmwareUpdater { data: &[u8], flash: &mut F, block_size: usize, - ) -> Result<(), F::Error> { + ) -> Result<(), FirmwareUpdaterError> { assert!(data.len() >= F::ERASE_SIZE); flash @@ -700,7 +859,10 @@ impl FirmwareUpdater { /// /// Using this instead of `write_firmware` allows for an optimized API in /// exchange for added complexity. - pub async fn prepare_update(&mut self, flash: &mut F) -> Result { + pub async fn prepare_update( + &mut self, + flash: &mut F, + ) -> Result { flash.erase((self.dfu.from) as u32, (self.dfu.to) as u32).await?; trace!("Erased from {} to {}", self.dfu.from, self.dfu.to); @@ -717,7 +879,11 @@ impl FirmwareUpdater { /// This is useful to check if the bootloader has just done a swap, in order /// to do verifications and self-tests of the new image before calling /// `mark_booted`. - pub fn get_state_blocking(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result { + pub fn get_state_blocking( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result { flash.read(self.state.from as u32, aligned)?; if !aligned.iter().any(|&b| b != SWAP_MAGIC) { @@ -727,12 +893,126 @@ impl FirmwareUpdater { } } + /// Verify the DFU given a public key. If there is an error then DO NOT + /// proceed with updating the firmware as it must be signed with a + /// corresponding private key (otherwise it could be malicious firmware). + /// + /// Mark to trigger firmware swap on next boot if verify suceeds. + /// + /// If the "ed25519-salty" feature is set (or another similar feature) then the signature is expected to have + /// been generated from a SHA-512 digest of the firmware bytes. + /// + /// If no signature feature is set then this method will always return a + /// signature error. + /// + /// # Safety + /// + /// The `_aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being read from + /// and written to. + #[cfg(feature = "_verify")] + pub fn verify_and_mark_updated_blocking( + &mut self, + _flash: &mut F, + _public_key: &[u8], + _signature: &[u8], + _update_len: usize, + _aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { + let _end = self.dfu.from + _update_len; + let _read_size = _aligned.len(); + + assert_eq!(_aligned.len(), F::WRITE_SIZE); + assert!(_end <= self.dfu.to); + + #[cfg(feature = "ed25519-dalek")] + { + use ed25519_dalek::{Digest, PublicKey, Sha512, Signature, SignatureError, Verifier}; + + let into_signature_error = |e: SignatureError| FirmwareUpdaterError::Signature(e.into()); + + let public_key = PublicKey::from_bytes(_public_key).map_err(into_signature_error)?; + let signature = Signature::from_bytes(_signature).map_err(into_signature_error)?; + + let mut digest = Sha512::new(); + + let mut offset = self.dfu.from; + let last_offset = _end / _read_size * _read_size; + + while offset < last_offset { + _flash.read(offset as u32, _aligned)?; + digest.update(&_aligned); + offset += _read_size; + } + + let remaining = _end % _read_size; + + if remaining > 0 { + _flash.read(last_offset as u32, _aligned)?; + digest.update(&_aligned[0..remaining]); + } + + public_key + .verify(&digest.finalize(), &signature) + .map_err(into_signature_error)? + } + #[cfg(feature = "ed25519-salty")] + { + use salty::constants::{PUBLICKEY_SERIALIZED_LENGTH, SIGNATURE_SERIALIZED_LENGTH}; + use salty::{PublicKey, Sha512, Signature}; + + fn into_signature_error(_: E) -> FirmwareUpdaterError { + FirmwareUpdaterError::Signature(signature::Error::default()) + } + + let public_key: [u8; PUBLICKEY_SERIALIZED_LENGTH] = _public_key.try_into().map_err(into_signature_error)?; + let public_key = PublicKey::try_from(&public_key).map_err(into_signature_error)?; + let signature: [u8; SIGNATURE_SERIALIZED_LENGTH] = _signature.try_into().map_err(into_signature_error)?; + let signature = Signature::try_from(&signature).map_err(into_signature_error)?; + + let mut digest = Sha512::new(); + + let mut offset = self.dfu.from; + let last_offset = _end / _read_size * _read_size; + + while offset < last_offset { + _flash.read(offset as u32, _aligned)?; + digest.update(&_aligned); + offset += _read_size; + } + + let remaining = _end % _read_size; + + if remaining > 0 { + _flash.read(last_offset as u32, _aligned)?; + digest.update(&_aligned[0..remaining]); + } + + let message = digest.finalize(); + let r = public_key.verify(&message, &signature); + trace!( + "Verifying with public key {}, signature {} and message {} yields ok: {}", + public_key.to_bytes(), + signature.to_bytes(), + message, + r.is_ok() + ); + r.map_err(into_signature_error)? + } + + self.set_magic_blocking(_aligned, SWAP_MAGIC, _flash) + } + /// Mark to trigger firmware swap on next boot. /// /// # Safety /// /// The `aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being written to. - pub fn mark_updated_blocking(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> { + #[cfg(not(feature = "_verify"))] + pub fn mark_updated_blocking( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { assert_eq!(aligned.len(), F::WRITE_SIZE); self.set_magic_blocking(aligned, SWAP_MAGIC, flash) } @@ -742,7 +1022,11 @@ impl FirmwareUpdater { /// # Safety /// /// The `aligned` buffer must have a size of F::WRITE_SIZE, and follow the alignment rules for the flash being written to. - pub fn mark_booted_blocking(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> { + pub fn mark_booted_blocking( + &mut self, + flash: &mut F, + aligned: &mut [u8], + ) -> Result<(), FirmwareUpdaterError> { assert_eq!(aligned.len(), F::WRITE_SIZE); self.set_magic_blocking(aligned, BOOT_MAGIC, flash) } @@ -752,7 +1036,7 @@ impl FirmwareUpdater { aligned: &mut [u8], magic: u8, flash: &mut F, - ) -> Result<(), F::Error> { + ) -> Result<(), FirmwareUpdaterError> { flash.read(self.state.from as u32, aligned)?; if aligned.iter().any(|&b| b != magic) { @@ -780,7 +1064,7 @@ impl FirmwareUpdater { data: &[u8], flash: &mut F, block_size: usize, - ) -> Result<(), F::Error> { + ) -> Result<(), FirmwareUpdaterError> { assert!(data.len() >= F::ERASE_SIZE); flash.erase( @@ -804,7 +1088,10 @@ impl FirmwareUpdater { /// /// Using this instead of `write_firmware_blocking` allows for an optimized /// API in exchange for added complexity. - pub fn prepare_update_blocking(&mut self, flash: &mut F) -> Result { + pub fn prepare_update_blocking( + &mut self, + flash: &mut F, + ) -> Result { flash.erase((self.dfu.from) as u32, (self.dfu.to) as u32)?; trace!("Erased from {} to {}", self.dfu.from, self.dfu.to); @@ -953,6 +1240,7 @@ mod tests { } #[test] + #[cfg(not(feature = "_verify"))] fn test_swap_state() { const STATE: Partition = Partition::new(0, 4096); const ACTIVE: Partition = Partition::new(4096, 61440); @@ -1022,6 +1310,7 @@ mod tests { } #[test] + #[cfg(not(feature = "_verify"))] fn test_separate_flash_active_page_biggest() { const STATE: Partition = Partition::new(2048, 4096); const ACTIVE: Partition = Partition::new(4096, 16384); @@ -1074,6 +1363,7 @@ mod tests { } #[test] + #[cfg(not(feature = "_verify"))] fn test_separate_flash_dfu_page_biggest() { const STATE: Partition = Partition::new(2048, 4096); const ACTIVE: Partition = Partition::new(4096, 16384); @@ -1133,6 +1423,55 @@ mod tests { assert_partitions(ACTIVE, DFU, STATE, 4096, 4); } + #[test] + #[cfg(feature = "_verify")] + fn test_verify() { + // The following key setup is based on: + // https://docs.rs/ed25519-dalek/latest/ed25519_dalek/#example + + use ed25519_dalek::Keypair; + use rand::rngs::OsRng; + + let mut csprng = OsRng {}; + let keypair: Keypair = Keypair::generate(&mut csprng); + + use ed25519_dalek::{Digest, Sha512, Signature, Signer}; + let firmware: &[u8] = b"This are bytes that would otherwise be firmware bytes for DFU."; + let mut digest = Sha512::new(); + digest.update(&firmware); + let message = digest.finalize(); + let signature: Signature = keypair.sign(&message); + + use ed25519_dalek::PublicKey; + let public_key: PublicKey = keypair.public; + + // Setup flash + + const STATE: Partition = Partition::new(0, 4096); + const DFU: Partition = Partition::new(4096, 8192); + let mut flash = MemFlash::<8192, 4096, 4>([0xff; 8192]); + + let firmware_len = firmware.len(); + + let mut write_buf = [0; 4096]; + write_buf[0..firmware_len].copy_from_slice(firmware); + NorFlash::write(&mut flash, DFU.from as u32, &write_buf).unwrap(); + + // On with the test + + let mut updater = FirmwareUpdater::new(DFU, STATE); + + let mut aligned = [0; 4]; + + assert!(block_on(updater.verify_and_mark_updated( + &mut flash, + &public_key.to_bytes(), + &signature.to_bytes(), + firmware_len, + &mut aligned, + )) + .is_ok()); + } struct MemFlash([u8; SIZE]); impl NorFlash @@ -1171,7 +1510,7 @@ mod tests { impl ReadNorFlash for MemFlash { - const READ_SIZE: usize = 4; + const READ_SIZE: usize = 1; fn read(&mut self, offset: u32, buf: &mut [u8]) -> Result<(), Self::Error> { let len = buf.len(); @@ -1194,7 +1533,7 @@ mod tests { impl AsyncReadNorFlash for MemFlash { - const READ_SIZE: usize = 4; + const READ_SIZE: usize = 1; type ReadFuture<'a> = impl Future> + 'a; fn read<'a>(&'a mut self, offset: u32, buf: &'a mut [u8]) -> Self::ReadFuture<'a> { diff --git a/examples/boot/application/nrf/Cargo.toml b/examples/boot/application/nrf/Cargo.toml index 9679bbc5..88899325 100644 --- a/examples/boot/application/nrf/Cargo.toml +++ b/examples/boot/application/nrf/Cargo.toml @@ -9,6 +9,7 @@ embassy-sync = { version = "0.1.0", path = "../../../../embassy-sync" } embassy-executor = { version = "0.1.0", path = "../../../../embassy-executor", features = ["nightly", "integrated-timers"] } embassy-time = { version = "0.1.0", path = "../../../../embassy-time", features = ["nightly"] } embassy-nrf = { version = "0.1.0", path = "../../../../embassy-nrf", features = ["time-driver-rtc1", "gpiote", "nightly"] } +embassy-boot = { version = "0.1.0", path = "../../../../embassy-boot/boot" } embassy-boot-nrf = { version = "0.1.0", path = "../../../../embassy-boot/nrf" } embassy-embedded-hal = { version = "0.1.0", path = "../../../../embassy-embedded-hal" } @@ -19,3 +20,7 @@ embedded-hal = { version = "0.2.6" } cortex-m = { version = "0.7.6", features = ["critical-section-single-core"] } cortex-m-rt = "0.7.0" + +[features] +ed25519-dalek = ["embassy-boot/ed25519-dalek"] +ed25519-salty = ["embassy-boot/ed25519-salty"] \ No newline at end of file diff --git a/examples/boot/application/nrf/README.md b/examples/boot/application/nrf/README.md index 5d45f629..9d6d2033 100644 --- a/examples/boot/application/nrf/README.md +++ b/examples/boot/application/nrf/README.md @@ -22,7 +22,7 @@ cp memory-bl.x ../../bootloader/nrf/memory.x # Flash bootloader cargo flash --manifest-path ../../bootloader/nrf/Cargo.toml --features embassy-nrf/nrf52840 --target thumbv7em-none-eabi --release --chip nRF52840_xxAA # Build 'b' -cargo build --release --bin b +cargo build --release --bin b --features embassy-nrf/nrf52840 # Generate binary for 'b' cargo objcopy --release --bin b --features embassy-nrf/nrf52840 --target thumbv7em-none-eabi -- -O binary b.bin ```