Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[keymgr, rtl] Reorganize keymgr advance payloads #22878

Merged
merged 1 commit into from
May 26, 2024

Conversation

ballifatih
Copy link
Contributor

I am having some issues with my environment and I need to runt this through CI.

It basically tries to address to #22565 by:

  • Not introducing extra seeds that incur extra area.
  • Cleaner diversifications for Keymgr calls, so that we can claim no message collisions for different use cases.

@ballifatih ballifatih requested a review from a team as a code owner April 29, 2024 13:40
@ballifatih ballifatih requested review from matutem and removed request for a team and matutem April 29, 2024 13:40
@ballifatih
Copy link
Contributor Author

Visual description of how advance payloads change with this PR:
image

Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx @ballifatih, RTL and DV change LGTM.

In the RTL, this is as simple as adding the revision seed to the material used for advancing into the OwnerIntermediate key and the Owner key.

DV seems to have been properly updated to cover this change: I ran util/dvsim/dvsim.py hw/ip/keymgr/dv/keymgr_sim_cfg.hjson -i all -rx 0.5 and got the usual pass rates.

That leaves the spec side, where I'd like explicit approval from @moidx and for which we should update doc/security/specs/identities_and_root_keys/identities_and_root_keys_DICE_fig1b.svg if we decide to merge this

Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me, thanks @ballifatih for working out the change and making sure DV can deal with this, too. I am supportive of merging this but don't want to make the final call as I don't know the design / relevant spec that well.

A solution to the issue raised in lowRISC#22565, which does the following
changes:

* Use REVISION_SEED only for 1st advance as the first block.
* Use CREATOR_ADV_SEED only for 2nd advance as the first block.
* Use OWNER_ADV_SEED only for 3rd advance as the first block.

 thereby ensuring that domain separation for KDF-KMAC is done
 correctly.

Signed-off-by: Fatih Balli <[email protected]>
@ballifatih ballifatih changed the title [keymgr, rtl] Reuse revision_seed in advance calls [keymgr, rtl] Reorganize keymgr advance payloads May 24, 2024
@ballifatih
Copy link
Contributor Author

Block-level DV results look similar to without this PR.
./util/dvsim/dvsim.py hw/ip/keymgr/dv/keymgr_sim_cfg.hjson -i all:

