Crypto enhancement (#594)

* froze adder inputs to fix issue 194 * removed deadcode found by FPV * removed sca_en from ecc * protect lfsr to stuck and remove zeroize from counter * Stop faulty ECC from continuing before zeroize * synchronized hmac_drbg READY to VALID by removing one cycle delay * code cleanup * remove one extra iteration in ecc_scalar_blinding * updated ecc error to be a pulse * MICROSOFT AUTOMATED PIPELINE: Stamp 'mojtaba_crypto_enhancement' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'mojtaba_crypto_enhancement' with updated timestamp and hash after successful run --------- Co-authored-by: Mojtaba Bisheh Niasar <“[email protected]”>
chipsalliance · Oct 18, 2024 · 2717847 · 2717847
1 parent 9f80a2b
commit 2717847
Show file tree

Hide file tree

Showing 15 changed files with 160 additions and 781 deletions.
diff --git a/.github/workflow_metadata/pr_hash b/.github/workflow_metadata/pr_hash
@@ -1 +1 @@
-ca1b84bffeaa6505df7813ec15b149c024eda5b4f6f72bbb8e7bf0a809e8620b7387021dfc397cd11953682ab2f798fd
+97174a8f4e139cd861cb3bbda2ce818e7d1dc2f237783d95448535e0204fde3de4d12e6dd81b16f56bd97711609540df
diff --git a/.github/workflow_metadata/pr_timestamp b/.github/workflow_metadata/pr_timestamp
@@ -1 +1 @@
-1727494929
+1728602320
diff --git a/src/doe/rtl/doe_core_cbc.sv b/src/doe/rtl/doe_core_cbc.sv
@@ -250,8 +250,6 @@ module doe_core_cbc(
  st_IV_engine_stars:
  begin
  if (IV_updated_delayed)
- IV_enc_state <= st_IV_engine_idle; 
- else if(enc_ready)
  IV_enc_state <= st_IV_engine_idle;
  else
  IV_enc_state <= st_IV_engine_stars; 

diff --git a/src/ecc/rtl/ecc_add_sub_mod_alter.sv b/src/ecc/rtl/ecc_add_sub_mod_alter.sv
@@ -76,23 +76,30 @@ module ecc_add_sub_mod_alter #(
  );
 
 
- assign sub_n = !sub_i;
  assign opb0 = sub_i ? ~opb_i : opb_i;
- assign opb1 = sub_i ? prime_i : ~prime_i;
 
  always_ff @(posedge clk or negedge reset_n) 
  begin
  if(!reset_n) begin
  r0_reg <= '0;
  carry0_reg <= '0;
+ sub_n <= '0;
+ opb1 <= '0;
  end
  else if (zeroize) begin
  r0_reg <= '0;
  carry0_reg <= '0;
+ sub_n <= '0;
+ opb1 <= '0;
  end
  else if (add_en_i) begin 
  r0_reg <= r0;
  carry0_reg <= carry0;
+ sub_n <= !sub_i;
+ if (sub_i)
+ opb1 <= prime_i;
+ else
+ opb1 <= ~prime_i;
  end
  end
 
@@ -110,6 +117,6 @@ module ecc_add_sub_mod_alter #(
 
  assign ready_o = push_result_reg[0];
 
- assign res_o = sub_n ? (carry0_reg ^ carry1)? r1 : r0 : (carry0_reg) ? r0 : r1;
+ assign res_o = sub_n ? (carry0_reg ^ carry1)? r1 : r0_reg : (carry0_reg) ? r0_reg : r1;
 
 endmodule
diff --git a/src/ecc/rtl/ecc_arith_unit.sv b/src/ecc/rtl/ecc_arith_unit.sv
@@ -46,7 +46,6 @@ module ecc_arith_unit
 
  // DATA PORT
  input wire [3 : 0] ecc_cmd_i,
- input wire sca_en_i,
  input wire [ADDR_WIDTH-1 : 0] addr_i,
  input wire wr_op_sel_i,
  input wire wr_en_i,
@@ -101,7 +100,6 @@ module ecc_arith_unit
  .reset_n(reset_n),
  .zeroize(zeroize),
  .ecc_cmd_i(ecc_cmd_i),
- .sca_en_i(sca_en_i),
  .digit_i(digit_in),
  .instr_o(ecc_instr_s),
  .req_digit_o(req_digit),

diff --git a/src/ecc/rtl/ecc_dsa_ctrl.sv b/src/ecc/rtl/ecc_dsa_ctrl.sv
@@ -142,9 +142,7 @@ module ecc_dsa_ctrl
  logic [REG_NUM_DWORDS-1 : 0][DATA_WIDTH-1:0] r_reg;
  logic [REG_NUM_DWORDS-1 : 0][DATA_WIDTH-1:0] s_reg;
  logic [REG_NUM_DWORDS-1 : 0][DATA_WIDTH-1:0] IV_reg;
- logic [REG_SIZE-1 : 0] lambda;
  logic [REG_SIZE-1 : 0] lambda_reg;
- logic [REG_SIZE-1 : 0] masking_rnd;
  logic [REG_SIZE-1 : 0] masking_rnd_reg;
  logic [REG_SIZE-1 : 0] pk_chk_reg;
 
@@ -153,7 +151,6 @@ module ecc_dsa_ctrl
 
  logic [REG_SIZE-1 : 0] scalar_in_reg;
  logic [REG_SIZE-1 : 0] scalar_rnd_reg;
- logic [(REG_SIZE+RND_SIZE)-1 : 0] scalar_out;
  logic [(REG_SIZE+RND_SIZE)-1 : 0] scalar_out_reg;
  logic scalar_sca_en;
  logic scalar_sca_busy_o;
@@ -164,10 +161,6 @@ module ecc_dsa_ctrl
  logic [REG_SIZE-1 : 0] hmac_drbg_result;
  logic hmac_busy;
 
- logic sca_point_rnd_en;
- logic sca_mask_sign_en;
- logic sca_scalar_rnd_en;
-
  //interface with kv client
  logic kv_privkey_write_en;
  logic [REG_OFFSET_W-1:0] kv_privkey_write_offset;
@@ -247,7 +240,6 @@ module ecc_dsa_ctrl
  .reset_n(reset_n),
  .zeroize(zeroize_reg),
  .ecc_cmd_i(pm_cmd_reg),
- .sca_en_i(sca_scalar_rnd_en),
  .addr_i(prog_instr.mem_addr),
  .wr_op_sel_i(prog_instr.opcode.op_sel),
 
@@ -274,9 +266,9 @@ module ecc_dsa_ctrl
  .privKey(privkey_reg),
  .hashed_msg(msg_reduced_reg),
  .IV(IV_reg),
- .lambda(lambda),
+ .lambda(lambda_reg),
  .scalar_rnd(scalar_rnd_reg),
- .masking_rnd(masking_rnd),
+ .masking_rnd(masking_rnd_reg),
  .drbg(hmac_drbg_result)
  );
 
