Kick out the ROM api and use the HAL ROM api - #47
Conversation
|
Not super pretty. /libs is a workspace, but we're doing things there a workspace is not good at. Might pull that apart in a later PR. |
| // Start as unused by default for safety; will be updated with real values from CMPA if read is successful. | ||
| soc_root_key_usage: [NbootRootKeyUsage::Unused; 4], | ||
| soc_root_key_type_and_length: NbootRootKeyType::EcdsaP384Mldsa87, //FIXED TO THIS because we are CNSA 2.0 compliant. | ||
| soc_lifecycle: 0, |
There was a problem hiding this comment.
LC as 0 is not a valid state, I know we are setting a default "unknown" here, but I still think we should use a strict starting point for proper diagnosis from the ROM API authenticate call (i.e. auth. fail vs invalid input).
There was a problem hiding this comment.
I disagree. Things just need to work. The only place this constant is used is a function called load_nboot_auth_parms_from_ifr. Except in its current form it may return ok without returning any data from the IFR. So if the lifecycle is not set, that's because something else already went wrong and we're returning a bad value anyways.
Also, this is all gonna change anyways when I tackle the IFR stuff
| parms.soc_ro_tnvm.soc_rkh.copy_from_slice(&image_rkth_words); | ||
| } else { | ||
| verify_warn!("Production: image RKTH differs; not copying, will not call verify "); | ||
| n_boot_api.nboot_context_deinit(&mut ctx); |
There was a problem hiding this comment.
This is more of a question, as soon as we return BootError, Nboot Drop implementation deinits the NBOOT APIs, right?
There was a problem hiding this comment.
Yes, the nboot api has a drop impl
|
|
||
| verify_trace!("begin auth"); | ||
| let status = n_boot_api.nboot_img_authenticate_romapi(&mut ctx, image_base, &mut sig_ok, &mut parms); | ||
| let status = n_boot_api |
There was a problem hiding this comment.
I would leave this comment in embassy HAL for the ROM API PR 6910 but it's closed:
Rom API success return is actually a 32-bit Hamming pattern. I think we need to retain that comparison for the success case. NXP does provide a full 32-bit value for glitch hardening, and we should take advantage (unless there are Rust specifics at play that I'm not aware of).
All other APIs do a return a simple 0 (Ok()) for success, NBOOT is the exception.
|
|
||
| verify_error!("Auth failed with status {:?}: {:?}", status, boot_error); | ||
| Err(boot_error) | ||
| Ok(true) |
There was a problem hiding this comment.
Since we have done a considerable amount of work up until this point to harden against glitching, are we losing all of that by returning a bool here, instead of returning a Hamming bit pattern like NbootBoolTrue that is 32 bits wide?
This would mean despite all the glitch resistance up until now, a single bit flip causes a jump to unauthenticated image.
There was a problem hiding this comment.
Right now, I don't care because of two things:
- I'm going to be changing more code later around the flow of the program
- I'm not convinced the current hardening actually fully protects us
So, my plan is to make it all simple and easy first and then analyze the threats we actually face and then mitigate that.
|
@diondokter At some point in one of your branches, you had a file named IFR.rs that defined the CMPA/CFPA flash regions. Will that file be making a comeback at some point or will the existing lifecycle.rs structure stay as is? |
|
@alamfarjadf Yes, the IFR is up next for me to do |
3 participants
Depends on embassy-rs/embassy#6910