Re: [edk2-devel] [GSoC proposal] Secure Image Loader

Fri, 16 Apr 2021 00:37:16 -0700 

Good day everyone,
Sorry for the late reply, but somehow my mail filter is malfunctioning. I have two folders for edk2-devel, one where all mails with me in To/CC go, and one for the rest. And somehow, multiple times in this conversation already, mails end up in the latter. While I try to figure out why this happens, if you ever feel like I take too long to answer, please ping me outside the list so I get a mail in my main inbox. Thanks and sorry! 


I'm still not sure whether I misunderstand your intention, or whether I 
am not properly communicating the design. This loader does *not* expect 
all images to be pitch-perfect in all situations. In fact, while I want 
to add more, it already has a couple of FixedAtBuild PCDs to ignore 
certain kinds of malformation. One example is a heavily malformed macOS 
bootloader binary from I think a decade ago, which is completely 
misaligned. For this we have what is currently called "tolerant load" (I 
will refactor and rename PCDs to make more sense once their 
functionality is stabilised), which ignores section alignment entirely. 
The code is also written in a tolerant fashion to my best knowledge, 
e.g. zero'ing the end of a section (i.e. when size of data in file < 
size of loaded section) always extends to the start of the next section, 
or the end of the buffer, if no such exist. So even when a binary is 
malformed, it still ensures the safest possible behaviour.



So, when encountering more broken binaries in the future (for which I 
already have mentioned the EFI image database I am planning to build, 
but obviously would gladly take any outside help I can getting it tested 
on as much real-world hardware as possible), I expect those PCDs to be 
tuned to accept them, and not to revert to known-broken code. The 
tightest of settings possible do not obviously increase security, but 
they are there to not take any chances. A good candidate for this is 
project Amaranth at ISP RAS. Real-world consumer platforms unlikely will 
benefit from some of the constraints, and they can be relaxed, possibly 
forever, just fine. As I have stated in the proposal, the defaults I 
will propose will aim for maximum compatibility, excluding obvious 
security threats. Of course I am trying to fix images like iPXE at the 
moment, but so long as there exist important external images that are 
likely to not be updated (iPXE I believe one could expect to just 
update, GPU OROMs not so much), I will also try my best to provide PCDs 
to have them be allowed. Honestly, the only constraint I can think of we 
do not allow relaxation of is virtual section order, which I never in my 
life have seen violated anywhere. By this I mean that sections are 
ordered with VirtualAddress in ascending order. Things like 
out-of-binary offsets or overlapping sections cannot be tolerated, sorry 
(not that I have seen them, but our loader is a little stricter than the 
old one with this).



In case I actually do misunderstand, the changes to the API are minimal 
(e.g. adding size parameters where there were none), so shimming the old 
loader into it will not be a huge issue. I also added a couple of 
functions to move all PE operations that are currently outside (sections 
for permissions, cert directory for SB, ...) inside, so that would 
merely be a copy+paste from other places of the code. Yet, I *strongly* 
would not like to implement this as the code is very flawed, and not 
just regarding security. If you believe(d) you needed the old loader to 
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 
<nathaniel.l.desim...@intel.com> 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 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.


Don’t forget Thunderbolt dongles, docks, and devices.

Thanks,

Andrew Fish


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 (#74197): https://edk2.groups.io/g/devel/message/74197
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]
-=-=-=-=-=-=-=-=-=-=-=-

























					Reply via email to