### Test Results
|  Stage  |                   Name                    | Tests                                |  Max Job Runtime  |  Simulated Time  |  Passing  |  Total  |  Pass Rate  |
|:-------:|:-----------------------------------------:|:-------------------------------------|:-----------------:|:----------------:|:---------:|:-------:|:-----------:|
|   V1    |                   smoke                   | keymgr_smoke                         |      41.740s      |     14.564ms     |    50     |   50    |  100.00 %   |
|   V1    |                  random                   | keymgr_random                        |      28.230s      |     1.542ms      |    50     |   50    |  100.00 %   |
|   V1    |               csr_hw_reset                | keymgr_csr_hw_reset                  |      1.150s       |    261.344us     |     5     |    5    |  100.00 %   |
|   V1    |                  csr_rw                   | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|   V1    |               csr_bit_bash                | keymgr_csr_bit_bash                  |      9.930s       |     1.041ms      |     5     |    5    |  100.00 %   |
|   V1    |               csr_aliasing                | keymgr_csr_aliasing                  |      9.740s       |     4.560ms      |     5     |    5    |  100.00 %   |
|   V1    |        csr_mem_rw_with_rand_reset         | keymgr_csr_mem_rw_with_rand_reset    |      1.740s       |     48.792us     |    20     |   20    |  100.00 %   |
|   V1    | regwen_csr_and_corresponding_lockable_csr | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_csr_aliasing                  |      9.740s       |     4.560ms      |     5     |    5    |  100.00 %   |
|   V1    |                                           | **TOTAL**                            |                   |                  |    155    |   155   |  100.00 %   |
|   V2    |              cfgen_during_op              | keymgr_cfg_regwen                    |      1.320m       |     2.120ms      |    50     |   50    |  100.00 %   |
|   V2    |                 sideload                  | keymgr_sideload                      |      48.240s      |     12.023ms     |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_sideload_kmac                 |      36.690s      |     1.656ms      |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_sideload_aes                  |      40.360s      |     3.030ms      |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_sideload_otbn                 |      20.650s      |     1.028ms      |    50     |   50    |  100.00 %   |
|   V2    |         direct_to_disabled_state          | keymgr_direct_to_disabled            |      16.320s      |     4.041ms      |    50     |   50    |  100.00 %   |
|   V2    |                lc_disable                 | keymgr_lc_disable                    |      11.290s      |    458.432us     |    50     |   50    |  100.00 %   |
|   V2    |            kmac_error_response            | keymgr_kmac_rsp_err                  |      14.440s      |     5.066ms      |    50     |   50    |  100.00 %   |
|   V2    |             invalid_sw_input              | keymgr_sw_invalid_input              |      25.420s      |     1.138ms      |    50     |   50    |  100.00 %   |
|   V2    |             invalid_hw_input              | keymgr_hwsw_invalid_input            |      1.134m       |     24.091ms     |    50     |   50    |  100.00 %   |
|   V2    |          sync_async_fault_cross           | keymgr_sync_async_fault_cross        |      19.540s      |     10.253ms     |    49     |   50    |   98.00 %   |
|   V2    |                stress_all                 | keymgr_stress_all                    |      4.776m       |     20.774ms     |    48     |   50    |   96.00 %   |
|   V2    |                 intr_test                 | keymgr_intr_test                     |      0.700s       |     17.619us     |    50     |   50    |  100.00 %   |
|   V2    |                alert_test                 | keymgr_alert_test                    |      0.820s       |     42.778us     |    50     |   50    |  100.00 %   |
|   V2    |           tl_d_oob_addr_access            | keymgr_tl_errors                     |      5.320s       |    204.383us     |    20     |   20    |  100.00 %   |
|   V2    |            tl_d_illegal_access            | keymgr_tl_errors                     |      5.320s       |    204.383us     |    20     |   20    |  100.00 %   |
|   V2    |          tl_d_outstanding_access          | keymgr_csr_hw_reset                  |      1.150s       |    261.344us     |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_csr_aliasing                  |      9.740s       |     4.560ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_same_csr_outstanding          |      3.760s       |    369.938us     |    20     |   20    |  100.00 %   |
|   V2    |            tl_d_partial_access            | keymgr_csr_hw_reset                  |      1.150s       |    261.344us     |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|         |                                           | keymgr_csr_aliasing                  |      9.740s       |     4.560ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_same_csr_outstanding          |      3.760s       |    369.938us     |    20     |   20    |  100.00 %   |
|   V2    |                                           | **TOTAL**                            |                   |                  |    737    |   740   |   99.59 %   |
|   V2S   |          sec_cm_additional_check          | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |                tl_intg_err                | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|         |                                           | keymgr_tl_intg_err                   |      7.800s       |    832.666us     |    20     |   20    |  100.00 %   |
|   V2S   |          shadow_reg_update_error          | keymgr_shadow_reg_errors             |      3.700s       |    327.514us     |    20     |   20    |  100.00 %   |
|   V2S   |    shadow_reg_read_clear_staged_value     | keymgr_shadow_reg_errors             |      3.700s       |    327.514us     |    20     |   20    |  100.00 %   |
|   V2S   |         shadow_reg_storage_error          | keymgr_shadow_reg_errors             |      3.700s       |    327.514us     |    20     |   20    |  100.00 %   |
|   V2S   |           shadowed_reset_glitch           | keymgr_shadow_reg_errors             |      3.700s       |    327.514us     |    20     |   20    |  100.00 %   |
|   V2S   |    shadow_reg_update_error_with_csr_rw    | keymgr_shadow_reg_errors_with_csr_rw |      13.710s      |     1.800ms      |    20     |   20    |  100.00 %   |
|   V2S   |             prim_count_check              | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |              prim_fsm_check               | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |           sec_cm_bus_integrity            | keymgr_tl_intg_err                   |      7.800s       |    832.666us     |    20     |   20    |  100.00 %   |
|   V2S   |           sec_cm_config_shadow            | keymgr_shadow_reg_errors             |      3.700s       |    327.514us     |    20     |   20    |  100.00 %   |
|   V2S   |          sec_cm_op_config_regwen          | keymgr_cfg_regwen                    |      1.320m       |     2.120ms      |    50     |   50    |  100.00 %   |
|   V2S   |        sec_cm_reseed_config_regwen        | keymgr_random                        |      28.230s      |     1.542ms      |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|   V2S   |      sec_cm_sw_binding_config_regwen      | keymgr_random                        |      28.230s      |     1.542ms      |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|   V2S   |     sec_cm_max_key_ver_config_regwen      | keymgr_random                        |      28.230s      |     1.542ms      |    50     |   50    |  100.00 %   |
|         |                                           | keymgr_csr_rw                        |      1.230s       |     31.016us     |    20     |   20    |  100.00 %   |
|   V2S   |       sec_cm_lc_ctrl_intersig_mubi        | keymgr_lc_disable                    |      11.290s      |    458.432us     |    50     |   50    |  100.00 %   |
|   V2S   |       sec_cm_constants_consistency        | keymgr_hwsw_invalid_input            |      1.134m       |     24.091ms     |    50     |   50    |  100.00 %   |
|   V2S   |        sec_cm_intersig_consistency        | keymgr_hwsw_invalid_input            |      1.134m       |     24.091ms     |    50     |   50    |  100.00 %   |
|   V2S   |         sec_cm_hw_key_sw_noaccess         | keymgr_random                        |      28.230s      |     1.542ms      |    50     |   50    |  100.00 %   |
|   V2S   |       sec_cm_output_keys_ctrl_redun       | keymgr_sideload_protect              |      22.520s      |     3.701ms      |    49     |   50    |   98.00 %   |
|   V2S   |          sec_cm_ctrl_fsm_sparse           | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |          sec_cm_data_fsm_sparse           | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |         sec_cm_ctrl_fsm_local_esc         | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |        sec_cm_ctrl_fsm_consistency        | keymgr_custom_cm                     |      21.260s      |    796.579us     |    50     |   50    |  100.00 %   |
|   V2S   |        sec_cm_ctrl_fsm_global_esc         | keymgr_lc_disable                    |      11.290s      |    458.432us     |    50     |   50    |  100.00 %   |
|   V2S   |           sec_cm_ctrl_ctr_redun           | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |         sec_cm_kmac_if_fsm_sparse         | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |         sec_cm_kmac_if_ctr_redun          | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |    sec_cm_kmac_if_cmd_ctrl_consistency    | keymgr_custom_cm                     |      21.260s      |    796.579us     |    50     |   50    |  100.00 %   |
|   V2S   |   sec_cm_kmac_if_done_ctrl_consistency    | keymgr_custom_cm                     |      21.260s      |    796.579us     |    50     |   50    |  100.00 %   |
|   V2S   |          sec_cm_reseed_ctr_redun          | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |   sec_cm_side_load_sel_ctrl_consistency   | keymgr_custom_cm                     |      21.260s      |    796.579us     |    50     |   50    |  100.00 %   |
|   V2S   |      sec_cm_sideload_ctrl_fsm_sparse      | keymgr_sec_cm                        |      8.140s       |     1.053ms      |     5     |    5    |  100.00 %   |
|   V2S   |         sec_cm_ctrl_key_integrity         | keymgr_custom_cm                     |      21.260s      |    796.579us     |    50     |   50    |  100.00 %   |
|   V2S   |                                           | **TOTAL**                            |                   |                  |    164    |   165   |   99.39 %   |
|   V3    |        stress_all_with_rand_reset         | keymgr_stress_all_with_rand_reset    |      22.040s      |     1.167ms      |    30     |   50    |   60.00 %   |
|   V3    |                                           | **TOTAL**                            |                   |                  |    30     |   50    |   60.00 %   |
|         |                                           | **TOTAL**                            |                   |                  |   1086    |  1110   |   97.84 %   |

