1147: Support codesigning in the firmware updater r=lulf a=huntc

This PR provides a method to verify that firmware has been SHA-512 hashed and signed with a private key given its public key. The implementation provides both [`ed25519-dalek`](https://github.com/dalek-cryptography/ed25519-dalek/blob/main/Cargo.toml) and [`salty`](https://github.com/ycrypto/salty) as the signature verifiers. Either of the `ed25519-dalek` and `ed25519-salty` features is required to enable the functionality from `embassy-boot`.

The `verify_and_mark_updated` method is used in place of `mark_updated` when signing is used via its feature. This avoids the accidental omission of validation where it has been declared as required at compile time. It also keeps the parity of calls at the same number to the previous situation.

The PR permits other types of signature verifiers in the future on the proviso that the [Signature trait](https://github.com/RustCrypto/traits/tree/master/signature) is supported.

Finally, I've updated the CI to include testing `embassy-boot`, which it was doing before. In addition, I've included a unit test for verification based on a `ed25519-dalek` documentation example. This tests both the `dalek` and `salty` implementations.

In terms of code size comparisons, `dalek` adds about 68KiB and `salty` adds about 20KiB. I'm using `salty` myself. I've also tested this out by signing my code with the OpenBSD `signify` utility and then verify it during firmware upload using `salty`.


Co-authored-by: huntc <huntchr@gmail.com>
This commit is contained in:
bors[bot] 2023-01-12 20:43:24 +00:00 committed by GitHub
commit b0c8c688c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 382 additions and 18 deletions

View File

@ -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

View File

@ -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 = []

View File

@ -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<E> From<E> 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<E> From<E> 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<F: AsyncNorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<State, F::Error> {
pub async fn get_state<F: AsyncNorFlash>(
&mut self,
flash: &mut F,
aligned: &mut [u8],
) -> Result<State, FirmwareUpdaterError> {
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<F: AsyncNorFlash>(
&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>(_: 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<F: AsyncNorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> {
#[cfg(not(feature = "_verify"))]
pub async fn mark_updated<F: AsyncNorFlash>(
&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<F: AsyncNorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> {
pub async fn mark_booted<F: AsyncNorFlash>(
&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<F: AsyncNorFlash>(&mut self, flash: &mut F) -> Result<FirmwareWriter, F::Error> {
pub async fn prepare_update<F: AsyncNorFlash>(
&mut self,
flash: &mut F,
) -> Result<FirmwareWriter, FirmwareUpdaterError> {
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<F: NorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<State, F::Error> {
pub fn get_state_blocking<F: NorFlash>(
&mut self,
flash: &mut F,
aligned: &mut [u8],
) -> Result<State, FirmwareUpdaterError> {
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<F: NorFlash>(
&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>(_: 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<F: NorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> {
#[cfg(not(feature = "_verify"))]
pub fn mark_updated_blocking<F: NorFlash>(
&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<F: NorFlash>(&mut self, flash: &mut F, aligned: &mut [u8]) -> Result<(), F::Error> {
pub fn mark_booted_blocking<F: NorFlash>(
&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<F: NorFlash>(&mut self, flash: &mut F) -> Result<FirmwareWriter, F::Error> {
pub fn prepare_update_blocking<F: NorFlash>(
&mut self,
flash: &mut F,
) -> Result<FirmwareWriter, FirmwareUpdaterError> {
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<const SIZE: usize, const ERASE_SIZE: usize, const WRITE_SIZE: usize>([u8; SIZE]);
impl<const SIZE: usize, const ERASE_SIZE: usize, const WRITE_SIZE: usize> NorFlash
@ -1171,7 +1510,7 @@ mod tests {
impl<const SIZE: usize, const ERASE_SIZE: usize, const WRITE_SIZE: usize> ReadNorFlash
for MemFlash<SIZE, ERASE_SIZE, WRITE_SIZE>
{
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<const SIZE: usize, const ERASE_SIZE: usize, const WRITE_SIZE: usize> AsyncReadNorFlash
for MemFlash<SIZE, ERASE_SIZE, WRITE_SIZE>
{
const READ_SIZE: usize = 4;
const READ_SIZE: usize = 1;
type ReadFuture<'a> = impl Future<Output = Result<(), Self::Error>> + 'a;
fn read<'a>(&'a mut self, offset: u32, buf: &'a mut [u8]) -> Self::ReadFuture<'a> {

View File

@ -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"]

View File

@ -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
```