[hw/csrng] Integrity protection for KV in CSRNG

[moidx]

This issue is to investigate integrity protection requirements for the DRBG internal state.

[tjaychen]

i think given that all the entropy blocks have passed D2S, this is probably relegated to future release.
Let me know if anyone objects.

[cdgori]

I'm closing out some emails that have been languishing in my inbox...

I think this is fine to move to future release. My only caveat would be if we plan to have any pre-tapeout FIPS review of the CSRNG, that feedback could force this forward.

@moidx do you remember what specifically triggered you to file this issue? (Or just a general concern?)

[johngt]

@moidx / @cdgori / @tjaychen - slightly older issue here, but just checking if this should be in M2?
I'm tagging as M3 but if this was something that needed to go in please change it back.

[moidx]

Moving to backlog as this was set as future release.

[andreaskurth]

Triaged for csrng, keeping Type:FutureRelease Not relevant to currently planned releases/milestones .

[msfschaffner]

To be assessed for pentesting testplan.

[vogelpi]

TODO:

• Check D2S review notes
• Discuss with @johannheyszl

[vogelpi]

I've studied the D2S review notes. The option of adding integrity protection for key and v inside was raised in the D2S meeting and we wanted to check with labs if this is advisable. It was mentioned in the discussion that loosing a couple of bits in the input (i.e. before AES) can be argued to be not that bad. It was also mentioned that adding integrity protection could be exploited itself, e.g., via SCA.

@johannheyszl and I talked about this and we believe that if integrity protection was added key and v inside CSRNG, it should also be added to the ENTROPY_SRC output and especially the EDNs. This is assuming it wouldn't introduce any other vulnerabilities. The later down the pipeline the entropy is manipulated, the worse the problem becomes.

Adding comprehensive integrity protection to the entropy complex is a huge task. We believe this should only be done if we have clear evidence that it's actually required and it doesn't introduce other vulnerabilities. I am thus labeling this as FutureRelease and removing PROD labels.

[msfschaffner]

Thanks @vogelpi, this sounds reasonable.

[johannheyszl]

For completeness, the pentesting plan contains a test geared towards this issue.