[keymgr] Reorganize diversification inputs

[ballifatih]

Description

One thing that bothers me about the organization of Keymgr RTL diversification constants/secrets is that it does not properly divide the input domain with respect to KMAC calls.

The idea behind diversification (a.k.a. domain separation) is that, when a user makes different KMAC calls for different use cases, each use of KMAC function can be interpreted as if a different (random) function. There are mainly three use cases (if not more) for Keymgr:

1. Generate HW-backed keys (such as AES, KMAC or OTBN).
2. Generate SW-backed keys for generic purposes
3. Compute another keymgr key in order to advance the state

Domain separation should guarantee that it should not be possible to construct the same KMAC message, so that we can argue that a collision of keys from different use cases is pure luck (e.g. as unlikely as for random functions).

Here is a visual description of how Keymgr constructs the message payload for different operations. Capitalized variable names represent the netlist constants that are used for diversification. Notice that the last advance call does not have proper diversification:

I would suggest the following:

• Add another diversification constant for the 3rd advance call as the first 256-bit.
• Move REVISION_SEED to the second 256-bit block. I also wonder if it makes sense to reuse this value for all advance calls as the second 256-bit block.
• sw_binding register value is moving around a lot during different advance calls. It might be better to fix its location as well (which might give SW ability to diversify further).

To recap, at the moment, it is hard to argue that KMAC call from the 3rd advance call is separated from any of the other KMAC calls in the same stage.

cc: @johannheyszl @moidx @jadephilipoom

[moidx]

I see value in adding an owner third stage seed for diversification purposes, but rate the priority for that change as P2-P3. @timothytrippel for visibility on the provisioning side.

Using a gate level constant for diversification in the third stage SGTM as well. I am not sure if this is a P2 or P3. @johannheyszl to flag this for further discussion.

We need to prioritize this against other cryptolib and pentesting work as @ballifatih would have to absorb both RTL and DV changes.

[ballifatih]

A correction: With the default synthesis parameters, CREATOR_ADV_SEED and OWNER_ADV_SEED from the drawing above are coming from flash, but not OTP.

For clarification, what I need is a way to be able to separate different KMAC calls from each other, but not necessarily secret values. I think the answer from the following post explains what I meant better:

https://crypto.stackexchange.com/questions/66969/what-is-meant-by-domain-separation-in-the-context-of-kdf

The smallest effort here is to keep these two flash secrets as inputs but use additional netlist constants as the first 256-bit chunks of the diversification data. In other words, add further 256-bit prefixes to advance payloads.

Without this change, I think it is hard to justify the correct use of KMAC for different purposes.

[ballifatih]

Further details and/or arguments are summarized in the following OT doc: https://docs.google.com/document/d/1YgrDGzkzv7ADkuwWWvStbqzcmrZywWxGYiaxI34FS6I

[vsukhoml]

What about integrating support for #22296 as part? It can provide complementary approach for key gen of FIPS key by swapping key & context when using KMAC.

I'd also prefer to use full 256-bit (vs. 224 bit with version for which I have no good use cases) when using KEYMGR for software generated keys.

As for diversification, it can be just 8-bit being added as part of label in SP800-108 $K{OUT} = KMAC(K{IN}, Context, L, Label)$ implicitly, depending on the destination.

[ballifatih]

What about integrating support for #22296 as part? It can provide complementary approach for key gen of FIPS key by swapping key & context when using KMAC.

I think this issue you are referring to is also important, but unlike this one, the HW changes and DV effort is large for that one. This issue in particular is low effort, because it only requires reorganizing hard coded variables in an array.

I'd also prefer to use full 256-bit (vs. 224 bit with version for which I have no good use cases) when using KEYMGR for software generated keys.

In this diagram, this value corresponds to salt. The size for this is already 256-bit in HW. 224-bit restriction is later imposed at SW level. We can discuss this further on cryptolib API level, if having 224-bit is really that inconvenient.

As for diversification, it can be just 8-bit being added as part of label in SP800-108 KOUT=KMAC(KIN,Context,L,Label) implicitly, depending on the destination.

I agree that diversification constants do not need to be large, and in ideal world, they would be just label input of KDF-KMAC and short for simplicity. However, the interface between Keymgr and KMAC cannot handle this flexibility at this moment. Also even if we use 8-bit diversification, message payloads are rounded up to multiples of 64-bits by 0-padding.

So overall I would say this issue tries to get rid of one of the potential problems with an emphasis on minimum HW modification effort.

[ballifatih]

As mentioned in #22624, it would also be nice to have another (at least) 32-bit diversification for key_mode. For HW keys, this is useful to distinguish between different OTBN use cases. For SW keys, it allows cryptolib to ensure that different keys do not get mixed up.

[vogelpi]

Thanks for creating the issue and pushing the discussion forward @ballifatih .

I had a look at the RTL in an attempt to assess the feasibility of the proposed changes. Even thought the change is mainly just about adding additional netlist constants, there is going the be a non-negligible area overhead for this. The reason is that the we feed the netlist constants through a set of prim_sec_anchor_buf cells in the design (most likely to allow tracking these signals in the back end flow for hardening purposes). As a result of this, any additional netlist constant added, would add 256 BUF cells + 256 MUX2 cells for the selection further downstream (or if not adding MUX2 cells, switching to MUX cells with 1 more input per additional seed). Note that due to the buffers, synthesis won't be able to optimize between the constants to reduce the number of MUX inputs. It's not possible and most likely also not desirable from a security view point.

In short, even though the proposed change is not going to add FFs, it's going to add a considerable amount of area that we most likely cannot absorb anymore at this stage. In addition, even though it's lower effort in terms of DD and DV compared to implementing context switching, it's additional effort that needs to be spent and we have tight deadline. I am thus proposing to NOT do this now but instead tag this as future release. This also gives us the time to discuss what is really needed.

[andreaskurth]

Just discussed in the triage meeting, and we agreed to move this to future release

[ballifatih]

Can we consider the alternative fix #22878 for this issue @vogelpi @andreaskurth? To recap what #22878 does:

• For the 1st advance calls, moves REVISION_SEED to the beginning of the message payload.
• For the 2nd and 3rd advance calls, reuses REVISION_SEED as the first prefix, such that all advance calls start with the same 256-bit value.
• Does not add a new RTL constant (hence no extra gates cost).
• The purpose of REVISION_SEED is defined as: https://github.com/lowRISC/opentitan/blob/7c92d49f77576a13a20fe6a508e47effca6193a2/doc/security/specs/identities_and_root_keys/README.md?plain=1#L158-L159 hence, the purpose is to include another revision-specific secret as part of DICE. Reusing this secret should not be a concern.

For future reference, the relevant standard section can be found in Section 6.5, Point 2 of SP 800-108r1.

[ballifatih]

Closing this issue as the merged PR resolves it.

For the record, this is what we converged on a as a solution:

The relevant docs are not yet updated, but they are tracked in a dedicated issue https://github.com/lowRISC/opentitan/issues/23323.