From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.18427.1669122624452553551 for ; Tue, 22 Nov 2022 05:10:24 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@zx2c4.com header.s=20210105 header.b=k/1AJTY7; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: srs0=2rl2=3w=zx2c4.com=jason@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B7A21616ED for ; Tue, 22 Nov 2022 13:10:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CD1C8C433C1 for ; Tue, 22 Nov 2022 13:10:22 +0000 (UTC) Authentication-Results: smtp.kernel.org; dkim=pass (1024-bit key) header.d=zx2c4.com header.i=@zx2c4.com header.b="k/1AJTY7" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zx2c4.com; s=20210105; t=1669122621; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=u6JZgDS2oUw2Ed+cysqJrf3oINCpzYfaa77YlmyCJoY=; b=k/1AJTY7tP2E1ZtGl43x2q98vc8CLr+YAtst3GDHa4KH82zvAKMWRkRu40ksd6IAd9woBJ MDv1USGSVslemD9oloxkPwDDjItKnHirGTzdlDpbw9yHMFUsy/YkEqZX1gTmYvBWjiKxzO ppRth51vxLjVhc7XE7ZNF3bw6uxA3Tc= Received: by mail.zx2c4.com (ZX2C4 Mail Server) with ESMTPSA id 74f10ed6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Tue, 22 Nov 2022 13:10:21 +0000 (UTC) Received: by mail-vs1-f46.google.com with SMTP id a6so14379346vsc.5 for ; Tue, 22 Nov 2022 05:10:21 -0800 (PST) X-Gm-Message-State: ANoB5pm+cKHytCsJ068N/68P5lIx87wE6u/VBtyfI4H3PcXiHtb1QumZ kVmWTDcB8Uzol/6wcjOmE8tfTObVFJ7/j1MdVuM= X-Google-Smtp-Source: AA0mqf4yyviO2pvqRZG9ZFbpmyDtJXP4742Y5XYGRjNJtatK85KSOpJyj915nGGVg0FYm+ebKCmGrI/1TXvV8144JJc= X-Received: by 2002:a67:f54e:0:b0:3b0:4e31:10f7 with SMTP id z14-20020a67f54e000000b003b04e3110f7mr2076731vsn.73.1669122619879; Tue, 22 Nov 2022 05:10:19 -0800 (PST) MIME-Version: 1.0 Received: by 2002:ab0:7606:0:b0:418:ce6c:9783 with HTTP; Tue, 22 Nov 2022 05:10:19 -0800 (PST) In-Reply-To: References: <20221110134738.3798618-1-ardb@kernel.org> <20221110134738.3798618-4-ardb@kernel.org> From: "Jason A. Donenfeld" Date: Tue, 22 Nov 2022 14:10:19 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH 3/3] OvmfPkg/OvmfX86: Enable RDRAND based EFI_RNG_PROTOCOL implementation To: Pedro Falcato Cc: devel@edk2.groups.io, ardb@kernel.org, Liming Gao , Rebecca Cran , Pierre Gondois , Leif Lindholm , Sami Mujawar , Gerd Hoffmann Content-Type: text/plain; charset="UTF-8" Hi Pedro, On 11/22/22, Pedro Falcato wrote: > 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 thing is, in low-entropy early boot scenarios, the alternative would be just having nothing. The kernel doesn't pause boot to wait for a good source....it just uses a bad one. So adding RDRAND in EFI helps. Really, more entropy from more sources as early as possible and as fast as possible only ever helps here. > also does proper sanity checking on it. No, there's nothing "proper" about it. It's a dumb basic thing. > > - EFI on actual baremetal firmware, as opposed to OVMF, already provides >> EFI, so this is par for the course. >> > > Hm? What do you mean? I meant already provides RDRAND, sorry. All x86 hardware with EFI has this enabled. > 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). Except as related several times now, it will only help and won't hurt. If you want to improve the RDRAND DXE, do it, but aside from the CPUID issue you raised, I don't think your "concerns" warrant holding up this patch. > > Also the CPUID check seems like an important step towards > not-breaking-old-CPUs. Yes. If what you say is true, this should be fixed asap. Do you intend to send a patch? > All I'm saying is that we shouldn't just hook up the RNG DXE driver without > carefully considering what the code is doing. To point out again, this is already hooked up to baremetal firmware. So if you see issues that are worth fixing, fix them, but it shouldn't hold up Ard's patchset. Jason