betrusted-io / xous-core

The Xous microkernel
Apache License 2.0
529 stars 85 forks source link

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

Open bunnie opened 7 months ago

bunnie commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Here are the impacted locations by this problem:

loader self-signing https://github.com/betrusted-io/xous-core/blob/cff2473771e71b2f09b76c38240e7cec494d99fd/services/root-keys/src/implementation.rs#L2683-L2792

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?): https://github.com/betrusted-io/xous-core/blob/cff2473771e71b2f09b76c38240e7cec494d99fd/tools/src/sign_image.rs#L65-L67

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.