Commit Graph

194 Commits

Author SHA1 Message Date
Peter Jones
25f6fd08cd try to show errors more usefully.
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-09-13 15:18:28 -04:00
Peter Jones
00753a0a28 Add some debugging data to the last malformed binary check...
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-09-13 15:16:43 -04:00
Peter Jones
1d39ada8cb Revert lots of Cryptlib updates.
OpenSSL changes quite a bit of the key validation, and most of the keys
I can find in the wild aren't marked as trusted by the new checker.

Intel noticed this too: https://github.com/vathpela/edk2/commit/f536d7c3ed
but instead of fixing the compatibility error, they switched their test
data to match the bug.

So that's pretty broken.

For now, I'm reverting OpenSSL 1.1.0e, because we need those certs in
the wild to work.

This reverts commit 513cbe2aea.
This reverts commit e9cc33d6f2.
This reverts commit 80d49f758e.
This reverts commit 9bc647e2b2.
This reverts commit ae75df6232.
This reverts commit e883479f35.
This reverts commit 97469449fd.
This reverts commit e39692647f.
This reverts commit 0f3dfc01e2.
This reverts commit 4da6ac8195.
This reverts commit d064bd7eef.
This reverts commit 9bc86cfd6f.
This reverts commit ab9a05a10f.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-31 15:13:58 -04:00
Peter Jones
eae64276ff Drain the OpenSSL error stack and report crypto verification errors
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-31 15:13:46 -04:00
Peter Jones
36d20ac0aa Init openssl so we can use its debug facilities.
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-31 15:13:45 -04:00
Peter Jones
78f6b007e7 Make msleep() be a thing
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-31 15:13:34 -04:00
Peter Jones
207dd7dc60 Add ENABLE_SHIM_CERT to make MokManager/fallback signing optional.
This makes shim not create its own keyring and sign MokManager and
fallback by default.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-11 15:18:39 -04:00
Peter Jones
bdc5d3ec9c Always measure all of MokSBState, MokList, and MokListX
Even if errors occurred, always try to measure all of our Mok entries.
This way we won't fail on e.g. MokList not being set.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-03 11:00:58 -04:00
Matthew Garrett
22f2737535 Measure stage 2 according to spec
We're currently measuring the raw second stage loader into PCR 9, but
we're closer to spec if we measure the semi-parsed PE into PCR 4. The
hash that's logged is the same as the hash used for the Authenticode
validation, so refactor shim.c a little to separate out the hash
generation.
2017-08-03 11:00:58 -04:00
Matthew Garrett
8af7c4caca Extend PCR 7
It's desirable to be able to use PCR 7 for all TPM policy on Secure Boot
systems, but right now Shim doesn't record any information about its
configuration or the signature used to launch the second stage loader. Add
support for that.
2017-08-01 12:54:49 -04:00
Lans Zhang
6d4498fb3b update verification_method if the loaded image is signed by shim/vendor cert
Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
2017-06-15 11:30:11 -04:00
Lans Zhang
71d927270a skip the error message when creating MokListRT if vendor cert is empty
EFI_NOT_FOUND will be returned when creating MokListRT if vendor cert is
empty. This is harmless, meaningless and skippable.

Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
2017-06-15 11:29:51 -04:00
John S. Gruber
f481019157 Fix buffer overrun / damaged options passed to second_stage.
start is a UCS-2 character pointer and loader_len is a number of bytes.
Adjust loader_len to count characters before adding to the start pointer.
2017-04-27 10:58:33 -04:00
Gary Lin
e39692647f shim: Remove the obsolete OBJ_cleanup
Signed-off-by: Gary Lin <glin@suse.com>
2017-04-11 10:42:19 -04:00
Lans Zhang
6dd948b57b generate_hash(): fix the regression
The commit 03b9f800 introduces an issue in case the gap between
SumOfBytesHashed and context->SecDir->VirtualAddress exists.

This would be a typo because a formal PE image always meet
SumOfBytesHashed + hashsize == context->SecDir->VirtualAddress either
the gap exists or not.

Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
2017-04-03 14:53:45 -04:00
Peter Jones
7a44b29edc Ignore BDS when it tells us we got our own path on the command line.
Sometimes we get our own path in LoadOptions for no clear reason.  Don't
execute it, just ignore it.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-04-03 14:53:45 -04:00
Peter Jones
d00ea5558e Fix some i386 type casting errors
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-03-27 14:16:42 -04:00
Peter Jones
29f3c91d4e shim: disambiguate our global image handle.
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-03-27 14:16:42 -04:00
Ard Biesheuvel
97022acd36 Use EfiLoaderCode memory for loading PE/COFF executables
Under a strict memory protection policy, UEFI may give out EfiLoaderData
memory with the XN attribute set. So use EfiLoaderCode explicitly.

