Which is where the pointer on ia32 for the Base Relocation Table should
be. It points to 0x8447, which isn't a particularly reasonable address as
numbers go, and happens to have this data there:
Peter Jones [Sat, 20 Sep 2014 18:03:03 +0000 (14:03 -0400)]
Generate a sane PE header on shim, fallback, and MokManager.
It turns out a7249a65 was masking a second problem - on some binaries,
when we actually don't have any base relocations at all, binutils'
"objcopy --target efi-app-x86_64" is generating a PE header with a base
relocations pointer that happily points into the middle of our text
section. So with shim processing base relocations correctly, it refuses
to load those binaries.
That wouldn't be so bad, except those binaries are MokManager.efi,
fallback.efi, and shim.efi, and sometimes they're .reloc, which we're
actually trying to handle correctly now because grub builds with a real
and valid .reloc table. So though I didn't think there was any hair
left on this yak, more shaving ensues.
With this change, instead of letting objcopy do whatever it likes, we
switch to "-O binary" and merely link in a header that's appropriate for
our binaries. This is the same method Ard wrote for aarch64, and it
seems to work fine in either place (modulo some minor changes.)
At some point this should be merged into gnu-efi instead of carrying our
own crt0-efi-x86_64.S, but that's a less immediate problem.
Peter Jones [Fri, 19 Sep 2014 20:46:01 +0000 (16:46 -0400)]
Fix our "in_protocol" printing.
When I merged 4bfb13d and fixed the conflicts, I managed to make the
in_protocol test exactly backwards, so that's why we don't currently see
error messages.
Peter Jones [Fri, 19 Sep 2014 15:48:56 +0000 (11:48 -0400)]
Don't call AuthenticodeVerify if vendor_cert_size is 0.
Actually check the size of our vendor cert quite early, so that there's
no confusion as to what's going on.
This isn't strictly necessary, in that in all cases if vendor_cert_size
is 0, then AuthenticodeVerify -> Pkcs7Verify() -> d2i_X509() will result
in a NULL "Cert", and it will return FALSE, and we'll reject the
signature, but better to avoid all that code in the first place. Belt
and suspenders and whatnot.
Peter Jones [Fri, 19 Sep 2014 15:37:35 +0000 (11:37 -0400)]
Make 64-on-32 maybe work on x86_64.
This is mostly based on a patch (https://github.com/mjg59/shim/issues/30)
from https://github.com/TBOpen , which refactors our __LP64__
tests to be tests of the header magic instead. I've simplified things
by using what we've pre-loaded into "context" and making some helper
functions so the conditionals in most of the code say what they do,
instead of how they work.
Note that we're only allowing that from in_protocol's loader - that is,
we'll let 64-bit grub load a 32-bit kernel or 32-bit grub load a 64-bit
kernel, but 32-bit shim isn't loading a 64-bit grub.
Peter Jones [Thu, 18 Sep 2014 22:34:38 +0000 (18:34 -0400)]
Actually refer to the base relocation table of our loaded image.
Currently when we process base relocations, we get the correct Data
Directory pointer from the headers (context->RelocDir), and that header
has been copied into our pristine allocated image when we copied up to
SizeOfHeaders. But the data it points to has not been mirrored in to
the new image, so it is whatever data AllocPool() gave us.
This patch changes relocate_coff() to refer to the base relocation table
from the image we loaded from disk, but apply the fixups to the new
copy.
I have no idea how x86_64 worked without this, but I can't make aarch64
work without it. I also don't know how Ard or Leif have seen aarch64
work. Maybe they haven't? Leif indicated on irc that they may have
only tested shim with simple "hello world" applications from gnu-efi;
they are certainly much less complex than grub.efi, and are generated
through a different linking process.
My only theory is that we're getting recycled data there pretty reliably
that just makes us /not/ process any relocations, but since our
ImageBase is 0, and I don't think we ever load grub with 0 as its base
virtual address, that doesn't follow. I'm open to any other ideas
anybody has.
I do know that on x86_64 (and presumably aarch64 as well), we don't
actually start seeing *symptoms* of this bug until the first chunk[0] of 94c9a77f is applied[1]. Once that is applied, relocate_coff() starts
seeing zero[2] for both RelocBase->VirtualAddress and
RelocBase->SizeOfBlock, because RelocBase is a (generated, relative)
pointer that only makes sense in the context of the original binary, not
our partial copy. Since RelocBase->SizeOfBlock is tested first,
relocate_base() gives us "Reloc block size is invalid"[3] and returns
EFI_UNSUPPORTED. At that point shim exits with an error.
[0] The second chunk of 94c9a77f patch makes no difference on this
issue.
[1] I don't see why at all.
[2] Which could really be any value since it's AllocatePool() and not
AllocateZeroPool() results, but 0 is all I've observed; I think
AllocatePool() has simply never recycled any memory in my test
cases.
[3] which is silent because perror() tries to avoid talking because that
has caused much crashing in the past; work needs to go in to 0.9 for
this.
Peter Jones [Wed, 27 Aug 2014 20:39:51 +0000 (16:39 -0400)]
Make sure we don't try to load a binary from a different arch.
Since in theory you could, for example, get an x86_64 binary signed that
also behaves as an ARM executable, we should be checking this before
people build on other architectures.
Peter Jones [Wed, 27 Aug 2014 17:26:23 +0000 (13:26 -0400)]
Don't name something exit().
On aarch64 due to some terrifying include chain we wind up with
Cryptlib's definition of exit here. I'm not a glutton for punishment,
so I'm just changing the name so it's not coliding.
Ard Biesheuvel [Wed, 13 Aug 2014 11:35:38 +0000 (13:35 +0200)]
Handle empty .reloc section in PE/COFF loader
On archs where no EFI aware objcopy is available, the generated PE/COFF
header contains a .reloc section which is completely empty. Handle this by
- returning early from relocate_coff() with EFI_SUCCESS,
- ignoring discardable sections in the section loader.
Ard Biesheuvel [Tue, 12 Aug 2014 13:33:20 +0000 (15:33 +0200)]
Factor out x86-isms and add cross compile support
This patch cleans up and refactors the Makefiles to better allow new
architectures to be added:
- remove unused Makefile definitions
- import Makefile definitions from top level rather than redefining
- move x86 specific CFLAGS to inside ifeq() blocks
- remove x86 inline asm
- allow $(FORMAT) to be overridden: this is necessary as there exists no
EFI or PE/COFF aware objcopy for ARM
Ard Biesheuvel [Tue, 12 Aug 2014 13:33:18 +0000 (15:33 +0200)]
CryptLib: undefine va_arg and friends before redefining them
Upstream GNU-EFI contains changes to efistdarg.h resulting in the va_start,
va_arg and va_end macros to be #defined unconditionally. Make sure we #undef
them before overriding the definitions.
If ca.crt was added into the certificate database, ca.crt would be the first
certificate in the signature. Because shim couldn't verify ca.crt with the
embedded shim.cer, it failed to load MokManager.efi.signed and
fallback.efi.signed.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
MokManager: handle the error status from ReadKeyStroke
On some machines, even though the key event was signaled, ReadKeyStroke
still got EFI_NOT_READY. This commit handles the error status to avoid
console_get_keystroke from returning unexpected keys.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
Conflicts:
MokManager.c
A non-DER encoding x509 certificate may be mistakenly enrolled into
db or MokList. This commit checks the first 4 bytes of the certificate
to ensure that it's DER encoding.
This commit also removes the iteration of the x509 signature list.
Per UEFI SPEC, each x509 signature list contains only one x509 certificate.
Besides, the size of certificate is incorrect. The size of the header must
be substracted from the signature size.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
The previous strategy is to locate the first available PXE_BASE_CODE
protocol and to fetch the second stage image from it, and this may
cause shim to fetch the wrong second stage image, i.e. grub.efi.
Consider the machine with the following boot order:
1. PXE Boot
2. Hard Drive
Assume that the EFI image, e.g. bootx64.efi, in the PXE server is
broken, then "PXE Boot" will fail and fallback to "Hard Drive". While
shim.efi in "Hard Drive" is loaded, it will find the PXE protocol is
available and fetch grub.efi from the PXE server, not grub.efi in the
disk.
This commit checks the DeviceHandle from Loaded Image. If the device
supports PXE, then shim fetches grub.efi with the PXE protocol. Otherwise,
shim loads grub.efi from the disk.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
[fallback] Try to boot the first boot option anyway
Some UEFI implementations never care the boot options, so the
restored boot options could be just ignored and this results in
endless reboot. To avoid this situation, this commit makes
fallback.efi to load the first matched boot option even if there
is no boot option to be restored. It may not be perfect, but at
least the bootloader is loaded...
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
set_boot_order() already copies the old BootOrder to the variable,
bootorder. Besides, we can adjust BootOrder when adding the newly
generated boot option. So, we don't have to copy the old one again
in update_boot_order(). This avoid the duplicate entries in BootOrder.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
Kees Cook [Mon, 3 Dec 2012 23:52:48 +0000 (15:52 -0800)]
additional bounds-checking on section sizes
This adds additional bounds-checking on the section sizes. Also adds
-Wsign-compare to the Makefile and replaces some signed variables with
unsigned counteparts for robustness.
Peter Jones [Fri, 14 Feb 2014 20:38:25 +0000 (15:38 -0500)]
Allow fallback to use the system's LoadImage/StartImage .
Track use of the system's LoadImage(), and when the next StartImage()
call is for an image the system verified, allow that to count as
participating, since it has been verified by the system's db.
Peter Jones [Fri, 31 Jan 2014 15:31:10 +0000 (10:31 -0500)]
[fallback] Attempt to re-use existing entries when possible.
Some firmwares seem to ignore our boot entries and put their fallback
entries back on top. Right now that results in a lot of boot entries
for our stuff, a la https://bugzilla.redhat.com/show_bug.cgi?id=995834 .
Instead of that happening, if we simply find existing entries that match
the entry we would create and move them to the top of the boot order,
the machine will continue to operate in failure mode (which we can't
avoid), but at least we won't create thousands of extra entries.
Peter Jones [Fri, 31 Jan 2014 15:30:24 +0000 (10:30 -0500)]
[fallback] For HD() device paths, use just the media node and later.
UEFI 2.x section 3.1.2 provides for "short-form device path", where the
first element specified is a "hard drive media device path", so that you
can move a disk around on different buses without invalidating your
device path. Fallback has not been using this option, though in most
cases efibootmgr has.
Note that we still keep the full device path, because LoadImage()
isn't necessarily the layer where HD() works - one some systems BDS is
responsible for resolving the full path and passes that to LoadImage()
instead. So we have to do LoadImage() with the full path.
Peter Jones [Fri, 15 Nov 2013 14:38:41 +0000 (09:38 -0500)]
Rewrite directory traversal allocation path so coverity can grok it.
The things we do for our tools. In this case, make the AllocatePool()
happen outside of a conditional, even though that conditional will
always bee satisfied. This way coverity won't think we're setting fi
to NULL and passing it to StrCaseCmp.
Peter Jones [Wed, 20 Nov 2013 17:20:23 +0000 (12:20 -0500)]
Fix path generation for Dhcpv4 bootloader.
Right now we always look for e.g. "\grubx64.efi", which is completely
wrong. This makes it look for the path shim was loaded from and modify
that to end in a sanitized version of our default loader name.
Matthew Garrett [Tue, 19 Nov 2013 15:15:55 +0000 (10:15 -0500)]
Don't hook system services if shim has no built-in keys
Shim should only need to enforce its security policy when its launching
binaries signed with its built-in key. Binaries signed by keys in db or
Mokdb should be able to rely on their own security policy.
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
Matthew Garrett [Tue, 19 Nov 2013 15:09:13 +0000 (10:09 -0500)]
Clarify meaning of insecure_mode
insecure_mode was intended to indicate that the user had explicity disabled
checks with mokutil, which means it wasn't the opposite of secure_mode().
Change the names to clarify this and don't show the insecure mode message
unless the user has explicitly enabled that mode.
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
Andrew Boie [Mon, 15 Apr 2013 21:11:17 +0000 (14:11 -0700)]
fix verify_mok()
() Fix the return value semantics. If the MokList doesn't
exist, we are OK. If the MokList was compromised but we
were able to erase it, that is OK too. Only if the list
can't be nuked do we return an error.
() Fix use of potentially uninitialized attribute variable
() Actually use the return value when called from verify_buffer.
Change-Id: If16df21d79c52a1726928df96d133390cde4cb7e Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Peter Jones [Wed, 6 Nov 2013 18:59:02 +0000 (13:59 -0500)]
Fix check logic for SetupMode variable.
After going back and inspecting this further, the logic for "SetupMode"
being present at all was incorrect. Also initialize our state earlier
so it's sure to always be set.
Peter Jones [Tue, 22 Oct 2013 17:36:54 +0000 (13:36 -0400)]
Don't reject all binaries without a certificate database.
If a binary isn't signed, but its hash is enrolled in db, it won't have
a certificate database. So in those cases, don't check it against
certificate databases in db/dbx/etc, but we don't need to reject it
outright.
Kees Cook [Mon, 3 Dec 2012 23:52:48 +0000 (15:52 -0800)]
additional bounds-checking on section sizes
This adds additional bounds-checking on the section sizes. Also adds
-Wsign-compare to the Makefile and replaces some signed variables with
unsigned counteparts for robustness.
Josh Boyer [Tue, 1 Oct 2013 15:49:22 +0000 (11:49 -0400)]
Add support for disabling db for verification
Provide a mechanism for a physically present end user to disable the use
of db when doing signature verification. This is handled by the OS passing
down a variable that contains a UINT32 and a SHA256 hash. If this variable
is present, MokManager prompts the user to choose whether to enable or
disable the use of db for verification purposes (depending on the value of
the UINT32). They are then asked to type the passphrase that matches the
hash. This then saves a boot services variable which is checked by shim,
and if set will cause shim to not use db for verification purposes. If
db is to be ignored, shim will export a runtime variable called
'MokIgnoreDB' for the OS to query at runtime.
Peter Jones [Tue, 1 Oct 2013 20:32:54 +0000 (16:32 -0400)]
If we fail to install our protocol, don't continue.
This shouldn't be exploitable unless you've got a way to make
InstallProtocol fail and still, for example, have memory free to
actually load and run something.
Peter Jones [Thu, 26 Sep 2013 13:44:50 +0000 (09:44 -0400)]
MokManager needs to disable the graphics console.
Without this patch, on some machines we never see MokManager's UI. This
protocol has never (I think?) been officially published, and yet I still
have new hardware that needs it.
Peter Jones [Mon, 9 Sep 2013 16:37:50 +0000 (12:37 -0400)]
Harden shim against non-participating bootloaders.
It works like this: during startup of shim, we hook into the system's
ExitBootServices() and StartImage(). If the system's StartImage() is
called, we automatically unhook, because we're chainloading to something
the system can verify.
When shim's verify is called, we record what kind of certificate the
image was verified against. If the call /succeeds/, we remove our
hooks.
If ExitBootServices() is called, we check how the bootloader verified
whatever it is loading. If it was verified by its hash, we unhook
everything and call the system's EBS(). If it was verified by
certificate, we check if it has called shim_verify(). If it has, we
unhook everything and call the system's EBS()
If the bootloader has not verified anything, and is itself verified by
a certificate, we display a security violation warning and halt the
machine.
Peter Jones [Mon, 9 Sep 2013 18:43:04 +0000 (14:43 -0400)]
Make vendor_cert/vendor_dbx actually replaceable by an external tool.
This moves them both to be computed at runtime from a pointer+offset
rather than just a pointer, so that their real address can be entirely
derived from the section they're in.
This means you can replace the whole .vendor_cert section with a new one
with certs that don't have the same size.
projects / efi-boot-shim.git / log