[otbn] Detect unexpected partial "secure" wipes

This commit adds a little bit of security hardening around signals used
for zeroing parts of the OTBN state during secure wipe. To this end, the
secure_wipe_running_o signal is registered inside the start stop
controller (to get distinct driving cells) and the critical signals
are compared against the secure_wipe_running signal at the point of
consumption.

This resolves lowRISC#12061.

Co-authored-by: Greg Chadwick <gac@lowrisc.org>
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>

hw/ip/otbn/rtl/otbn_alu_bignum.sv

@@ -100,7 +100,9 @@ module otbn_alu_bignum
 
   output logic                        reg_intg_violation_err_o,
 
-  input logic                         sec_wipe_mod_urnd_i,
+  input  logic                        sec_wipe_mod_urnd_i,
+  input  logic                        sec_wipe_running_i,
+  output logic                        sec_wipe_err_o,
 
   input  flags_t                      mac_operation_flags_i,
   input  flags_t                      mac_operation_flags_en_i,
@@ -963,6 +965,9 @@ module otbn_alu_bignum
   assign reg_intg_violation_err_o = mod_used & |(mod_intg_err);
   `ASSERT_KNOWN(RegIntgErrKnown_A, reg_intg_violation_err_o)
 
+  // Detect and signal unexpected secure wipe signals.
+  assign sec_wipe_err_o = sec_wipe_mod_urnd_i & ~sec_wipe_running_i;
+
   // Blanking Assertions
   // All blanking assertions are reset with predec_error or overall error in the whole system
   // -indicated by operation_commit_i port- as OTBN does not guarantee blanking in the case

hw/ip/otbn/rtl/otbn_controller.sv

@@ -147,6 +147,7 @@ module otbn_controller
   input  logic secure_wipe_ack_i,
   input  logic sec_wipe_zero_i,
   input  logic secure_wipe_running_i,
+  input  logic sec_wipe_err_i,
 
   input  logic        state_reset_i,
   output logic [31:0] insn_cnt_o,
@@ -193,6 +194,7 @@ module otbn_controller
   logic executing;
   logic state_error, state_error_d, state_error_q;
   logic spurious_secure_wipe_ack_q, spurious_secure_wipe_ack_d;
+  logic sec_wipe_err_q, sec_wipe_err_d;
   logic mubi_err_q, mubi_err_d;
 
   logic                     insn_fetch_req_valid_raw;
@@ -349,6 +351,18 @@ module otbn_controller
                                        ~secure_wipe_running_q &
                                        ~secure_wipe_running_i);
 
+  // Detect and latch unexpected secure wipe signals.
+  always_ff @(posedge clk_i or negedge rst_ni) begin
+    if (!rst_ni) begin
+      sec_wipe_err_q <= 1'b0;
+    end else begin
+      sec_wipe_err_q <= sec_wipe_err_d;
+    end
+  end
+  assign sec_wipe_err_d = sec_wipe_err_q |
+                          sec_wipe_err_i |
+                          (sec_wipe_zero_i & ~secure_wipe_running_i);
+
   // Stall a cycle on loads to allow load data writeback to happen the following cycle. Stall not
   // required on stores as there is no response to deal with.
   assign mem_stall = lsu_load_req_raw;
@@ -568,7 +582,7 @@ module otbn_controller
   assign fatal_software_err       = software_err & software_errs_fatal_i;
   assign bad_internal_state_err   = |{state_error_d, loop_hw_err, rf_base_call_stack_hw_err_i,
                                       rf_bignum_spurious_we_err, spurious_secure_wipe_ack_q,
-                                      mubi_err_q};
+                                      sec_wipe_err_q, mubi_err_q};
   assign reg_intg_violation_err   = rf_bignum_intg_err | ispr_rdata_intg_err;
   assign key_invalid_err          = ispr_rd_bignum_insn & insn_valid_i & key_invalid;
   assign illegal_insn_err         = illegal_insn_static | rf_indirect_err;

hw/ip/otbn/rtl/otbn_core.sv

@@ -146,6 +146,7 @@ module otbn_core
   logic [31:0]              rf_base_wr_data_no_intg_ctrl;
   logic [BaseIntgWidth-1:0] rf_base_wr_data_intg;
   logic                     rf_base_wr_data_intg_sel, rf_base_wr_data_intg_sel_ctrl;
+  logic                     rf_base_wr_sec_wipe_err;
   logic [4:0]               rf_base_rd_addr_a;
   logic                     rf_base_rd_en_a;
   logic [BaseIntgWidth-1:0] rf_base_rd_data_a_intg;
@@ -157,6 +158,7 @@ module otbn_core
   logic                     rf_base_call_stack_hw_err;
   logic                     rf_base_intg_err;
   logic                     rf_base_spurious_we_err;
+  logic                     rf_base_sec_wipe_err;
 
   alu_base_operation_t  alu_base_operation;
   alu_base_comparison_t alu_base_comparison;
@@ -185,6 +187,7 @@ module otbn_core
   logic [WLEN-1:0]    rf_bignum_wr_data_no_intg_ctrl;
   logic [ExtWLEN-1:0] rf_bignum_wr_data_intg;
   logic               rf_bignum_wr_data_intg_sel, rf_bignum_wr_data_intg_sel_ctrl;
+  logic               rf_bignum_wr_sec_wipe_err;
   logic [WdrAw-1:0]   rf_bignum_rd_addr_a;
   logic               rf_bignum_rd_en_a;
   logic [ExtWLEN-1:0] rf_bignum_rd_data_a_intg;
@@ -200,6 +203,7 @@ module otbn_core
   logic [WLEN-1:0]       alu_bignum_operation_result;
   logic                  alu_bignum_selection_flag;
   logic                  alu_bignum_reg_intg_violation_err;
+  logic                  alu_bignum_sec_wipe_err;
 
   mac_bignum_operation_t mac_bignum_operation;
   logic [WLEN-1:0]       mac_bignum_operation_result;
@@ -208,6 +212,7 @@ module otbn_core
   logic                  mac_bignum_en;
   logic                  mac_bignum_commit;
   logic                  mac_bignum_reg_intg_violation_err;
+  logic                  mac_bignum_sec_wipe_err;
 
   ispr_e                       ispr_addr;
   logic [31:0]                 ispr_base_wdata;
@@ -255,6 +260,7 @@ module otbn_core
   logic sec_wipe_acc_urnd;
   logic sec_wipe_mod_urnd;
   logic sec_wipe_zero;
+  logic sec_wipe_err;
 
   logic zero_flags;
 
@@ -416,6 +422,12 @@ module otbn_core
      ispr_predec_error                                                                           |
      rd_predec_error;
 
+  assign sec_wipe_err = |{rf_base_wr_sec_wipe_err,
+                          rf_base_sec_wipe_err,
+                          rf_bignum_wr_sec_wipe_err,
+                          alu_bignum_sec_wipe_err,
+                          mac_bignum_sec_wipe_err};
+
   // Controller: coordinate between functional units, prepare their inputs (e.g. by muxing between
   // operand sources), and post-process their outputs as needed.
   otbn_controller #(
@@ -544,6 +556,7 @@ module otbn_core
     .secure_wipe_ack_i     (secure_wipe_ack),
     .sec_wipe_zero_i       (sec_wipe_zero),
     .secure_wipe_running_i (secure_wipe_running_o),
+    .sec_wipe_err_i        (sec_wipe_err),
 
     .state_reset_i       (state_reset),
     .insn_cnt_o          (insn_cnt),
@@ -684,6 +697,7 @@ module otbn_core
 
     .state_reset_i         (state_reset),
     .sec_wipe_stack_reset_i(sec_wipe_zero),
+    .sec_wipe_running_i    (secure_wipe_running_o),
 
     .wr_addr_i         (rf_base_wr_addr),
     .wr_en_i           (rf_base_wr_en),
@@ -703,7 +717,8 @@ module otbn_core
     .call_stack_sw_err_o(rf_base_call_stack_sw_err),
     .call_stack_hw_err_o(rf_base_call_stack_hw_err),
     .intg_err_o         (rf_base_intg_err),
-    .spurious_we_err_o  (rf_base_spurious_we_err)
+    .spurious_we_err_o  (rf_base_spurious_we_err),
+    .sec_wipe_err_o     (rf_base_sec_wipe_err)
   );
 
   assign rf_base_wr_addr         = sec_wipe_base ? sec_wipe_addr : rf_base_wr_addr_ctrl;
@@ -726,6 +741,8 @@ module otbn_core
     end
   end
 
+  assign rf_base_wr_sec_wipe_err = sec_wipe_base & ~secure_wipe_running_o;
+
   otbn_alu_base u_otbn_alu_base (
     .clk_i,
     .rst_ni,
@@ -799,6 +816,8 @@ module otbn_core
     end
   end
 
+  assign rf_bignum_wr_sec_wipe_err = sec_wipe_wdr_q & ~secure_wipe_running_o;
+
   otbn_alu_bignum u_otbn_alu_bignum (
     .clk_i,
     .rst_ni,
@@ -830,6 +849,8 @@ module otbn_core
     .reg_intg_violation_err_o(alu_bignum_reg_intg_violation_err),
 
     .sec_wipe_mod_urnd_i(sec_wipe_mod_urnd),
+    .sec_wipe_running_i (secure_wipe_running_o),
+    .sec_wipe_err_o     (alu_bignum_sec_wipe_err),
 
     .mac_operation_flags_i   (mac_bignum_operation_flags),
     .mac_operation_flags_en_i(mac_bignum_operation_flags_en),
@@ -858,6 +879,8 @@ module otbn_core
 
     .urnd_data_i        (urnd_data),
     .sec_wipe_acc_urnd_i(sec_wipe_acc_urnd),
+    .sec_wipe_running_i (secure_wipe_running_o),
+    .sec_wipe_err_o     (mac_bignum_sec_wipe_err),
 
     .mac_en_i    (mac_bignum_en),
     .mac_commit_i(mac_bignum_commit),

hw/ip/otbn/rtl/otbn_mac_bignum.sv

@@ -22,8 +22,10 @@ module otbn_mac_bignum
   input  mac_predec_bignum_t mac_predec_bignum_i,
   output logic               predec_error_o,
 
-  input logic [WLEN-1:0] urnd_data_i,
-  input logic            sec_wipe_acc_urnd_i,
+  input  logic [WLEN-1:0] urnd_data_i,
+  input  logic            sec_wipe_acc_urnd_i,
+  input  logic            sec_wipe_running_i,
+  output logic            sec_wipe_err_o,
 
   output logic [ExtWLEN-1:0] ispr_acc_intg_o,
   input  logic [ExtWLEN-1:0] ispr_acc_wr_data_intg_i,
@@ -236,5 +238,7 @@ module otbn_mac_bignum
   assign predec_error_o = |{expected_op_en     != mac_predec_bignum_i.op_en,
                             expected_acc_rd_en != mac_predec_bignum_i.acc_rd_en};
 
+  assign sec_wipe_err_o = sec_wipe_acc_urnd_i & ~sec_wipe_running_i;
+
   `ASSERT(NoISPRAccWrAndMacEn, ~(ispr_acc_wr_en_i & mac_en_i))
 endmodule

