From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web08.5882.1618558625325293232 for ; Fri, 16 Apr 2021 00:37:06 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=JoLY4wIT; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id EB5D7240106 for ; Fri, 16 Apr 2021 09:37:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1618558622; bh=cFPY1RLeShnhunhjxjpABQIW5+L4FRpc5hx9N8uD4uc=; h=Subject:To:Cc:From:Date:From; b=JoLY4wIT2KZH59neO2fz3/I/efTnjshbZpN9614Hgq9QwttMn9e3OYjt1P0oLbMnp CYzjJt1ZMIkC2RKmEHeJRX3WjpbjL/IxKm6+h6LCbGc2q7iFa2ohndz0T83ieSH+/b RDub9naMaNAY6gWaj8y0ut9VHe87rCq+rpPgQcwvkvDOSHMfvx6f4Thdv8zvnzV9Rk 8mI370SrcMrVURhRcpkJSG5i9WX+tTegvG4awepwE/rIAKk+3TnPb7LOVjunXPyXYG mwl3DPwB0Nf+QE03MF/iUWZjxKDWMUwCHzFKwKTrRYDaT0gmSvshSQLwixNzEqTL7l Blx30RjAynyqg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4FM7MJ1yC5z6tmN; Fri, 16 Apr 2021 09:36:59 +0200 (CEST) Subject: Re: [edk2-devel] [GSoC proposal] Secure Image Loader To: Andrew Fish , "Desimone, Nathaniel L" , Mike Kinney Cc: "devel@edk2.groups.io" , Laszlo Ersek References: <055bcd6f-c055-25a8-f258-6581ccbbd591@posteo.de> <4fa3c0fc-8865-447e-96ec-7286b0fd9a7f@posteo.de> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <347b92a5-cf68-222e-33a1-c918dd242e0c@posteo.de> Date: Fri, 16 Apr 2021 07:36:59 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Good day everyone, Sorry for the late reply, but somehow my mail filter is malfunctioning.=20 I have two folders for edk2-devel, one where all mails with me in To/CC=20 go, and one for the rest. And somehow, multiple times in this=20 conversation already, mails end up in the latter. While I try to figure=20 out why this happens, if you ever feel like I take too long to answer,=20 please ping me outside the list so I get a mail in my main inbox. Thanks= =20 and sorry! I'm still not sure whether I misunderstand your intention, or whether I=20 am not properly communicating the design. This loader does *not* expect=20 all images to be pitch-perfect in all situations. In fact, while I want=20 to add more, it already has a couple of FixedAtBuild PCDs to ignore=20 certain kinds of malformation. One example is a heavily malformed macOS=20 bootloader binary from I think a decade ago, which is completely=20 misaligned. For this we have what is currently called "tolerant load" (I= =20 will refactor and rename PCDs to make more sense once their=20 functionality is stabilised), which ignores section alignment entirely.=20 The code is also written in a tolerant fashion to my best knowledge,=20 e.g. zero'ing the end of a section (i.e. when size of data in file <=20 size of loaded section) always extends to the start of the next section,= =20 or the end of the buffer, if no such exist. So even when a binary is=20 malformed, it still ensures the safest possible behaviour. So, when encountering more broken binaries in the future (for which I=20 already have mentioned the EFI image database I am planning to build,=20 but obviously would gladly take any outside help I can getting it tested= =20 on as much real-world hardware as possible), I expect those PCDs to be=20 tuned to accept them, and not to revert to known-broken code. The=20 tightest of settings possible do not obviously increase security, but=20 they are there to not take any chances. A good candidate for this is=20 project Amaranth at ISP RAS. Real-world consumer platforms unlikely will= =20 benefit from some of the constraints, and they can be relaxed, possibly=20 forever, just fine. As I have stated in the proposal, the defaults I=20 will propose will aim for maximum compatibility, excluding obvious=20 security threats. Of course I am trying to fix images like iPXE at the=20 moment, but so long as there exist important external images that are=20 likely to not be updated (iPXE I believe one could expect to just=20 update, GPU OROMs not so much), I will also try my best to provide PCDs=20 to have them be allowed. Honestly, the only constraint I can think of we= =20 do not allow relaxation of is virtual section order, which I never in my= =20 life have seen violated anywhere. By this I mean that sections are=20 ordered with VirtualAddress in ascending order. Things like=20 out-of-binary offsets or overlapping sections cannot be tolerated, sorry= =20 (not that I have seen them, but our loader is a little stricter than the= =20 old one with this). In case I actually do misunderstand, the changes to the API are minimal=20 (e.g. adding size parameters where there were none), so shimming the old= =20 loader into it will not be a huge issue. I also added a couple of=20 functions to move all PE operations that are currently outside (sections= =20 for permissions, cert directory for SB, ...) inside, so that would=20 merely be a copy+paste from other places of the code. Yet, I *strongly*=20 would not like to implement this as the code is very flawed, and not=20 just regarding security. If you believe(d) you needed the old loader to=20 accept malformed images, I hope I could explain why I believe otherwise. Thank your for your feedback and considerations! Best regards, Marvin On 13.04.21 20:14, Andrew Fish wrote: > >> On Apr 13, 2021, at 11:04 AM, Desimone, Nathaniel L wrote: >> >> 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 a= lso thinking the same thing of shimming the old loader into the new interfa= ce. I completely agree with you that it is unlikely the new loader is "brok= en"... it is more likely that broken images exist out in the world somewher= e 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 image= s are found, it can take some time to get them fixed. A lot of times they c= ome from outside vendors and the source code for those binaries is not read= ily 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 completel= y 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 t= hat contains a broken PE/COFF header which cannot be fixed. >> > Don=E2=80=99t forget Thunderbolt dongles, docks, and devices. > > Thanks, > > Andrew Fish > >> Thanks, >> Nate >> >> -----Original Message----- >> From: Marvin H=C3=A4user >> Sent: Tuesday, April 13, 2021 12:32 AM >> To: Desimone, Nathaniel L >> Cc: Kinney, Michael D ; devel@edk2.groups.i= o; Laszlo Ersek ; Andrew Fish >> 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 solut= ions 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 t= he deprecated interfaces macro, for out-of-tree code, however. The new libr= ary interfaces will probably be something like PeCoffLib2, PeCoffDebugLib2,= maybe more. >> >> I'm also not sure what on-the-fly switching would accomplish, as testin= g 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 t= he new code is broken. >> >> Instead of debugging with two libraries, I rather want to make sure thi= s 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, c= hecking 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 e= specially tested, please let me know. >> >> Best regards, >> Marvin >> >> 13.04.2021 02:56:22 Desimone, Nathaniel L : >> >>> 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 e= cosystem time to test the new loader and correct any issues that arise from= its introduction. >>> >>> Best Regards, >>> Nate >>> >>> -----Original Message----- >>> From: Kinney, Michael D >>> Sent: Monday, April 12, 2021 5:20 PM >>> To: Marvin H=C3=A4user ; devel@edk2.groups.io; >>> Desimone, Nathaniel L ; Laszlo Ersek >>> ; Andrew Fish ; Kinney, Michael D >>> >>> 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 i= nstance of the PE/COFF Library, so both the existing one and the new one ar= e available to the ecosystem. >>> >>> This allows you to be successful in submitting code outlined in your p= roposal and gives the ecosystem time to evaluate and decide when/if to swit= ch from the old to the new. >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: Marvin H=C3=A4user >>>> Sent: Monday, April 12, 2021 10:22 AM >>>> To: devel@edk2.groups.io; Desimone, Nathaniel L >>>> ; Laszlo Ersek ; >>>> Andrew Fish ; Kinney, Michael D >>>> >>>> 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 ap= proach. >>>> >>>> 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 pa= sses tomorrow. >>>> >>>> Thank you in advance! >>>> >>>> Best regards, >>>> Marvin >>>> >>>> [1] >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3315 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3316 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3317 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3318 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3319 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3320 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3321 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3322 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3323 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3324 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3326 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3327 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3328 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3329 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3330 >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D3331 >>>> >>>> [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 a= gree 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 me= ntion and write up formal ECRs for the UEFI spec changes that you believe a= re 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 On Behalf Of >>>>>> Marvin H=C3=A4user >>>>>> Sent: Sunday, April 4, 2021 4:02 PM >>>>>> To: devel@edk2.groups.io; Laszlo Ersek ; Andrew >>>>>> Fish ; Kinney, Michael D >>>>>> >>>>>> 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=C3=A4user, 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 agai= n 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 proposa= l 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 p= rotocols (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 d= ue 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>>=20 >>>>> >>>>>