@ballifatih
Copy link
Contributor Author

chip_sw_keymgr_key_derivation and chip_sw_keymgr_sideload_kmac results also look good:

./util/dvsim/dvsim.py hw/top_earlgrey/dv/chip_sim_cfg.hjson -i chip_sw_keymgr_key_derivation
INFO: [dvsim] [proj_root]: /workspaces/repo
INFO: [SimCfg] [scratch_path]: [chip] [/workspaces/repo/scratch/keymgr_reorg_cnsts/chip_earlgrey_asic-sim-vcs]
ERROR: [Regression] Test "chip_plic_all_irqs" added to regression "xcelium_ci_0" not found!
ERROR: [Regression] Test "chip_sw_aes_prng_reseed" added to regression "V2" not found!
ERROR: [Regression] Test "chip_plic_all_irqs" added to regression "V2" not found!
ERROR: [Regression] Test "chip_sw_entropy_src_fuse_en_fw_read_test" added to regression "V2" not found!
ERROR: [Regression] Test "chip_sw_aes_force_prng_reseed" added to regression "V2" not found!
ERROR: [Regression] Test "//sw/device/tests:spi_passthru_test" added to regression "V3" not found!
ERROR: [Regression] Test "//sw/device/tests:i2c_host_override_test" added to regression "V3" not found!
ERROR: [Regression] Test "rom_e2e_self_hash" added to regression "V3" not found!
INFO: [FlowCfg] [results]: [chip]:
## CHIP Simulation Results
### Friday May 24 2024 23:11:29 UTC
### GitHub Revision: [`84bc81e436`](https://github.com/lowrisc/opentitan/tree/84bc81e436830200090a9581257b34064bda2239)<br>Foundry Revision: `0e5d1ff`
### Branch: keymgr_reorg_cnsts
### [Testplan](https://opentitan.org/book/hw/top_earlgrey/data/chip_testplan.html)
### Simulator: VCS