hw/ip/otbn/rtl/otbn_rf_base.sv

@@ -38,6 +38,7 @@ module otbn_rf_base
 
   input  logic                     state_reset_i,
   input  logic                     sec_wipe_stack_reset_i,
+  input  logic                     sec_wipe_running_i,
 
   input  logic [4:0]               wr_addr_i,
   input  logic                     wr_en_i,
@@ -59,7 +60,8 @@ module otbn_rf_base
   output logic                     call_stack_sw_err_o,
   output logic                     call_stack_hw_err_o,
   output logic                     intg_err_o,
-  output logic                     spurious_we_err_o
+  output logic                     spurious_we_err_o,
+  output logic                     sec_wipe_err_o
 );
   localparam int unsigned CallStackRegIndex = 1;
   localparam int unsigned CallStackDepth = 8;
@@ -233,4 +235,6 @@ module otbn_rf_base
   // secure wipe.
   `ASSERT(OtbnRfBaseRdAKnown, rd_en_a_i && !pop_stack_a |-> !$isunknown(rd_data_a_raw_intg))
   `ASSERT(OtbnRfBaseRdBKnown, rd_en_b_i && !pop_stack_b |-> !$isunknown(rd_data_b_raw_intg))
+
+  assign sec_wipe_err_o = sec_wipe_stack_reset_i & ~sec_wipe_running_i;
 endmodule

