KeyVault fixes and JTAG fix (#528)

* patch for kv exfiltration locking api registers from being modified by uc when data is loaded from the keyvault updating smoke tests to attempt to corrupt the kv data to test the lock * updating kv smoke test to use keyvault for block register * adding multi block hmac keyvault test content * updating keyvault section of the hardware spec to explicitly call out the key locking/clearing inside the crypto function Also detailing the requirement that each iteration of a multi block operation must program the keyvault read/write operation * corrected the expected tag to match the expected output of the hmac block * adding multiblock test to l0 and nightly directed regressions * preventing commands from being issued while key is being copied to the crypto engine * changing the masking to just cover the idle case, no need to check for data present * added busy signal to crypto engines with key access multiple busy signals trigger a fatal error zeroize keyvault reads when read has an error updated ras test to include testing crypto error case * adding new port for busy signals and crypto errors to all the unit level testbenches * fixing jtag aperture to allow access to veer jtag registers only when debug is unlocked jtag path to soc ifc registers is unchanged * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <[email protected]> * [ENV] Disable wget HSTS in ROM test (#527) * Disable HSTS (https is hardcoded in makefile, no MIM attack here) * MICROSOFT AUTOMATED PIPELINE: Stamp 'cwhitehead-msft-rom-wget-hsts-disable' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating hardware spec for crypto error fatal error fixing some typos and doc nits updating covergroups to include new crypto error fatal error bit * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Apply suggested feedback * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Removed multiple write scenarios in kv * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating register description for internal fw update reset wait cycle count to indicate that 5 is the minimum value allowed updating kv definition description to clarify that SHA is no longer a valid destination * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <[email protected]> Co-authored-by: Michael Norris <[email protected]> Co-authored-by: Michael Norris <[email protected]> Co-authored-by: Caleb <[email protected]> Co-authored-by: Caleb Whitehead <[email protected]> Co-authored-by: Kiran Upadhyayula <[email protected]>
chipsalliance · Jun 14, 2024 · 44bdcf5 · 44bdcf5
1 parent 77d2a15
commit 44bdcf5
Show file tree

Hide file tree

Showing 60 changed files with 1,087 additions and 200 deletions.
diff --git a/.github/workflow_metadata/pr_hash b/.github/workflow_metadata/pr_hash
@@ -1 +1 @@
-3e1ac9d915dbc1b961bea53177a00994a83506b8a048e76f9425fc5bdcbcee319af1d15feded88f64d73a7d08dffbdd1
+1e8bb6c06e021ead6816c6df79671ce4a6618b3af11abb6b9eab96d3ce7d13a43649b5e72d6ca4b9afc7c13c500d23d0
diff --git a/.github/workflow_metadata/pr_timestamp b/.github/workflow_metadata/pr_timestamp
@@ -1 +1 @@
-1718252040
+1718313162
diff --git a/docs/CaliptraHardwareSpecification.md b/docs/CaliptraHardwareSpecification.md
@@ -160,8 +160,8 @@ Vector 0 is reserved by the RISC-V processor and may not be used, so vector assi
 | ECC (Notifications)                                 | 4                | 7                                               |
 | HMAC (Errors)                                       | 5                | 8                                               |
 | HMAC (Notifications)                                | 6                | 7                                               |
-| KeyVault (Errors)                                   | 7                | 8                                               |
-| KeyVault (Notifications)                            | 8                | 7                                               |
+| Key Vault (Errors)                                  | 7                | 8                                               |
+| Key Vault (Notifications)                           | 8                | 7                                               |
 | SHA512 (Errors)                                     | 9                | 8                                               |
 | SHA512 (Notifications)                              | 10               | 7                                               |
 | SHA256 (Errors)                                     | 11               | 8                                               |
@@ -430,7 +430,7 @@ Caliptra provides a clock gating feature that turns off clocks when the microcon
 | :------------------- | :---------------- | :------------------------ |
 | CPTRA_CLK_GATING_EN  | 0x300300bc        | Register bit to enable or disable the clock gating feature. |
 
-When enabled, halting the microcontroller turns off clocks to all of the cryptographic subsystem, the vaults (keyvault, PCR vault, and data vault), mailbox SRAM, SoC interface, and peripherals subsystem. The Watchdog timer and SoC registers run on the gated RDC clock. The RV core implements its own clock gating mechanism. Halting the core automatically turns off its clock.
+When enabled, halting the microcontroller turns off clocks to all of the cryptographic subsystem, the vaults (key vault, PCR vault, and data vault), mailbox SRAM, SoC interface, and peripherals subsystem. The Watchdog timer and SoC registers run on the gated RDC clock. The RV core implements its own clock gating mechanism. Halting the core automatically turns off its clock.
 
 There are a total of 4 clocks in Caliptra: ungated clock, gated clock, gated RDC clock, and gated SoC IFC clock. The following table shows the different modules and their designated clocks.
 
@@ -1456,10 +1456,10 @@ FW must set a last cycle flag before running the last iteration of the SHA engin
 
 Key Vault (KV) is a register file that stores the keys to be used by the microcontroller, but this register file is not observed by the microcontroller. Each cryptographic function has a control register and functional block designed to read from and write to the KV.  
 
-| KV register                      | Description                                              |
-| :------------------------------- | :------------------------------------------------------- |
-| Key Control\[7:0\]               | 8 Control registers, 32 bits each                        |
-| Key Entry\[7:0\]\[15:0\]\[31:0\] | 8 Key entries, 512 bits each <br>No read or write access |
+| KV register                       | Description                                               |
+| :-------------------------------- | :-------------------------------------------------------- |
+| Key Control\[31:0\]               | 32 Control registers, 32 bits each                        |
+| Key Entry\[31:0\]\[11:0\]\[31:0\] | 32 Key entries, 384 bits each <br>No read or write access |
 
 ### Key vault functional block
 
@@ -1485,14 +1485,18 @@ The destination valid field is programmed by FW in the cryptographic block gener
 
 A generic block is instantiated in each cryptographic block to enable access to KV. 
 
-Each input to a cryptographic engine can have a key vault read block associated with it. The KV read block takes in a keyvault read control register that drives an FSM to copy an entry from the keyvault into the appropriate input register of the cryptographic engine.
+Each input to a cryptographic engine can have a key vault read block associated with it. The KV read block takes in a key vault read control register that drives an FSM to copy an entry from the key vault into the appropriate input register of the cryptographic engine.
 
-Each output generated by a cryptographic engine can have its result copied to a slot in the keyvault. The KV write block takes in a keyvault write control register. This register drives an FSM to copy the result from the cryptographic engine into the appropriate keyvault entry. It also programs a control field for that entry to indicate where that entry can be used.
+Each output generated by a cryptographic engine can have its result copied to a slot in the key vault. The KV write block takes in a key vault write control register. This register drives an FSM to copy the result from the cryptographic engine into the appropriate key vault entry. It also programs a control field for that entry to indicate where that entry can be used.
 
 After programming the key vault read control, FW needs to query the associated key vault read status to confirm that the requested key was copied successfully. After valid is set and the error field reports success, the key is ready to be used.
 
 Similarly, after programming the key vault write control and initiating the cryptographic function that generates the key to be written, FW needs to query the associated key vault write status to confirm that the requested key was generated and written successfully.
 
+When a key is read from the key vault, the API register is locked and any result generated from the cryptographic block is not readable by firmware. The digest can only be sent to the key vault by appropriately programming the key vault write controls. After the cryptographic block completes its operation, the lock is cleared and the key is cleared from the API registers.
+
+If multiple iterations of the cryptographic function are required, the key vault read and write controls must be programmed for each iteration. This ensures that the lock is set and the digest is not readable.
+
 The following tables describe read, write, and status values for key vault blocks.
 
 | KV Read Ctrl Reg     | Description                                                                                                                            |
@@ -1547,11 +1551,11 @@ A de-obfuscation engine (DOE) is used in conjunction with AES cryptography to de
 
 The following tables describe DOE register and control fields.
 
-| DOE Register | Address    | Description                                                                                                                    |
-| :----------- | :--------- | :----------------------------------------------------------------------------------------------------------------------------- |
-| IV           | 0x10000000 | 128 bit IV for DOE flow. Stored in big-endian representation.                                                                  |
-| CTRL         | 0x10000010 | Controls for DOE flows.                                                                                                        |
-| STATUS       | 0x10000014 | Valid indicates the command is done and results are stored in keyvault. Ready indicates the core is ready for another command. |
+| DOE Register | Address    | Description                                                                                                                     |
+| :----------- | :--------- | :------------------------------------------------------------------------------------------------------------------------------ |
+| IV           | 0x10000000 | 128 bit IV for DOE flow. Stored in big-endian representation.                                                                   |
+| CTRL         | 0x10000010 | Controls for DOE flows.                                                                                                         |
+| STATUS       | 0x10000014 | Valid indicates the command is done and results are stored in key vault. Ready indicates the core is ready for another command. |
 
 | DOE Ctrl Fields  | Reset        | Description                                                                                                                                  |
 | :--------------- | :----------- | :------------------------------------------------------------------------------------------------------------------------------------------- |
@@ -1581,9 +1585,10 @@ Data vault is a set of generic scratch pad registers with specific lock function
 
 The following table describes cryptographic errors. 
 
-| Errors     | Error type         | Description                                                                                                                                               |
-| :--------- | :----------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| ECC_R_ZERO | HW_ERROR_NON_FATAL | Indicates a non-fatal error in ECC signing if the computed signature R is equal to 0. FW should change the message or privkey to perform a valid signing. |
+| Errors       | Error type         | Description                                                                                                                                               |
+| :----------- | :----------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| ECC_R_ZERO   | HW_ERROR_NON_FATAL | Indicates a non-fatal error in ECC signing if the computed signature R is equal to 0. FW should change the message or privkey to perform a valid signing. |
+| CRYPTO_ERROR | HW_ERROR_FATAL     | Indicates a fatal error due to multiple cryptographic operations occurring simultaneously. FW must only operate one cryptographic engine at a time.       |
 
 # Terminology
 

diff --git a/src/doe/rtl/doe_cbc.sv b/src/doe/rtl/doe_cbc.sv
@@ -66,6 +66,8 @@ module doe_cbc
 
    output logic clear_obf_secrets,
 
+  output logic busy_o,
+
    //interface with kv
    output kv_write_t kv_write,
    input  logic debugUnlock_or_scan_mode_switch
@@ -287,6 +289,8 @@ doe_reg i_doe_reg (
 assign error_intr = hwif_out.intr_block_rf.error_global_intr_r.intr;
 assign notif_intr = hwif_out.intr_block_rf.notif_global_intr_r.intr;
 
+always_comb busy_o = flow_in_progress | ~core_ready;
+
 endmodule // doe
 
 //======================================================================

diff --git a/src/doe/rtl/doe_ctrl.sv b/src/doe/rtl/doe_ctrl.sv
@@ -59,6 +59,8 @@ module doe_ctrl
 
     output logic clear_obf_secrets,
 
+    output logic busy_o,
+
     // Interrupt
     output logic error_intr,
     output logic notif_intr,
@@ -91,6 +93,7 @@ module doe_ctrl
         .read_data(doe_read_data[31:0]),
         .error_intr(error_intr),
         .notif_intr(notif_intr),
+        .busy_o(busy_o),
         .clear_obf_secrets(clear_obf_secrets),
         .kv_write(kv_write),
         .debugUnlock_or_scan_mode_switch(debugUnlock_or_scan_mode_switch)

diff --git a/src/doe/tb/doe_cbc_tb.sv b/src/doe/tb/doe_cbc_tb.sv
@@ -148,7 +148,7 @@ module doe_cbc_tb();
              .obf_uds_seed(obf_uds_seed),
 
              .kv_write(),
-
+             .busy_o(),
              .error_intr(),
              .notif_intr()
             );

diff --git a/src/ecc/rtl/ecc_dsa_ctrl.sv b/src/ecc/rtl/ecc_dsa_ctrl.sv
@@ -79,6 +79,8 @@ module ecc_dsa_ctrl
     //PCR Signing
     input pcr_signing_t pcr_signing_data,
 
+    output logic busy_o,
+
     // Interrupts (from ecc_reg)
     output logic error_intr,
     output logic notif_intr,
@@ -178,9 +180,12 @@ module ecc_dsa_ctrl
     logic kv_privkey_ready, kv_privkey_done;
     logic kv_seed_ready, kv_seed_done ;
     logic kv_write_ready, kv_write_done;
-    //KV Read Data Present
-    logic kv_read_data_present;
-    logic kv_read_data_present_set, kv_read_data_present_reset;
+    //KV Seed Data Present
+    logic kv_seed_data_present;
+    logic kv_seed_data_present_set, kv_seed_data_present_reset;
+    //KV Key Data Present
+    logic kv_key_data_present;
+    logic kv_key_data_present_set, kv_key_data_present_reset;
 
     kv_read_ctrl_reg_t kv_privkey_read_ctrl_reg;
     kv_read_ctrl_reg_t kv_seed_read_ctrl_reg;
@@ -309,7 +314,8 @@ module ecc_dsa_ctrl
 
     // read the registers written by sw
     always_comb begin
-        cmd_reg = hwif_out.ECC_CTRL.CTRL.value;
+        //Mask the command if KV clients are not idle
+        cmd_reg = hwif_out.ECC_CTRL.CTRL.value & {2{kv_seed_ready}} & {2{kv_privkey_ready}};
         zeroize_reg = hwif_out.ECC_CTRL.ZEROIZE.value || debugUnlock_or_scan_mode_switch;
 
         sca_point_rnd_en  = 1'b1;
@@ -324,23 +330,27 @@ module ecc_dsa_ctrl
             privkey_we_reg      <= '0;
             privkey_we_reg_ff   <= '0;
             kv_reg    <= '0;
-            kv_read_data_present <= '0;
+            kv_seed_data_present <= '0;
+            kv_key_data_present <= '0;
         end
         else if (zeroize_reg) begin
             privkey_we_reg      <= '0;
             privkey_we_reg_ff   <= '0;
             kv_reg    <= '0;
-            kv_read_data_present <= '0;
+            kv_seed_data_present <= '0;
+            kv_key_data_present <= '0;
         end
         //Store private key here before pushing to keyvault
         else begin
             privkey_we_reg <= hw_privkey_we;
             privkey_we_reg_ff <= privkey_we_reg;
-            if (privkey_out_we & (dest_keyvault | kv_read_data_present))
+            if (privkey_out_we & (dest_keyvault | kv_seed_data_present))
                 kv_reg <= read_reg;
 
-            kv_read_data_present <= kv_read_data_present_set ? '1 :
-                                    kv_read_data_present_reset ? '0 : kv_read_data_present;
+            kv_seed_data_present <= kv_seed_data_present_set ? '1 :
+                                    kv_seed_data_present_reset ? '0 : kv_seed_data_present;
+            kv_key_data_present  <= kv_key_data_present_set ? '1 :
+                                    kv_key_data_present_reset ? '0 : kv_key_data_present;
         end
     end
 
@@ -368,13 +378,16 @@ module ecc_dsa_ctrl
             //don't store the private key generated in sw accessible register if it's going to keyvault
             privkey_reg[dword] = hwif_out.ECC_PRIVKEY_IN[11-dword].PRIVKEY_IN.value;
             hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.we = (pcr_sign_mode | (kv_privkey_write_en & (kv_privkey_write_offset == dword))) & !zeroize_reg;
-            hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.next = pcr_sign_mode ? pcr_signing_data.pcr_signing_privkey[dword] : kv_privkey_write_en? kv_privkey_write_data : read_reg[11-dword];
-            hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.hwclr = zeroize_reg;
+            hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.next = pcr_sign_mode       ? pcr_signing_data.pcr_signing_privkey[dword] : 
+                                                            kv_privkey_write_en ? kv_privkey_write_data : 
+                                                                                  read_reg[11-dword];
+            hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.hwclr = zeroize_reg | kv_key_data_present_reset | (kv_privkey_error == KV_READ_FAIL);
+            hwif_in.ECC_PRIVKEY_IN[dword].PRIVKEY_IN.swwe = dsa_ready_reg & ~kv_key_data_present;
         end 
 
         for (int dword=0; dword < 12; dword++)begin
             //If keyvault is not enabled, grab the sw value as usual
-            hwif_in.ECC_PRIVKEY_OUT[dword].PRIVKEY_OUT.we = (privkey_out_we & ~(dest_keyvault | kv_read_data_present)) & !zeroize_reg;
+            hwif_in.ECC_PRIVKEY_OUT[dword].PRIVKEY_OUT.we = (privkey_out_we & ~(dest_keyvault | kv_seed_data_present)) & !zeroize_reg;
             hwif_in.ECC_PRIVKEY_OUT[dword].PRIVKEY_OUT.next = read_reg[11-dword];
             hwif_in.ECC_PRIVKEY_OUT[dword].PRIVKEY_OUT.hwclr = zeroize_reg;
         end
@@ -383,7 +396,8 @@ module ecc_dsa_ctrl
             seed_reg[dword] = hwif_out.ECC_SEED[11-dword].SEED.value;
             hwif_in.ECC_SEED[dword].SEED.we = (kv_seed_write_en & (kv_seed_write_offset == dword)) & !zeroize_reg;
             hwif_in.ECC_SEED[dword].SEED.next = kv_seed_write_data;
-            hwif_in.ECC_SEED[dword].SEED.hwclr = zeroize_reg | kv_read_data_present_reset;
+            hwif_in.ECC_SEED[dword].SEED.hwclr = zeroize_reg | kv_seed_data_present_reset | (kv_seed_error == KV_READ_FAIL);
+            hwif_in.ECC_SEED[dword].SEED.swwe  = dsa_ready_reg & ~kv_seed_data_present;
         end
 
         for (int dword=0; dword < 12; dword++)begin
@@ -454,7 +468,7 @@ module ecc_dsa_ctrl
     end
 
 
-    always_comb hwif_in.ECC_CTRL.CTRL.hwclr = |hwif_out.ECC_CTRL.CTRL.value;
+    always_comb hwif_in.ECC_CTRL.CTRL.hwclr = |cmd_reg;
     always_comb hwif_in.ECC_CTRL.PCR_SIGN.hwclr = hwif_out.ECC_CTRL.PCR_SIGN.value;
 
     // TODO add other interrupt hwset signals (errors)
@@ -490,9 +504,12 @@ module ecc_dsa_ctrl
     `CALIPTRA_KV_READ_CTRL_REG2STRUCT(kv_seed_read_ctrl_reg, ecc_kv_rd_seed_ctrl)
     `CALIPTRA_KV_WRITE_CTRL_REG2STRUCT(kv_write_ctrl_reg, ecc_kv_wr_pkey_ctrl)
 
-    //Force result into KV reg whenever source came from KV
-    always_comb kv_read_data_present_set = kv_seed_read_ctrl_reg.read_en;
-    always_comb kv_read_data_present_reset = kv_read_data_present & privkey_out_we;
+    //Detect keyvault data coming in to lock api registers and protect outputs
+    always_comb kv_seed_data_present_set = kv_seed_read_ctrl_reg.read_en;
+    always_comb kv_seed_data_present_reset = kv_seed_data_present & ecc_status_done_p;
+
+    always_comb kv_key_data_present_set = kv_privkey_read_ctrl_reg.read_en;
+    always_comb kv_key_data_present_reset = kv_key_data_present & ecc_status_done_p;
 
     always_comb pcr_sign_mode = hwif_out.ECC_CTRL.PCR_SIGN.value;
 
@@ -900,4 +917,6 @@ module ecc_dsa_ctrl
         .dest_done(kv_write_done)
     );
 
+always_comb busy_o = ~dsa_ready_reg | ~kv_write_ready | ~kv_seed_ready | ~kv_privkey_ready;
+
 endmodule
diff --git a/src/ecc/rtl/ecc_reg.rdl b/src/ecc/rtl/ecc_reg.rdl
@@ -112,7 +112,7 @@ addrmap ecc_reg {
         default hw = rw;
         default we = true;
         default resetsignal = reset_b;
-        field {desc = "Input seed field"; swwe = ecc_ready; hwclr;} SEED[32] = 32'b0;
+        field {desc = "Input seed field"; swwe; hwclr;} SEED[32] = 32'b0;
 
     } ECC_SEED[12] @0x00000080;
 
@@ -181,7 +181,7 @@ addrmap ecc_reg {
         default hw = rw;
         default we = true;
         default resetsignal = reset_b;
-        field {desc = "Private key field"; swwe = ecc_ready; hwclr;} PRIVKEY_IN[32] = 32'b0;
+        field {desc = "Private key field"; swwe; hwclr;} PRIVKEY_IN[32] = 32'b0;
 
     } ECC_PRIVKEY_IN[12] @0x00000580;