lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.56k stars 762 forks source link

[rv_core_ibex] Bit-flip in Register File Muxing #20715

Closed nasahlpa closed 8 months ago

nasahlpa commented 10 months ago

Depending on how the synthesis tools map this logic: https://github.com/lowRISC/opentitan/blob/14fb28c1e274a0e4a153d74f36bf730412cddc24/hw/vendor/lowrisc_ibex/rtl/ibex_register_file_ff.sv#L133-L134 a bit error could change which register file value is routed to the rdata_a_o & rdata_b_o ports and then forwarded to SecureIbex. As the same faulty value is provided to the main and shadow core, lockstep does not detect this induced error.

As an example, on a Yosys synthesized netlist with a single fault-induced bit-flip, we were able to achieve the following: r_addr_a_i = 5, expected rdata_a_o = rf_reg[5] to r_addr_a_i = 5, retrieved rdata_a_o = rf_reg[15] No alert was triggered.

The fix I am currently working on transforms rdata_a/b_o into one-hot encoded signals using prim_onehot_enc. The integrity of these signals is checked using prim_onehot_check. The final muxing then is conducted using prim_onehot_mux.

A bit-flip into the one-hot encoded signal is detected by the checker. Although a bit flip inside the one-hot MUX is still possible, only a single bit is manipulated, which then is detected by the ECC checker in SecureIbex.

I'll open the PR with the fix in the Ibex repository and link it here.

nasahlpa commented 10 months ago

A solution is provided in #2117.

The formal verification tool could verify that the vulnerability is indeed fixed. The area overhead is negligible (~1% with Yosys + Nangate45).

However, one issue still is open: https://github.com/lowRISC/opentitan/blob/78dee87b4ab0047d3815ae76a0012dbc49e9a9ed/hw/vendor/lowrisc_ibex/rtl/ibex_register_file_ff.sv#L117 As the we_r0_dummy signal is not protected, a fault injected there could change which value is returned. Not sure if we should protect this or not. Apart from this issue, the RF seems now to be OK in terms of FI security.

FYI @vogelpi , @msfschaffner, @johannheyszl.

vogelpi commented 9 months ago

Thanks for opening this issue and working on the fix @nasahlpa ! The PR looks good to me.

As for the we_r0_dummy signal: my understanding is that the dummy R0 is used to randomize the power consumption of dummy instructions. But the dummy instructions are not committed. Thus, glitching we_r0_dummy is of little value to adversaries. What matters is that dummy instructions cannot be converted to regular instructions and vice versa, this is covered by the dummy_instr_id_i signal which is protected by the lockstep countermeasure as well. So this should be fine.

nasahlpa commented 9 months ago

The fix got merged into the Ibex repository (#PR2117). Next step would be to apply the change also here to Ibex used in OpenTitan.

@vogelpi, regarding the dummy instruction - I agree, as we've discussed offline.

GregAC commented 8 months ago

Closing as this is fixed in Ibex and the fix has been vendored into OT