[rtl] Fix FI vulnerability in RF

As described in #20715, a single fault-induced bit-flip inside the
register file could change which of the register file value is
provided to Ibex.

This PR fixes this issue by (i) encoding raddr_a/b to one-hot
encoded signals, (ii) checking these signals for faults, and
(iii) using an one-hot encoded MUX to select which register file
value is forwarded to rdata_a/b.

Area increases by ~1% (Yosys + Nangate45 synthesis).

I conducted a formal fault injection verification at the Yosys
netlist to ensure that the issue really is fixed.

Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>

doc/03_reference/security.rst

@@ -90,6 +90,15 @@ When Ibex is configured with the SecureIbex parameter, the write enable signal i
 This can be useful to detect fault injection attacks.
 No attempt is made to correct detected errors, but an internal major alert is signaled for the system to take action.
 
+Register file read addresses glitch detection
+-------------------------------------------
+
+When Ibex is configured with the SecureIbex parameter, the read addresses provided to the register file are converted to one-hot encoded signals, and a one-hot encoded MUX is used to select the register to read from.
+By using one-hot encoding checkers, glitches in the one-hot encoded signals are detected.
+Bit-flips inside the plain read addresses before the one-hot conversion happens are detected by the dual core lockstep.
+This can be useful to detect fault injection attacks.
+No attempt is made to correct detected errors, but an internal major alert is signaled for the system to take action.
+
 ICache ECC
 ----------
 

dv/uvm/core_ibex/common/prim/prim_and2.sv

@@ -0,0 +1,27 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// Abstract primitives wrapper.
+//
+// This file is a stop-gap until the DV file list is generated by FuseSoC.
+// Its contents are taken from the file which would be generated by FuseSoC.
+// https://github.com/lowRISC/ibex/issues/893
+
+module prim_and2 #(
+  parameter int Width = 1
+) (
+  input        [Width-1:0] in0_i,
+  input        [Width-1:0] in1_i,
+  output logic [Width-1:0] out_o
+);
+
+if (1) begin : gen_generic
+  prim_generic_and2 #(
+    .Width(Width)
+  ) u_impl_generic (
+    .*
+  );
+end
+
+endmodule

dv/uvm/core_ibex/ibex_dv.f

