Add bounds checking for apob_read

[jamesmunns]

Currently WIP, waiting to validate on real hardware before marking this ready for merge.

[jamesmunns]

Digging into whether this is practically reproducable to verify the fix was effective:

1. apob read requests are only honored when the ApobState is Waiting, which is the default state on reboot. Initially, this made me think that we might not be able to trigger this on a real sled, because the state becomes Locked when the PSP has booted and the writes are committed. This made me look into the grapefruit schematic, and realize this flash chip is mapped through the FPGA, though I did validate that these parts are all populated on a grapefruit. BUT, reads only occur if there is a valid read slot present, which would require successfully writing a valid image to the flash first before the state machine would allow arbitrary reads. HOWEVER:
2. We don't actually need the ApobState::read to occur, we ONLY need the sp_comms request to be processed: the issue is "upstream" of any actual apob actions. We only need task/host-sp-comms to trigger the invalid slicing, so our attack surface is potentially more viable:
   1. handle_usart_notification needs to be running, and needs to decide that we have read until uart_rx_until_maybe_packet returns true
   2. process_message needs to decide this isn't a "debug message", and dispatch to process_ipcc_message
   3. parse_received_message needs to decode a valid HostToSp message
   4. parse_received_message doesn't consider HostToSp::ApobRead a sequence break, so it continues into the request handling
   5. ServerImpl::apob_read is called with the unvalidated header.sequence, offset, and size
   6. ServerImpl::apob_read requires a size that is LESS than u32::MAX, but greater than the bounds of TxBuf::len(). TODO: Figure this out, but we can probably pass 0xFFFF_FFFF or something equally unreasonably large
   7. The panic is triggered due to slicing TxBuf to [..0xFFFF_FFFF], which is out of bounds
3. This means I need to figure out how to ensure that the SP enters the IPCC processing loop, and if there's any setup/discussion to have, or if I can just boot and fire the malicious packet immediately
   1. Starting from host-sp-comms::main(), the server immediately starts by claiming static resources, sets our status to Status::SP_TASK_RESTARTED, sets the USART IRQ mask, and runs dispatch with the server
   2. Claiming static resources doesn't seem to do anything that would inhibit this
   3. Setting our status does set (low/active) an interrupt GPIO for the CPU, I'm unsure if this does anything to inhibit actions in the idolatry machinery. There is an InOrderHostSpCommsImpl that looks like it considers this, I'm not sure where this is generated yet. host-sp-comms.idol has a getter and setter, and mentions that set_status is "only done with hiffy", so even if it does inhibit something, we might just need to clear the bits with a hiffy call. Sending HostToSp::AckSpStart would also clear this, so I'm going to assume for now this isn't a mandatory setup step for this repro
4. Okay, but the idol server deals with IPC, and the methods we want to handle originate from external uart comms. It feels like we are really only processing those get/set status calls as IPC, and are handling UART dispatch as a side effect of <ServerImpl as NotificationHandler>::handle_notification, which calls handle_usart_notification when the USART IRQ mask is set. This pops from the UART FIFO, which looks like direct driver control in response to the IRQ. handle_usart_notification has a good state diagram for this.
   • TODO: See if there's a possible pathology of sending a periodic RST zero in the middle of the host sending us some data after first waking up, and if we care about that. I'm not certain I see somewhere where receiving a byte resets the timer, so if the host boots 199 ticks into a 200 tick periodic timer, we might send a RST before fully receiving a request from the host, potentially causing it to read that as an invalid reply. This might be ignored at the host anyway (it's not an error, just a periodic cobs sentinel).
5. Okay, so this path DOES seem viable, and could work either on a real sled or a grapefruit. However, folks were saying ipcc-rs doesn't have handlers for APOB messages, so we probably need to manually craft a payload for this. I need to:
   • For a Grapefruit:
     1. Just cold boot the system
     2. Send a manually crafted HostToSp::ApobRead message
        • Do we need to get anything correct in the header? Sequence/generation numbers?
        • Use an FTDI in some kind of raw binary write mode?
     3. Observe the reset via humility, maybe checking a changed task generation number if this task will be restarted by Jefe
   • For a Sled:
     1. Cold boot the system
     2. Send a manually crafted HostToSp::ApobRead message
        • How do I do this without ipcc-rs?
     3. Observe the reset via humility, maybe checking a changed task generation number if this task will be restarted by Jefe
6. I think I prefer using a grapefruit, restarting host-sp-comms is likely to be destructive, we raise a processor interrupt GPIO on the start of this task, I'm unsure if this causes a system reset, hang, etc., if done unexpectedly.
7. For manually crafting the packet: parse_received_message has the logic for this. It needs to be/have:
   • A corncobs wrapped message
   • a host_sp_messages message, with the right MAGIC, version V1, and a NOT have SEQ_REPLY set
   • A valid fletcher16 checksum, this is enforced by host_sp_messages::deserialize, but is also created by host_sp_messages::serialize for
   • I think this is hubpack? lib/host-sp-messages is the crate I want for this.

[cbiffle]

Do you prefer let/else to ok_or here?

[jamesmunns]

tbh I have a slight/weak preference to let-else here, but some of that is baseless preference, and some of that is habit because the .into() of ? can sometimes codegen slightly larger. That being said, I probably wouldn't refactor ok_or if I saw it.

[cbiffle]

You may not know the answer to this either, since you just moved this code, but...why are these types distinct?

[jamesmunns]

use drv_hf_api::ApobReadError;
use host_sp_messages::ApobReadResult;

I think they are just different types from different sources, IPC vs IPCC types? I think there are probably just two different wire schemas we're marshalling between here, not 100% certain though.