Messages in this thread | | | From | "Reshetova, Elena" <> | Subject | RE: [PATCH v2 2/2] x86/coco: Require seeding RNG with RDRAND on CoCo systems | Date | Fri, 16 Feb 2024 07:57:19 +0000 |
| |
> Hi Elena, > > On Thu, Feb 15, 2024 at 07:30:32AM +0000, Reshetova, Elena wrote: > > Should we just go back to the approach to add one more check in > random_init_early() > > to panic in the CoCo case if both rdseed and rdrand fails to give us anything? > > Yea, no, definitely not. That is, in my opinion, completely backwards > and leads to impossible maintainability. CoCo is not some special > snowflake that gets to sprinkle random conditionals in generic code. > > First, consider the motivation for doing this: > - This is to abort on a physical defective CPU bug -- presumably a > highly implausible thing to ever happen. > - This is for a threat model that few people are really compelled by > anyway, e.g. it's whack-a-mole season with host->guest vectors. > - This is for a single somewhat obscure configuration of a single > architecture with a feature only available on certain chipsets. > - This is not an "intrinsic" problem that necessitates plumbing complex > policy logic all over the place, but for a very special > driver-specific case. > > Rather, what this patch does is... > > > Now with this patch, the logic becomes > > Your description actually wasn't quite accurate so I'll write it out > (and I'll clarify in the commit message/comments for v3 - my fault for > being vague): > > 1. At early boot, x86/CoCo is initialized. As part of that > initialization, it makes sure it can get 256 bits of RDRAND output > and adds it to the pool, in exactly the same way that the SD card > driver adds inserted memory card serial numbers to the pool.
Yes, my mental picture that random_init_early() is called before setup_arch() was obviously wrong, I should have checked it explicitly. So, yes, coco_random_init() happens first, which actually now has a nice side-effect that on coco platforms we drop HW CPU output even earlier in the entropy pool (Yay!). Which at this point would be almost perfect, *if* we could also count this entropy drop and allow ChaCha seeding to benefit straight from this early drop of entropy.
How about changing this to use add_hwgenerator_randomness()? And adjust cc_random_init() to try rdseed first and only fallback to rdrand if it fails? I envision that a counter argument to this would be "we only count entropy from HW CPU RNG, if we trust CPU RNG", but in CoCo case we *do trust CPU* and this is the output of true HW RNG that we are mixing in the pool per definition.
Best Regards, Elena.
| |