hw/ip/otbn/rtl/otbn_start_stop_control.sv

@@ -79,6 +79,7 @@ module otbn_start_stop_control
   logic mubi_err_q, mubi_err_d;
   logic urnd_reseed_err_q, urnd_reseed_err_d;
   logic secure_wipe_error_q, secure_wipe_error_d;
+  logic secure_wipe_running_q, secure_wipe_running_d;
   logic skip_reseed_q;
 
   logic addr_cnt_inc;
@@ -165,7 +166,7 @@ module otbn_start_stop_control
     sec_wipe_zero_o           = 1'b0;
     addr_cnt_inc              = 1'b0;
     secure_wipe_ack_o         = 1'b0;
-    secure_wipe_running_o     = 1'b0;
+    secure_wipe_running_d     = 1'b0;
     state_error_d             = state_error_q;
     allow_secure_wipe         = 1'b0;
     expect_secure_wipe        = 1'b0;
@@ -176,7 +177,7 @@ module otbn_start_stop_control
 
     unique case (state_q)
       OtbnStartStopStateInitial: begin
-        secure_wipe_running_o = 1'b1;
+        secure_wipe_running_d = 1'b1;
         urnd_reseed_req_o     = 1'b1;
         if (rma_request) begin
           // If we get an RMA request before the URND got reseeded, proceed with the initial secure
