On 08/04/22 11:58, Ard Biesheuvel wrote: > On Thu, 4 Aug 2022 at 07:55, Laszlo Ersek <ler...@redhat.com> wrote: >> >> On 08/04/22 04:52, Yuan Yu wrote: >>> Currently networking can only be enabled/disabled at compile time. This >>> patch series will add support to disable VirtIo net at runtime even if >>> the functionality is built into binary at compile time. >>> >>> This will enable VMM to reduce attack surface without recompilation. >>> >>> The changes can be seen at: >>> https://github.com/yyu/edk2/tree/network_cfg_lib_v1 >>> >>> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> >>> Cc: Jordan Justen <jordan.l.jus...@intel.com> >>> Cc: Laszlo Ersek <ler...@redhat.com> >>> Cc: Anthony Perard <anthony.per...@citrix.com> >>> Cc: Julien Grall <jul...@xen.org> >>> >>> Yuan Yu (2): >>> OvmfPkg: Introduce NetworkCfgLib >>> OvmfPkg: Use PcdNetworkSupport to enable/disable VirtIo net >>> >>> OvmfPkg/OvmfPkg.dec | 3 ++ >>> OvmfPkg/OvmfPkgX64.dsc | 7 ++++- >>> OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.inf | 29 ++++++++++++++++++ >>> OvmfPkg/VirtioNetDxe/VirtioNet.inf | 3 ++ >>> OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.c | 32 ++++++++++++++++++++ >>> OvmfPkg/VirtioNetDxe/EntryPoint.c | 10 ++++++ >>> 6 files changed, 83 insertions(+), 1 deletion(-) >>> create mode 100644 OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.inf >>> create mode 100644 OvmfPkg/Library/NetworkCfgLib/NetworkCfgLib.c >>> >> >> Well I've not been reviewing upstream edk2 patches for a while, but the >> virtio-net driver is still very close to my heart, so this patch kind of >> hits a nerve. >> > > Welcome back old friend! > >> I think I disagree with the idea and the implementation both. >> >> Minimally, the idea needs a much better elaboration -- what is the >> threat model? Do you want to protect the host from the guest, or the >> guest from the host? Or something else? How does controlling a single >> SNP driver via fw_cfg (which is also dictated by the host) help? >> > > I have to confess that I was the one who suggested this approach to > Yuan internally, but mainly to get the discussion going, as I was > anticipating some pushback, just not from you :-)
Heh, sorry about that :) > > 'Reducing the attack surface' is probably not the most accurate > characterization of the purpose. We are simply looking for a way to > disable network boot from the vmm/host side without affecting > how/which network interfaces the guest exposes to the OS. > >> Regarding the implementation: there is much more to networking in edk2 >> than VirtioNetDxe. UEFI driver binaries (SNP drivers) built from iPXE >> can be passed in via the NICs' option ROMs. SNP drivers can be loaded >> from the UEFI system partition (for example, Intel's binary-only driver >> for QEMU's e1000* cards). >> >> If you can control this fw_cfg switch from the VMM side, you can also >> control the VMM enough to simply *not give* a virtio-net device to the >> guest. Then the driver (it being a UEFI driver following the UEFI driver >> model) will simply not have anything to bind. >> > > Sure, but then the OS will lose networking as well. We just want to > remove the ability to network boot without impacting anything else > that relies on virtio-net > >> Sorry I find this approach very wrong. If you really need it for your >> particular VMM, I kind of suggest not upstreaming this patch. I see it >> as a step backwards for the upstream project. >> > > If there are better ways to achieve this, we're all ears, but I think > that having a PCD which could either be fixed at build and compiled > out completely, or be set via a NULL library resolution, or even be > wired to a menu option (using PcdsDynamicHii] is a rather low-impact > but flexible way to go about this. How about using PCDs, but at a higher level in the edk2 network stack (regardless of the SNP driver(s) used)? Not that I'm a huge fan of them, but we already have PcdIPv4PXESupport and PcdIPv6PXESupport, and they can be set for OVMF and ArmVirtQemu via fw_cfg. If both PCDs are "PXE_DISABLED" (0), then PxeBcDriverEntryPoint() [NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c] exits early. See: - https://bugzilla.tianocore.org/show_bug.cgi?id=1695 - https://bugzilla.tianocore.org/show_bug.cgi?id=2681 So that's at least "prior art". If similar PCDs could be introduced for other kinds of network boot (I think there's only HTTP(S)v[46] to speak of [*]), i.e. in "NetworkPkg.dec", then I guess setting those from fw_cfg in ArmVirtQemu and OVMF could be fine. I assume HttpBootDxe would be the driver to short-circuit. [*] Hmm, maybe not just that. Do we consider booting off an iSCSI device "network boot"? Perhaps where you want to "cut the stack" is Ip4Dxe/Ip6Dxe. Prior art with PXE does suggest PCDs for the higher-level network boot drivers. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92112): https://edk2.groups.io/g/devel/message/92112 Mute This Topic: https://groups.io/mt/92808627/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