@@ -45,6 +45,8 @@
 ${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_clock_mux2.sv
 ${LOWRISC_IP_DIR}/ip/prim_generic/rtl/prim_generic_flop.sv
 ${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_flop.sv
+${LOWRISC_IP_DIR}/ip/prim_generic/rtl/prim_generic_and2.sv
+${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_and2.sv
 
 // Shared lowRISC code
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_cipher_pkg.sv
@@ -64,6 +66,8 @@
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_72_64_enc.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_72_64_dec.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_check.sv
+${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_enc.sv
+${LOWRISC_IP_DIR}/ip/prim/rtl/prim_onehot_mux.sv
 ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_mubi_pkg.sv
 
 // ibex CORE RTL files

dv/uvm/core_ibex/tb/core_ibex_tb_top.sv

@@ -336,4 +336,12 @@ module core_ibex_tb_top;
     assign dut.u_ibex_top.gen_regfile_ff.register_file_i.gen_wren_check.u_prim_onehot_check.
           unused_assert_connected = 1;
   end
+
+  // Disable the assertion for onhot check in case RdataMuxCheck (set by SecureIbex) is enabled.
+  if (SecureIbex) begin : gen_disable_rdata_mux_check
+    assign dut.u_ibex_top.gen_regfile_ff.register_file_i.gen_rdata_mux_check.
+          u_prim_onehot_check_raddr_a.unused_assert_connected = 1;
+    assign dut.u_ibex_top.gen_regfile_ff.register_file_i.gen_rdata_mux_check.
+          u_prim_onehot_check_raddr_b.unused_assert_connected = 1;
+  end
 endmodule

ibex_top.core

@@ -10,11 +10,13 @@ filesets:
     depend:
       - lowrisc:ibex:ibex_pkg
       - lowrisc:ibex:ibex_core
+      - lowrisc:prim:and2
       - lowrisc:prim:buf
       - lowrisc:prim:clock_mux2
       - lowrisc:prim:flop
       - lowrisc:prim:ram_1p_scr
       - lowrisc:prim:onehot_check
+      - lowrisc:prim:onehot
     files:
       - rtl/ibex_register_file_ff.sv # generic FF-based
       - rtl/ibex_register_file_fpga.sv # FPGA

rtl/ibex_register_file_ff.sv

@@ -15,6 +15,7 @@ module ibex_register_file_ff #(
   parameter int unsigned          DataWidth         = 32,
   parameter bit                   DummyInstructions = 0,
   parameter bit                   WrenCheck         = 0,
+  parameter bit                   RdataMuxCheck     = 0,
   parameter logic [DataWidth-1:0] WordZeroVal       = '0
 ) (
   // Clock and Reset
@@ -39,7 +40,7 @@ module ibex_register_file_ff #(
   input  logic [DataWidth-1:0] wdata_a_i,
   input  logic                 we_a_i,
 
-  // This indicates whether spurious WE are detected.
+  // This indicates whether spurious WE or non-one-hot encoded raddr are detected.
   output logic                 err_o
 );
 
@@ -49,6 +50,8 @@ module ibex_register_file_ff #(
   logic [DataWidth-1:0] rf_reg   [NUM_WORDS];
   logic [NUM_WORDS-1:0] we_a_dec;
 
+  logic oh_raddr_a_err, oh_raddr_b_err, oh_we_err;
+
   always_comb begin : we_a_decoder
     for (int unsigned i = 0; i < NUM_WORDS; i++) begin
       we_a_dec[i] = (waddr_a_i == 5'(i)) ? we_a_i : 1'b0;
@@ -78,12 +81,12 @@ module ibex_register_file_ff #(
       .oh_i(we_a_dec_buf),
       .addr_i(waddr_a_i),
       .en_i(we_a_i),
-      .err_o
+      .err_o(oh_we_err)
     );
   end else begin : gen_no_wren_check
     logic unused_strobe;
     assign unused_strobe = we_a_dec[0]; // this is never read from in this case
-    assign err_o = 1'b0;
+    assign oh_we_err = 1'b0;
   end
 
   // No flops for R0 as it's hard-wired to 0
@@ -130,8 +133,106 @@ module ibex_register_file_ff #(
     assign rf_reg[0] = WordZeroVal;
   end
 
-  assign rdata_a_o = rf_reg[raddr_a_i];
-  assign rdata_b_o = rf_reg[raddr_b_i];
+  if (RdataMuxCheck) begin : gen_rdata_mux_check
+    // Encode raddr_a/b into one-hot encoded signals.
+    logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
+    logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_a (
+      .in_i  (raddr_a_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_a)
+    );
+
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_b (
+      .in_i  (raddr_b_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_b)
+    );
+
+    // Buffer the one-hot encoded signals so that the checkers
+    // are not optimized.
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_a (
+      .in_i (raddr_onehot_a),
+      .out_o(raddr_onehot_a_buf)
+    );
+
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_b (
+      .in_i (raddr_onehot_b),
+      .out_o(raddr_onehot_b_buf)
+    );
+
+    // SEC_CM: DATA_REG_SW.GLITCH_DETECT
+    // Check the one-hot encoded signals for glitches.
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      // When AddrCheck=1 also EnableCheck needs to be 1.
+      .EnableCheck(1)
+    ) u_prim_onehot_check_raddr_a (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_a_buf),
+      .addr_i (raddr_a_i),
+      // Set enable=1 as address is always valid.
+      .en_i   (1'b1),
+      .err_o  (oh_raddr_a_err)
+    );
+
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      // When AddrCheck=1 also EnableCheck needs to be 1.
+      .EnableCheck(1)
+    ) u_prim_onehot_check_raddr_b (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_b_buf),
+      .addr_i (raddr_b_i),
+      // Set enable=1 as address is always valid.
+      .en_i   (1'b1),
+      .err_o  (oh_raddr_b_err)
+    );
+
+    // MUX register to rdata_a/b_o according to raddr_a/b_onehot.
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_a_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (rf_reg),
+      .sel_i (raddr_onehot_a),
+      .out_o (rdata_a_o)
+    );
+
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_b_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (rf_reg),
+      .sel_i (raddr_onehot_b),
+      .out_o (rdata_b_o)
+    );
+  end else begin : gen_no_rdata_mux_check
+    assign rdata_a_o = rf_reg[raddr_a_i];
+    assign rdata_b_o = rf_reg[raddr_b_i];
+    assign oh_raddr_a_err = 1'b0;
+    assign oh_raddr_b_err = 1'b0;
+  end
+
+  assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;
 
   // Signal not used in FF register file
   logic unused_test_en;

rtl/ibex_register_file_fpga.sv

@@ -16,6 +16,7 @@ module ibex_register_file_fpga #(
     parameter int unsigned          DataWidth         = 32,
     parameter bit                   DummyInstructions = 0,
     parameter bit                   WrenCheck         = 0,
+    parameter bit                   RdataMuxCheck     = 0,
     parameter logic [DataWidth-1:0] WordZeroVal       = '0
 ) (
   // Clock and Reset
@@ -37,7 +38,7 @@ module ibex_register_file_fpga #(
   input  logic [DataWidth-1:0] wdata_a_i,
   input  logic                 we_a_i,
 
-  // This indicates whether spurious WE are detected.
+  // This indicates whether spurious WE or non-one-hot encoded raddr are detected.
   output logic                 err_o
 );
 
