Hi Nate, Yes. A FixedAtBuild PCD or a FeatureFlag PCD is one way. However, that approach does add some risk to the existing PE/COFF loader functionality.
My suggestion was to add a new library instance as a peer to the existing library instance. I believe Marvin is also suggesting some changes to the PE/COFF Library Class APIs, which would be more challenging to support old and news. I agree with Andrew that the proposed changes to the PE/COFF Library Class need to be reviewed to see if there is a path to support old and new. Mike > -----Original Message----- > From: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com> > Sent: Tuesday, April 13, 2021 11:05 AM > To: Marvin Häuser <mhaeu...@posteo.de> > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io; > Laszlo Ersek <ler...@redhat.com>; Andrew Fish > <af...@apple.com> > Subject: RE: [edk2-devel] [GSoC proposal] Secure Image Loader > > Hi Marvin, > > What Mike and I were thinking is having a FixedAtBuildPcd which chooses > whether to use the new loader or the old loader at > compile time. We were also thinking the same thing of shimming the old loader > into the new interface. I completely agree > with you that it is unlikely the new loader is "broken"... it is more likely > that broken images exist out in the world > somewhere and that we won't know that they are broken until someone tries to > build their system's firmware with the new > loader included. Once the broken images are found, it can take some time to > get them fixed. A lot of times they come from > outside vendors and the source code for those binaries is not readily > available. Because of that, there may be a need to > fallback to the old loader in the interim period while a fixed binary is > being acquired. > > This could become very difficult for OpROMs on PCIe add-in cards since they > are stored on a separate device rom and most > of the time are completely non-upgradable. We should think about how we can > handle the case where we find an old graphics > or network card built in 2014 that has a UEFI OpROM that contains a broken > PE/COFF header which cannot be fixed. > > Thanks, > Nate > > -----Original Message----- > From: Marvin Häuser <mhaeu...@posteo.de> > Sent: Tuesday, April 13, 2021 12:32 AM > To: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com> > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; devel@edk2.groups.io; > Laszlo Ersek <ler...@redhat.com>; Andrew Fish > <af...@apple.com> > Subject: Re: [edk2-devel] [GSoC proposal] Secure Image Loader > > Hey Mike, > Hey Nate, > > I'm not 100 % sure I get what you mean. The interfaces of the two solutions > are not compatible (however I could write > wrapper code to shim the old into the new I suppose?), so on-the-fly > switching between the two would be inconvenient. I do > plan to keep the old library around and guard it with the deprecated > interfaces macro, for out-of-tree code, however. The > new library interfaces will probably be something like PeCoffLib2, > PeCoffDebugLib2, maybe more. > > I'm also not sure what on-the-fly switching would accomplish, as testing with > the old loader allows broken images to be > loaded, i.e. just because something "works" with the old code but not the > new, it doesn't mean that the new code is > broken. > > Instead of debugging with two libraries, I rather want to make sure this > thing is rock-solid before even sending out the > patches. For this I would like to build a small EFI file database to parse > and load from userland, checking for image > acceptance and memory safety. This would include several versions of Windows > and macOS bootloaders, Option ROMs (NVIDIA > and AMD GOP, iPXE), tools (memtest), and so on. If you have anything you want > to have especially tested, please let me > know. > > Best regards, > Marvin > > 13.04.2021 02:56:22 Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>: > > > Hi Marvin, > > > > I agree with Mike K that having both the new strict loader and the old > > loader co-exist for some time may be the best > option. That will give the ecosystem time to test the new loader and correct > any issues that arise from its introduction. > > > > Best Regards, > > Nate > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kin...@intel.com> > > Sent: Monday, April 12, 2021 5:20 PM > > To: Marvin Häuser <mhaeu...@posteo.de>; devel@edk2.groups.io; > > Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Laszlo Ersek > > <ler...@redhat.com>; Andrew Fish <af...@apple.com>; Kinney, Michael D > > <michael.d.kin...@intel.com> > > Subject: RE: [edk2-devel] [GSoC proposal] Secure Image Loader > > > > Hi Marvin, > > > > If it has not already been considered, one option is to submit a new > > instance of the PE/COFF Library, so both the > existing one and the new one are available to the ecosystem. > > > > This allows you to be successful in submitting code outlined in your > > proposal and gives the ecosystem time to evaluate > and decide when/if to switch from the old to the new. > > > > Mike > > > >> -----Original Message----- > >> From: Marvin Häuser <mhaeu...@posteo.de> > >> Sent: Monday, April 12, 2021 10:22 AM > >> To: devel@edk2.groups.io; Desimone, Nathaniel L > >> <nathaniel.l.desim...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > >> Andrew Fish <af...@apple.com>; Kinney, Michael D > >> <michael.d.kin...@intel.com> > >> Subject: Re: [edk2-devel] [GSoC proposal] Secure Image Loader > >> > >> Good day Nate, > >> > >> As you seem to be mostly in charge of the GSoC side of things, I > >> direct this at you, but of course everyone is welcome to comment. > >> > >> I think I finished my first round of investigations just in time for > >> the deadline. You can find a list of BZs attached[1]. Please note > >> that > >> 1) some are declared security issues and may not be publicly > >> accessible, and 2) that this list is far from complete. I only added > >> items that are > >> a) not implicitly fixed by "simply" deploying the new Image Loader > >> (*some* important prerequisites are listed), and b) I am confident > >> are actual issues (or things to consider) I believe to know how to > >> approach. > >> > >> I have taken notes about more things, e.g. the existence of the > >> security architectural protocols, which I could not find a rationale > >> for. I can prepare something for this matter, but it really needs an > >> active discussion with some of the core people. I'm not sure delayed > >> e-mail discussion is going to be enough, but there is an official IRC > >> I suppose. :) I hope we can work something out for this. > >> > >> I also hope this makes it clearer why I don't believe that we need to > >> "fill" 10 weeks, but rather the opposite. This is not a matter of > >> replacing a library instance, but the whole surrounding ecosystem > >> needs to follow for the changes to make sense. And as I tried to make > >> clear in my discussion with Michael Brown, I am not keen on > >> preserving backwards-compatibility with platform code (i.e. PEI, DXE, > >> things we consider "internal"), as most of it should be controlled by > >> EDK II already. This of course does *not* include user code (OROMs, > >> bootloaders, ...), for which I want to provide the *option* to lock > >> some of them out for security, but with sane defaults that will > >> ensure good compatibility. > >> > >> I'd like to thank Michael Brown for his cooperation and support, > >> because we recently landed changes in iPXE to allow for the strictest > >> image format and permission constraints currently possible[2]. > >> > >> I will have to rework the submitted proposal to reflect the new > >> knowledge. To be honest, seeing how the BZs kept rolling in, I am not > >> convinced an amazing amount of mainlining can be accomplished during > >> the > >> 10 weeks. It may have to suffice to have a publicly accessible > >> prototype (e.g. OVMF) and a subset of the planned patches on the list. > >> I hope you can manage to provide some feedback before the deadline passes > >> tomorrow. > >> > >> Thank you in advance! > >> > >> Best regards, > >> Marvin > >> > >> [1] > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3315 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3316 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3317 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3318 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3319 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3320 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3321 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3322 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3323 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3324 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3326 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3327 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3328 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3329 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3330 > >> https://bugzilla.tianocore.org/show_bug.cgi?id=3331 > >> > >> [2] https://github.com/ipxe/ipxe/pull/313 > >> > >> On 06.04.21 11:41, Nate DeSimone wrote: > >>> Hi Marvin, > >>> > >>> Great to meet you and welcome back! Glad you hear you are > >>> interested! Completing a formal verification of a PE/COFF > >> loader is certainly impressive. Was this done with some sort of > >> automated theorem proving? It would seem a rather arduous task doing > >> an inductive proof for an algorithm like that by hand! I completely agree > >> with you that getting a formally verified > PE/COFF loader into mainline is undoubtably valuable and would pay security > dividends for years to come. > >>> > >>> Admittedly, this is an area of computer science that I don't have a > >>> great deal of experience with. The furthest I have > >> gone on this topic is writing out proofs for simple algorithms on > >> exams in my Algorithms class in college. Regardless you have a much > >> better idea of what the current status is of the work that you and > >> Vitaly have done. I guess my only question is do you think there is > >> sufficient work remaining to fill the 10 week GSoC development window? > >> Certainly we can use some of that time to > perform the code reviews you mention and write up formal ECRs for the UEFI > spec changes that you believe are needed. > >>> > >>> Thank you for sending the application and alerting us to the great > >>> work you and Vitaly have done! I'll read your paper > >> more closely and come back with any questions I still have. > >>> > >>> With Best Regards, > >>> Nate > >>> > >>>> -----Original Message----- > >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > >>>> Marvin Häuser > >>>> Sent: Sunday, April 4, 2021 4:02 PM > >>>> To: devel@edk2.groups.io; Laszlo Ersek <ler...@redhat.com>; Andrew > >>>> Fish <af...@apple.com>; Kinney, Michael D > >>>> <michael.d.kin...@intel.com> > >>>> Subject: [edk2-devel] [GSoC proposal] Secure Image Loader > >>>> > >>>> Good day everyone, > >>>> > >>>> I'll keep the introduction brief because I've been around for a > >>>> while now. :) I'm Marvin Häuser, a third-year Computer Science > >>>> student from TU Kaiserslautern, Germany. Late last year, my > >>>> colleague Vitaly from ISP RAS and me introduced a formally verified > >>>> Image Loader for UEFI usage at ISP RAS Open[1] due to various > >>>> defects we outlined in the corresponding paper. Thank you once again > >>>> Laszlo for your *incredible* review work on the > publication part. > >>>> > >>>> I now want to make an effort to mainline it, preferably as part of > >>>> the current Google Summer of Code event. To be clear, my internship > >>>> at ISP RAS has concluded, and while Vitaly will be available for > >>>> design discussion, he has other priorities at the moment and the > >>>> practical part will be on me. I have previously submitted a proposal via > >>>> the GSoC website for your review. > >>>> > >>>> There are many things to consider: > >>>> 1. The Image Loader is a core component, and there needs to be a > >>>> significant level of quality and security assurance. > >>>> 2. Being consumed by many packages, the proposed patch set will > >>>> take a lot of time to review and integrate. > >>>> 3. During my initial exploration, I discovered defective PPIs and > >>>> protocols (e.g. > >>>> returning data with no corresponding size) originating from the > >>>> UEFI PI and UEFI specifications. Changes need to be discussed, > >>>> settled on, and submitted to the UEFI Forum. > >>>> 4. Some UEFI APIs like the Security Architecture protocols are > >>>> inconveniently abstract, see 5. > >>>> 5. Some of the current code does not use the existing context, or > >>>> accesses it outside of the exposed APIs. The control flow of the > >>>> dispatchers may need to be adapted to make the context available to > >>>> appropriate APIs. > >>>> > >>>> But obviously there are not only unpleasant considerations: > >>>> A. The Image Loader is mostly formally verified, and only very few > >>>> changes will be required from the last proven state. This gives a > >>>> lot of trust in its correctness and safety. > >>>> B. All outlined defects that are of critical nature have been fixed > >>>> successfully. > >>>> C. The Image Loader has been tested with real-world code loading > >>>> real-world OSes on thousands of machines in the past few months, > >>>> including rejecting malformed images (configurable by PCD). > >>>> D. The new APIs will centralise everything PE, reducing code > >>>> duplication and potentially unsafe operations. > >>>> E. Centralising and reduced parse duplication may improve overall > >>>> boot performance. > >>>> F. The code has been coverage-tested to not contain dead code. > >>>> G. The code has been fuzz-tested including sanitizers to not invoke > >>>> undefined behaviour. > >>>> H. I already managed to identify a malformed image in OVMF with its > >>>> help (incorrectly reported section alignment of an Intel IPXE > >>>> driver). A fix will be submitted shortly. > >>>> I. I plan to support PE section permissions, allowing for read-only > >>>> data segments when enabled. > >>>> > >>>> There are likely more points for both lists, but I hope this gives > >>>> a decent starting point for discussion. What are your thoughts on > >>>> the matter? I strongly encourage everyone to read the section > >>>> regarding defects of our publication[2] to better understand the > >>>> motivation. The vague points above can of course be elaborated in due > >>>> time, as you see fit. > >>>> > >>>> Thank you for your time! > >>>> > >>>> Best regards, > >>>> Marvin > >>>> > >>>> > >>>> [1] https://github.com/mhaeuser/ISPRASOpen-SecurePE > >>>> [2] https://arxiv.org/pdf/2012.05471.pdf > >>>> > >>>> > >>>> > >>>> > >>> > >>> > >>> > >>> > >>> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74061): https://edk2.groups.io/g/devel/message/74061 Mute This Topic: https://groups.io/mt/81853302/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
