[rv_dm] D2S Signoff

[andreaskurth]

As part of Earlgrey-PROD.M2, we signed rv_dm off at D2 but not yet at D2S. We have to go through the D2S checklist focusing on the changes since Earlgrey-ES TO. In particular, https://github.com/lowRISC/opentitan/commit/94612fe0957eb151866d32dc99eeab6ffb4e6e6e needs to be reviewed for security impact.

[andreaskurth]

@vogelpi to review RTL code and hand over to @rswarbrick; expected early next week

[vogelpi]

I've finally managed to do the security review of 94612fe (late debug enable), a49f888 (update pulp_riscv_dbg) and b3c7206 (implement NDM reset tracking). These are the main changes since ES as outlined in https://github.com/lowRISC/opentitan/issues/20972#issuecomment-2000634000 .

In my view, these commits don't contain anything unexpected and don't add risk. 94612fe (late debug enable) has been very carefully implemented given the relevance/impact of the feature. All relevant checks have been implemented using hardening comparators and prim_bufs in between to avoid aggressive synthesis optimizations defeating the hardening.

In short, I am happy to sign RV_DM off at D2S. @rswarbrick , would you mind moving this forward and taking care of the remaining steps for the sign-off please?

[rswarbrick]

Here are some notes that I have made following the light-weight signoff process (applying it to D2S for rv_dm). The structure starts with the flow described in the light-weight signoff, but then works through the standard D2S signoff procedure, focused on changes since ES.

