fix a critical bug that prevents root key initialization

This is going to require users to update their devices before
doing a key initialization.

The problem is that we can't trust the length of data presented
to us to self-sign, so we sign the whole area. In v0.9.2 of the
loader, a check was introduced to check the putative length of
the signed area against the actual length of the signed area,
by encoding the length of the signed area within the signed
region.

This would require the rootkeys algorithm to also encode that
new length at the end of the signed area, but this was never
done. However, this bug was never found during testing probably
because I failed to completely erase the original key provisioning
(I was more fixated on checking that existing keys were not damaged by
firmware updates at the time), and thus did not trigger a full
rootkey provisioning cycle when I did the test. I had only just
caught it now while porting the PDDB from simulation to real
hardware.
This commit is contained in:
bunnie 2022-01-03 19:06:21 +08:00
parent 0844b3e961
commit fe04f5684f
3 changed files with 30 additions and 1 deletions

View file

@ -353,11 +353,25 @@ pub fn validate_xous_img(xous_img_offset: *const u32) -> bool {
if sig.version != 1 || (sig.version != protected_version) {
gfx.msg("Check fail: mismatch on signature record version numbering.\n\r", &mut cursor);
println!("Check fail: mismatch on signature record version numbering.\n\r");
println!("sig.version: {}", sig.version);
println!("protected_version: {}", protected_version);
// a little insight to help debug what went wrong.
for words in image[signed_len as usize - 16 .. ].chunks(4) {
let word = u32::from_le_bytes(words.try_into().unwrap());
println!("{:x}", word);
}
die();
}
if protected_len != signed_len - 4 {
gfx.msg("Check fail: mismatch on header length vs protected length.\n\r", &mut cursor);
println!("Check fail: mismatch on header length vs protected length.\n\r");
println!("signed_len - 4: {}", signed_len - 4);
println!("protected_len: {}", protected_len);
// a little insight to help debug what went wrong.
for words in image[signed_len as usize - 16 .. ].chunks(4) {
let word = u32::from_le_bytes(words.try_into().unwrap());
println!("{:x}", word);
}
die();
}

View file

@ -3,6 +3,8 @@ pub(crate) const SERVER_NAME_KEYS: &str = "_Root key server and update manag
pub(crate) const ROOTKEY_MODAL_NAME: &'static str = "rootkeys modal";
#[allow(dead_code)]
pub(crate) const ROOTKEY_MENU_NAME: &'static str = "rootkeys menu";
#[allow(dead_code)]
pub(crate) const SIG_VERSION: u32 = 1;
#[allow(dead_code)]
#[derive(num_derive::FromPrimitive, num_derive::ToPrimitive, Debug)]

View file

@ -1820,6 +1820,19 @@ impl<'a> RootKeys {
// use most of the available space for the kernel, so by the time we're done maybe only 10-20% of the space is empty.
let kernel_len = kernel_region.len() - SIGBLOCK_SIZE as usize;
// we need to update our actual length because the initial burn doesn't set this
let mut len_data = [0u8; 8];
for (&src, dst) in SIG_VERSION.to_le_bytes().iter().zip(len_data[..4].iter_mut()) {
*dst = src;
}
for (&src, dst) in (kernel_len as u32 - 4).to_le_bytes().iter().zip(len_data[4..].iter_mut()) {
*dst = src;
}
log::info!("kernel len area before: {:x?}", &(self.kernel()[kernel_region.len()-8..]));
self.spinor.patch(self.kernel(), self.kernel_base(), &len_data, kernel_region.len() as u32 - 8)
.expect("couldn't patch length area");
log::info!("kernel len area after: {:x?}", &(self.kernel()[kernel_region.len()-8..]));
(signing_key.sign(&kernel_region[SIGBLOCK_SIZE as usize..]), kernel_len as u32)
}
@ -1988,7 +2001,7 @@ impl<'a> RootKeys {
// map a structure onto the signature region, so we can do something sane when writing stuff to it
let mut signature: &mut SignatureInFlash = unsafe{(sig_region.as_mut_ptr() as *mut SignatureInFlash).as_mut().unwrap()}; // this pointer better not be null, we just created it!
signature.version = 1;
signature.version = SIG_VERSION;
signature.signed_len = len;
signature.signature = sig.to_bytes();
log::debug!("sig: {:x?}", sig.to_bytes());