### Test Results
|  Stage  |             Name              | Tests                         |  Max Job Runtime  |  Simulated Time  |  Passing  |  Total  |  Pass Rate  |
|:-------:|:-----------------------------:|:------------------------------|:-----------------:|:----------------:|:---------:|:-------:|:-----------:|
|   V2    |  chip_sw_flash_keymgr_seeds   | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    | chip_sw_keymgr_key_derivation | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    |    chip_sw_kmac_app_keymgr    | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    |   chip_sw_lc_ctrl_broadcast   | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    |     chip_sw_otp_ctrl_keys     | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    |   chip_sw_otp_ctrl_entropy    | chip_sw_keymgr_key_derivation |      19.868m      |     10.988ms     |     3     |    3    |  100.00 %   |
|   V2    |                               | **TOTAL**                     |                   |                  |     3     |    3    |  100.00 %   |
|         |                               | **TOTAL**                     |                   |                  |     3     |    3    |  100.00 %   |


INFO: [FlowCfg] [scratch_path]: [chip] [/workspaces/repo/scratch/keymgr_reorg_cnsts/chip_earlgrey_asic-sim-vcs]

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                                                                                              
00:03:47  [    build    ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%                                                                                                                                                                                          
00:24:40  [     run     ]: [Q: 0, D: 0, P: 3, F: 0, K: 0, T: 3] 100%                                                                                                                                                                                          



./util/dvsim/dvsim.py hw/top_earlgrey/dv/chip_sim_cfg.hjson -i chip_sw_keymgr_sideload_kmac
INFO: [dvsim] [proj_root]: /workspaces/repo
INFO: [SimCfg] [scratch_path]: [chip] [/workspaces/repo/scratch/keymgr_reorg_cnsts/chip_earlgrey_asic-sim-vcs]
ERROR: [Regression] Test "chip_plic_all_irqs" added to regression "xcelium_ci_0" not found!
ERROR: [Regression] Test "chip_sw_aes_prng_reseed" added to regression "V2" not found!
ERROR: [Regression] Test "chip_plic_all_irqs" added to regression "V2" not found!
ERROR: [Regression] Test "chip_sw_entropy_src_fuse_en_fw_read_test" added to regression "V2" not found!
ERROR: [Regression] Test "chip_sw_aes_force_prng_reseed" added to regression "V2" not found!
ERROR: [Regression] Test "//sw/device/tests:spi_passthru_test" added to regression "V3" not found!
ERROR: [Regression] Test "rom_e2e_self_hash" added to regression "V3" not found!
ERROR: [Regression] Test "//sw/device/tests:i2c_host_override_test" added to regression "V3" not found!
INFO: [FlowCfg] [results]: [chip]:
## CHIP Simulation Results
### Friday May 24 2024 23:50:26 UTC
### GitHub Revision: [`84bc81e436`](https://github.com/lowrisc/opentitan/tree/84bc81e436830200090a9581257b34064bda2239)<br>Foundry Revision: `0e5d1ff`
### Branch: keymgr_reorg_cnsts
### [Testplan](https://opentitan.org/book/hw/top_earlgrey/data/chip_testplan.html)
### Simulator: VCS

### Test Results
|  Stage  |             Name             | Tests                        |  Max Job Runtime  |  Simulated Time  |  Passing  |  Total  |  Pass Rate  |
|:-------:|:----------------------------:|:-----------------------------|:-----------------:|:----------------:|:---------:|:-------:|:-----------:|
|   V2    | chip_sw_keymgr_sideload_kmac | chip_sw_keymgr_sideload_kmac |      21.328m      |     12.557ms     |     3     |    3    |  100.00 %   |
|   V2    |                              | **TOTAL**                    |                   |                  |     3     |    3    |  100.00 %   |
|         |                              | **TOTAL**                    |                   |                  |     3     |    3    |  100.00 %   |


INFO: [FlowCfg] [scratch_path]: [chip] [/workspaces/repo/scratch/keymgr_reorg_cnsts/chip_earlgrey_asic-sim-vcs]

          [   legend    ]: [Q: queued, D: dispatched, P: passed, F: failed, K: killed, T: total]                                                                                                                                                              
00:02:48  [    build    ]: [Q: 0, D: 0, P: 1, F: 0, K: 0, T: 1] 100%                                                                                                                                                                                          
00:24:51  [     run     ]: [Q: 0, D: 0, P: 3, F: 0, K: 0, T: 3] 100% 

@ballifatih
Copy link
Contributor Author

With the last push, keymgr advance messages are organized as below:

image

@moidx
Copy link
Contributor

moidx commented May 25, 2024

Thanks @ballifatih!

@andreaskurth andreaskurth merged commit 2cf28c4 into lowRISC:master May 26, 2024
32 checks passed
@ballifatih ballifatih deleted the keymgr_reorg_cnsts branch May 27, 2024 12:14
ballifatih added a commit to ballifatih/opentitan that referenced this pull request Aug 23, 2024
For better alignment with the NIST standards, we
have updated how diversification inputs are consumed
during advance calls. The RTL/DV change was resolved
in lowRISC#22878.

This commit updates the identity and keys documentation
to match these previously merged changes.

Signed-off-by: Fatih Balli <[email protected]>
timothytrippel pushed a commit that referenced this pull request Oct 2, 2024
For better alignment with the NIST standards, we
have updated how diversification inputs are consumed
during advance calls. The RTL/DV change was resolved
in #22878.

This commit updates the identity and keys documentation
to match these previously merged changes.

Signed-off-by: Fatih Balli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants