summaryrefslogtreecommitdiff
path: root/MokManager.c
AgeCommit message (Collapse)Author
2025-01-17MokManager: remove redundant logical constraintsHeinrich Schuchardt
CertList->SignatureSize is of type UINT32 which is always positive. If CertList->SignatureListSize == 0, then CertList->SignatureListSize <= CertList->SignatureSize is also true. Remove the redundant CertList->SignatureListSize == 0 checks. A message "Corrupted signature list" is better suited then "Invalid MOK detected! Ignoring MOK List." in this case. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
2022-05-04Allow MokListTrusted to be enabled by defaultEric Snowberg
Within previous versions of shim the MokListTrusted var did not exist. The user had to opt in to using the feature. Change the default behavior to an opt out model. Since old shims will not have the BS MokListTrusted set, use inverse logic that sets the MokListTrustedRT to 1 when the boot service variable is missing. Many Linux distros carry out of tree patches to trust the mok keys by default. These out of tree patches can be dropped when using a Linux kernel that supports MokListTrustedRT. Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
2021-12-10Minor coverity fixes15.5-rc2Peter Jones
- one missing free - one minor deadcode issue - two unchecked allocations - one debug hexdump of a variable we just freed Signed-off-by: Peter Jones <pjones@redhat.com>
2021-11-03Introduce a new MOK variable called MokListTrustedRTEric Snowberg
Introduce a new MOK variable called MokListTrustedRT. It allows an end-user to decide if they want to trust MOKList keys within the soon to be booted Linux kernel. This variable does not change any functionality within shim itself. When Linux boots, if MokListTrustedRT is set and EFI_VARIABLE_NON_VOLATILE is not set, keys in MokListRT are loaded into the .machine keyring instead of the .platform keyring. Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
2021-10-12console: add a clear_screen() primitivePeter Jones
Several places in e.g. MokManager and our console library use ST->ConOut->ClearScreen directly, without checking for the existence of a console output device. This patch adds function to our console library to do that correctly, instead of using the bug-prone ad hoc implementation everywhere. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-09-07cleanup: always use BS and RT, not gBS and gRTPeter Jones
This just makes one less thing we have to make sure is the same between the test harnesses and the runtime code. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-03-10Restructure our includes.Peter Jones
This re-structures our includes so we can be sure everything is always including all the system headers in a uniform, predictable way. Temporarily it also adds a bunch of junk at all the places we use variadic functions to specifically pick either the MS (cdecl) or ELF ABIs. I'm not 100% sure that's all correct (see later patch) but it's enough to allow this to build. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Fix all the places we need UNUSED on arguments.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-16console: Move the countdown function to console.cGary Lin
Move the countdown function from MokManager to console.c to make the function public Also make console_save_and_set_mode() and console_restore_mode() public Signed-off-by: Gary Lin <glin@suse.com>
2021-02-16Fix up a bunch of our license statements and add SPDX most placesPeter Jones
The license statements in our source files were getting to be a giant mess, and mostly they all just say the same thing. I've switched most of it to SPDX labels, but left copyright statements in place (where they were not obviously incorrect copy-paste jobs that I did...). If there's some change here you don't think is valid, let me know and we can fix it up together. Signed-off-by: Peter Jones <pjones@redhat.com>
2020-10-15Fix some mokmanager deletion pathsPeter Jones
This fixes several codepaths where MokList and MokListX are supposed to be deleted, but are not. It also adds debug logging to much of the deletion codepath.
2020-07-23MokManager: fix a wrong allocation failure check.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com> Upstream: pr#212
2020-07-23MokManager: fix uninitialized valuePeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com> Upstream: pr#212
2020-07-23MokManager: avoid -Werror=address-of-packed-memberJonas Witschel
When compiling with GCC 9, there are a couple of errors of the form MokManager.c: In function ‘write_back_mok_list’: MokManager.c:1056:19: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1056 | if (CompareGuid(&(list[i].Type), &X509_GUID) == 0) | ^~~~~~~~~~~~~~~ Copying the member of the packed struct to a temporary variable and pointing to that variable solves the problem. Upstream-commit-id: 58532e12e9a
2020-07-23MokManager: console mode modification for hi-dpi screen devicesIvan Hu
There are lots of hi-dpi laptops nowadays, as doing mok enrollment, the font is too small to see. https://bugs.launchpad.net/ubuntu/+source/shim/+bug/1822043 This patch checks if the resolution is larger than Full HD (1920x1080) and current console output columns and rows is in a good mode. Then swith the console output to a better mode. Signed-off-by: Ivan Hu <ivan.hu@canonical.com> Upstream-commit-id: cf05af6d899
2020-07-23MokManager: Use CompareMem on MokListNode.Type instead of CompareGuidGary Lin
Fix the errors from gcc9 '-Werror=address-of-packed-member' https://github.com/rhboot/shim/issues/161 Signed-off-by: Gary Lin <glin@suse.com> Upstream-commit-id: aaa09b35e73
2020-07-23Work around stuff -Waddress-of-packed-member finds.Peter Jones
In MokManager we get a lot of these: ../src/MokManager.c:1063:19: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1063 | if (CompareGuid(&(list[i].Type), &X509_GUID) == 0) | ^~~~~~~~~~~~~~~ The reason for this is that gnu-efi takes EFI_GUID * as its argument instead of VOID *, and there's nothing telling the compiler that it doesn't have alignment constraints on the input, so the compiler wants it to have 16-byte alignment. Just use CompareMem() for these, as that's all CompareGuid is calling anyway. Signed-off-by: Peter Jones <pjones@redhat.com> Upstream-commit-id: 08c14376b59
2020-07-23MokManager: Stop using EFI_VARIABLE_APPEND_WRITEGary Lin
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> Upstream-commit-id: f442c8424b4
2020-07-23Let MokManager follow a MokTimeout var for timeout length for the promptMathieu Trudel-Lapierre
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> Upstream-commit-id: 93708c11083
2018-04-05Audit get_variable() calls for correct FreePool() use.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-20Revert "MokManager: stop using StrnCat"Peter Jones
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>
2018-03-13MokManager: stop using StrnCatHans de Goede
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>
2018-03-12console: Do not set EFI console to textmode until something is printedHans de Goede
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>
2018-03-12console: Add console_print and console_print_at helpersHans de Goede
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>
2018-03-12Don't use uefi_call_wrapper(), ever.Peter Jones
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>
2018-03-12MokManager: get rid of struct menu_item, it is unused.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12MokManager: Fix a conditional on an uninitialized value in one error path.Peter Jones
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>
2018-03-12MokManager: handle mok parameter allocations better.Peter Jones
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>
2018-03-12MokManager: use EFI_ERROR() instead of comparing to EFI_SUCCESS.Peter Jones
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>
2018-03-12MokManager: check_der_suffix(): use StrnCat() on our suffix checker.Peter Jones
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>
2018-03-12MokManager: Lindent (and other reformats) the whole file.Peter Jones
I'm just tired of all the little quirks. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12Get rid of all the places we cast to (CHAR16 *[])Peter Jones
Lindent gets confused by these, and they're hard to read anyway. Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12Fix some "if (x < 0)" tests where x is UINTN.Peter Jones
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>
2018-03-12Don't have tons of local guid definitions for no reason at all.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2018-03-12Move includes around to clean the source tree up a bit.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2017-08-31Revert lots of Cryptlib updates.Peter Jones
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>
2017-08-18Rework looping in enter_mok_menu(), to allow multiple MOK changesMathieu Trudel-Lapierre
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>
2017-04-11MokManager: Update to new openssl APIGary Lin
X509_get_notBefore -> X509_getm_notBefore X509_get_notAfter -> X509_getm_notAfter Signed-off-by: Gary Lin <glin@suse.com>
2016-09-21MokManager: list Extended Key Usage OIDsMathieu Trudel-Lapierre
Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
2016-09-09MokManager: free new_data after useGary Lin
new_data in write_db() wasn't freed after SetVariable. Signed-off-by: Gary Lin <glin@suse.com>
2016-09-09MokManager: Try APPEND_WRITE firstGary Lin
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>
2016-09-09MokManager: Remove the usage of APPEND_WRITEGary Lin
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>
2016-09-06Update to openssl to 1.0.2eGary Lin
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>
2015-11-17MokManager: Fix a -Wsign-compare bug on i?86Peter Jones
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>
2015-06-16MokManager: Nerf SHA-1 again for actual hashes and signatures.Peter Jones
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>
2015-06-16MokManager: fix comparison between signed and unsigned integerGary Ching-Pang Lin
Patch from Johannes Segitz <jsegitz@suse.com>
2015-06-16MokManager: Discard the list contains an invalid signatureGary Ching-Pang Lin
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2015-06-16MokManager: Support SHA224, SHA384, and SHA512Gary Ching-Pang Lin
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2015-06-16MokManager: Add more key list safe checksGary Ching-Pang Lin
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
2015-06-16MokManager: fix the return value and typeGary Ching-Pang Lin
There are some functions that the return value and the type didn't match. Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>