Commit Graph

127 Commits

Author SHA1 Message Date
Gary Ching-Pang Lin
875eb1b9d5 Simplify the checking of SB and DB states
MokSBState and MokDBState are just 1 byte variables, so a UINT8
local variable is sufficient to include the content.

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>

Conflicts:
	shim.c
2014-06-25 10:58:23 -04:00
Peter Jones
9ea3d9b401 Make sure we default to assuming we're locked down.
If "SecureBoot" exists but "SetupMode" does not, assume "SetupMode" says
we're not in Setup Mode.

Signed-off-by: Peter Jones <pjones@redhat.com>
2014-06-25 10:55:56 -04:00
Gary Ching-Pang Lin
7a72592b75 Check the secure variables with the lib functions
There are functions defined in lib to check the secure variables.
Use the functions to shun the duplicate code.

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>

Conflicts:
	shim.c
2014-06-25 10:55:12 -04:00
Peter Jones
3b41442227 Explain the logic in secure_mode() better.
I was getting confused reading it, and I wrote it, so clearly it needs
more commentry.

Signed-off-by: Peter Jones <pjones@redhat.com>
2014-06-25 10:46:52 -04:00
Gary Ching-Pang Lin
fe8527aaa6 Free the string from DevicePathToStr
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>

Conflicts:
	shim.c
2014-06-25 10:33:25 -04:00
Gary Ching-Pang Lin
e50cfe371f Silence the functions of shim protocol
When grub2 invokes the functions of shim protocol in gfx mode,
OutputString in shim could distort the screen.

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>

Conflicts:
	shim.c

(modified by pjones to include some newer Prints that weren't there when
Gary did the initial work here.)
2014-06-25 10:30:38 -04:00
Gary Ching-Pang Lin
95c6743e4c No newline for console_notify
The newlines are for Print(), not console_notify().

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>

Conflicts:
	shim.c
2014-06-25 10:12:43 -04:00
Gary Ching-Pang Lin
c902256046 Remove grubpath in generate_path()
The variable is not used anymore.

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2014-06-25 09:56:27 -04:00
Gary Ching-Pang Lin
b8070380ee Check the first 4 bytes of the certificate
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>
2014-06-25 09:55:49 -04:00
Gary Ching-Pang Lin
da49ac6d69 Fetch the netboot image from the same device
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>
2014-06-25 09:53:23 -04:00
Peter Jones
a63d665fb8 Get rid of SectionCache in generate_hash(), it is unused.
Signed-off-by: Peter Jones <pjones@redhat.com>
2014-04-11 15:07:45 -04:00
Peter Jones
16a8356350 Kees' patch missed the offset adjustment to PEHdr.
In read_header, we adjust context->PEHdr's address by doshdr->e_lfanew.
If we're going to recompute that address, we have to adjust it here
too.

Signed-off-by: Peter Jones <pjones@redhat.com>
2014-04-11 15:05:24 -04:00
Kees Cook
47a9d2c908 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.

Signed-off-by: Kees Cook <kees@ubuntu.com>
2014-04-11 14:41:22 -04:00
Peter Jones
cf90edfff5 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.

Signed-off-by: Peter Jones <pjones@redhat.com>
2014-02-14 17:48:01 -05:00
Matthew Garrett
7c1f49dacc 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>
2013-11-19 10:20:34 -05:00
Matthew Garrett
e60f118155 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>
2013-11-19 10:20:34 -05:00
Andrew Boie
acacfca319 shim: improve error messages
%r when used in Print() will show a string representation of
an EFI_STATUS code.

Change-Id: I6db47f5213454603bd66177aca378ad01e9f0bd4
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
2013-11-12 10:32:48 -05:00
Mohanraj S
b6a12d99be shim.c: Add support for hashing/relocation of 32-bit binaries
Change-Id: Ib93305f7f1691d1b142567507df1058de62dde06
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
2013-11-12 10:25:23 -05:00
Andrew Boie
42426e6eae 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>
2013-11-12 10:24:01 -05:00
Peter Jones
0948ac0971 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.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-11-06 13:59:02 -05:00
Peter Jones
6b1f8796ff Don't free GetVariable() return data without checking the status code.
This breaks every machine from before Secure Boot was a thing.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-30 16:36:01 -04:00
Peter Jones
321797142e We should be checking both mok and the system's SB settings
When we call hook_system_services(), we're currently only checking mok's
setting.  We should use secure_mode() instead so it'll check both.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-28 10:41:03 -04:00
Peter Jones
cf718e1940 Revert "additional bounds-checking on section sizes"
This reverts commit 21e40f0174.

In principle I like the idea of what's going on here, but
generate_hash() really does need to have the expected result.
2013-10-23 10:50:36 -04:00
Peter Jones
8044a321f9 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.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-22 13:40:08 -04:00
Kees Cook
a0df78b73f 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.

