This commit fixes 2 issues with the TPM support code:
1) Remove "REQUIRE_TPM ?=" line from the Makefile, further down the Makefile
checks if REQUIRE_TPM is undefined, but the above line sets it to an empty
string, which is not the same as undefined. Without this handle_image fails
after the tpm_log_pe() call even if REQUIRE_TPM=1 once was not set when
building the shim
2) When secure-boot is disabled then shim_verify() would exit with the
status of tpm_log_pe(), which on systems with a TPM is an error. Combined
with the recent change to always install the shim protocols, this causes
grub to refuse to boot any kernel since the verify() call now always fails.
This commit fixes this by explicitly setting status = EFI_SUCCESS when
secure-boot is disabled.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Currently the shim_lock protocol is only installed when SecureBoot is enabled.
However, having Verify just measure into the TPM without SecureBoot is a useful
feature.
Signed-off-by: Tamas K Lengyel <lengyelt@ainfosec.com>
Currently the only measurement the shim logs in the TPM is that of the EFI
application it directly loads. However, there are no measurements being taken
of application that are being verified through the shim_lock protocol. In this
patch we extend PCR4 for any binary for which Verify is being called through
the shim_lock protocol.
Signed-off-by: Tamas K Lengyel <lengyelt@ainfosec.com>
Make sure if we chainload things, a chainloaded bootloader will be able to use
the latest systab replacements and protocols. They need to match for things
to validate correctly.
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
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>
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>
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.
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
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>
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>
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>
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>
