linux-kernel - RE: [PATCH 1/2] x86/random: Retry on RDSEED failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM8PR11MB5750E38A8B2BCE66AF7F9812E77D2@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Tue, 30 Jan 2024 13:10:20 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: "Jason A. Donenfeld" <Jason@...c4.com>, "Kirill A. Shutemov"
	<kirill.shutemov@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
	"Theodore Ts'o" <tytso@....edu>, Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@...ux.intel.com>, "Nakajima, Jun"
	<jun.nakajima@...el.com>, Tom Lendacky <thomas.lendacky@....com>, "Kalra,
 Ashish" <ashish.kalra@....com>, Sean Christopherson <seanjc@...gle.com>,
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] x86/random: Retry on RDSEED failure

 
> Hi Kirill,
> 
> I've been following the other discussion closely thinking about the
> matter, but I suppose I'll jump in here directly on this patch, if
> this is the approach the discussion is congealing around.
> 
> A comment below:
> 
> On Tue, Jan 30, 2024 at 9:30 AM Kirill A. Shutemov
> <kirill.shutemov@...ux.intel.com> wrote:
> >  static inline bool __must_check rdseed_long(unsigned long *v)
> >  {
> > +       unsigned int retry = RDRAND_RETRY_LOOPS;
> >         bool ok;
> > -       asm volatile("rdseed %[out]"
> > -                    CC_SET(c)
> > -                    : CC_OUT(c) (ok), [out] "=r" (*v));
> > -       return ok;
> > +
> > +       do {
> > +               asm volatile("rdseed %[out]"
> > +                            CC_SET(c)
> > +                            : CC_OUT(c) (ok), [out] "=r" (*v));
> > +
> > +               if (ok)
> > +                       return true;
> > +       } while (--retry);
> > +
> > +       return false;
> >  }
> 
> So, my understanding of RDRAND vs RDSEED -- deliberately leaving out
> any cryptographic discussion here -- is roughly that RDRAND will
> expand the seed material for longer, while RDSEED will mostly always
> try to sample more bits from the environment. AES is fast, while
> sampling is slow, so RDRAND gives better performance and is less
> likely to fail, whereas RDSEED always has to wait on the hardware to
> collect some bits, so is more likely to fail.

The internals of Intel DRBG behind RDRAND/RDSEED has been publicly
documented, so the structure is no secret. Please see [1] for overall
structure and other aspects. So, yes, your overall understanding is correct
(there are many more details though). 

[1] https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html


> 
> For that reason, most of the usage of RDRAND and RDSEED inside of
> random.c is something to the tune of `if (!rdseed(out)) rdrand(out);`,
> first trying RDSEED but falling back to RDRAND if it's busy. That
> still seems to me like a reasonable approach, which this patch would
> partly undermine (in concert with the next patch, which I'll comment
> on in a follow up email there).

I agree that for the purpose of extracting entropy for Linux RNG falling
back to RDRAND (current behavior) is perfectly ok, so I think you are doing
the right thing. However, in principle it is not always the case, there are
situations when a fallback to RDRAND should not be used, but it is also
true that the user of this interface should know/understand this situation. 

> 
> So maybe this patch #1 (of 2) can be dropped?

Before we start debating this patchset, what is your opinion on the original
problem we raised for CoCo VMs when both RDRAND/RDSEED are made to
fail deliberately? 

Best Regards,
Elena.