@@ -293,21 +285,10 @@ module ecc_dsa_ctrl
  .en_i(scalar_sca_en),
  .data_i(scalar_in_reg),
  .rnd_i(scalar_rnd_reg[RND_SIZE-1 : 0]),
- .data_o(scalar_out),
+ .data_o(scalar_out_reg),
  .busy_o(scalar_sca_busy_o)
  );
 
- //----------------------------------------------------------------
- // side-channel config update
- // Update functionality for SCA registers in the core.
- //----------------------------------------------------------------
-
- always_comb 
- begin : SCA_config
- scalar_out_reg = (sca_scalar_rnd_en)? scalar_out : (REG_SIZE+RND_SIZE)'(scalar_in_reg << RND_SIZE);
- lambda_reg = (sca_point_rnd_en)? lambda : ONE_CONST;
- masking_rnd_reg = (sca_mask_sign_en)? masking_rnd : ZERO_CONST;
- end // SCA_config
 
  //----------------------------------------------------------------
  // ecc_reg_update
@@ -319,10 +300,6 @@ module ecc_dsa_ctrl
  //Mask the command if KV clients are not idle
  cmd_reg = {hwif_out.ECC_CTRL.DH_SHAREDKEY.value, hwif_out.ECC_CTRL.CTRL.value} & {3{kv_seed_ready}} & {3{kv_privkey_ready}};
  zeroize_reg = hwif_out.ECC_CTRL.ZEROIZE.value || debugUnlock_or_scan_mode_switch;
-
- sca_point_rnd_en = 1'b1;
- sca_mask_sign_en = 1'b1;
- sca_scalar_rnd_en = 1'b1;
  end
 
  //there is a clk cycle memory read delay between hw_privkey_we and read_reg
@@ -481,7 +458,7 @@ module ecc_dsa_ctrl
  always_comb hwif_in.ECC_CTRL.PCR_SIGN.hwclr = hwif_out.ECC_CTRL.PCR_SIGN.value;
 
  // TODO add other interrupt hwset signals (errors)
- always_comb hwif_in.intr_block_rf.error_internal_intr_r.error_internal_sts.hwset = error_flag_reg;
+ always_comb hwif_in.intr_block_rf.error_internal_intr_r.error_internal_sts.hwset = error_flag_edge;
  always_comb hwif_in.intr_block_rf.notif_internal_intr_r.notif_cmd_done_sts.hwset = ecc_status_done_p;
 
 