At the same time, use a page based allocation rather than a pool
allocation, which is more appropriate when loading PE/COFF images.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
2017-02-28 13:37:23 -05:00
Peter Jones
9f2c83e60e Also just check for access denied anyway.
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-02-06 16:49:28 -05:00
Peter Jones
6ebf9b8704 Ensure all of the SB verification returns the same error code.
Previously we were returning EFI_ACCESS_DENIED at some places and
EFI_SECURITY_VIOLATION at others.  When we're checking whether to run
MokManager, we're checking EFI_SECURITY_VIOLATION, which is more or less
analogous with what the spec says StartImage() returns.  So we should
always have that as the return code.

I believe this will fix github issue #44.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-02-06 13:34:20 -05:00
Ivan Hu
07bda58596 shim: fix the mirroring MokSBState fail
Some machines have already embedded MokSBStateRT varaible with
EFI_VARIABLE_NON_VOLATILE attribute, and some users might disable shim
vailidation manually by creating MokSBStateRT. It causes mirroring MokSBState
fail because the variable cannot be set with different attribute again, and gets
error massage every time when booting.

Fix it with checking the MokSBStateRT existence and deleting it before
mirroring it.

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
2017-02-06 11:16:24 -05:00
Peter Jones
03b9f800b9 generate_hash(): make check_size() set an error, and verify SecDir size.
Currently generate_hash() attempts to include any trailing data at the
end of the binary in the resulting digest, but it won't include such
data if the size computed is wrong because context->SecDir->Size is
invalid.  In this case the return code is EFI_SUCCESS, and the hash will
match any a binary as if the Attribute Certificate Table and anything
after it are missing.  This is wrong.

Signed-off-by: Peter Jones <pjones@redhat.com>
2017-02-06 11:16:24 -05:00
Mathieu Trudel-Lapierre
6c180c6004 shim: verify Extended Key Usage flags
For starters; don't allow the "module signing" OID; which ought to
only ever be used for signing kernel modules, not signing EFI binaries.

Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
2016-09-21 13:32:53 -04:00
Peter Jones
af13b3efc9 Fix up a merge error in 467878f3e0.
In the branch I wrote the code on, "size" was a thing.  On this branch
it isn't.

Signed-off-by: Peter Jones <pjones@redhat.com>
2016-09-09 12:07:26 -04:00
Peter Jones
2de084689f verify_buffer: check that the value of cert->Hdr.dwLength is reasonable
Signed-off-by: Peter Jones <pjones@redhat.com>
2016-09-09 11:16:17 -04:00
Peter Jones
b8e27b3cfe Minor formatting fix
Signed-off-by: Peter Jones <pjones@redhat.com>
2016-09-06 15:19:08 -04:00
Sachin Agrawal
d241bbbdbf Use authenticode signature length from WIN_CERTIFICATE structure.
Authenticode Certificate length is available in Certificate Table
(inside PE header) and also in signature header(WIN_CERTIFICATE) itself.
Code in 'check_backlist()' method uses length from signature header,
whereas, AuthenticodeVerify() call inside 'verify_buffer()' method uses
the length in signature header. This causes a security vulnerability issue :

Good Scenario : Assume shim1.crt is used for signing grub.efi and
shim1.crt is embedded inside shim.efi. Also, assume shim1.crt got
compromised and therefore it was added in 'dbx' database. Now, when
shim.efi will attempt to load grub.efi, it will fail loading with
log message "Binary is blacklisted" because 'check_blacklist' call
will detect the presence of 'shim1.crt' in 'dbx'.

Vulnerable Scenario : Similar as above. Add 'shim1.crt' in dbx database.
Also, tamper the earlier signed grub.efi file by placing 0x0000 in the
WIN_CERTIFICATE.dwLength.
(Open grub.efi/vmlinuz signed binary with hex editor.
 Go to 0x128 address and read out the address from 0x128 until
 0x12B in little Indian order from right to left.
 Jump to the address from 0x128 address area.
 First 8bytes are the signature header area which consist of
 signature size(4bytes), revision(2bytes) and type(2bytes).
 So tamper the first 4 bytes for signature size and save the binary.
)
With this tampered grub.efi, shim.efi loads it successfully because
'check_blacklist()' call fails to detect the presence of shim1.crt in 'dbx'
database.


