lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.57k stars 770 forks source link

[hw/kmac] key integrity checks #6546

Open moidx opened 3 years ago

moidx commented 3 years ago

Consider placing integrity checks on keys. Not high priority since keys are already masked.

Integrity options to consider: ECC, parity, digest, None.

tjaychen commented 2 years ago

@moidx i believe the conclusion for this is that we will not have integrity checks, but it would be good to duplicate in software space to ensure results are consistent.

We've also discussed the idea of adding keymgr known answer tests to ensure the seeds are programmed correctly.

tjaychen commented 2 years ago

just an addition. One item we added was that the secret key inside keymgr now comes with integrity checks. Since that one cannot be easily replayed for correctness.

For the remainder of this issue, it looks more like a software guidance question now, so I've moved it to the software hotlist for closure.

tjaychen commented 2 years ago

specifically for kmac v2s, i do not think this needs to be gating.

cindychip commented 2 years ago

specifically for kmac v2s, i do not think this needs to be gating.

We also want to check if this is still planning for M2?

tjaychen commented 2 years ago

i don't think so for now. But let me double check something and get back to you.

tjaychen commented 2 years ago

okay dug up an old comment, it looks like we do plan on having software redundancy.

If that is the case, then the integrity checking between keymgr / kmac isn't super critical. We also do plan on having a known answer test of some kind, so earlier in the derivation flow I think is okay as well.

@jadephilipoom @johannheyszl

jadephilipoom commented 2 years ago

okay dug up an old comment, it looks like we do plan on having software redundancy.

Yep, I am planning on software redundancy for this in the cryptolib. The measure discussed in that comment (running verify/decrypt after sign/encrypt and running decrypt/verify/mac twice) does impose a significant performance cost. However, it it protects against a wide variety of fault attacks as well as key integrity issues, so I think there's a good chance we'd want to do it even if integrity checks were in place.

If that is the case, then the integrity checking between keymgr / kmac isn't super critical. We also do plan on having a known answer test of some kind, so earlier in the derivation flow I think is okay as well.

I'm curious -- how will a known answer test work for keymgr, since the keys rely on hardware-encoded secrets?

tjaychen commented 2 years ago

@jadephilipoom have a look at #10323 The KAT idea is a bit of a misnomer. We're not actually trying to compare keymgr internal state. Rather it's more we get keymgr to generate something during provisioning, and store that something in OTP.

Later, when we get keymgr back to the same state, we can go through the same exercise to see if the same answer can be generated. So it's "known" in that sense, but it relies on having non-volatile storage.

jadephilipoom commented 2 years ago

Should we close this issue for now, or at least remove the M2 label, since it seems to me there's no action required for M2?

tjaychen commented 2 years ago

o yes this should probably be future release. I don't think we intend to do this for M3 either since we've planned a keymgr KAT anyways.

cindychip commented 1 year ago

Triaged: keys are masked. Can be for future release.

johannheyszl commented 9 months ago

My understanding is this is a cryptolib feature, re-labeling accordingly.