Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change loader signature to ed25519-prehash (next-rev hw) #503

Open
bunnie opened this issue Feb 7, 2024 · 2 comments
Open

Change loader signature to ed25519-prehash (next-rev hw) #503

bunnie opened this issue Feb 7, 2024 · 2 comments
Assignees
Labels
Next Rev Hardware Issues to be resolved on the next rev of hardware

Comments

@bunnie
Copy link
Member

bunnie commented Feb 7, 2024

This is a note mainly for @bunnie when implementing the next-gen hardware.

We should switch the loader verification to the ed25519-ph scheme. It is now standardized and using the pre-hash mechanism gives us the flexibility we need to parcel out the loader for fast hardware computation without having to re-implement tricky crypto APIs.

This is not done on the 1st-gen Precursor devices because it would involve a SoC update and a tricky re-factor of the extremely tiny, highly optimized ROM bootloader, which itself involves plenty of dangerous tricks to get it to fit into such a small space.

Basically, "don't fix it if it ain't broke", but "do it better next time".

And, the issue will hopefully help me remember to do it better next time, since I revisit the issue board regularly for old reminders like this.

@bunnie bunnie added the Next Rev Hardware Issues to be resolved on the next rev of hardware label Feb 7, 2024
@bunnie bunnie self-assigned this Feb 7, 2024
@bunnie
Copy link
Member Author

bunnie commented Feb 7, 2024

Also, fwiw, I heartily agree with the code comments in the ed25519 crate, ranting about the implementation of the ph scheme and not being able to specify your own nonce, but for my own reason: splitting the hash op is terrible for hardware hashers:

https://github.com/dalek-cryptography/curve25519-dalek/blob/4ac84dd0668b1d2e51654fcdffe4ae6a687bef00/ed25519-dalek/src/signing.rs#L871-L882

Included in full here because links to other repos don't inline:

        // This is the dumbest, ten-years-late, non-admission of fucking up the
        // domain separation I have ever seen.  Why am I still required to put
        // the upper half "prefix" of the hashed "secret key" in here?  Why
        // can't the user just supply their own nonce and decide for themselves
        // whether or not they want a deterministic signature scheme?  Why does
        // the message go into what's ostensibly the signature domain separation
        // hash?  Why wasn't there always a way to provide a context string?
        //
        // ...
        //
        // This is a really fucking stupid bandaid, and the damned scheme is
        // still bleeding from malleability, for fuck's sake.

Glad I'm not alone in writing cathartic rants in code comments.

@bunnie
Copy link
Member Author

bunnie commented Feb 9, 2024

Here are the impacted locations by this problem:

loader self-signing

