From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by mx.groups.io with SMTP id smtpd.web11.17922.1669121159569968408 for ; Tue, 22 Nov 2022 04:45:59 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=qvz9G7aZ; spf=pass (domain: gmail.com, ip: 209.85.216.44, mailfrom: pedro.falcato@gmail.com) Received: by mail-pj1-f44.google.com with SMTP id k5so13156459pjo.5 for ; Tue, 22 Nov 2022 04:45:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=XblrgwpFYof0GTvrmvZe8OJC0F//Qx+TuBLjZrmz7Qs=; b=qvz9G7aZ9WcATeOA21pbt5ntXwBGeZKyGrMtAIFwUa8NaaJkxjJAK5C0ijQYb+74Hz ZJbvMxKggxnpvrdNXxuQL91vFsyf8rARMJezxjhXfNDu0k6CgrC1xBiaEpygquj9cS1T 6O4qS8djJ+NDto2zTrN0CTZdMpF1fP/j5E3J7sRC96xh4Y0U2ERbIEgvKs9boFeIJGg+ nqSX4418Zn/ZZpg5NDbtoENR+uVWY7sIJmnVGwI9aTD9HCqaJVZv9OfPlfGYebh3IgsT bKWUiP4PpcTqhPQvEIotCsB7EUs2qSdPVLXWzbYsyVWMUDe1utROJKeSsT/TbSCzv6j0 dw5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XblrgwpFYof0GTvrmvZe8OJC0F//Qx+TuBLjZrmz7Qs=; b=zeZZocMNyyRz2PpfQrHF62+jsZlU/HLB+AjpXdR8UMVfVTmLmwsO5+wHk8XI4otIUM CDUDX2rEriK9Z//lQQwL8Cr3uJsZTe5jRleRNvlCtSrnwH1/b9qg0mEPzdf4LrkIsBqQ BKWC/kgvw0+aXIOCoGHXFFXAFvPZU/9pl4LT2RJaFjz/gmovQKUkgwmJ8o3lu0ZgUB4g Vg5NUZIidYELtAwL+cS9i0Wef0UGWNAy2k3VUvJhVOUtwRtdNNsgZdkZRdHEu+Add5+H PwAFdJuuI9xN8GTUaiW0i5voB61knJDXEAUcGfuDib1fkf0F6akbSoMRRX2UN+/43vMF n2uQ== X-Gm-Message-State: ANoB5pkSn4GXOyxqJ9IGy046uCfvNWmIauJQe0zjT+wwTkKQvFajQ/Ui rQ/yYR4HxaKtbM+xTBO9MdQ5mpPOHbtZQTKbLvw= X-Google-Smtp-Source: AA0mqf5SdBjeNE7iNcnARTz3X5Fgo+PRGP2lPJzZMW/V55SijuxkhX6/dHHwHngpcsHzjeb4h2ZoCx8GfDBMVcZSS38= X-Received: by 2002:a17:902:ef93:b0:189:3998:17d4 with SMTP id iz19-20020a170902ef9300b00189399817d4mr676003plb.25.1669121158841; Tue, 22 Nov 2022 04:45:58 -0800 (PST) MIME-Version: 1.0 References: <20221110134738.3798618-1-ardb@kernel.org> <20221110134738.3798618-4-ardb@kernel.org> In-Reply-To: From: "Pedro Falcato" Date: Tue, 22 Nov 2022 12:45:47 +0000 Message-ID: Subject: Re: [edk2-devel] [PATCH 3/3] OvmfPkg/OvmfX86: Enable RDRAND based EFI_RNG_PROTOCOL implementation To: "Jason A. Donenfeld" Cc: devel@edk2.groups.io, ardb@kernel.org, Liming Gao , Rebecca Cran , Pierre Gondois , Leif Lindholm , Sami Mujawar , Gerd Hoffmann Content-Type: multipart/alternative; boundary="000000000000c28a8f05ee0e8d5c" --000000000000c28a8f05ee0e8d5c Content-Type: text/plain; charset="UTF-8" On Tue, Nov 22, 2022 at 12:20 PM Jason A. Donenfeld wrote: > Hi Pedro, > > On Tue, Nov 22, 2022 at 12:35 PM Pedro Falcato > wrote: > > Given this patch plus the corresponding linux-efi patches wrt RNG, I'm > > mildly concerned about buggy RDRAND implementations compromising the > > kernel's RNG. Is this not a concern? > Hi, Thank you for your thorough response, glad to have you in this thread. Speaking with my kernel RNG maintainer hat on, no, this is not really a > concern, for several reasons: > > - The kernel's RNG takes input from multiple sources, continuously, and > tries to mix in new inputs rather quickly, especially at early boot. > I am aware, but I'm more scared when it comes to very early boot (think linux's EFI stub or some other bootloader) I can see how an ill-advised RNG_PROTOCOL user can try to exclusively rely on it (if it's available, which I don't believe it is atm on non-virtio-rng OVMF) vs mixing in the few sources you can get at that point, making important things like KASLR addresses or possibly even a stack canary 100% guessable. - The kernel will use RDRAND on its own, even in the case that EFI > doesn't provide it, so it's not gaining anything here. > Yes, but as you said, the kernel mixes RDRAND with a lot of other entropy sources and also does proper sanity checking on it. - EFI on actual baremetal firmware, as opposed to OVMF, already provides > EFI, so this is par for the course. > Hm? What do you mean? > - Most of those RDRAND bugs have concerned coming in and out of various > sleep states, which doesn't really apply to early boot EFI. > > - And again, just to reinforce the first point, the kernel takes inputs > from many sources. Having EFI provide its own thing -- via RDRAND or > any other mechanism -- is complementary and will only help. > > Regarding your "corresponding linux-efi patches wrt RNG", I'm not quite > sure what you're referring to. If anything, recent work during this > cycle has been aimed around shuffling more sources of randomness in from > elsewhere. The EFI RNG protocol stuff has been in there already for a > long time. So maybe you misunderstood those? Or I'm misunderstanding > what you're referring to? > Ah yes, I haven't been paying much close attention to the RNG patches themselves but now that I took a closer look I can see you're right. As a general point, this question of "do we have enough entropy?" or > "are we initialized yet?" is an impossible proposition. Entropy > estimation is impossible, and is only ever a guess, and that guess can > be sometimes wrong, even wildly wrong. So the kernel is increasingly > moving away from /relying/ on that, and is more focused on getting more > sources faster -- incorporating anything it can find, and mixing it into > the output stream more continuously. To that end, if EFI's got a DXE to > do something or another, please hook it up. > > Lastly, I think the concern you raised is inappropriate for Ard's > patchset, as it actually doesn't apply to it at all. This patchset is > about hooking an existing DXE up to OVMF, one that is hooked up > elsewhere, but wasn't for OVMF. This alone has nothing to do with the > concern. Rather, the concerns you raised are about the DXE itself. So to > that end, perhaps you should start a new thread or send some patches or > do something to the DXE that you're concerned about (e.g. a basic boring > power-on selftest like what the kernel has or something, if you're extra > worried). Or maybe not, for the reasons I listed above of things being > basically fine. > I know, I'm not yelling at Ard for the (questionable?) choices done in the BaseRngLib code, but I'm concerned this patch may negatively influence any sort of early boot RNG, particularly for the more naive users of RNG_PROTOCOL, by providing the possibly flawed RDRAND code. If the efi subsystem/EFISTUB code handles this case well by still mixing in whatever sources it can get before using this entropy, then that's great, but providing things like a non-random RNG_PROTOCOL sounds very broken and very unsafe to me (again invoking that possible KASLR at very early boot example). Also the CPUID check seems like an important step towards not-breaking-old-CPUs. All I'm saying is that we shouldn't just hook up the RNG DXE driver without carefully considering what the code is doing. Thanks, Pedro --000000000000c28a8f05ee0e8d5c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Nov 22, 2022 at 12:20 PM Jason A.= Donenfeld <Jason@zx2c4.com> w= rote:
Hi Pedro,

