| Age | Commit message (Collapse) | Author |
|---|
|
X509_get_notBefore -> X509_getm_notBefore
X509_get_notAfter -> X509_getm_notAfter
Signed-off-by: Gary Lin <glin@suse.com>
|
|
When writing MokList with EFI_VARIABLE_APPEND_WRITE, some HP laptops
may just return EFI_SUCCESS without writing the content into the flash,
so we have no way to detect if MokList is updated or not. Now we always
read MokList first and write it back with the new content.
https://github.com/rhboot/shim/issues/105
Signed-off-by: Gary Lin <glin@suse.com>
|
|
This timeout can have the values [-1,0..0x7fff]; where -1 means "no timeout",
with MokManager going directly to the menu, and is capped to 0x7fff to avoid
unecessary long timeouts. The default remains 10, which will be used whenever
the MokTimeout variable isn't set.
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This reverts commit 6aa5a62515d62139a2d3b34626fac8910e864a3d.
Everything Hans said was correct. But StrnCat() is in gnu-efi 3.0.8,
and using just StrCpy() here confuses coverity. I'd rather have a CI
page that's not completely full of chaff, but a little bit of redundancy
in the code.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
StrnCat is not available in gnu-efi-3.0.5 (I did not check if it does
actually exists in 3.0.6). Moreover using strcat on a buffer where we've
just done: "buf[0] = '\0'" is a bit silly, we might as well drop the 0
termination and just use strcpy.
It seems there also is no StrnCpy in gnu-efi-3.0.5, but we are passing in
a pointer to the end of file_name minus 4, so strcpy will consume only
4 bytes anyways and there is no need for the "n".
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
Remove the setup_console(1) calls from shim and instead make lib/console.c
make that call when necessary. This avoids shim forcing the EFI console to
switch to text-mode if nothing is printed.
This commit also modifies MokManager to work the same way for consistency,
even though MokManager will always print something.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
This is a preparation commit for removing the setup_console(1) calls from
MokManager and shim so that we don't force the EFI console to switch to
text-mode.
This commit replaces all direct calls to Print / PrintAt with calls to
the new helpers (no functional changes) so that we can delay calling
setup_console(1) till the first Print call in a follow-up patch.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
I'm pretty done with typing uefi_call_wrapper() and counting arguments
every time. Instead, just make the compiler error if we don't have
ms_abi. Also, make it so nothing can use uefi_call_wrapper() directly.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
clang-analyzer says:
MokManager.c:1431:6: warning: Branch condition evaluates to a garbage value
if (mok)
^~~
MokManager.c:1433:6: warning: Branch condition evaluates to a garbage value
if (del_key)
^~~~~~~
And it's right; if we take the first error exit in the function, those
never get initialized. This patch sets them to NULL to begin with.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Covscan daftly claims:
288. var_compare_op: Comparing MokSB to null implies that MokSB might be null.
2330 if (MokSB) {
2331 menu_strings[i] = L"Change Secure Boot state";
2332 menu_item[i] = MOK_CHANGE_SB;
2333 i++;
2334 }
2335
...
2358 choice = console_select(perform_mok_mgmt, menu_strings, 0);
2359 if (choice < 0)
2360 goto out;
...
2362 switch (menu_item[choice]) {
...
2395 case MOK_CHANGE_SB:
CID 182841 (#1 of 1): Dereference after null check
(FORWARD_NULL)293. var_deref_model: Passing null pointer MokSB to
mok_sb_prompt, which dereferences it. [show details]
2396 efi_status = mok_sb_prompt(MokSB, MokSBSize);
Which is, of course, entirely false, beause for menu_item[choice] to be
MOK_CHANGE_SB, MokSB must be !NULL. And then:
252. Condition efi_status == 0, taking true branch.
2397 if (efi_status == EFI_SUCCESS)
2398 MokSB = NULL;
This guarantees it won't be in the list the next time through the loop.
This adds tests for NULLness before mok_sb_prompt(), just to make it
more clear to covscan what's going on.
Also do the same thing for all of:
MOK_CHANGE_SB
MOK_SET_PW
MOK_CHANGE_DB
MOK_ENROLL_MOKX
MOK_DELETE_MOKX
I also Lindent-ed everything I had to touch.
Three other minor errors are also fixed:
1) the loop in enter_mok_menu() leaked the menu allocations each time
through the loop
2) mok_sb_prompt(), mok_pw_prompt(), and mok_db_prompt() all call
FreePool() on their respective variables (MokSB, etc), and
check_mok_request() also calls FreePool() on these. This sounds
horrible, but it turns out it's not an issue, because they only free
them in their EFI_SUCCESS paths, and enter_mok_menu() resets the
system if any of the mok_XX_prompt() calls actually returned
EFI_SUCCESS, so we never get back to check_mok_request() for it to do
its FreePool() calls.
3) the loop in enter_mok_menu() winds up introducing a double free in
the call to free_menu(), but we also can't hit this bug, because all
the exit paths from the loop are "goto out" (or return error) rather
than actually exiting on the loop conditional.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Also consistently name our status variable "efi_status" unless there's a
good reason not to, such as already having another one of those.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
We know it's legit already because we computed the pointer from the end,
but covscan gets confused, and we have StrnCat, so we should just use it
anyway.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
I'm just tired of all the little quirks.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Lindent gets confused by these, and they're hard to read anyway.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Obviously, these are not correct. Most of them are just useless; one
can be changed to a more useful test.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.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 513cbe2aea689bf968f171f894f3d4cdb43524d5.
This reverts commit e9cc33d6f2b7f35c6f5e349fd83fb9ae0bc66226.
This reverts commit 80d49f758ead0180bfe6161931838e0578248303.
This reverts commit 9bc647e2b23bcfd69a0077c0717fbc454c919a57.
This reverts commit ae75df6232ad30f3e8736e9449692d58a7439260.
This reverts commit e883479f35644d17db7efed710657c8543cfcb68.
This reverts commit 97469449fda5ba933a64280917e776487301a127.
This reverts commit e39692647f78e13d757ddbfdd36f440d5f526050.
This reverts commit 0f3dfc01e2d5e7df882c963dd8dc4a0dfbfc96ad.
This reverts commit 4da6ac819510c7cc4ba21d7a735d69b45daa5873.
This reverts commit d064bd7eef201f26cb926450a76260b5187ac689.
This reverts commit 9bc86cfd6f9387f0da9d5c0102b6aa5627e91c91.
This reverts commit ab9a05a10f16b33f7ee1e9da360c7801eebdb9d2.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Rather than looping once through the possible actions (MokNew, MokDel, etc.),
revise the logic so that instead of rebooting immediately we get back to the
main menu setting a flag to replace "Continue booting" with a proper reboot.
Getting back to the menu means we can go make other changes before rebooting.
For instance, you might want to enable validation, but beforehand you also
need to enroll a MOK. You can already do so from userland; except the requests
were cleared as soon as one of them was processed.
This involves some extra cleanup of the states to avoid running the same
request more than once, removing the option from the menu once it's done, and
changing prompting functions to return an EFI_STATUS so we can better track
whether the process has succeeded.
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
|
|
X509_get_notBefore -> X509_getm_notBefore
X509_get_notAfter -> X509_getm_notAfter
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
|
|
new_data in write_db() wasn't freed after SetVariable.
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Try to append the MOK/MOKX list first and then fallback to the normal
SetVariable if the firmware doesn't support EFI_VARIABLE_APPEND_WRITE.
Signed-off-by: Gary Lin <glin@suse.com>
|
|
We got the bug report about the usage of APPEND_WRITE that may cause the
failure when writing a variable in Lenovo machines. Although
EFI_VARIABLE_APPEND_WRITE already exists in the UEFI spec for years,
unfortunately, some vendors just ignore it and never implement the
attribute. This commit removes the usage of EFI_VARIABLE_APPEND_WRITE to
make MokManager work on those machines.
https://github.com/rhinstaller/shim/issues/55
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Also update Cryptlib to edk2 r19218
- Undefine NO_BUILTIN_VA_FUNCS in Cryptlib/OpenSSL/ for x86_64 to use
the gcc builtins and remove all EFIAPI from the functions
- Move the most of defines into the headers instead of Makefile
- Remove the global variable 'timeval'
- Remove the unused code: crypto/pqueue/* and crypto/ts/*
- Include bn.h in MokManager.c due to the changes in openssl
Signed-off-by: Gary Lin <glin@suse.com>
|
|
My favorite part of -Wsign-compare is how it shows different results on
different arches for no obvious reason.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Nobody should be deploying SHA-1. No hardware deploys it, and the rate
of change on https://en.wikipedia.org/wiki/SHA-1#Attacks is wildly
uninspiring.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Patch from Johannes Segitz <jsegitz@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
There are some functions that the return value and the type
didn't match.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Add SHA1 hash support and amend the code to make it easier to support
other SHA digests.
|
|
match_hash() requests the number of keys in a list and it was
mistakenly replaced with the size of the Mok node. This would
made MokManager to remove the whole Mok node instead of one
hash.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
MokSize of the hash signature list includes the owner GUID,
so we should not add the 16bytes compensation.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
also return to the previous entry in the list
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
The new blacklist, MokListX, stores the keys and hashes that are
banned.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
Basically they messed around with stdarg some and now we need to do it
the other way.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
|
We build with -Werror=signed-compare in fedora/rhel rpms, and this
showed up.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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
|
|
LibDeleteVariable assumes that the variable is RT+NV and it
won't work on a BS+NV variable.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
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>
|
|
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>
|
