From 7248f47ca81e8b45d4e6335e4657f3d9987484f3 Mon Sep 17 00:00:00 2001 From: bunnie Date: Mon, 11 Jul 2022 16:11:32 +0800 Subject: [PATCH] fix self-signing to reflect extra data for semvers the loader is a little funky, and requires some maintainer hand-holding due to the split of the page ownership between the root-keys crate and the graphics-server crate. Improved docs so I don't forget this detail in the future. --- services/graphics-server/src/main.rs | 6 +++++- services/root-keys/src/implementation.rs | 7 +++++-- tools/src/bin/sign-image.rs | 10 ++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/services/graphics-server/src/main.rs b/services/graphics-server/src/main.rs index 9b35f4e17..f41e7f5bd 100644 --- a/services/graphics-server/src/main.rs +++ b/services/graphics-server/src/main.rs @@ -472,7 +472,11 @@ fn wrapped_main() -> ! { .expect("couldn't ack that bulk read pointer was reset"); }), Some(Opcode::BulkReadFonts) => { - let fontlen = fontmap::FONT_TOTAL_LEN as u32 + 8; + // this also needs to reflect in root-keys/src/implementation.rs @ sign_loader() + let fontlen = fontmap::FONT_TOTAL_LEN as u32 + + 16 // minver + + 16 // current ver + + 8; // sig ver + len let mut buf = unsafe { Buffer::from_memory_message_mut(msg.body.memory_message_mut().unwrap()) }; diff --git a/services/root-keys/src/implementation.rs b/services/root-keys/src/implementation.rs index 960f5a16f..e4fdf2509 100644 --- a/services/root-keys/src/implementation.rs +++ b/services/root-keys/src/implementation.rs @@ -1410,8 +1410,8 @@ impl<'a> RootKeys { pb.update_text(t!("rootkeys.init.signing_loader", xous::LANG)); pb.rebase_subtask_percentage(35, 85); let (loader_sig, loader_len) = self.sign_loader(&keypair, Some(&mut pb)); - log::debug!("loader signature: {:x?}", loader_sig.to_bytes()); - log::debug!("loader len: {} bytes", loader_len); + log::info!("loader signature: {:x?}", loader_sig.to_bytes()); + log::info!("loader len: {} bytes", loader_len); // commit the signatures pb.update_text(t!("rootkeys.init.commit_signatures", xous::LANG)); @@ -1912,6 +1912,9 @@ impl<'a> RootKeys { xous::LOADER_CODE_LEN - SIGBLOCK_SIZE + graphics_server::fontmap::FONT_TOTAL_LEN as u32 + // these also need to be updated in graphics-server/src/main.rs @ Some(Opcode::BulkReadfonts) + + 16 // for the minimum compatible semver + + 16 // for the current semver + 8; // two u32 words are appended to the end, which repeat the "version" and "length" fields encoded in the signature block // this is a huge hash, so, get a hardware hasher, even if it means waiting for it diff --git a/tools/src/bin/sign-image.rs b/tools/src/bin/sign-image.rs index b5eb71572..f625e0a9a 100644 --- a/tools/src/bin/sign-image.rs +++ b/tools/src/bin/sign-image.rs @@ -31,6 +31,16 @@ where let semver: [u8; 16] = SemVer::from_git() .map_err(|_| Error::new(ErrorKind::Other, "error parsing current Git rev"))? .into(); + // extra data appended here needs to be reflected in two places in Xous: + // 1. root-keys/src/implementation.rs @ sign-loader() + // 2. graphics-server/src/main.rs @ Some(Opcode::BulkReadfonts) + // This is because memory ownership is split between two crates for performance reasons: + // the direct memory page of fonts belongs to the graphics server, to avoid having to send + // a message on every font lookup. However, the keys reside in root-keys, so therefore, + // a bulk read operation has to shuttle font data back to the root-keys crate. Of course, + // the appended metadata is in the font region, so, this data has to be shuttled back. + // The graphics server is also entirely naive to how much cryptographic data is in the font + // region, and I think it's probably better for it to stay that way. source.append(&mut minver_bytes.to_vec()); source.append(&mut semver.to_vec()); for &b in LOADER_VERSION.to_le_bytes().iter() {