[keymgr] D2(S) Signoff

[msfschaffner]

Description

Ensure D2(S) signoff criteria are still maintained (this is not a focus area block).

[andreaskurth]

Commits since Earlgrey-ES tapeout

git rev-parse HEAD

c73929e4d6e864bad72c456b265a089225a5c8e2

git log Earlgrey-M2.5.2-RC0..HEAD --oneline hw/ip/keymgr

• 52da8f707d [keymgr] Add missing CM annotation --> RTL changed but only a CM label added
• 0d14dd693f [docs] Fix repetitions of the definite article --> no RTL changed; spelling fixed in spec
• 56399241e7 Revert "[edn] Move prim_edn_req out of prim" --> reverts 3b4e36e01c below
• c721c51c13 [rtl, prim] Add 'commit' functionality to prim_count --> RTL changed but no functional difference
• e81b5885e5 [keymgr/otp_ctrl] Add support for creator/owner seeds --> RTL changes with minimal functional impact for keymgr:
  1. In otp_ctrl_pkg::otp_keymgr_key_t the creator root key shares got renamed and the valid signal got split into one valid signal per share.
  2. keymgr can now take the creator and owner seed from flash instead of OTP iff the UseOtpSeedsInsteadOfFlash synthesis parameter is enabled, which is disabled in Earlgrey -- so this does not change functionality for Earlgrey.
• 61a237e197 [util/reggen] reverse order of substruct generation --> RTL changed but no functional difference
• 3b4e36e01c [edn] Move prim_edn_req out of prim --> reverted by 56399241e7 above
• de31bdf1c2 [reggen] Remove the devmode input --> RTL changed but no functional difference
• 963a5006cc [doc] Minor tweak to md sanitisation code --> no RTL changed; spec docs changed but changes should be structural only
• 975a6eb927 [adc_ctrl,dv] Tidy up access to intr_state in env_cfg files --> no RTL nor specification changed
• 1b16ca2122 [reggen] Add mubi support SWAccess that sets/clears a reg --> RTL changed but no functional difference
• 59f8142826 [doc] Moved badges over to using hosted images --> no RTL nor specification changed
• 8ba7e45f4e [SiVal] Test plan updates for keymgr --> no RTL nor specification changed
• 19c05ad821 [doc] keymgr registers and interfaces now use CMDGEN --> no RTL changed; spec docs changed but changes should be structural only
• 7688e714e8 [reggen] Add initial support for version and cip_id hjson fields --> no RTL changed
• fbd888eea8 Revert "[reggen] Add CIP_IDs and bump all major versions" --> reverts commit below
• 0ba10b3cd3 [reggen] Add CIP_IDs and bump all major versions --> reverted by commit above
• e47df29f3e [misc] Use lc_tx_t testing functions at endpoints --> RTL changed but no functional difference

Issues closed since the Earlgrey-ES tapeout

https://github.com/lowRISC/opentitan/issues?q=is%3Aissue+is%3Aclosed+closed%3A%3E2023-06-27+keymgr

• 2136

  --> not planned

• 5416

  --> bug in prim_subreg that got resolved

• 7614

  --> resolved for Earlgrey-ES tapeout already but got closed afterwards

• 8947

  --> not planned

• 10970

  --> top-level test

• 12323

  --> not planned

• 14047

  --> resolved in #21372: keymgr now gets a unique diversification value (from lc_ctrl) for each group of life cycle states

• 14596

  --> kmac

• 15473

  --> not planned for Earlgrey-PROD

• 15783

  --> flash_ctrl

• 15894

  --> edn

• 16072

  --> not planned

• 16562

  --> unrelated to keymgr

• 16805

  --> test issue

• 18228

  --> test issue

• 19232

  --> kmac

• 20108

  --> bazel

• 20120

  --> top-level test

• 20467

  --> keymgr_dpe

• 20468

  --> keymgr_dpe

• 20828

  --> kmac

• 20887

  --> CM annotations

• 20978

  --> kmac

• 20983

  --> otp_ctrl

• 20990

  --> pwrmgr

• 21018

  --> otp_ctrl

• 21025

  --> pwrmgr

• 21044

  --> top_darjeeling

• 21455

  --> SiVal

• 21504

  --> SiVal

Currently open issues

https://github.com/lowRISC/opentitan/issues?q=is%3Aissue+is%3Aopen+keymgr

• 3479

  --> future release / backlog

• 6220

  --> ROM

• 6543

  --> kmac

• 6546

  --> cryptolib