Signed-off-by: Kees Cook <kees@ubuntu.com>
2013-10-22 11:23:51 -04:00
Peter Jones
98a9957866 Unhook system services as we exit.
If we never find a valid thing to boot, we need to undo the weird things
we've done.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-04 15:31:48 -04:00
Peter Jones
4ab978a369 Try to actually make debug printing look reasonable.
Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-04 11:51:09 -04:00
Peter Jones
7de74e6734 Do more strict checking on PE Headers.
Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-04 11:51:09 -04:00
Peter Jones
a3beb2a6f7 Improve PE image bounds checking.
Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-03 17:04:45 -04:00
Peter Jones
0fb089ee14 Add ident-like blobs to shim.efi for version checking.
I feel dirty.
2013-10-03 11:11:09 -04:00
Josh Boyer
47ebeb6262 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.

Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
2013-10-02 11:29:34 -04:00
Peter Jones
a847e33aaf Fix wrong type on console_error() call.
Stupid L"".

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-02 10:00:11 -04:00
Peter Jones
1d56305945 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.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-01 16:33:58 -04:00
Peter Jones
bb2fe4cfb3 Conditionalize overriding the security policy.
Make OVERRIDE_SECURITY_POLICY a build option.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-01 14:03:16 -04:00
Peter Jones
417077f8de Merge console_control.h and console.h
Since these are topically the same thing, they can live together.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-01 14:03:16 -04:00
Peter Jones
bc71a15ed5 Make verbose stuff use console_notify
Signed-off-by: Peter Jones <pjones@redhat.com>
2013-10-01 14:03:16 -04:00
Peter Jones
4185c7d67e Include shim's vendor_cert in MokListRT
There needs to be some way to communicate to the kernel that it's a
trusted key, and since this mechanism already exists, it's by far the
easiest.
2013-10-01 14:03:16 -04:00
Peter Jones
cbef697a96 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.
2013-10-01 14:03:16 -04:00
Peter Jones
a1f2863584 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.
2013-10-01 14:03:16 -04:00
Gary Ching-Pang Lin
59dcd9d1b8 integrate security override 2013-09-26 11:58:03 -04:00
Gary Ching-Pang Lin
7f0208a0f9 Merge variable retrieving functions 2013-09-26 11:58:02 -04:00
Gary Ching-Pang Lin
53862ddace Merge signature.h into efiauthenticated.h and guid.h
Conflicts:
	shim.c
2013-09-26 11:58:02 -04:00
Gary Ching-Pang Lin
ca2e00d067 Free unused memory space 2013-09-26 11:58:02 -04:00
Gary Ching-Pang Lin
d71240bfff Correct the certificate count of the signature list 2013-09-26 11:58:02 -04:00
Peter Jones
e053c22701 Since different distros name grub*.efi differently, make it compile-time.
Basically, if you don't want grub.efi, you do:

make 'DEFAULT_LOADER=\\\\grubx64.efi'

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-09-26 11:58:02 -04:00
Gary Ching-Pang Lin
436afcc276 Remove double-separators from the bootpath 2013-09-26 11:58:01 -04:00
Gary Ching-Pang Lin
f9f81a22dd Fix the broken bootpath
- The file path from DevicePathToStr may use slash as the file
  seperator. Change all slashes to backslashes to avoid the strange
  bootpath.
- Remove the redundant backslashes.
- ImagePath no longer requires the leading backslash.
- Fix a memory leak

Based on the patch from Michal Marek <mmarek@suse.com>
2013-09-26 11:58:01 -04:00
Steve Langasek
fbc486b50d Pass the right arguments to EFI_PXE_BASE_CODE_TFTP_READ_FILE
A wrong pointer was being passed to EFI_PXE_BASE_CODE_TFTP_READ_FILE,
preventing us from getting the file size back from the tftp call, ensuring
that we don't have enough information to properly secureboot-validate the
retrieved image.
2013-09-24 12:05:21 -04:00
Peter Jones
cb59de3847 Make SHIM_LOCK_GUID a first-class object with a symbol.
Right now the CA is checking if shim builds expose a particular version
of the shim protocol.  To do this, they're looking for SHIM_LOCK_GUID's
value in the resulting binary.

Currently, with SHIM_LOCK_GUID as a macro that gets assigned to local
variables, that means they have to compensate for mov instructions mixed
in with the actual value.  This is completely absurd, so promote it to a
first-class object with a symbol to make it both easy to find and
continuous.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-09-23 10:40:49 -04:00
Peter Jones
e75294e569 Don't print things on the screen by default when everything works.
There's no point to this text, and it generally confuses people.

Signed-off-by: Peter Jones <pjones@redhat.com>
2013-09-16 09:27:08 -04:00