pub fn sign_loader(
&self,
signing_key: &SigningKey,
maybe_pb: Option<&mut ProgressBar>,
) -> (Signature, u32) {
let maybe_pb = maybe_pb.map(|pb| {
pb.rebase_subtask_work(0, 2);
pb
});
let loader_len = 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
// compute the nonce. This is a small hash, so use a software hasher.
let mut nonce_hasher = Sha512Sw::new();
nonce_hasher.update(signing_key.as_bytes());
let hazmat = nonce_hasher.finalize();
let hash_prefix = &hazmat.as_slice()[32..];
// this is a huge hash, so, get a hardware hasher, even if it means waiting for it
let mut hasher = Sha512Hw::new();
// prime the hash with the nonce derived from the upper bits of the secret key's hash.
hasher.update(hash_prefix);
{
// this is the equivalent of hasher.update(&message)
let loader_region = self.loader_code();
// the loader data starts one page in; the first page is reserved for the signature itself
hasher.update(&loader_region[SIGBLOCK_SIZE as usize..]);
// now get the font plane data
self.gfx.bulk_read_restart(); // reset the bulk read pointers on the gfx side
let bulkread = BulkRead::default();
let mut buf = xous_ipc::Buffer::into_buf(bulkread)
.expect("couldn't transform bulkread into aligned buffer");
// this form of loop was chosen to avoid the multiple re-initializations and copies that would be
// entailed in our usual idiom for pasing buffers around. instead, we create a single
// buffer, and re-use it for every iteration of the loop.
loop {
buf.lend_mut(self.gfx.conn(), self.gfx.bulk_read_fontmap_op())
.expect("couldn't do bulkread from gfx");
let br = buf.as_flat::<BulkRead, _>().unwrap();
hasher.update(&br.buf[..br.len as usize]);
if br.len != bulkread.buf.len() as u32 {
log::trace!("non-full block len: {}", br.len);
}
if br.len < bulkread.buf.len() as u32 {
// read until we get a buffer that's not fully filled
break;
}
}
}
let maybe_pb = maybe_pb.map(|pb| {
pb.increment_work(1);
pb
});
let r = Scalar::from_hash(hasher);
let R: CompressedEdwardsY = EdwardsPoint::mul_base(&r).compress();
let mut hasher = Sha512Hw::new();
hasher.update(R.as_bytes());
hasher.update(signing_key.verifying_key().as_bytes());
{
// this is the equivalent of hasher.update(&message)
let loader_region = self.loader_code();
// the loader data starts one page in; the first page is reserved for the signature itself
hasher.update(&loader_region[SIGBLOCK_SIZE as usize..]);
// now get the font plane data
self.gfx.bulk_read_restart(); // reset the bulk read pointers on the gfx side
let bulkread = BulkRead::default();
let mut buf = xous_ipc::Buffer::into_buf(bulkread)
.expect("couldn't transform bulkread into aligned buffer");
// this form of loop was chosen to avoid the multiple re-initializations and copies that would be
// entailed in our usual idiom for pasing buffers around. instead, we create a single
// buffer, and re-use it for every iteration of the loop.
loop {
buf.lend_mut(self.gfx.conn(), self.gfx.bulk_read_fontmap_op())
.expect("couldn't do bulkread from gfx");
let br = buf.as_flat::<BulkRead, _>().unwrap();
hasher.update(&br.buf[..br.len as usize]);
if br.len != bulkread.buf.len() as u32 {
log::trace!("non-full block len: {}", br.len);
}
if br.len < bulkread.buf.len() as u32 {
// read until we get a buffer that's not fully filled
break;
}
}
}
if let Some(pb) = maybe_pb {
pb.increment_work(1);
}
let k = Scalar::from_hash(hasher);
let s = &(&k * &signing_key.to_scalar()) + &r;
let mut signature_bytes: [u8; 64] = [0u8; 64];
signature_bytes[..32].copy_from_slice(&R.as_bytes()[..]);
signature_bytes[32..].copy_from_slice(&s.as_bytes()[..]);
(Signature::from_bytes(&signature_bytes), loader_len)
}

bootstrap signing by the image creator
This would need to be updated to use the "prehash" version of signing, instead of the regular signing method (note there is a prehash implementation just below it, so we'd be condensing the API?):

let signing_key =
Ed25519KeyPair::from_pkcs8_maybe_unchecked(&private_key.contents).map_err(|e| format!("{}", e))?;
let signature = signing_key.sign(&source);

low level bootloader verification

This all has to fit in 32k of code space. The API looks innocent:

https://github.com/betrusted-io/betrusted-soc/blob/65927da7349b7b7f8dba8e91df2a01cc5cc0f535/boot/betrusted-boot/src/main.rs#L413-L414

...but the magic happens here:

https://github.com/betrusted-io/betrusted-soc/blob/65927da7349b7b7f8dba8e91df2a01cc5cc0f535/boot/Cargo.toml#L14-L31

Basically, the crates are all monkey-patched to use hardware acceleration, and the hardware accelerators are hand-initialized before the calls (and cleaned up afterwards) to ensure it works for the context of the boot environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Next Rev Hardware Issues to be resolved on the next rev of hardware
Projects
None yet
Development

No branches or pull requests

1 participant