To avoid duplicating the work that has already been done for D2 (see issue notes for #20972), I'm pointing there for the full list of commits, issues closed etc. This was made using version b3c7206.

Of course, there might be a further delta since then. The current head of master is 6e8108c51.

Light-weight signoff flow

Commits since ES that are relevant to D2S signoff

The D2 signoff review listed commits since ES. Filtering this, I find the following commits that I believe might affect D2S status for rv_dm:

• b3c72069c2 [rv_dm] Implement NDM reset tracking
• 94612fe095 [rv_dm] Implement late debug enablement mechanism

A list of all further changes can be found with git log b3c7206..6e8108c51 --oneline hw/ip/rv_dm:

• 3a0b57c90b [rv_dm,dv] Fix typo in rv_dm_abstractcmd_status_vseq
• 6319948b9a [rv_dm,doc] Expand table in theory_of_operation.md
• acb896a4ab [rv_dm,dv] Slightly tidy up interface connections in tb.sv
• b0dd18cde7 [rv_dm,lint] Waive unused port in rv_dm.sv
• 30d7e787c7 (tag: Earlgrey-PROD-M2-RC5, tag: Earlgrey-PROD-M2) Add the project name to the copyright header
• 919341eb22 (tag: Earlgrey-PROD-M2-RC4) [rv_dm] Sign off at D2

None of these changes touch the RTL, so these do not need further discussion for D2S.

Issues closed since the Earlgrey-ES tapeout

Most of the closed issues listed in the D2 signoff are unrelated to D2S. Only the following issue seems worth tracking for D2S signoff:

• 20829

  • Implemented by 94612fe

A list of issues closed in the intervening time can be found with the filter is:issue is:closed closed:>2024-03-14 rv_dm:

• 22357

  • DV issue that was closed as unnecessary

• 21716

  • Lint bug fixed by adding waivers

• 21517

  • DV bug

• 20972

  • Signoff

• 22038

  • Fixed by updating vendored pulp code

• 15123

  • Fixed in b3c7206

The two issues relevant to D2S are fixed by code that will be discussed properly below.

Currently open issues

There is a list of open issues in the D2 signoff (for "then-open issues"). Looking through the list, none of them seem relevant for D2S signoff. Running the search again and discarding "signoff" items gives the following list of new issues:

• 22823

  • Issue found in FPGA tests. Will not affect security properties.

• 22728

  • DV issue

• 22545

  • Lint issue

Summary

Following the light-weight signoff flow, the only two items that are worth investigating in detail are the NDM reset tracking and late debug enablement mechanism changes. The text below goes through the standard D2S signoff items, but focusing on those changes.

"Less light-weight" signoff (focusing on the changes found above)

SEC_CM_ASSETS_LISTED

Looking through the two changes found above results in some countermeasure work that has been described (and implemented):

• Late debug enablement:
  • This feature is documented in theory_of_operation.md. The way this is driven by an 8-bit mubi signal is explained, which feeds into the D2 SEC_CM_DOCUMENTED list item.
  • The hardening that is applied to this logic is listed in the hjson text as two new items: LC_DFT_EN.INTERSIG.MUBI and OTP_DIS_RV_DM_LATE_DEBUG.INTERSIG.MUBI. The weakened lifecycle gating is also now described carefully in the description of DM_EN.CTRL.LC_GATED.
• NDM reset tracking:
  • This isn't covered by any explicit countermeasure in rv_dm itself.
  • Latching of the reset request through an NDM reset is handled by pinmux (as documented in the rv_dm theory_of_operation.md)

I'm convinced that any new assets are indeed listed. The new items in the list are:

• LC_DFT_EN.INTERSIG.MUBI
• OTP_DIS_RV_DM_LATE_DEBUG.INTERSIG.MUBI
• DM_EN.CTRL.LC_GATED

SEC_CM_IMPLEMENTED

• LC_DFT_EN.INTERSIG.MUBI:
  • The signal whose hardening is described is the lc_dft_en_i input port.
  • This is encoded as an lc_tx_t. It gets synchronised through a prim_lc_sync.
  • The multi-bit encoding isn't skipped at any point. The signal gets replicated into lc_hw_debug_en_gated.
  • The copies of that signal are then passed as a mubi signal or explicitly checked with lc_tx_test_true_strict and the hand-off from here is unchanged from the behaviour in Earlgrey A0.
• OTP_DIS_RV_DM_LATE_DEBUG.INTERSIG.MUBI:
  • The signal whose hardening is described is the otp_dis_rv_dm_late_debug_i input.
  • (Minor bug: The countermeasure line is attached to the wrong input port, and currently points at pinmux_hw_debug_en_i. This is fixed by #22902.)
  • This is indeed a multi-bit signal (mubi8_t).
  • Is correctly used by checking with mubi8_test_true_strict
• DM_EN.CTRL.LC_GATED:
  • A lot of the description of this countermeasure in rv_dm.hjson is really describing a feature of rv_dm, rather than the hardening applied to how the feature is implemented.
  • I've tried to look at the implementation and how this has changed since ES.
  • The fiddly bit that has changed is a hardened multiplexer. This is annotated with the relevant CM tag in rv_dm.sv and is implemented by replicating the mubi comparators that feed into lc_hw_debug_en_gated_raw.

This analysis matches the results of the review that @vogelpi has done (described here).

SEC_CM_RND_CNST

The changes since ES have not added any compile-time random netlist constants, so this item does not apply to the changes.

SEC_CM_NON_RESET_FLOPS

No change since ES: there is no new security-critical information being stored.

SEC_CM_SHADOW_REGS

The only state that was added that is security-critical has been added with multi-bit encodings and replicated. There are no registers storing this state, so no shadow registers needed.

SEC_CM_RTL_REVIEWED

This has been performed by @vogelpi.

SEC_CM_COUNCIL_REVIEWED

This task was performed for ES and I don't believe there have been any substantive changes since which would need a follow-up review from the security council.

Summary

I am now convinced that rv_dm is at the D2S lifecycle stage again and will file a PR bumping the hjson stage accordingly.

[vogelpi]

Thanks @rswarbrick for putting this together. I agree with your assessment. Regarding the SEC_CM_COUNCIL_REVIEWED item it's worth noting that the relevant changes have been implemented in accordance with the RFC reviewed by security folks (in particular @moidx @johannheyszl and myself), the Security WG and TC. Thus, I believe these changes had sufficient visibility.