@@ -203,7 +204,8 @@ module otbn_start_stop_control
           if (rma_request) begin
             // Do not reseed URND before secure wipe for RMA, as the entropy complex may not be able
             // to provide entropy at this point.
-            state_d = OtbnStartStopSecureWipeWdrUrnd;
+            secure_wipe_running_d = 1'b1;
+            state_d               = OtbnStartStopSecureWipeWdrUrnd;
             // As we don't reseed URND, there's no point in doing two rounds of wiping, so we
             // pretend that the first round is already the second round.
             wipe_after_urnd_refresh_d = MuBi4True;
@@ -225,7 +227,7 @@ module otbn_start_stop_control
             // wait for the ACK and then do a secure wipe.
             allow_secure_wipe     = 1'b1;
             expect_secure_wipe    = 1'b1;
-            secure_wipe_running_o = 1'b1;
+            secure_wipe_running_d = 1'b1;
             if (urnd_reseed_ack_i) begin
               state_d = OtbnStartStopSecureWipeWdrUrnd;
             end
@@ -242,7 +244,7 @@ module otbn_start_stop_control
             // wait for the ACK and then do a secure wipe.
             allow_secure_wipe     = 1'b1;
             expect_secure_wipe    = 1'b1;
-            secure_wipe_running_o = 1'b1;
+            secure_wipe_running_d = 1'b1;
             if (urnd_reseed_ack_i) begin
               state_d = OtbnStartStopSecureWipeWdrUrnd;
             end
@@ -254,7 +256,8 @@ module otbn_start_stop_control
         allow_secure_wipe = 1'b1;
 
         if (stop) begin