On Tue, Nov 22, 2022 at 12:35 PM Pedro Falcato <pedro.falcato@gmail.com> wrote:=
> Given this patch plus the corresponding linux-efi patches wrt RNG, I&#= 39;m
> mildly concerned about buggy RDRAND implementations compromising the > kernel's RNG. Is this not a concern?
=C2=A0
Hi,

Thank you for your thorough respo= nse, glad to have you in this thread.

Speaking with my kernel RNG maintainer hat on, no, this is not really a
concern, for several reasons:

- The kernel's RNG takes input from multiple sources, continuously, and=
=C2=A0 tries to mix in new inputs rather quickly, especially at early boot.=

I am aware, but I'm more scared wh= en it comes to very early boot (think linux's EFI stub or some other bo= otloader) I can see how
an ill-advised RNG_PROTOCOL user can try = to exclusively rely on it (if it's available, which I don't believe= it is atm on non-virtio-rng OVMF) vs
mixing in the few sources y= ou can get at that point, making important things like KASLR addresses or p= ossibly even a stack canary 100% guessable.

- The kernel will use RDRAND on its own, even in the case that EFI
=C2=A0 doesn't provide it, so it's not gaining anything here.

Yes, but as you said, the kernel mixes RDRAN= D with a lot of other entropy sources and also does proper sanity checking = on it.