@@ -674,19 +651,16 @@ module ecc_dsa_ctrl
 
  always_ff @(posedge clk or negedge reset_n) 
  begin : error_detection
- if(!reset_n) begin
- error_flag_reg <= '0;
- end
- else if(zeroize_reg) begin
- error_flag_reg <= '0;
- end
- else begin
- error_flag_reg <= error_flag;
- end
+ if(!reset_n)
+ error_flag_reg <= 1'b0;
+ else if(zeroize_reg)
+ error_flag_reg <= 1'b0;
+ else if (error_flag)
+ error_flag_reg <= 1'b1;
  end // error_detection
-
- assign error_flag_edge = error_flag & (!error_flag_reg);
 
+ assign error_flag_edge = error_flag & (!error_flag_reg);
+
  assign privkey_input_outofrange = signing_process & ((privkey_reg == 0) | (privkey_reg >= GROUP_ORDER));
  assign r_output_outofrange = signing_process & (hw_r_we & (read_reg == 0));
  assign s_output_outofrange = signing_process & (hw_s_we & (read_reg == 0));
@@ -738,15 +712,22 @@ module ecc_dsa_ctrl
  verifying_process <= 0;
  sharedkey_process <= 0;
  end
+ else if (error_flag | error_flag_reg) begin
+ prog_cntr <= ECC_NOP;
+ cycle_cnt <= '0;
+ pm_cmd_reg <= '0;
+ ecc_valid_reg <= 0;
+ scalar_G_sel <= 0;
+ hmac_mode <= '0;
+ hmac_init <= 0;
+ scalar_sca_en <= 0;
+ keygen_process <= 0;
+ signing_process <= 0;
+ verifying_process <= 0;
+ sharedkey_process <= 0;
+ end
  else begin
- if (error_flag_edge) begin
- prog_cntr <= ECC_NOP;
- cycle_cnt <= 2'd3;
- pm_cmd_reg <= '0;
- scalar_sca_en <= 0;
- hmac_init <= 0;
- end
- else if (subcomponent_busy) begin //Stalled until sub-component is done
+ if (subcomponent_busy) begin //Stalled until sub-component is done
  prog_cntr <= prog_cntr;
  cycle_cnt <= 2'd3;
  pm_cmd_reg <= '0;

diff --git a/src/ecc/rtl/ecc_hmac_drbg_interface.sv b/src/ecc/rtl/ecc_hmac_drbg_interface.sv
@@ -38,8 +38,7 @@
 
 module ecc_hmac_drbg_interface#(
  parameter REG_SIZE = 384,