@@ -47,11 +48,114 @@ module ibex_register_file_fpga #(
   logic [DataWidth-1:0] mem[NUM_WORDS];
   logic we; // write enable if writing to any register other than R0
 
-  // async_read a
-  assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
+  logic [DataWidth-1:0] mem_o_a, mem_o_b;
 
-  // async_read b
-  assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
+  // WE strobe and one-hot encoded raddr alert.
+  logic oh_raddr_a_err, oh_raddr_b_err, oh_we_err;
+  assign err_o = oh_raddr_a_err || oh_raddr_b_err || oh_we_err;
+
+  if (RdataMuxCheck) begin : gen_rdata_mux_check
+    // Encode raddr_a/b into one-hot encoded signals.
+    logic [NUM_WORDS-1:0] raddr_onehot_a, raddr_onehot_b;
+    logic [NUM_WORDS-1:0] raddr_onehot_a_buf, raddr_onehot_b_buf;
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_a (
+      .in_i  (raddr_a_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_a)
+    );
+
+    prim_onehot_enc #(
+      .OneHotWidth(NUM_WORDS)
+    ) u_prim_onehot_enc_raddr_b (
+      .in_i  (raddr_b_i),
+      .en_i  (1'b1),
+      .out_o (raddr_onehot_b)
+    );
+
+    // Buffer the one-hot encoded signals so that the checkers
+    // are not optimized.
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_a (
+      .in_i (raddr_onehot_a),
+      .out_o(raddr_onehot_a_buf)
+    );
+
+    prim_buf #(
+      .Width(NUM_WORDS)
+    ) u_prim_buf_raddr_b (
+      .in_i (raddr_onehot_b),
+      .out_o(raddr_onehot_b_buf)
+    );
+
+    // SEC_CM: DATA_REG_SW.GLITCH_DETECT
+    // Check the one-hot encoded signals for glitches.
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      // When AddrCheck=1 also EnableCheck needs to be 1.
+      .EnableCheck(1)
+    ) u_prim_onehot_check_raddr_a (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_a_buf),
+      .addr_i (raddr_a_i),
+      // Set enable=1 as address is always valid.
+      .en_i   (1'b1),
+      .err_o  (oh_raddr_a_err)
+    );
+
+    prim_onehot_check #(
+      .AddrWidth(ADDR_WIDTH),
+      .OneHotWidth(NUM_WORDS),
+      .AddrCheck(1),
+      // When AddrCheck=1 also EnableCheck needs to be 1.
+      .EnableCheck(1)
+    ) u_prim_onehot_check_raddr_b (
+      .clk_i,
+      .rst_ni,
+      .oh_i   (raddr_onehot_b_buf),
+      .addr_i (raddr_b_i),
+      // Set enable=1 as address is always valid.
+      .en_i   (1'b1),
+      .err_o  (oh_raddr_b_err)
+    );
+
+    // MUX register to rdata_a/b_o according to raddr_a/b_onehot.
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_a_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (mem),
+      .sel_i (raddr_onehot_a),
+      .out_o (mem_o_a)
+    );
+
+    prim_onehot_mux  #(
+      .Width(DataWidth),
+      .Inputs(NUM_WORDS)
+    ) u_rdata_b_mux (
+      .clk_i,
+      .rst_ni,
+      .in_i  (mem),
+      .sel_i (raddr_onehot_b),
+      .out_o (mem_o_b)
+    );
+
+    assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem_o_a;
+    assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem_o_b;
+  end else begin : gen_no_rdata_mux_check
+    // async_read a
+    assign rdata_a_o = (raddr_a_i == '0) ? '0 : mem[raddr_a_i];
+
+    // async_read b
+    assign rdata_b_o = (raddr_b_i == '0) ? '0 : mem[raddr_b_i];
+  end
 
   // we select
   assign we = (waddr_a_i == '0) ? 1'b0 : we_a_i;
@@ -60,9 +164,9 @@ module ibex_register_file_fpga #(
   // This checks for spurious WE strobes on the regfile.
   if (WrenCheck) begin : gen_wren_check
     // Since the FPGA uses a memory macro, there is only one write-enable strobe to check.
-    assign err_o = we && !we_a_i;
+    assign oh_we_err = we && !we_a_i;
   end else begin : gen_no_wren_check
-    assign err_o = 1'b0;
+    assign oh_we_err = 1'b0;
   end
 
   // Note that the SystemVerilog LRM requires variables on the LHS of assignments within