Re: [PATCH 1/2] x86/random: Retry on RDSEED failure

[Jason A. Donenfeld]

Hi Ted, Kirill,

On Fri, Feb 02, 2024 at 10:39:27AM -0500, Theodore Ts'o wrote:
> On Fri, Feb 02, 2024 at 07:25:42AM +0000, Reshetova, Elena wrote:
> > This is a great summary of options, thank you Jason!
> > My proposal would be to wait on result of our internal investigation 
> > before proceeding to choose the approach. 
> 
> I'm happy for the option "Do nothing for now", but if we do want to do
> something in the absence of more detailed information, I'd suggest
> doing something simple for first, on the theory that it doesn't make
> things worse, and we can always do something more complicated if it
> turns out to be needed.
> 
> In that vein, my suggestion is:
> 
> > > Solution B) BUG_ON(is_early_boot && is_coco_system) in the RDRAND
> > > failure path (> 10 retries).
> > > 
> > > This is slightly less simple than A, because we have to plumb
> > > CoCo-detection through to the RDRAND helper. [Side note: I feel
> > > ridiculous typing 'CoCo'.] Systems-wise, I don't see drawbacks.
> > > RNG-wise, the drawback is that this doesn't help deal with secure
> > > reseeding later in time, which is a RNG property that we otherwise
> > > enjoy.
> 
> If there isn't a global variable we can test to see if Confidential
> Compute is enabled, I suspect we should just add one.  I would assume
> that /dev/random isn't the only place where we might need to do
> whether Confidential Compute is enabled.
> 
> So I don't think plumbing CC into the /dev/random code, and since we
> are only doing this in early boot, I wouldn't put it in the RDRAND
> helper, but rather in the caller of the RDRAND helper that gets used
> in the early boot path.

Yea, actually, I had a pretty similar idea for something like that
that's very non-invasive, where none of this even touches the RDRAND
core code, much less random.c. Specifically, we consider "adding some
extra RDRAND to the pool" like any other driver that wants to add some
of its own seeds to the pool, with add_device_randomness(), a call that
lives in various driver code, doesn't influence any entropy readiness
aspects of random.c, and can safely be sprinkled in any device or
platform driver.

Specifically what I'm thinking about is something like:

void coco_main_boottime_init_function_somewhere_deep_in_arch_code(void)
{
  // [...]
  // bring up primary CoCo nuts
  // [...]

  /* CoCo requires an explicit RDRAND seed, because the host can make the
   * rest of the system deterministic.
   */
  unsigned long seed[32 / sizeof(long)];
  size_t i, longs;
  for (i = 0; i < ARRAY_SIZE(seed); i += longs) {
    longs = arch_get_random_longs(&seed[i], ARRAY_SIZE(seed) - i);
    /* If RDRAND is being DoS'd, panic, because we can't ensure
     * confidentiality.
     */
    BUG_ON(!longs);
  }
  add_device_randomness(seed, sizeof(seed));
  memzero_explicit(seed, sizeof(seed));

  // [...]
  // do other CoCo things
  // [...]
}

I would have no objection to the CoCo people adding something like this
and would give it my Ack, but more importantly, my Ack for that doesn't
even matter, because add_device_randomness() is pretty innocuous.

So Kirill, if nobody else here objects to that approach, and you want to
implement it in some super minimal way like that, that would be fine
with me. Or maybe we want to wait for that internal inquiry at Intel to
return some answers first. But either way, this might be an easy
approach that doesn't add too much complexity.

Jason