[kmac] kmac does not check key validation in sw initiated sequence

[cindychip]

Hi Eunchan,

I have two questions regarding kmac ErrKeyNotValid error: 1). Right now this error ErrKeyNotValid is specifically triggered when design is configured to KMAC mode. However, if design is in other mode (such as Cshake128), user can still use keymgr key input when configure the sideload field to 1 right? In that case, if keymgr's key is not valid, shall we flag a ErrKeyNotValid error? (I think current design does not flag any error)

2). Follow up with the previous question, in that case, design seems to take the invalid input keymgr.key from the interface, and continues the cshake operation. However, in DV we will drive X as data in the sideload interface when key is not valid. Then the X will propagates to the data, and eventually I saw an assertion error. How do you think we can proceed with this issue? a). Drive all 0s or 1s as key output when keymgr is not valid? b). Constrain DV to not drive X when key is not valid? (I think this is less desirable) c). You probably have some better ideas..

Please corrent me if I misunderstand anything :)

smart ID: smart:dc11c7ec-e875-47bf-8d06-9e129c1668ed path: /workspace/mnt/opentitan/scratch/fix_kmac_err/kmac_masked-sim-vcs/0.kmac_masked_error/out/

Thanks, Cindy

[eunchan]

Hi Eunchan,

I have two questions regarding kmac ErrKeyNotValid error: 1). Right now this error ErrKeyNotValid is specifically triggered when design is configured to KMAC mode. However, if design is in other mode (such as Cshake128), user can still use keymgr key input when configure the sideload field to 1 right? In that case, if keymgr's key is not valid, shall we flag a ErrKeyNotValid error? (I think current design does not flag any error)

Only when the key is needed, KMAC checks the side load key is valid or not. cSHAKE, SHAKE, SHA mode does not use the key. 2). Follow up with the previous question, in that case, design seems to take the invalid input keymgr.key from the interface, and continues the cshake operation. However, in DV we will drive X as data in the sideload interface when key is not valid. Then the X will propagates to the data, and eventually I saw an assertion error. How do you think we can proceed with this issue? a). Drive all 0s or 1s as key output when keymgr is not valid? b). Constrain DV to not drive X when key is not valid? (I think this is less desirable) c). You probably have some better ideas..

In cSHAKE mode, it shouldn't use key at all. Let me check the waveform. Please educate me if I misunderstand anything :)

smart ID: smart:dc11c7ec-e875-47bf-8d06-9e129c1668ed path: /workspace/mnt/opentitan/scratch/fix_kmac_err/kmac_masked-sim-vcs/0.kmac_masked_error/out/ https://user-images.githubusercontent.com/11466553/153093991-0d2aad94-494a-4dc4-96d6-ab591f32eb6c.png

[cindychip]

After the discussion with Eunchan, this sequence actually is in KMAC MODE. So we plan to support it by enhancing the RTL to check key validation in sw initiated sequence.

[eunchan]

So, based on the discussion, the test case is SW initiated KMAC operation with sideload key. This version of KMAC does not check the Key validity for SW initiated operation. But it is good to have.

Brief review of the design seems like adding a condition when SW initiated the request, then check keymgr_key_en_i and reg_kmac_en_i is a good start. But the actual handling of the error should differ from AppIntf error handling. The current error state can't handle the SW initiated error gracefully.

[tjaychen]

@eunchan / @cindychip does it make sense to move this to future release?

[weicaiyang]

just debugged a chip-level case with @cindychip, and we saw this could happen in a real case.

If kmac starts an operation with sideload enabled, but forget to configure keymgr to provide the sideload key, the kmac operation will still complete without any error. kmac seems to use all 0s as the key in this case.

[tjaychen]

@jadephilipoom in lieu of larger changes, maybe we could consider an all 0 or all 1 key kind of an error case... this could probably be done in either hardware or software.

[weicaiyang]

@jadephilipoom in lieu of larger changes, maybe we could consider an all 0 or all 1 key kind of an error case... this could probably be done in either hardware or software.

@tjaychen If SW doesn't configure keymgr to provide sideload key before start a kmac operation, key with all 0s is used and SW doesn't know the digest is generated with an invalid key (since no error is reported), right?

[tjaychen]

yes that's right. I was originally thinking software could read it out to check, but I forgot the kmac key shares are write-only by software. So if were to check, it would have to be from hardware.

Let's get @jadephilipoom opinion to see if this is something we should explicitly check for.

[jadephilipoom]

I think it would be nice to have a hardware-side check. From software all I can do is see that the key manager says it's done writing the sideloaded key, but I can't check that this loading was successful in any way since I can't read the KMAC key registers. I can make sure that SW always checks the key manager status, of course, but it would be a helpful extra layer of assurance if the hardware could alert me that something went wrong (including potentially a fault attack that prevented the key from getting written).

[tjaychen]

thanks @jadephilipoom . Since this is already marked future release, we should probably revisit it in the future. But it does seem better to have related hw checks.

[cindychip]

Triaged: will discuss with #16855

[ballifatih]

estimate 16

[johngt]

@cindychip - it was mentioned in your comment of Feb 28 that you would discuss with #16855 - what priority should this be at?

[moidx]

Moving to M2.5.Backlog, for consideration for M3. Priority to be reassessed then.

[msfschaffner]

@moidx how important do you think this check is, and should we do it for prod or not?

[vogelpi]

Jumping in here as I came across this when scoping the effort for M2. IMO, we should definitely do this: KMAC should not use invalid sideload keys. It can probably be done together with #16855 as part of M3. I am reassigning this to M3 as this won't have big area or physical design impact.

[andreaskurth]

@andreaskurth has a PR to address this: #22794. Expected completion early next week.