On 16 February 2018 at 19:22, Peter Jones <pjo...@redhat.com> wrote: > On Fri, Feb 16, 2018 at 10:48:32AM -0800, Joe Konno wrote: >> On Fri, Feb 16, 2018 at 11:18:12AM +0000, Ard Biesheuvel wrote: >> > On 16 February 2018 at 11:08, Borislav Petkov <b...@alien8.de> wrote: >> > > On Fri, Feb 16, 2018 at 10:58:47AM +0000, Ard Biesheuvel wrote: >> > >> By your own reasoning above, that's a no-no as well. >> > > >> > > I'm sure we can come up with some emulation - the same way we did the >> > > BIOS emulation. >> > > >> > >> But thanks for your input. Anyone else got something constructive to >> > >> contribute? >> > > >> > > The not-breaking userspace is constructive contribution. The last >> > > paragraph is my usual rant. >> > > >> > >> > Fair enough. And I am not disagreeing with you either. >> > >> > So question to Joe: is it well defined which variables may exhibit >> > this behavior? >> >> For brevity's sake, "not yet." Folks-- e.g. firmware writers and >> equipment makers-- may use SMIs more (or less) than others. So, how many >> SMIs generated by the reference shell script can, and will, vary >> platform to platform. I and my colleagues are digging into this. > > As a first guess: anything generated during boot is probably not an > SMI. Everything else is probably an SMI. In fact, I would expect that > for most systems, the entire list of things that *don't* generate an SMI > (aside from a few IBV specific variables) is all the variables in Table > 10 of the UEFI spec that don't have the NV flag. > >> > Given that UEFI variables are GUID scoped, would whitelisting certain >> > GUIDs (the ones userland currently relies on to be readable my >> > non-privileged users) and making everything else user-only solve this >> > problem as well? >> >> Perhaps. I don't want us chasing white rabbits just yet, but the >> whitelist is but one approach under consideration. We may see some other >> patches or RFCs about caching and/or shadowing variable values in >> efivarfs to reduce the number of direct EFI reads, with the goal of >> reducing how many SMIs are generated. >> >> Any obvious EFI variables that userspace tools have come to depend on-- >> those which normal, unprivileged users need to read-- are helpful inputs >> to this discussion. > > So, our big userland consumers are efibootmgr, fwupdate, and > "systemctl reboot --firmware-setup". efibootmgr and fwupdate can do the > "show the current status" half of their job as a user right now, but > they rely on root for the other half anyway. I don't think we normally > use libfwup as non-root even for reading status. So basically, the use > case from userland that this will effect looks like: > > efibootmgr -v > *scratch head* > sudo su - > efibootmgr -b 0000 -B > efibootmgr -b 0000 -c -L "fixed entry" ... > exit > > I don't feel really bad about people having to move the third line up to > the top of that. It's also not a use case you can have very much of: it > means you manually booted without any valid boot entries. fallback.efi > would have created a valid boot entry if you didn't do it manually. > > "systemctl reboot --firmware-setup" effectively runs as root in all > cases. > > The only thing we really must ensure to preserve the other workflows > is making sure creat() and open() with 0644 doesn't return an error (it > obviously won't be preserved across a reboot), because that would break > the existing tools. But I don't see anything in this patchset which > will cause that. > > tl;dr: I think changing everything to 0600 is probably completely fine, > and whitelisting is probably pointless.
This is why I was leaning towards applying these patches: not breaking userland is an important rule, but it does not imply every aspect of behavior observable by userland is set in stone. In other words, I agree with Peter that making this change does not *break* userland in a way anyone is likely to care deeply about. Adding a caching layer to efivarfs for this purpose is really overkill IMHO. Also, we need this only on x86, so I'd like to see it proposed in a way that allows it to be compiled out for architectures that have no need for it.