linux-kernel - RE: [PATCH v2 1/1] x86/hyperv: Use Hyper-V entropy to seed guest random number generator

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:
 <BN7PR02MB4148FA0075DCB43F1971485BD4292@BN7PR02MB4148.namprd02.prod.outlook.com>
Date: Thu, 14 Mar 2024 04:30:50 +0000
From: Michael Kelley <mhklinux@...look.com>
To: "Jason A. Donenfeld" <Jason@...c4.com>
CC: "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>, "wei.liu@...nel.org"
	<wei.liu@...nel.org>, "decui@...rosoft.com" <decui@...rosoft.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "mingo@...hat.com"
	<mingo@...hat.com>, "bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "hpa@...or.com"
	<hpa@...or.com>, "arnd@...db.de" <arnd@...db.de>, "tytso@....edu"
	<tytso@....edu>, "x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Subject: RE: [PATCH v2 1/1] x86/hyperv: Use Hyper-V entropy to seed guest
 random number generator

From: Jason A. Donenfeld <Jason@...c4.com>
> 
> Hi Michael,
> 
> On Thu, Mar 14, 2024 at 12:30:06AM +0000, Michael Kelley wrote:
> > OK, fair enough.  But just to double-check:  When this is called,
> > the EFI RNG protocol has already invoked add_bootloader_randomness(),
> >  and this line has been output:
> >
> > [    0.000000] random: crng init done
> >
> > I don't see an obvious problem with calling add_bootloader_randomness()
> > again, but wanted to confirm.
> 
> Yea, that's very much okay. It'll just get added as extra, which can't
> hurt.
> 
> > Also, if we're adding this ACPI-based randomness for VMs that
> > boot via EFI, then for consistency we should use it on Hyper-V
> > based ARM64 VMs as well.
> 
> Agreed.
> 
> > Both bounds are just a check for bogusness.  Having the hypervisor
> > provide just 4 bytes (for example) of randomness seems like
> > there might be something weird going on.  But widening the bounds
> > is fine with me.  I'll use "8" and "SZ_4K".
> 
> Ahh, as a sanity check that seems like a reasonable heuristic.
> 
> > > > +	for (i = 0; i < length; i++) {
> > > > +		header->checksum += randomdata[i];
> > > > +		randomdata[i] = 0;
> > > > +	}
> > >
> > > Seems dangerous for kexec and such. What if, in addition to zeroing out
> > > the actual data, you also set header->length to 0, so that it doesn't
> > > get used again as 32 bytes of known zeros?
> >
> > What's your take on the whole idea of zero'ing the random data?   I
> > saw the EFI RNG protocol handling was doing something roughly
> > similiar.  But yes, good point about kexec().  Zeroing the header->length
> > would make sense to prevent any re-use.
> 
> Yes, I think zeroing it out is the right call. I wonder, though, what's
> the deal with the checksum adjustment? Should we be checking the
> checksum before using the random data? And do we have to adjust it like
> that at the end, or can we just zero out the entire header (including
> length) along with the random data?
> 

By default, Linux doesn't verify checksums when accessing ACPI tables
during early boot, though you can add "acpi_force_table_verification"
to the kernel boot line.  The default is shown in dmesg like this:

[    0.004419] ACPI: Early table checksum verification disabled

The checksum of all tables is checked slightly later in boot, though
it's after my entropy code has run.  Without the checksum fixup,
this error is output:

[    0.053752] ACPI BIOS Warning (bug): Incorrect checksum in table
		[OEM0] - 0x8B, should be 0x82 (20230628/utcksum-58)

At this point, the checksum error doesn't really matter, but I
don't want the warning showing up.  I need to experiment a
bit, but probably the best approach is to set the data length to
zero (and adjust the checksum) while leaving the rest of the ACPI
table header intact.  It will be more difficult to make the table
disappear entirely as it appears in a global list of ACPI tables.

Michael