Signed-off-by: Sachin Agrawal <sachin.agrawal@intel.com>
2016-09-06 15:06:51 -04:00
Benjamin Antin
7052e75307 Don't close file twice in should_use_fallback error path
When fallback.efi is not present, the should_use_fallback error path
attempts to close a file that has already been closed, resulting in a
hang. This issue only affects certain systems.

This is a regression from version 0.8 and was introduced by commit
4794822.

Signed-off-by: Benjamin Antin <ben.antin@endlessm.com>
2016-09-06 14:57:33 -04:00
Gary Lin
cc1fe3c669 shim: remove unused variable
Fix the compilation error from gcc:

shim.c: In function ‘handle_image’:
shim.c:1121:15: error: unused variable ‘size’ [-Werror=unused-variable]
  unsigned int size;
               ^~~~

Signed-off-by: Gary Lin <glin@suse.com>
2016-09-06 14:56:36 -04:00
Lans Zhang
9249fc2849 Fix the size of MokDBState
MokDBState is a 8-bit unsigned integer. Looks like a typo here.

Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
2016-09-06 14:50:52 -04:00
Gary Ching-Pang Lin
3d79bcb265 Add the optional HTTPBoot support
This commit adds the basic support for HTTPBoot, i.e. to fetch
the next stage loader with the HTTP protocol.

It requires gnu-efi >= 3.0.3 to support the URI device path and
Ip4Config2 or Ip6Config protocol support in the UEFI implementation.

To build shim.efi with HTTPBoot support:
make ENABLE_HTTPBOOT=1 shim.efi

Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2016-09-06 14:49:52 -04:00
Peter Jones
467878f3e0 read_header/handle_image: treat uninitialized file alignment as PAGE_SIZE 2016-09-06 14:44:50 -04:00
Peter Jones
6f04092060 Make fallback and mokmanager know about multi-arch.
On baytrail, we've got 32-bit firmware, 32-bit efi utilities, and 64-bit
kernel.  So since most distros will want 32+64 EFI media booting a
64-bit kernel, we have to name them better on the filesystem.

Signed-off-by: Peter Jones <pjones@redhat.com>
2016-09-06 14:39:15 -04:00
Peter Jones
14a59055aa shim: make the PE loader less overzealous on rejections 2016-06-09 15:32:37 -04:00
Matthew Garrett
22b58f2455 Measure state and second stage into TPM
Add support for measuring the MOK database and secure boot state into a
TPM, and do the same for the second stage loader. This avoids a hole in
TPM measurement between the firmware and the second stage loader.
2016-05-11 11:11:05 -04:00
Ivan Hu
085d56c464 shim: dealing with only one string on loadoption
The second stage set is not working after commit
3322257e61 for those which load option
only have one string.

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
2016-05-11 11:10:17 -04:00
Mathieu Trudel-Lapierre
8f1bd605d0 shim: mirror MokSBState in runtime so the kernel can make use of it.
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
2016-03-22 11:14:31 -04:00
Peter Jones
edeb313e6e shim: check for EFI\BOOT\BOOT${ARCH}.EFI as well as the leading \ version
I found a machine whose BDS gives us relative paths, yay!  The rest of
the code still works without that leading slash, so just make it one
more item we let through our StrnCaseCmp() filter.

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:40:29 -05:00
Peter Jones
4794822464 shim: fix resource leak on should_use_fallback() error path
ExitBootServices() and Exit() should both clean these up anyway, but we
should do the right thing nonetheless.

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:40:23 -05:00
Peter Jones
2e65561938 shim: if generate_path() gets a full path, just return it.
We decide if it's a full path by if it starts with \\EFI\\.  That's
quite lazy, but we can't just check \\ like you'd hope, because we need
to stay compatible with what we've set as DEFAULT_LOADER in the past,
and I don't feel like writing the full path traversal file test.

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:40:01 -05:00
Peter Jones
cf5f75fa14 shim: fix a wrong-abi call to Stall() and ResetSystem()
Woops.  The net outcome of these is going to be a sleep of unknown
duration, followed by either a) ResetSystem() with some random selection
of warm/cold boot, or b) ResetSystem() returning an error and shim
returning error from efi_main().

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:39:54 -05:00
Peter Jones
3322257e61 shim: handle BDS's li->LoadOptions and Shell's li->LoadOptions .
Load options are a giant pain in the ass, because the shell is a giant
piece of junk.  If we're invoked from the EFI shell, we get something
like this:

00000000 5c 00 45 00 36 00 49 00 5c 00 66 00 65 00 64 00 |\.E.F.I.\.f.e.d.|
00000010 6f 00 72 00 61 00 5c 00 73 00 68 00 69 00 6d 00 |o.r.a.\.s.h.i.m.|
00000020 78 00 36 00 34 00 2e 00 64 00 66 00 69 00 20 00 |x.6.4...e.f.i. .|
00000030 5c 00 45 00 46 00 49 00 5c 00 66 00 65 00 64 00 |\.E.F.I.\.f.e.d.|
00000040 6f 00 72 00 61 00 5c 00 66 00 77 00 75 00 70 00 |o.r.a.\.f.w.u.p.|
00000050 64 00 61 00 74 00 65 00 2e 00 65 00 66 00 20 00 |d.a.t.e.e.f.i. .|
00000060 00 00 66 00 73 00 30 00 3a 00 5c 00 00 00       |..f.s.0.:.\...|

which is just some paths rammed together separated by a UCS-2 NUL. But
if we're invoked from BDS, we get something more like:

00000000 01 00 00 00 62 00 4c 00 69 00 6e 00 75 00 78 00 |....b.L.i.n.u.x.|
00000010 20 00 46 00 69 00 72 00 6d 00 77 00 61 00 72 00 | .F.i.r.m.w.a.r.|
00000020 65 00 20 00 55 00 70 00 64 00 61 00 74 00 65 00 |e. .U.p.d.a.t.e.|
00000030 72 00 00 00 40 01 2a 00 01 00 00 00 00 08 00 00 |r.....*.........|
00000040 00 00 00 00 00 40 06 00 00 00 00 00 1a 9e 55 bf |.....@........U.|
00000050 04 57 f2 4f b4 4a ed 26 4a 40 6a 94 02 02 04 04 |.W.O.:.&J@j.....|
00000060 34 00 5c 00 45 00 46 00 49 00 5c 00 66 00 65 00 |4.\.E.F.I.f.e.d.|
00000070 64 00 6f 00 72 00 61 00 5c 00 73 00 68 00 69 00 |o.r.a.\.s.h.i.m.|
00000080 6d 00 78 00 36 00 34 00 2e 00 65 00 66 00 69 00 |x.6.4...e.f.i...|
00000090 00 00 7f ff 40 00 20 00 5c 00 66 00 77 00 75 00 |...... .\.f.w.u.|
000000a0 70 00 78 00 36 00 34 00 2e 00 65 00 66 00 69 00 |p.x.6.4...e.f.i.|
000000b0 00 00                                           |..|

which is clearly an EFI_LOAD_OPTION filled in halfway reasonably.  In
short, the UEFI shell is still a useless piece of junk.

So anyway, try to determine which one we've got and handle it
appropriately.

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:39:34 -05:00
Peter Jones
e22b8561d7 Fix unsigned int overflow on our i386 debug hook test.
Signed-off-by: Peter Jones <pjones@redhat.com>
2015-11-17 11:39:16 -05:00
Peter Jones
70ce2c4204 Improve our debuginfo path print
Signed-off-by: Peter Jones <pjones@redhat.com>
2015-06-30 14:19:57 -04:00
Peter Jones
0abed15aa8 Only be verbose the first time secure_mode() is called.
It's annoying to find out we're not in SB mode over and over.  Really it
is.

Signed-off-by: Peter Jones <pjones@redhat.com>
2015-06-29 14:41:21 -04:00
Peter Jones
a031960750 Add a conditional point for a debugger to attach.
Signed-off-by: Peter Jones <pjones@redhat.com>
2015-06-29 14:41:21 -04:00
Peter Jones
f7c34e9b5f Don't print anything or delay when start_image() succeeds.
Signed-off-by: Peter Jones <pjones@redhat.com>
2015-06-29 14:41:21 -04:00
Gary Ching-Pang Lin
7cb3ee9213 Make shim to check MokXAuth for MOKX reset
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2015-06-16 11:41:32 -04:00
Gary Ching-Pang Lin
b8d1bc6e98 Verify the EFI images with MOK blacklist
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2015-06-16 11:41:32 -04:00