- parameter [REG_SIZE-1 : 0] GROUP_ORDER = 384'hffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973,
- parameter [REG_SIZE-1 : 0] LFSR_INIT_SEED = 384'hc48555929cd58779f4819c1e6570c2ef20bccd503284e2d366f3273a66e9719b07ac999c80740d6277af88ceb4c3029c // a random value
+ parameter [REG_SIZE-1 : 0] GROUP_ORDER = 384'hffffffffffffffffffffffffffffffffffffffffffffffffc7634d81f4372ddf581a0db248b0a77aecec196accc52973
  )
  (
  // Clock and reset.
@@ -115,8 +114,7 @@ module ecc_hmac_drbg_interface#(
 
  hmac_drbg #(
  .REG_SIZE(REG_SIZE),
- .HMAC_DRBG_PRIME(GROUP_ORDER),
- .LFSR_INIT_SEED(LFSR_INIT_SEED)
+ .HMAC_DRBG_PRIME(GROUP_ORDER)
  ) 
  hmac_drbg_i (
  .clk(clk),
@@ -200,14 +198,14 @@ module ecc_hmac_drbg_interface#(
  scalar_rnd_reg <= '0;
  masking_rnd_reg <= '0;
  drbg_reg <= '0;
- lfsr_seed_reg <= LFSR_INIT_SEED;
+ lfsr_seed_reg <= '0;
  end
  else if (zeroize) begin
  lambda_reg <= '0;
  scalar_rnd_reg <= '0;
  masking_rnd_reg <= '0;
  drbg_reg <= '0;
- lfsr_seed_reg <= LFSR_INIT_SEED;
+ lfsr_seed_reg <= '0;
  end
  else
  if (hmac_done_edge) begin
@@ -223,7 +221,7 @@ module ecc_hmac_drbg_interface#(
  scalar_rnd_reg <= '0;
  masking_rnd_reg <= '0;
  drbg_reg <= '0;
- lfsr_seed_reg <= LFSR_INIT_SEED;
+ lfsr_seed_reg <= '0;
  end
  endcase
  end
@@ -263,8 +261,6 @@ module ecc_hmac_drbg_interface#(
  begin : counter_reg_update
  if (!reset_n)
  counter_reg <= '0;
- else if (zeroize)
- counter_reg <= '0;
  else
  counter_reg <= counter_reg + 1;
  end // counter_reg_update

diff --git a/src/ecc/rtl/ecc_pm_ctrl.sv b/src/ecc/rtl/ecc_pm_ctrl.sv
@@ -54,7 +54,6 @@ module ecc_pm_ctrl
 
  // from arith_unit
  input wire [3 : 0] ecc_cmd_i,
- input wire sca_en_i,
  input wire digit_i,
  output pm_instr_struct_t instr_o,
  output logic req_digit_o,
@@ -160,19 +159,19 @@ module ecc_pm_ctrl
  default : begin stalled <= 1'b0; stall_cntr <= '0; end
  endcase
  end
- else if ((!stalled) | (stalled & (stall_cntr == 0))) begin
+ else begin
  stalled <= 0;
  unique case (prog_cntr)
  NOP : begin // Waiting for new valid command
  ecc_cmd_reg <= ecc_cmd_i;
  unique case (ecc_cmd_i)
  KEYGEN_CMD : begin // keygen
- mont_cntr <= (sca_en_i)? Secp384_SCA_MONT_COUNT : Secp384_MONT_COUNT;
+ mont_cntr <= Secp384_SCA_MONT_COUNT;
  prog_cntr <= PM_INIT_G_S;
  end 
 
  SIGN_CMD : begin // signing
- mont_cntr <= (sca_en_i)? Secp384_SCA_MONT_COUNT : Secp384_MONT_COUNT;
+ mont_cntr <= Secp384_SCA_MONT_COUNT;
  prog_cntr <= PM_INIT_G_S;
  end 
 
@@ -194,7 +193,7 @@ module ecc_pm_ctrl
  end
 
  DH_SHARED_CMD : begin // DH shared key
- mont_cntr <= (sca_en_i)? Secp384_SCA_MONT_COUNT : Secp384_MONT_COUNT;
+ mont_cntr <= Secp384_SCA_MONT_COUNT;
  prog_cntr <= PM_INIT_DH_S;
  end
 

diff --git a/src/ecc/rtl/ecc_scalar_blinding.sv b/src/ecc/rtl/ecc_scalar_blinding.sv
@@ -60,7 +60,7 @@ module ecc_scalar_blinding #(
  // Equivalent to $ceil(REG_SIZE/RADIX) + 1
  localparam REG_DIG_NUM = (((REG_SIZE + RADIX) - 1) / RADIX) + 1; //13
  localparam RND_DIG_NUM = (((RND_SIZE + RADIX) - 1) / RADIX) + 1; //7
- localparam FULL_DIG_NUM = REG_DIG_NUM + RND_DIG_NUM; //20
+ localparam FULL_DIG_NUM = REG_DIG_NUM + RND_DIG_NUM - 1; //19
 
  localparam FULL_REG_SIZE = REG_DIG_NUM * RADIX;
  localparam FULL_RND_SIZE = RND_DIG_NUM * RADIX;
@@ -225,14 +225,14 @@ module ecc_scalar_blinding #(
 
  always_ff @(posedge clk or negedge reset_n) begin
  if (!reset_n) begin
- product_idx_reg <= FULL_DIG_NUM[P_ARR_WIDTH-1 : 0] - 1;
+ product_idx_reg <= FULL_DIG_NUM[P_ARR_WIDTH-1 : 0];
  operand_idx_reg <= '0;
  shift_state <= 0;
  add1_cin <= 0;
  carry_garbage_bits0 <= '0;
  end
  else if (zeroize) begin
- product_idx_reg <= FULL_DIG_NUM[P_ARR_WIDTH-1 : 0] - 1;
+ product_idx_reg <= FULL_DIG_NUM[P_ARR_WIDTH-1 : 0];
  operand_idx_reg <= '0;
  shift_state <= 0;
  add1_cin <= 0;
@@ -245,7 +245,7 @@ module ecc_scalar_blinding #(
  add1_cin <= 0;
  end
  else begin
- if (product_idx < (FULL_DIG_NUM-1)) begin
+ if (product_idx < FULL_DIG_NUM) begin
  if (shift_state) begin
  product_idx_reg <= product_idx + 1;
  if (product_idx < (REG_DIG_NUM-1))
@@ -272,7 +272,7 @@ module ecc_scalar_blinding #(
 
  assign accu_store = (accu_done)? 0 : (!shift_state);
  assign accu_shift = (accu_done)? 0 : shift_state;
- assign accu_done = (product_idx == (FULL_DIG_NUM-1));
+ assign accu_done = (product_idx == FULL_DIG_NUM);
 
  // Determines which a and b is pushed through the multiplier
  always_comb begin