• 7667

  --> edn

• 7901

  --> future release / backlog

• 8120

  --> D2S & M3; thus we expect security hardening changes for keymgr and should not sign off D2S yet

• 8458

  --> V2S & M5

• 9206

  --> doc improvement

• 10323

  --> ROM_EXT

• 10704

  --> kmac

• 10724

  --> kmac

• 11172

  --> future release / backlog

• 14171

  --> top-level test

• 14518

  --> DIF

• 15088

  --> Integrated

• 15590

  --> cryptolib

• 16070

  --> top-level test

• 16636

  --> top-level test

• 16767

  --> cryptolib

• 16792

  --> ROM_EXT

• 16855

  --> kmac

• 16952

  --> cryptolib

• 17330

  --> future release / backlog

• 17341

  --> ROM_EXT

• 17472

  --> V3 & M5

• 17585

  --> future release / backlog

• 17711

  --> cryptolib

• 17773

  --> V3 & M5

• 17776

  --> M3

• 17785

  --> cryptolib

• 18013

  --> V3 & M5

• 18344

  --> V3 & M5

• 18644

  --> M5

• 19361

  --> not directly related to keymgr

• 19444

  --> SiVal

• 19588

  --> ROM_EXT

• 19595

  --> ROM_EXT

• 19797

  --> build system

• 19805

  --> SiVal

• 19875

  --> cryptolib

• 20023

  --> ROM

• 20194

  --> SiVal

• 20414

  --> future release / backlog

• 20442

  --> SiVal

• 20819

  --> keymgr_dpe

• 20981

  --> this issue

• 21016

  --> V2(S) signoff

• 21094

  --> keymgr_dpe

• 21222

  --> SiVal

• 21299

  --> keymgr_dpe

• 21500

  --> SiVal

• 21501

  --> SiVal

• 21502

  --> SiVal

• 21503

  --> SiVal

• 21505

  --> SiVal

• 21555

  --> ROM

• 21583

  --> ROM

• 21706

  --> SiVal

• 21936

  --> cryptolib

• 21937

  --> SW

• 22117

  --> feature request under discussion, not gating this signoff on it

• 22214

  --> cryptolib

• 22283

  --> looks like a provisioning issue rather than a HW issue; otherwise feature request to be discussed, not gating this signoff on it

• 22296

  --> feature request under discussion, not gating this signoff on it

• 22297

  --> feature request under discussion, not gating this signoff on it

• 22301

  --> cryptolib

Summary

Re RTL changes, e81b5885e5 is the only RTL change with minimal functional impact; the only functional change is that each creator root key share now has its own valid signal. The two valid signals get ANDed in keymgr_ctrl before u_key_valid_sync. This doesn't create glitch problems because the signals only ever transition from 0 to 1. DV has been changed to take this into account.

Re closed design issues since Earlgrey-ES tapeout, #19146 fixed a bug in prim_subreg_arb (#5146), which is relevant for keymgr, and #21372 changed lc_ctrl so that keymgr now gets a unique diversification value for each group of life cycle states (resolving #14047). Top-level tests cover these changes.

Re open design issues, #8120 is an open item for D2S, so we should not sign off D2S at this point. #22117, #22296, and #22297 could be open items for D2 but they are under discussion, so I suggest we don't gate this signoff on them. If we decide that at least one of those items need a keymgr design change, that could be a minor change as part of D3 or, if it's a major change, we will have to re-open D2.

In conclusion, I think this analysis supports signing keymgr off at D2 but not D2S. I'll create a separate issue for keymgr D2S signoff as part of M3.

[andreaskurth]

@vogelpi: May I ask you for feedback on my analysis and D2 signoff approval if you agree?

[vogelpi]

Thanks for putting this together @andreaskurth !

To be honest, I am in favor of signing of D2S. This is the state keymgr was before and as you pointed out, no significant changes have gone in since the last sign-off. So there is IMO no need to go back down again. I agree that there are a couple of security hardening changes we want to do for M3. But all of them are minor. We also keep adding security improvements to other blocks after hitting D2S.

D2S just says security countermeasures are implemented and this is the case for keymgr. The things coming in M3 are not about adding new countermeasures but minor improvements to existing things.

So to summarize: signing of at D2 is definitely okay, signing of at D2S would be preferred from my side. It will save us further paper work in M3.

[andreaskurth]

Alright, given that (1) keymgr has been signed off at D2S before and the changes since then don't revert D2S and (2) #8120 is tracked for M3, I agree that we can sign keymgr off at D2S again.