- Most of those RDRAND bugs have concerned coming in and out of various
=C2=A0 sleep states, which doesn't really apply to early boot EFI.

- And again, just to reinforce the first point, the kernel takes inputs
=C2=A0 from many sources. Having EFI provide its own thing -- via RDRAND or=
=C2=A0 any other mechanism -- is complementary and will only help.

Regarding your "corresponding linux-efi patches wrt RNG", I'm= not quite
sure what you're referring to. If anything, recent work during this
cycle has been aimed around shuffling more sources of randomness in from elsewhere. The EFI RNG protocol stuff has been in there already for a
long time. So maybe you misunderstood those? Or I'm misunderstanding what you're referring to?

Ah yes, I= haven't been paying much close attention to the RNG patches themselves= but now that I took a closer look I can see you're right.
As a general point, this question of "do we have enough entropy?"= or
"are we initialized yet?" is an impossible proposition. Entropy estimation is impossible, and is only ever a guess, and that guess can
be sometimes wrong, even wildly wrong. So the kernel is increasingly
moving away from /relying/ on that, and is more focused on getting more
sources faster -- incorporating anything it can find, and mixing it into the output stream more continuously. To that end, if EFI's got a DXE to=
do something or another, please hook it up.

Lastly, I think the concern you raised is inappropriate for Ard's
patchset, as it actually doesn't apply to it at all. This patchset is about hooking an existing DXE up to OVMF, one that is hooked up
elsewhere, but wasn't for OVMF. This alone has nothing to do with the concern. Rather, the concerns you raised are about the DXE itself. So to that end, perhaps you should start a new thread or send some patches or
do something to the DXE that you're concerned about (e.g. a basic borin= g
power-on selftest like what the kernel has or something, if you're extr= a
worried). Or maybe not, for the reasons I listed above of things being
basically fine.

I know, I'm not yel= ling at Ard for the (questionable?) choices done in the BaseRngLib code, bu= t I'm concerned this patch may negatively influence any sort of early b= oot RNG,
particularly for the more naive users of RNG_PROTOCOL, b= y providing the possibly flawed RDRAND code. If the efi subsystem/EFISTUB c= ode handles this case well by still mixing
in whatever sources it= can get before using this entropy, then that's great, but providing th= ings like a non-random RNG_PROTOCOL sounds very broken and very unsafe to m= e (again invoking
that possible KASLR at very early boot example)= .

Also the CPUID check seems like an important ste= p towards not-breaking-old-CPUs.
All I'm saying is that w= e shouldn't just hook up the RNG DXE driver without carefully consideri= ng what the code is doing.

Thanks,
Pedro
--000000000000c28a8f05ee0e8d5c--