-          state_d = OtbnStartStopSecureWipeWdrUrnd;
+          secure_wipe_running_d = 1'b1;
+          state_d               = OtbnStartStopSecureWipeWdrUrnd;
         end
       end
       // SEC_CM: DATA_REG_SW.SEC_WIPE
@@ -266,7 +269,7 @@ module otbn_start_stop_control
         sec_wipe_wdr_urnd_o   = 1'b1;
         allow_secure_wipe     = 1'b1;
         expect_secure_wipe    = 1'b1;
-        secure_wipe_running_o = 1'b1;
+        secure_wipe_running_d = 1'b1;
 
         // Count one extra cycle when wiping the WDR, because the wipe signals to the WDR
         // (`sec_wipe_wdr_o` and `sec_wipe_wdr_urnd_o`) are flopped once but the wipe signals to the
@@ -291,7 +294,7 @@ module otbn_start_stop_control
         addr_cnt_inc          = 1'b1;
         allow_secure_wipe     = 1'b1;
         expect_secure_wipe    = 1'b1;
-        secure_wipe_running_o = 1'b1;
+        secure_wipe_running_d = 1'b1;
         // The first two clock cycles are used to write random data to accumulator and modulus.
         sec_wipe_acc_urnd_o   = (addr_cnt_q == 6'b000000);
         sec_wipe_mod_urnd_o   = (addr_cnt_q == 6'b000001);
@@ -305,23 +308,24 @@ module otbn_start_stop_control
       // Writing zeros to the CSRs and reset the stack. The other registers are intentionally not
       // overwritten with zero.
        OtbnStartStopSecureWipeAllZero: begin
-        sec_wipe_zero_o       = 1'b1;
-        allow_secure_wipe     = 1'b1;
-        expect_secure_wipe    = 1'b1;
-        secure_wipe_running_o = 1'b1;
+        sec_wipe_zero_o    = 1'b1;
+        allow_secure_wipe  = 1'b1;
+        expect_secure_wipe = 1'b1;
 
         // Leave this state after a single cycle, which is sufficient to reset the CSRs and the
         // stack.
         if (mubi4_test_false_strict(wipe_after_urnd_refresh_q)) begin
           // This is the first round of wiping with random numbers, refresh URND and do a second
           // round.
           state_d = OtbnStartStopStateUrndRefresh;
+          secure_wipe_running_d     = 1'b1;
           wipe_after_urnd_refresh_d = MuBi4True;
         end else begin
           // This is the second round of wiping with random numbers, so the secure wipe is
           // complete.
           state_d = OtbnStartStopSecureWipeComplete;
-          secure_wipe_ack_o = 1'b1;
+          secure_wipe_running_d = 1'b0;
+          secure_wipe_ack_o     = 1'b1;
         end
       end
       OtbnStartStopSecureWipeComplete: begin
@@ -396,11 +400,13 @@ module otbn_start_stop_control
 
   always_ff @(posedge clk_i or negedge rst_ni) begin
     if (!rst_ni) begin
-      addr_cnt_q           <= 6'd0;
-      init_sec_wipe_done_q <= 1'b0;
+      addr_cnt_q            <= 6'd0;
+      init_sec_wipe_done_q  <= 1'b0;
+      secure_wipe_running_q <= 1'b1;
     end else begin
-      addr_cnt_q           <= addr_cnt_d;
-      init_sec_wipe_done_q <= init_sec_wipe_done_d;
+      addr_cnt_q            <= addr_cnt_d;
+      init_sec_wipe_done_q  <= init_sec_wipe_done_d;
+      secure_wipe_running_q <= secure_wipe_running_d;
     end
   end
 
@@ -454,6 +460,8 @@ module otbn_start_stop_control
 
   assign rma_ack_o = rma_ack_q;
 
+  assign secure_wipe_running_o = secure_wipe_running_q;
+
   `ASSERT(StartStopStateValid_A,
       state_q inside {OtbnStartStopStateInitial,
                       OtbnStartStopStateHalt,