From 1bc1cd96e43f73d75b4a0346532c86260cd33748 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Sat, 13 Oct 2012 01:07:43 -0400 Subject: [PATCH 01/26] Add section headers Provide a little more contextual information when people are in shim menus. --- MokManager.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/MokManager.c b/MokManager.c index 1d84a2d..f538a56 100644 --- a/MokManager.c +++ b/MokManager.c @@ -15,6 +15,9 @@ #define EFI_VARIABLE_APPEND_WRITE 0x00000040 +#define CERT_STRING L"Select an X509 certificate to enroll:\n\n" +#define HASH_STRING L"Select a file to trust:\n\n" + struct menu_item { CHAR16 *text; INTN (* callback)(void *data, void *data2, void *data3); @@ -678,7 +681,8 @@ static INTN mok_deletion_prompt (void *MokNew, void *data2, void *data3) { return 0; } -static UINTN draw_menu (struct menu_item *items, UINTN count) { +static UINTN draw_menu (CHAR16 *header, UINTN lines, struct menu_item *items, + UINTN count) { UINTN i; uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); @@ -688,6 +692,9 @@ static UINTN draw_menu (struct menu_item *items, UINTN count) { Print(L"%s UEFI key management\n\n", SHIM_VENDOR); + if (header) + Print(L"%s", header); + for (i = 0; i < count; i++) { uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, items[i].colour | EFI_BACKGROUND_BLACK); @@ -697,7 +704,7 @@ static UINTN draw_menu (struct menu_item *items, UINTN count) { uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, ST->ConOut, 0, 0); uefi_call_wrapper(ST->ConOut->EnableCursor, 2, ST->ConOut, TRUE); - return 2; + return 2 + lines; } static void free_menu (struct menu_item *items, UINTN count) { @@ -711,7 +718,8 @@ static void free_menu (struct menu_item *items, UINTN count) { FreePool(items); } -static void run_menu (struct menu_item *items, UINTN count, UINTN timeout) { +static void run_menu (CHAR16 *header, UINTN lines, struct menu_item *items, + UINTN count, UINTN timeout) { UINTN index, pos = 0, wait = 0, offset; EFI_INPUT_KEY key; EFI_STATUS status; @@ -722,7 +730,7 @@ static void run_menu (struct menu_item *items, UINTN count, UINTN timeout) { while (1) { uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); - offset = draw_menu (items, count); + offset = draw_menu (header, lines, items, count); uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, @@ -783,7 +791,7 @@ static void run_menu (struct menu_item *items, UINTN count, UINTN timeout) { items[pos].callback(items[pos].data, items[pos].data2, items[pos].data3); - draw_menu (items, count); + draw_menu (header, lines, items, count); pos = 0; break; } @@ -937,6 +945,7 @@ static INTN directory_callback (void *data, void *data2, void *data3) { EFI_FILE *dir; CHAR16 *filename = data; EFI_FILE *root = data2; + BOOLEAN hash = !!data3; status = uefi_call_wrapper(root->Open, 5, root, &dir, filename, EFI_FILE_MODE_READ, 0); @@ -1023,7 +1032,10 @@ static INTN directory_callback (void *data, void *data2, void *data3) { buffer = NULL; } - run_menu(dircontent, dircount, 0); + if (hash) + run_menu(HASH_STRING, 2, dircontent, dircount, 0); + else + run_menu(CERT_STRING, 2, dircontent, dircount, 0); return 0; } @@ -1035,6 +1047,7 @@ static INTN filesystem_callback (void *data, void *data2, void *data3) { UINTN dircount = 0, i = 0; struct menu_item *dircontent; EFI_FILE *root = data; + BOOLEAN hash = !!data3; uefi_call_wrapper(root->SetPosition, 2, root, 0); @@ -1117,7 +1130,10 @@ static INTN filesystem_callback (void *data, void *data2, void *data3) { buffersize = 0; } - run_menu(dircontent, dircount, 0); + if (hash) + run_menu(HASH_STRING, 2, dircontent, dircount, 0); + else + run_menu(CERT_STRING, 2, dircontent, dircount, 0); return 0; } @@ -1128,6 +1144,7 @@ static INTN find_fs (void *data, void *data2, void *data3) { UINTN OldSize, NewSize; EFI_HANDLE **filesystem_handles; struct menu_item *filesystems; + BOOLEAN hash = !!data3; uefi_call_wrapper(BS->LocateHandleBuffer, 5, ByProtocol, &fs_guid, NULL, &count, &filesystem_handles); @@ -1208,7 +1225,10 @@ static INTN find_fs (void *data, void *data2, void *data3) { uefi_call_wrapper(BS->FreePool, 1, filesystem_handles); - run_menu(filesystems, count, 0); + if (hash) + run_menu(HASH_STRING, 2, filesystems, count, 0); + else + run_menu(CERT_STRING, 2, filesystems, count, 0); return 0; } @@ -1275,7 +1295,7 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, menucount++; - run_menu(menu_item, menucount, 10); + run_menu(NULL, 0, menu_item, menucount, 10); uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); From 9272bc5b847bf30f89d9bef49fcc5b04c780460e Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:41:52 -0400 Subject: [PATCH 02/26] Add support for disabling signature verification Provide a mechanism for a physically present end user to disable 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 signature validation (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 skip verification of signatures. --- MokManager.c | 179 ++++++++++++++++++++++++++++++++++++++++++--------- shim.c | 29 +++++---- 2 files changed, 166 insertions(+), 42 deletions(-) diff --git a/MokManager.c b/MokManager.c index f538a56..fb20352 100644 --- a/MokManager.c +++ b/MokManager.c @@ -8,6 +8,7 @@ #define PASSWORD_MAX 16 #define PASSWORD_MIN 8 +#define SB_PASSWORD_LEN 8 #ifndef SHIM_VENDOR #define SHIM_VENDOR L"Shim" @@ -32,6 +33,11 @@ typedef struct { UINT8 *Mok; } __attribute__ ((packed)) MokListNode; +typedef struct { + UINT32 MokSBState; + UINT8 hash[SHA256_DIGEST_SIZE]; +} __attribute__ ((packed)) MokSBvar; + static EFI_INPUT_KEY get_keystroke (void) { EFI_INPUT_KEY key; @@ -681,6 +687,96 @@ static INTN mok_deletion_prompt (void *MokNew, void *data2, void *data3) { return 0; } +static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { + EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; + EFI_STATUS efi_status; + UINTN MokSBSize = (UINTN)data2; + MokSBvar *var = MokSB; + CHAR16 password[SB_PASSWORD_LEN]; + UINT8 fail_count = 0; + UINT8 hash[SHA256_DIGEST_SIZE]; + UINT32 length; + CHAR16 line[1]; + UINT8 sbval = 1; + + LibDeleteVariable(L"MokSB", &shim_lock_guid); + + if (MokSBSize != sizeof(MokSBvar)) { + Print(L"Invalid MokSB variable contents\n"); + return -1; + } + + while (fail_count < 3) { + Print(L"Enter Secure Boot passphrase: "); + get_line(&length, password, SB_PASSWORD_LEN, 0); + + if (length != SB_PASSWORD_LEN) { + Print(L"Invalid password length\n"); + fail_count++; + continue; + } + + efi_status = compute_pw_hash(NULL, 0, password, + SB_PASSWORD_LEN, hash); + + if (efi_status != EFI_SUCCESS) { + Print(L"Unable to generate password hash\n"); + fail_count++; + continue; + } + + if (CompareMem(var->hash, hash, SHA256_DIGEST_SIZE) != 0) { + Print(L"Password doesn't match\n"); + fail_count++; + continue; + } + + break; + } + + if (fail_count >= 3) { + Print(L"Password limit reached\n"); + return -1; + } + + if (var->MokSBState == 0) { + Print(L"Disable Secure Boot? (y/n): "); + } else { + Print(L"Enable Secure Boot? (y/n): "); + } + + do { + get_line (&length, line, 1, 1); + + if (line[0] == 'Y' || line[0] == 'y') { + if (var->MokSBState == 0) { + efi_status = uefi_call_wrapper(RT->SetVariable, + 5, L"MokSBState", + &shim_lock_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS, + 1, &sbval); + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to set Secure Boot state\n"); + return -1; + } + } else { + LibDeleteVariable(L"MokSBState", + &shim_lock_guid); + } + + Print(L"Press a key to reboot system\n"); + Pause(); + uefi_call_wrapper(RT->ResetSystem, 4, EfiResetWarm, + EFI_SUCCESS, 0, NULL); + Print(L"Failed to reboot\n"); + return -1; + } + } while (line[0] != 'N' && line[0] != 'n'); + + return -1; +} + static UINTN draw_menu (CHAR16 *header, UINTN lines, struct menu_item *items, UINTN count) { UINTN i; @@ -1234,11 +1330,12 @@ static INTN find_fs (void *data, void *data2, void *data3) { } static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, - UINTN MokNewSize) + UINTN MokNewSize, void *MokSB, + UINTN MokSBSize) { struct menu_item *menu_item; UINT32 MokAuth = 0; - UINTN menucount = 0; + UINTN menucount = 3, i = 0; EFI_STATUS efi_status; EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; UINT8 auth[SHA256_DIGEST_SIZE]; @@ -1253,47 +1350,59 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, MokAuth = 1; if (MokNew || MokAuth) - menu_item = AllocateZeroPool(sizeof(struct menu_item) * 4); - else - menu_item = AllocateZeroPool(sizeof(struct menu_item) * 3); + menucount++; + + if (MokSB) + menucount++; + + menu_item = AllocateZeroPool(sizeof(struct menu_item) * menucount); if (!menu_item) return EFI_OUT_OF_RESOURCES; - menu_item[0].text = StrDuplicate(L"Continue boot"); - menu_item[0].colour = EFI_WHITE; - menu_item[0].callback = NULL; + menu_item[i].text = StrDuplicate(L"Continue boot"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = NULL; - menucount++; + i++; if (MokNew || MokAuth) { if (!MokNew) { - menu_item[1].text = StrDuplicate(L"Delete MOK"); - menu_item[1].colour = EFI_WHITE; - menu_item[1].callback = mok_deletion_prompt; + menu_item[i].text = StrDuplicate(L"Delete MOK"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = mok_deletion_prompt; } else { - menu_item[1].text = StrDuplicate(L"Enroll MOK"); - menu_item[1].colour = EFI_WHITE; - menu_item[1].data = MokNew; - menu_item[1].data2 = (void *)MokNewSize; - menu_item[1].callback = mok_enrollment_prompt_callback; + menu_item[i].text = StrDuplicate(L"Enroll MOK"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].data = MokNew; + menu_item[i].data2 = (void *)MokNewSize; + menu_item[i].callback = mok_enrollment_prompt_callback; } - menucount++; + i++; } - menu_item[menucount].text = StrDuplicate(L"Enroll key from disk"); - menu_item[menucount].colour = EFI_WHITE; - menu_item[menucount].callback = find_fs; - menu_item[menucount].data3 = (void *)FALSE; + if (MokSB) { + menu_item[i].text = StrDuplicate(L"Change Secure Boot state"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = mok_sb_prompt; + menu_item[i].data = MokSB; + menu_item[i].data2 = (void *)MokSBSize; + i++; + } - menucount++; + menu_item[i].text = StrDuplicate(L"Enroll key from disk"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = find_fs; + menu_item[i].data3 = (void *)FALSE; - menu_item[menucount].text = StrDuplicate(L"Enroll hash from disk"); - menu_item[menucount].colour = EFI_WHITE; - menu_item[menucount].callback = find_fs; - menu_item[menucount].data3 = (void *)TRUE; + i++; - menucount++; + menu_item[i].text = StrDuplicate(L"Enroll hash from disk"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = find_fs; + menu_item[i].data3 = (void *)TRUE; + + i++; run_menu(NULL, 0, menu_item, menucount, 10); @@ -1305,12 +1414,15 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; - UINTN MokNewSize = 0; + UINTN MokNewSize = 0, MokSBSize = 0; void *MokNew = NULL; + void *MokSB = NULL; MokNew = LibGetVariableAndSize(L"MokNew", &shim_lock_guid, &MokNewSize); - enter_mok_menu(image_handle, MokNew, MokNewSize); + MokSB = LibGetVariableAndSize(L"MokSB", &shim_lock_guid, &MokSBSize); + + enter_mok_menu(image_handle, MokNew, MokNewSize, MokSB, MokSBSize); if (MokNew) { if (LibDeleteVariable(L"MokNew", &shim_lock_guid) != EFI_SUCCESS) { @@ -1318,6 +1430,13 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) } FreePool (MokNew); } + + if (MokSB) { + if (LibDeleteVariable(L"MokSB", &shim_lock_guid) != EFI_SUCCESS) { + Print(L"Failed to delete MokSB\n"); + } + FreePool (MokNew); + } LibDeleteVariable(L"MokAuth", &shim_lock_guid); return EFI_SUCCESS; diff --git a/shim.c b/shim.c index 4eab87a..bffad13 100644 --- a/shim.c +++ b/shim.c @@ -1033,7 +1033,7 @@ done: EFI_STATUS check_mok_request(EFI_HANDLE image_handle) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; - EFI_STATUS efi_status; + EFI_STATUS moknew_status, moksb_status, efi_status; UINTN size = sizeof(UINT32); UINT32 MokNew; UINT32 attributes; @@ -1041,22 +1041,27 @@ EFI_STATUS check_mok_request(EFI_HANDLE image_handle) if (!secure_mode()) return EFI_SUCCESS; - efi_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokNew", - &shim_lock_guid, &attributes, - &size, (void *)&MokNew); + moknew_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokNew", + &shim_lock_guid, &attributes, + &size, (void *)&MokNew); - if (efi_status != EFI_SUCCESS && efi_status != EFI_BUFFER_TOO_SMALL) - goto done; + moksb_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokSB", + &shim_lock_guid, &attributes, + &size, (void *)&MokNew); - efi_status = start_image(image_handle, MOK_MANAGER); + if (moknew_status == EFI_SUCCESS || + moknew_status == EFI_BUFFER_TOO_SMALL || + moksb_status == EFI_SUCCESS || + moksb_status == EFI_BUFFER_TOO_SMALL) { + efi_status = start_image(image_handle, MOK_MANAGER); - if (efi_status != EFI_SUCCESS) { - Print(L"Failed to start MokManager\n"); - goto done; + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to start MokManager\n"); + return efi_status; + } } -done: - return efi_status; + return EFI_SUCCESS; } EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *passed_systab) From 9eaadb0d115eaf452f105398b300194939164664 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 03/26] Skip signature checking if insecure If we're configured to run untrusted code, print a message and skip the validation checks. --- shim.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/shim.c b/shim.c index bffad13..39ad9bb 100644 --- a/shim.c +++ b/shim.c @@ -54,6 +54,8 @@ extern UINT32 vendor_cert_size; #define EFI_IMAGE_SECURITY_DATABASE_GUID { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f }} +static UINT8 insecure_mode; + typedef enum { DATA_FOUND, DATA_NOT_FOUND, @@ -360,6 +362,9 @@ static BOOLEAN secure_mode (void) UINT8 sb, setupmode; UINT32 attributes; + if (insecure_mode) + return FALSE; + status = get_variable(L"SecureBoot", global_var, &attributes, &charsize, (void *)&sb); @@ -1038,9 +1043,6 @@ EFI_STATUS check_mok_request(EFI_HANDLE image_handle) UINT32 MokNew; UINT32 attributes; - if (!secure_mode()) - return EFI_SUCCESS; - moknew_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokNew", &shim_lock_guid, &attributes, &size, (void *)&MokNew); @@ -1064,6 +1066,36 @@ EFI_STATUS check_mok_request(EFI_HANDLE image_handle) return EFI_SUCCESS; } +static EFI_STATUS check_mok_sb (void) +{ + EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; + EFI_STATUS status = EFI_SUCCESS; + void *MokSBState = NULL; + UINTN MokSBStateSize = 0; + UINT32 attributes; + + status = get_variable(L"MokSBState", shim_lock_guid, &attributes, + &MokSBStateSize, &MokSBState); + + if (status != EFI_SUCCESS) + return EFI_ACCESS_DENIED; + + if (attributes & EFI_VARIABLE_RUNTIME_ACCESS) { + Print(L"MokSBState is compromised! Clearing it\n"); + if (LibDeleteVariable(L"MokSBState", &shim_lock_guid) != EFI_SUCCESS) { + Print(L"Failed to erase MokSBState\n"); + } + status = EFI_ACCESS_DENIED; + } else { + if (*(UINT8 *)MokSBState == 1) { + insecure_mode = 1; + } + } + + return status; +} + + EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *passed_systab) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; @@ -1079,6 +1111,13 @@ EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *passed_systab) InitializeLib(image_handle, systab); + check_mok_sb(); + + if (insecure_mode) { + Print(L"Booting in insecure mode\n"); + uefi_call_wrapper(BS->Stall, 1, 2000000); + } + efi_status = check_mok_request(image_handle); efi_status = mirror_mok_list(); From d08ea5363cec6f0159112aec1658d57fbaf1e471 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 04/26] Pause on callback failures If a callback returns any kind of failure, wait for a keypress in order to give the user an opportunity to read any failure messages. --- MokManager.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/MokManager.c b/MokManager.c index fb20352..88a08f7 100644 --- a/MokManager.c +++ b/MokManager.c @@ -819,6 +819,7 @@ static void run_menu (CHAR16 *header, UINTN lines, struct menu_item *items, UINTN index, pos = 0, wait = 0, offset; EFI_INPUT_KEY key; EFI_STATUS status; + INTN ret; if (timeout) wait = 10000000; @@ -885,8 +886,13 @@ static void run_menu (CHAR16 *header, UINTN lines, struct menu_item *items, return; } - items[pos].callback(items[pos].data, items[pos].data2, - items[pos].data3); + ret = items[pos].callback(items[pos].data, + items[pos].data2, + items[pos].data3); + if (ret < 0) { + Print(L"Press a key to continue\n"); + Pause(); + } draw_menu (header, lines, items, count); pos = 0; break; From 801d1b936be96f0d22fd5b91af973cafc1fcb68c Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 05/26] Add MOK password auth Add support for setting an MOK password. The OS passes down a password hash. MokManager then presents an option for setting a password. Selecting it prompts the user for the same password again. If they match, the hash is enrolled into a boot services variable and MokManager will prompt for the password whenever it's started. --- MokManager.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++-- shim.c | 16 +++-- 2 files changed, 180 insertions(+), 9 deletions(-) diff --git a/MokManager.c b/MokManager.c index 88a08f7..18992f2 100644 --- a/MokManager.c +++ b/MokManager.c @@ -777,6 +777,86 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { return -1; } + +static INTN mok_pw_prompt (void *MokPW, void *data2, void *data3) { + EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; + EFI_STATUS efi_status; + UINTN MokPWSize = (UINTN)data2; + UINT8 fail_count = 0; + UINT8 hash[SHA256_DIGEST_SIZE]; + CHAR16 password[PASSWORD_MAX]; + UINT32 length; + CHAR16 line[1]; + + if (MokPWSize != SHA256_DIGEST_SIZE) { + Print(L"Invalid MokPW variable contents\n"); + return -1; + } + + LibDeleteVariable(L"MokPW", &shim_lock_guid); + + while (fail_count < 3) { + Print(L"Confirm MOK passphrase: "); + get_line(&length, password, PASSWORD_MAX, 0); + + if ((length < PASSWORD_MIN) || (length > PASSWORD_MAX)) { + Print(L"Invalid password length\n"); + fail_count++; + continue; + } + + efi_status = compute_pw_hash(NULL, 0, password, + SB_PASSWORD_LEN, hash); + + if (efi_status != EFI_SUCCESS) { + Print(L"Unable to generate password hash\n"); + fail_count++; + continue; + } + + if (CompareMem(MokPW, hash, SHA256_DIGEST_SIZE) != 0) { + Print(L"Password doesn't match\n"); + fail_count++; + continue; + } + + break; + } + + if (fail_count >= 3) { + Print(L"Password limit reached\n"); + return -1; + } + + Print(L"Set MOK password? (y/n): "); + + do { + get_line (&length, line, 1, 1); + + if (line[0] == 'Y' || line[0] == 'y') { + efi_status = uefi_call_wrapper(RT->SetVariable, 5, + L"MokPWStore", + &shim_lock_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS, + MokPWSize, MokPW); + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to set MOK password\n"); + return -1; + } + + Print(L"Press a key to reboot system\n"); + Pause(); + uefi_call_wrapper(RT->ResetSystem, 4, EfiResetWarm, + EFI_SUCCESS, 0, NULL); + Print(L"Failed to reboot\n"); + return -1; + } + } while (line[0] != 'N' && line[0] != 'n'); + + return 0; +} + static UINTN draw_menu (CHAR16 *header, UINTN lines, struct menu_item *items, UINTN count) { UINTN i; @@ -1335,9 +1415,67 @@ static INTN find_fs (void *data, void *data2, void *data3) { return 0; } +static BOOLEAN verify_pw(void) +{ + EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; + EFI_STATUS efi_status; + CHAR16 password[PASSWORD_MAX]; + UINT8 fail_count = 0; + UINT8 hash[SHA256_DIGEST_SIZE]; + UINT8 pwhash[SHA256_DIGEST_SIZE]; + UINTN size = SHA256_DIGEST_SIZE; + UINT32 length; + UINT32 attributes; + + efi_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokPWStore", + &shim_lock_guid, &attributes, &size, + pwhash); + + /* + * If anything can attack the password it could just set it to a + * known value, so there's no safety advantage in failing to validate + * purely because of a failure to read the variable + */ + if (efi_status != EFI_SUCCESS) + return TRUE; + + if (attributes & EFI_VARIABLE_RUNTIME_ACCESS) + return TRUE; + + while (fail_count < 3) { + Print(L"Enter MOK password: "); + get_line(&length, password, PASSWORD_MAX, 0); + + if (length < PASSWORD_MIN || length > PASSWORD_MAX) { + Print(L"Invalid password length\n"); + fail_count++; + continue; + } + + efi_status = compute_pw_hash(NULL, 0, password, length, hash); + + if (efi_status != EFI_SUCCESS) { + Print(L"Unable to generate password hash\n"); + fail_count++; + continue; + } + + if (CompareMem(pwhash, hash, SHA256_DIGEST_SIZE) != 0) { + Print(L"Password doesn't match\n"); + fail_count++; + continue; + } + + return TRUE; + } + + Print(L"Password limit reached\n"); + return FALSE; +} + static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, UINTN MokNewSize, void *MokSB, - UINTN MokSBSize) + UINTN MokSBSize, void *MokPW, UINTN MokPWSize) { struct menu_item *menu_item; UINT32 MokAuth = 0; @@ -1348,6 +1486,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, UINTN auth_size = SHA256_DIGEST_SIZE; UINT32 attributes; + if (verify_pw() == FALSE) + return EFI_ACCESS_DENIED; + efi_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokAuth", &shim_lock_guid, &attributes, &auth_size, auth); @@ -1361,6 +1502,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, if (MokSB) menucount++; + if (MokPW) + menucount++; + menu_item = AllocateZeroPool(sizeof(struct menu_item) * menucount); if (!menu_item) @@ -1396,6 +1540,15 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, i++; } + if (MokPW) { + menu_item[i].text = StrDuplicate(L"Set MOK password"); + menu_item[i].colour = EFI_WHITE; + menu_item[i].callback = mok_pw_prompt; + menu_item[i].data = MokPW; + menu_item[i].data2 = (void *)MokPWSize; + i++; + } + menu_item[i].text = StrDuplicate(L"Enroll key from disk"); menu_item[i].colour = EFI_WHITE; menu_item[i].callback = find_fs; @@ -1420,15 +1573,19 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokNew, static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; - UINTN MokNewSize = 0, MokSBSize = 0; + UINTN MokNewSize = 0, MokSBSize = 0, MokPWSize = 0; void *MokNew = NULL; void *MokSB = NULL; + void *MokPW = NULL; MokNew = LibGetVariableAndSize(L"MokNew", &shim_lock_guid, &MokNewSize); - MokSB = LibGetVariableAndSize(L"MokSB", &shim_lock_guid, &MokSBSize); + MokSB = LibGetVariableAndSize(L"MokSB", &shim_lock_guid, &MokSBSize); - enter_mok_menu(image_handle, MokNew, MokNewSize, MokSB, MokSBSize); + MokPW = LibGetVariableAndSize(L"MokPW", &shim_lock_guid, &MokPWSize); + + enter_mok_menu(image_handle, MokNew, MokNewSize, MokSB, MokSBSize, + MokPW, MokPWSize); if (MokNew) { if (LibDeleteVariable(L"MokNew", &shim_lock_guid) != EFI_SUCCESS) { @@ -1443,6 +1600,14 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) } FreePool (MokNew); } + + if (MokPW) { + if (LibDeleteVariable(L"MokPW", &shim_lock_guid) != EFI_SUCCESS) { + Print(L"Failed to delete MokPW\n"); + } + FreePool (MokNew); + } + LibDeleteVariable(L"MokAuth", &shim_lock_guid); return EFI_SUCCESS; diff --git a/shim.c b/shim.c index 39ad9bb..dbe5e84 100644 --- a/shim.c +++ b/shim.c @@ -1038,23 +1038,29 @@ done: EFI_STATUS check_mok_request(EFI_HANDLE image_handle) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; - EFI_STATUS moknew_status, moksb_status, efi_status; + EFI_STATUS moknew_status, moksb_status, mokpw_status, efi_status; UINTN size = sizeof(UINT32); - UINT32 MokNew; + UINT32 MokVar; UINT32 attributes; moknew_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokNew", &shim_lock_guid, &attributes, - &size, (void *)&MokNew); + &size, (void *)&MokVar); moksb_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokSB", &shim_lock_guid, &attributes, - &size, (void *)&MokNew); + &size, (void *)&MokVar); + + mokpw_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokPW", + &shim_lock_guid, &attributes, + &size, (void *)&MokVar); if (moknew_status == EFI_SUCCESS || moknew_status == EFI_BUFFER_TOO_SMALL || moksb_status == EFI_SUCCESS || - moksb_status == EFI_BUFFER_TOO_SMALL) { + moksb_status == EFI_BUFFER_TOO_SMALL || + mokpw_status == EFI_SUCCESS || + mokpw_status == EFI_BUFFER_TOO_SMALL) { efi_status = start_image(image_handle, MOK_MANAGER); if (efi_status != EFI_SUCCESS) { From 37635f541465578e29c2c6e908a24e74f240d728 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 06/26] Clean up timeout counter handling Reduce menu redrawing by only redrawing the invalidated section of the menu during the timeout countdown. --- MokManager.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/MokManager.c b/MokManager.c index 18992f2..23ed80e 100644 --- a/MokManager.c +++ b/MokManager.c @@ -894,6 +894,28 @@ static void free_menu (struct menu_item *items, UINTN count) { FreePool(items); } +static void update_time (UINTN position, UINTN timeout) +{ + uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, ST->ConOut, 0, + position); + + uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, + EFI_BLACK | EFI_BACKGROUND_BLACK); + + Print(L" ", timeout); + + uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, ST->ConOut, 0, + position); + + uefi_call_wrapper(ST->ConOut->SetAttribute, 2, ST->ConOut, + EFI_WHITE | EFI_BACKGROUND_BLACK); + + if (timeout > 1) + Print(L"Booting in %d seconds\n", timeout); + else if (timeout) + Print(L"Booting in %d second\n", timeout); +} + static void run_menu (CHAR16 *header, UINTN lines, struct menu_item *items, UINTN count, UINTN timeout) { UINTN index, pos = 0, wait = 0, offset; @@ -904,23 +926,10 @@ static void run_menu (CHAR16 *header, UINTN lines, struct menu_item *items, if (timeout) wait = 10000000; + offset = draw_menu (header, lines, items, count); + while (1) { - uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); - - offset = draw_menu (header, lines, items, count); - - uefi_call_wrapper(ST->ConOut->SetAttribute, 2, - ST->ConOut, - EFI_WHITE | EFI_BACKGROUND_BLACK); - - if (timeout) { - uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, - ST->ConOut, 0, count + 1 + offset); - if (timeout > 1) - Print(L"Booting in %d seconds\n", timeout); - else - Print(L"Booting in %d second\n", timeout); - } + update_time(count + offset + 1, timeout); uefi_call_wrapper(ST->ConOut->SetCursorPosition, 3, ST->ConOut, 0, pos + offset); From 16c512f9b5930da742e4014dbe26d3c895d689c3 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 07/26] Don't print SHA1 sum when calculating file fingerprints There's no point in printing the SHA1 of a SHA256... --- MokManager.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/MokManager.c b/MokManager.c index 23ed80e..bc62cd0 100644 --- a/MokManager.c +++ b/MokManager.c @@ -317,8 +317,8 @@ static void show_mok_info (void *Mok, UINTN MokSize) return; if (MokSize != 48) { - if (X509ConstructCertificate(Mok, MokSize, (UINT8 **) &X509Cert) && - X509Cert != NULL) { + if (X509ConstructCertificate(Mok, MokSize, + (UINT8 **) &X509Cert) && X509Cert != NULL) { show_x509_info(X509Cert); X509_free(X509Cert); } else { @@ -326,6 +326,20 @@ static void show_mok_info (void *Mok, UINTN MokSize) ((UINT32 *)Mok)[0]); return; } + + efi_status = get_sha1sum(Mok, MokSize, hash); + + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to compute MOK fingerprint\n"); + return; + } + + Print(L" Fingerprint (SHA1):\n "); + for (i = 0; i < SHA1_DIGEST_SIZE; i++) { + Print(L" %02x", hash[i]); + if (i % 10 == 9) + Print(L"\n "); + } } else { Print(L"SHA256 hash:\n "); for (i = 0; i < SHA256_DIGEST_SIZE; i++) { @@ -335,19 +349,7 @@ static void show_mok_info (void *Mok, UINTN MokSize) } Print(L"\n"); } - efi_status = get_sha1sum(Mok, MokSize, hash); - if (efi_status != EFI_SUCCESS) { - Print(L"Failed to compute MOK fingerprint\n"); - return; - } - - Print(L" Fingerprint (SHA1):\n "); - for (i = 0; i < SHA1_DIGEST_SIZE; i++) { - Print(L" %02x", hash[i]); - if (i % 10 == 9) - Print(L"\n "); - } Print(L"\n"); } From 34f0c4abc0a0e43656155be587428639bbf87710 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: [PATCH 08/26] Clear screen before prompting We were drawing prompts on top of existing text, which was less than ideal. --- MokManager.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MokManager.c b/MokManager.c index bc62cd0..08e1d36 100644 --- a/MokManager.c +++ b/MokManager.c @@ -708,6 +708,8 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { return -1; } + uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); + while (fail_count < 3) { Print(L"Enter Secure Boot passphrase: "); get_line(&length, password, SB_PASSWORD_LEN, 0); @@ -797,6 +799,8 @@ static INTN mok_pw_prompt (void *MokPW, void *data2, void *data3) { LibDeleteVariable(L"MokPW", &shim_lock_guid); + uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); + while (fail_count < 3) { Print(L"Confirm MOK passphrase: "); get_line(&length, password, PASSWORD_MAX, 0); @@ -1453,6 +1457,8 @@ static BOOLEAN verify_pw(void) if (attributes & EFI_VARIABLE_RUNTIME_ACCESS) return TRUE; + uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); + while (fail_count < 3) { Print(L"Enter MOK password: "); get_line(&length, password, PASSWORD_MAX, 0); From 5f0a358b6349aa9bae3da562c928c920065afd17 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 23 Oct 2012 11:47:41 -0400 Subject: [PATCH 09/26] Support a vendor-specific DBX list. In some rare corner cases, it's useful to add a blacklist of things that were allowed by a copy of shim that was never signed by the UEFI signing service. In these cases it's okay for them to go into a local dbx, rather than taking up precious flash. Signed-off-by: Peter Jones --- Makefile | 7 +++-- dbx.S | 32 +++++++++++++++++++++ shim.c | 86 ++++++++++++++++++++++++++++++++++++++++---------------- 3 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 dbx.S diff --git a/Makefile b/Makefile index bea0ebd..af49678 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ LDFLAGS = -nostdlib -znocombreloc -T $(EFI_LDS) -shared -Bsymbolic -L$(EFI_PATH VERSION = 0.1 TARGET = shim.efi MokManager.efi -OBJS = shim.o cert.o +OBJS = shim.o cert.o dbx.o SOURCES = shim.c shim.h signature.h PeImage.h MOK_OBJS = MokManager.o MOK_SOURCES = MokManager.c shim.h @@ -41,7 +41,10 @@ shim.o: $(SOURCES) cert.o : cert.S $(CC) $(CFLAGS) -c -o $@ $< -shim.so: $(OBJS) Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a cert.o +dbx.o : dbx.S + $(CC) $(CFLAGS) -c -o $@ $< + +shim.so: $(OBJS) Cryptlib/libcryptlib.a Cryptlib/OpenSSL/libopenssl.a $(LD) -o $@ $(LDFLAGS) $^ $(EFI_LIBS) MokManager.o: $(SOURCES) diff --git a/dbx.S b/dbx.S new file mode 100644 index 0000000..d19123c --- /dev/null +++ b/dbx.S @@ -0,0 +1,32 @@ +#if defined(VENDOR_DBX_FILE) + .globl vendor_dbx_size + .data + .align 1 + .type vendor_dbx_size, @object + .size vendor_dbx_size, 4 +vendor_dbx_size: + .long .L0 - vendor_dbx + .globl vendor_dbx + .data + .align 1 + .type vendor_dbx, @object + .size vendor_dbx_size, vendor_dbx_size-vendor_dbx +vendor_dbx: +.incbin VENDOR_DBX_FILE +.L0: +#else + .globl vendor_dbx + .bss + .type vendor_dbx, @object + .size vendor_dbx, 1 +vendor_dbx: + .zero 1 + + .globl vendor_dbx_size + .data + .align 4 + .type vendor_dbx_size, @object + .size vendor_dbx_size, 4 +vendor_dbx_size: + .long 1 +#endif diff --git a/shim.c b/shim.c index dbe5e84..eb3cf52 100644 --- a/shim.c +++ b/shim.c @@ -51,6 +51,8 @@ static EFI_STATUS (EFIAPI *entry_point) (EFI_HANDLE image_handle, EFI_SYSTEM_TAB */ extern UINT8 vendor_cert[]; extern UINT32 vendor_cert_size; +extern EFI_SIGNATURE_LIST *vendor_dbx; +extern UINT32 vendor_dbx_size; #define EFI_IMAGE_SECURITY_DATABASE_GUID { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f }} @@ -209,26 +211,16 @@ static EFI_STATUS relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context, return EFI_SUCCESS; } -static CHECK_STATUS check_db_cert(CHAR16 *dbname, EFI_GUID guid, - WIN_CERTIFICATE_EFI_PKCS *data, UINT8 *hash) +static CHECK_STATUS check_db_cert_in_ram(EFI_SIGNATURE_LIST *CertList, + UINTN dbsize, + WIN_CERTIFICATE_EFI_PKCS *data, + UINT8 *hash) { - EFI_STATUS efi_status; - EFI_SIGNATURE_LIST *CertList; EFI_SIGNATURE_DATA *Cert; - UINTN dbsize = 0; UINTN CertCount, Index; - UINT32 attributes; BOOLEAN IsFound = FALSE; - void *db; EFI_GUID CertType = EfiCertX509Guid; - efi_status = get_variable(dbname, guid, &attributes, &dbsize, &db); - - if (efi_status != EFI_SUCCESS) - return VAR_NOT_FOUND; - - CertList = db; - while ((dbsize > 0) && (dbsize >= CertList->SignatureListSize)) { if (CompareGuid (&CertList->SignatureType, &CertType) == 0) { CertCount = (CertList->SignatureListSize - CertList->SignatureHeaderSize) / CertList->SignatureSize; @@ -250,34 +242,44 @@ static CHECK_STATUS check_db_cert(CHAR16 *dbname, EFI_GUID guid, CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); } - FreePool(db); - if (IsFound) return DATA_FOUND; return DATA_NOT_FOUND; } -static CHECK_STATUS check_db_hash(CHAR16 *dbname, EFI_GUID guid, UINT8 *data, - int SignatureSize, EFI_GUID CertType) +static CHECK_STATUS check_db_cert(CHAR16 *dbname, EFI_GUID guid, + WIN_CERTIFICATE_EFI_PKCS *data, UINT8 *hash) { + CHECK_STATUS rc; EFI_STATUS efi_status; EFI_SIGNATURE_LIST *CertList; - EFI_SIGNATURE_DATA *Cert; UINTN dbsize = 0; - UINTN CertCount, Index; UINT32 attributes; - BOOLEAN IsFound = FALSE; void *db; efi_status = get_variable(dbname, guid, &attributes, &dbsize, &db); - if (efi_status != EFI_SUCCESS) { + if (efi_status != EFI_SUCCESS) return VAR_NOT_FOUND; - } CertList = db; + rc = check_db_cert_in_ram(CertList, dbsize, data, hash); + + FreePool(db); + + return rc; +} + +static CHECK_STATUS check_db_hash_in_ram(EFI_SIGNATURE_LIST *CertList, + UINTN dbsize, UINT8 *data, + int SignatureSize, EFI_GUID CertType) +{ + EFI_SIGNATURE_DATA *Cert; + UINTN CertCount, Index; + BOOLEAN IsFound = FALSE; + while ((dbsize > 0) && (dbsize >= CertList->SignatureListSize)) { CertCount = (CertList->SignatureListSize - CertList->SignatureHeaderSize) / CertList->SignatureSize; Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); @@ -302,19 +304,53 @@ static CHECK_STATUS check_db_hash(CHAR16 *dbname, EFI_GUID guid, UINT8 *data, CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); } - FreePool(db); - if (IsFound) return DATA_FOUND; return DATA_NOT_FOUND; } +static CHECK_STATUS check_db_hash(CHAR16 *dbname, EFI_GUID guid, UINT8 *data, + int SignatureSize, EFI_GUID CertType) +{ + EFI_STATUS efi_status; + EFI_SIGNATURE_LIST *CertList; + UINT32 attributes; + UINTN dbsize = 0; + void *db; + + efi_status = get_variable(dbname, guid, &attributes, &dbsize, &db); + + if (efi_status != EFI_SUCCESS) { + return VAR_NOT_FOUND; + } + + CertList = db; + + CHECK_STATUS rc = check_db_hash_in_ram(CertList, dbsize, data, + SignatureSize, CertType); + FreePool(db); + return rc; + +} + static EFI_STATUS check_blacklist (WIN_CERTIFICATE_EFI_PKCS *cert, UINT8 *sha256hash, UINT8 *sha1hash) { EFI_GUID secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID; + if (check_db_hash_in_ram(vendor_dbx, vendor_dbx_size, sha256hash, + SHA256_DIGEST_SIZE, EfiHashSha256Guid) == + DATA_NOT_FOUND) + return EFI_ACCESS_DENIED; + if (check_db_hash_in_ram(vendor_dbx, vendor_dbx_size, sha1hash, + SHA1_DIGEST_SIZE, EfiHashSha1Guid) == + DATA_NOT_FOUND) + return EFI_ACCESS_DENIED; + if (check_db_cert_in_ram(vendor_dbx, vendor_dbx_size, cert, + sha256hash) == DATA_NOT_FOUND) + return EFI_ACCESS_DENIED; + if (check_db_hash(L"dbx", secure_var, sha256hash, SHA256_DIGEST_SIZE, EfiHashSha256Guid) == DATA_FOUND) return EFI_ACCESS_DENIED; From 3a5933619965b4ba8c3aa0e3abbb4755bb80dc78 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 13:00:40 -0400 Subject: [PATCH 10/26] Fix key database parsing The pointer to the certificate needs to be incremented by the size of the entire certificate, not just the certificate data. --- MokManager.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/MokManager.c b/MokManager.c index 08e1d36..282fbc6 100644 --- a/MokManager.c +++ b/MokManager.c @@ -108,7 +108,7 @@ static MokListNode *build_mok_list(UINT32 num, void *Data, UINTN DataSize) { (CompareGuid (&CertList->SignatureType, &HashType) != 0)) { dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *)((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); continue; } @@ -116,7 +116,7 @@ static MokListNode *build_mok_list(UINT32 num, void *Data, UINTN DataSize) { (CertList->SignatureSize != 48)) { dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *)((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); continue; } @@ -129,7 +129,7 @@ static MokListNode *build_mok_list(UINT32 num, void *Data, UINTN DataSize) { count++; dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); } return list; @@ -414,7 +414,7 @@ static UINT8 list_keys (void *MokNew, UINTN MokNewSize) Print(L"Doesn't look like a key or hash\n"); dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); continue; } @@ -423,14 +423,14 @@ static UINT8 list_keys (void *MokNew, UINTN MokNewSize) Print(L"Doesn't look like a valid hash\n"); dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); continue; } MokNum++; dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + - CertList->SignatureSize); + CertList->SignatureListSize); } keys = build_mok_list(MokNum, MokNew, MokNewSize); From d5a2d9ea083f329f71a9cc5c3aac45b60fa60b93 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 13:01:25 -0400 Subject: [PATCH 11/26] Clean up checks for MokManager entry Add a helper function and tidy up the calls for getting into MokManager --- shim.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/shim.c b/shim.c index dbe5e84..9ff4b8b 100644 --- a/shim.c +++ b/shim.c @@ -1035,32 +1035,30 @@ done: return efi_status; } -EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +static BOOLEAN check_var(CHAR16 *varname) { + EFI_STATUS efi_status; EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; - EFI_STATUS moknew_status, moksb_status, mokpw_status, efi_status; UINTN size = sizeof(UINT32); UINT32 MokVar; UINT32 attributes; - moknew_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokNew", - &shim_lock_guid, &attributes, - &size, (void *)&MokVar); + efi_status = uefi_call_wrapper(RT->GetVariable, 5, varname, + &shim_lock_guid, &attributes, + &size, (void *)&MokVar); - moksb_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokSB", - &shim_lock_guid, &attributes, - &size, (void *)&MokVar); + if (efi_status == EFI_SUCCESS || efi_status == EFI_BUFFER_TOO_SMALL) + return TRUE; - mokpw_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokPW", - &shim_lock_guid, &attributes, - &size, (void *)&MokVar); + return FALSE; +} - if (moknew_status == EFI_SUCCESS || - moknew_status == EFI_BUFFER_TOO_SMALL || - moksb_status == EFI_SUCCESS || - moksb_status == EFI_BUFFER_TOO_SMALL || - mokpw_status == EFI_SUCCESS || - mokpw_status == EFI_BUFFER_TOO_SMALL) { +EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +{ + EFI_STATUS efi_status; + + if (check_var(L"MokNew") || check_var(L"MokSB") || + check_var(L"MokPW") || check_var(L"MokAuth")) { efi_status = start_image(image_handle, MOK_MANAGER); if (efi_status != EFI_SUCCESS) { From 868d5b903801fded444d03a92c7a371af6fe32ce Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 13:01:48 -0400 Subject: [PATCH 12/26] Delete MokList properly A cut and paste error meant that attempts to delete MokList were instead appending a zero-length addition. --- MokManager.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MokManager.c b/MokManager.c index 282fbc6..9e2fdf8 100644 --- a/MokManager.c +++ b/MokManager.c @@ -615,8 +615,7 @@ static EFI_STATUS store_keys (void *MokNew, UINTN MokNewSize, int authenticate) efi_status = uefi_call_wrapper(RT->SetVariable, 5, L"MokList", &shim_lock_guid, EFI_VARIABLE_NON_VOLATILE - | EFI_VARIABLE_BOOTSERVICE_ACCESS - | EFI_VARIABLE_APPEND_WRITE, + | EFI_VARIABLE_BOOTSERVICE_ACCESS, 0, NULL); } else { /* Write new MOK */ From 79a5aa039dc185870b2159418b0eda30731c141d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 15:43:10 -0400 Subject: [PATCH 13/26] Update image validation enable/disable Update this to match the new mokutil behaviour --- MokManager.c | 70 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/MokManager.c b/MokManager.c index 9e2fdf8..d764bf6 100644 --- a/MokManager.c +++ b/MokManager.c @@ -35,7 +35,8 @@ typedef struct { typedef struct { UINT32 MokSBState; - UINT8 hash[SHA256_DIGEST_SIZE]; + UINT32 PWLen; + CHAR16 Password[PASSWORD_MAX]; } __attribute__ ((packed)) MokSBvar; static EFI_INPUT_KEY get_keystroke (void) @@ -693,12 +694,13 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { EFI_STATUS efi_status; UINTN MokSBSize = (UINTN)data2; MokSBvar *var = MokSB; - CHAR16 password[SB_PASSWORD_LEN]; - UINT8 fail_count = 0; + CHAR16 password[1]; + UINT8 correct = 0, fail_count = 0; UINT8 hash[SHA256_DIGEST_SIZE]; UINT32 length; CHAR16 line[1]; UINT8 sbval = 1; + UINT8 pos; LibDeleteVariable(L"MokSB", &shim_lock_guid); @@ -709,32 +711,23 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); - while (fail_count < 3) { - Print(L"Enter Secure Boot passphrase: "); - get_line(&length, password, SB_PASSWORD_LEN, 0); + while (correct < 3) { + RandomBytes (&pos, sizeof(pos)); - if (length != SB_PASSWORD_LEN) { - Print(L"Invalid password length\n"); + pos = pos % var->PWLen; + + Print(L"Enter password character %d: ", pos + 1); + get_line(&length, password, 1, 0); + + if (password[0] != var->Password[pos]) { + Print(L"Invalid character\n"); fail_count++; - continue; + } else { + correct++; } - efi_status = compute_pw_hash(NULL, 0, password, - SB_PASSWORD_LEN, hash); - - if (efi_status != EFI_SUCCESS) { - Print(L"Unable to generate password hash\n"); - fail_count++; - continue; - } - - if (CompareMem(var->hash, hash, SHA256_DIGEST_SIZE) != 0) { - Print(L"Password doesn't match\n"); - fail_count++; - continue; - } - - break; + if (fail_count >= 3) + break; } if (fail_count >= 3) { @@ -1629,12 +1622,39 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) return EFI_SUCCESS; } +static EFI_STATUS setup_rand (void) +{ + EFI_TIME time; + EFI_STATUS efi_status; + UINT64 seed; + BOOLEAN status; + + efi_status = uefi_call_wrapper(RT->GetTime, 2, &time, NULL); + + if (efi_status != EFI_SUCCESS) + return efi_status; + + seed = ((UINT64)time.Year << 48) | ((UINT64)time.Month << 40) | + ((UINT64)time.Day << 32) | ((UINT64)time.Hour << 24) | + ((UINT64)time.Minute << 16) | ((UINT64)time.Second << 8) | + ((UINT64)time.Daylight); + + status = RandomSeed((UINT8 *)&seed, sizeof(seed)); + + if (!status) + return EFI_ABORTED; + + return EFI_SUCCESS; +} + EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *systab) { EFI_STATUS efi_status; InitializeLib(image_handle, systab); + setup_rand(); + efi_status = check_mok_request(image_handle); return efi_status; From 254c04bcddd722fd839962b46e6207b743aa432b Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 15:43:29 -0400 Subject: [PATCH 14/26] Fix password hash calculation This was hardcoded, rather than being based on the actual password length, resulting in incorrect hashes being generated. --- MokManager.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MokManager.c b/MokManager.c index d764bf6..f7504d0 100644 --- a/MokManager.c +++ b/MokManager.c @@ -803,8 +803,7 @@ static INTN mok_pw_prompt (void *MokPW, void *data2, void *data3) { continue; } - efi_status = compute_pw_hash(NULL, 0, password, - SB_PASSWORD_LEN, hash); + efi_status = compute_pw_hash(NULL, 0, password, length, hash); if (efi_status != EFI_SUCCESS) { Print(L"Unable to generate password hash\n"); From 6e05b32d0717ebc2541048de7269ce807a2fa12d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 23:46:44 -0400 Subject: [PATCH 15/26] Add another missing screen clearing Another case where we were drawing text over existing text. --- MokManager.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MokManager.c b/MokManager.c index f7504d0..f9acb0c 100644 --- a/MokManager.c +++ b/MokManager.c @@ -664,7 +664,9 @@ static UINTN mok_enrollment_prompt (void *MokNew, UINTN MokNewSize, int auth) { } static INTN mok_enrollment_prompt_callback (void *MokNew, void *data2, - void *data3) { + void *data3) +{ + uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); return mok_enrollment_prompt(MokNew, (UINTN)data2, TRUE); } From cbe214072bfa49f177e33f7e98f6e0431b4832eb Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 00:09:08 -0400 Subject: [PATCH 16/26] Miscellaneous small fixups Fixes for some small bugs noticed during review --- shim.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/shim.c b/shim.c index 0cd89b4..447cf87 100644 --- a/shim.c +++ b/shim.c @@ -82,8 +82,7 @@ static EFI_STATUS get_variable (CHAR16 *name, EFI_GUID guid, UINT32 *attributes, return efi_status; } - if (allocate) - *buffer = AllocatePool(*size); + *buffer = AllocatePool(*size); if (!*buffer) { Print(L"Unable to allocate variable buffer\n"); @@ -545,7 +544,8 @@ static EFI_STATUS generate_hash (char *data, int datasize, if (!hashbase) { Print(L"Malformed section header\n"); - return EFI_INVALID_PARAMETER; + status = EFI_INVALID_PARAMETER; + goto done; } if (!(Sha256Update(sha256ctx, hashbase, hashsize)) || @@ -683,9 +683,19 @@ static EFI_STATUS read_header(void *data, unsigned int datasize, EFI_IMAGE_DOS_HEADER *DosHdr = data; EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr = data; + if (datasize < sizeof(EFI_IMAGE_DOS_HEADER)) { + Print(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) PEHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((char *)data + DosHdr->e_lfanew); + if ((((UINT8 *)PEHdr - (UINT8 *)data) + sizeof(EFI_IMAGE_OPTIONAL_HEADER_UNION)) > datasize) { + Print(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + if (PEHdr->Te.Signature != EFI_IMAGE_NT_SIGNATURE) { Print(L"Unsupported image type\n"); return EFI_UNSUPPORTED; @@ -712,6 +722,16 @@ static EFI_STATUS read_header(void *data, unsigned int datasize, context->FirstSection = (EFI_IMAGE_SECTION_HEADER *)((char *)PEHdr + PEHdr->Pe32.FileHeader.SizeOfOptionalHeader + sizeof(UINT32) + sizeof(EFI_IMAGE_FILE_HEADER)); context->SecDir = (EFI_IMAGE_DATA_DIRECTORY *) &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; + if (context->ImageSize < context->SizeOfHeaders) { + Print(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (((UINT8 *)context->SecDir - (UINT8 *)data) > (datasize - sizeof(EFI_IMAGE_DATA_DIRECTORY))) { + Print(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + if (context->SecDir->VirtualAddress >= datasize) { Print(L"Malformed security header\n"); return EFI_INVALID_PARAMETER; @@ -831,7 +851,7 @@ static EFI_STATUS generate_path(EFI_LOADED_IMAGE *li, CHAR16 *ImagePath, bootpath[i+1] = '\0'; - if (bootpath[i-i] == '\\') + if (i == 0 || bootpath[i-i] == '\\') bootpath[i] = '\0'; *PathName = AllocatePool(StrSize(bootpath) + StrSize(ImagePath)); @@ -904,6 +924,7 @@ static EFI_STATUS load_image (EFI_LOADED_IMAGE *li, void **data, &buffersize, fileinfo); if (efi_status == EFI_BUFFER_TOO_SMALL) { + FreePool(fileinfo); fileinfo = AllocatePool(buffersize); if (!fileinfo) { Print(L"Unable to allocate file info buffer\n"); @@ -946,6 +967,8 @@ static EFI_STATUS load_image (EFI_LOADED_IMAGE *li, void **data, *datasize = buffersize; + FreePool(fileinfo); + return EFI_SUCCESS; error: if (*data) { @@ -983,7 +1006,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) EFI_STATUS efi_status; EFI_LOADED_IMAGE *li, li_bak; EFI_DEVICE_PATH *path; - CHAR16 *PathName; + CHAR16 *PathName = NULL; void *data = NULL; int datasize; @@ -1019,10 +1042,16 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) goto done; } - efi_status = uefi_call_wrapper(entry_point, 3, image_handle, systab); + efi_status = uefi_call_wrapper(entry_point, 2, image_handle, systab); CopyMem(li, &li_bak, sizeof(li_bak)); done: + if (PathName) + FreePool(PathName); + + if (data) + FreePool(data); + return efi_status; } From 832e5161b5bf9bba3e46ee203d5a131fc8b087c8 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 00:10:29 -0400 Subject: [PATCH 17/26] Boot unsigned binaries if we're not in secure mode read_header would fail if the binary was unsigned, even if we weren't then going to verify the signature. Move that check to the verify function instead. --- shim.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/shim.c b/shim.c index 447cf87..2ba7e5a 100644 --- a/shim.c +++ b/shim.c @@ -625,6 +625,11 @@ static EFI_STATUS verify_buffer (char *data, int datasize, WIN_CERTIFICATE_EFI_PKCS *cert; unsigned int size = datasize; + if (context->SecDir->Size == 0) { + Print(L"Empty security header\n"); + return EFI_INVALID_PARAMETER; + } + cert = ImageAddress (data, size, context->SecDir->VirtualAddress); if (!cert) { @@ -737,11 +742,6 @@ static EFI_STATUS read_header(void *data, unsigned int datasize, return EFI_INVALID_PARAMETER; } - if (context->SecDir->Size == 0) { - Print(L"Empty security header\n"); - return EFI_INVALID_PARAMETER; - } - return EFI_SUCCESS; } From 8a1690683f2d5691e3a58bef2a24f347d762f91d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 01:05:45 -0400 Subject: [PATCH 18/26] Improve signature validation enable/disable The logic used in checking the signature validation password was a bit ugly. Improve that so it behaves rather more as expected. --- MokManager.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/MokManager.c b/MokManager.c index f9acb0c..2fbda84 100644 --- a/MokManager.c +++ b/MokManager.c @@ -696,13 +696,12 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { EFI_STATUS efi_status; UINTN MokSBSize = (UINTN)data2; MokSBvar *var = MokSB; - CHAR16 password[1]; - UINT8 correct = 0, fail_count = 0; - UINT8 hash[SHA256_DIGEST_SIZE]; + CHAR16 pass1, pass2, pass3; + UINT8 fail_count = 0; UINT32 length; CHAR16 line[1]; UINT8 sbval = 1; - UINT8 pos; + UINT8 pos1, pos2, pos3; LibDeleteVariable(L"MokSB", &shim_lock_guid); @@ -713,23 +712,37 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); - while (correct < 3) { - RandomBytes (&pos, sizeof(pos)); + while (fail_count < 3) { + RandomBytes (&pos1, sizeof(pos1)); + pos1 = (pos1 % var->PWLen); - pos = pos % var->PWLen; + do { + RandomBytes (&pos2, sizeof(pos2)); + pos2 = (pos2 % var->PWLen); + } while (pos2 == pos1); - Print(L"Enter password character %d: ", pos + 1); - get_line(&length, password, 1, 0); + do { + RandomBytes (&pos3, sizeof(pos3)); + pos3 = (pos3 % var->PWLen) ; + } while (pos3 == pos2 || pos3 == pos1); - if (password[0] != var->Password[pos]) { + Print(L"Enter password character %d: ", pos1 + 1); + get_line(&length, &pass1, 1, 0); + + Print(L"Enter password character %d: ", pos2 + 1); + get_line(&length, &pass2, 1, 0); + + Print(L"Enter password character %d: ", pos3 + 1); + get_line(&length, &pass3, 1, 0); + + if (pass1 != var->Password[pos1] || + pass2 != var->Password[pos2] || + pass3 != var->Password[pos3]) { Print(L"Invalid character\n"); fail_count++; } else { - correct++; - } - - if (fail_count >= 3) break; + } } if (fail_count >= 3) { From d77f421bccf00722192a0cc90dddae05b1b74f91 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 01:14:50 -0400 Subject: [PATCH 19/26] Clean up password setting Permit clearing of the password, and avoid a case where choosing not to set a password would result in an error message on exit. Fix the same problem with MokSB. --- MokManager.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/MokManager.c b/MokManager.c index 2fbda84..eb5bb91 100644 --- a/MokManager.c +++ b/MokManager.c @@ -703,8 +703,6 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { UINT8 sbval = 1; UINT8 pos1, pos2, pos3; - LibDeleteVariable(L"MokSB", &shim_lock_guid); - if (MokSBSize != sizeof(MokSBvar)) { Print(L"Invalid MokSB variable contents\n"); return -1; @@ -776,6 +774,8 @@ static INTN mok_sb_prompt (void *MokSB, void *data2, void *data3) { &shim_lock_guid); } + LibDeleteVariable(L"MokSB", &shim_lock_guid); + Print(L"Press a key to reboot system\n"); Pause(); uefi_call_wrapper(RT->ResetSystem, 4, EfiResetWarm, @@ -804,10 +804,25 @@ static INTN mok_pw_prompt (void *MokPW, void *data2, void *data3) { return -1; } - LibDeleteVariable(L"MokPW", &shim_lock_guid); - uefi_call_wrapper(ST->ConOut->ClearScreen, 1, ST->ConOut); + SetMem(hash, SHA256_DIGEST_SIZE, 0); + + if (CompareMem(MokPW, hash, SHA256_DIGEST_SIZE) == 0) { + Print(L"Clear MOK password? (y/n): "); + + do { + get_line (&length, line, 1, 1); + + if (line[0] == 'Y' || line[0] == 'y') { + LibDeleteVariable(L"MokPWStore", &shim_lock_guid); + LibDeleteVariable(L"MokPW", &shim_lock_guid); + } + } while (line[0] != 'N' && line[0] != 'n'); + + return 0; + } + while (fail_count < 3) { Print(L"Confirm MOK passphrase: "); get_line(&length, password, PASSWORD_MAX, 0); @@ -857,6 +872,8 @@ static INTN mok_pw_prompt (void *MokPW, void *data2, void *data3) { return -1; } + LibDeleteVariable(L"MokPW", &shim_lock_guid); + Print(L"Press a key to reboot system\n"); Pause(); uefi_call_wrapper(RT->ResetSystem, 4, EfiResetWarm, From ba00aadb4534b0d0012c5b6154882504e6b591db Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 30 Oct 2012 10:35:36 -0400 Subject: [PATCH 20/26] Initialize the size of vendor dbx as 0 The size of vendor dbx must be 0 if there is no vendor dbx provided or the functions of db check will crash. --- dbx.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbx.S b/dbx.S index d19123c..a26fc38 100644 --- a/dbx.S +++ b/dbx.S @@ -28,5 +28,5 @@ vendor_dbx: .type vendor_dbx_size, @object .size vendor_dbx_size, 4 vendor_dbx_size: - .long 1 + .long 0 #endif From 8b7685b212c2bbb2cd487a1171b07669dbe52a20 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 30 Oct 2012 10:35:36 -0400 Subject: [PATCH 21/26] Check the vendor blacklist correctly --- shim.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shim.c b/shim.c index 0cd89b4..81e4231 100644 --- a/shim.c +++ b/shim.c @@ -341,14 +341,14 @@ static EFI_STATUS check_blacklist (WIN_CERTIFICATE_EFI_PKCS *cert, if (check_db_hash_in_ram(vendor_dbx, vendor_dbx_size, sha256hash, SHA256_DIGEST_SIZE, EfiHashSha256Guid) == - DATA_NOT_FOUND) + DATA_FOUND) return EFI_ACCESS_DENIED; if (check_db_hash_in_ram(vendor_dbx, vendor_dbx_size, sha1hash, SHA1_DIGEST_SIZE, EfiHashSha1Guid) == - DATA_NOT_FOUND) + DATA_FOUND) return EFI_ACCESS_DENIED; if (check_db_cert_in_ram(vendor_dbx, vendor_dbx_size, cert, - sha256hash) == DATA_NOT_FOUND) + sha256hash) == DATA_FOUND) return EFI_ACCESS_DENIED; if (check_db_hash(L"dbx", secure_var, sha256hash, SHA256_DIGEST_SIZE, From 5a8d573fb1349a6a1e7708b97934694d5ff735c4 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 30 Oct 2012 16:14:02 -0400 Subject: [PATCH 22/26] Add documentation of the Mok variables Brief overview of the function and format of the various variables used by Shim and MokManager. --- MokVars.txt | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 MokVars.txt diff --git a/MokVars.txt b/MokVars.txt new file mode 100644 index 0000000..74f0908 --- /dev/null +++ b/MokVars.txt @@ -0,0 +1,51 @@ +Variables used by Shim and Mokmanager + +Request variables: + +MokPW: Set by MokUtil when setting a password. A SHA-256 hash of the +UCS-2 representation of the password. The user will be asked to +re-enter the password to confirm. If the hash of the entered password +matches the contents of MokPW, the user will be prompted to copy MokPW +into MokPWState. BS,RT,NV + +MokSB: Set by MokUtil when requesting a change in state of signature +validation. A packed structure as follows: + +typedef struct { + UINT32 MokSBState; + UINT32 PWLen; + CHAR16 Password[PASSWORD_MAX]; +} __attribute__ ((packed)) MokSBvar; + +If MokSBState is 0, the user will be prompted to disable signature +validation. Otherwise, the user will be prompted to enable it. PWLen +is the length of the password, in characters. Password is a UCS-2 +representation of the password. The user will be prompted to enter +three randomly chosen characters from the password. If successful, +they will then be prompted to change the signature validation +according to MokSBState. BS,RT,NV + +MokNew: Set by MokUtil when requesting the addition or removal of keys +from MokList. Is an EFI_SIGNATURE_LIST as described in the UEFI +specification. BS,RT,NV + +MokAuth: A hash dependent upon the contents of MokNew and the sealing +password. The user's password in UCS-2 form should be appended to the +contents of MokNew and a SHA-256 hash generated and stored in MokAuth. +The hash will be regenerated by MokManager after the user is requested +to enter their password to confirm enrolment of the keys. If the hash +matches MokAuth, the user will be prompted to enrol the keys. BS,RT,NV + +State variables: + +MokList: A list of whitelisted keys and hashes. An EFI_SIGNATURE_LIST +as described in the UEFI specification. BS,NV + +MokListRT: A copy of MokList made available to the kernel at runtime. RT + +MokSBState: An 8-bit unsigned integer. If 1, shim will switch to +insecure mode. BS,NV + +MokPWStore: A SHA-256 representation of the password set by the user +via MokPW. The user will be prompted to enter this password in order +to interact with MokManager. From ed711b02ec18fecbf8b627b563e8cdfe1253170a Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 09:46:51 -0400 Subject: [PATCH 23/26] Fix up some types Type-checking the UEFI calls picked up a couple of problems. Fix them up. --- MokManager.c | 6 +++--- shim.c | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/MokManager.c b/MokManager.c index eb5bb91..5802d27 100644 --- a/MokManager.c +++ b/MokManager.c @@ -1362,7 +1362,7 @@ static INTN find_fs (void *data, void *data2, void *data3) { EFI_GUID fs_guid = SIMPLE_FILE_SYSTEM_PROTOCOL; UINTN count, i; UINTN OldSize, NewSize; - EFI_HANDLE **filesystem_handles; + EFI_HANDLE *filesystem_handles = NULL; struct menu_item *filesystems; BOOLEAN hash = !!data3; @@ -1383,7 +1383,7 @@ static INTN find_fs (void *data, void *data2, void *data3) { filesystems[0].colour = EFI_YELLOW; for (i=1; iHandleProtocol, 3, fs, &fs_guid, - &fs_interface); + (void **)&fs_interface); if (status != EFI_SUCCESS || !fs_interface) continue; diff --git a/shim.c b/shim.c index 2f7f8c2..8fbc070 100644 --- a/shim.c +++ b/shim.c @@ -890,7 +890,8 @@ static EFI_STATUS load_image (EFI_LOADED_IMAGE *li, void **data, device = li->DeviceHandle; efi_status = uefi_call_wrapper(BS->HandleProtocol, 3, device, - &simple_file_system_protocol, &drive); + &simple_file_system_protocol, + (void **)&drive); if (efi_status != EFI_SUCCESS) { Print(L"Failed to find fs\n"); @@ -1011,7 +1012,7 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath) int datasize; efi_status = uefi_call_wrapper(BS->HandleProtocol, 3, image_handle, - &loaded_image_protocol, &li); + &loaded_image_protocol, (void **)&li); if (efi_status != EFI_SUCCESS) { Print(L"Unable to init protocol\n"); From 6f1616265333a7f9b611f7c7ea7f6e0a3054806d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:12:20 -0400 Subject: [PATCH 24/26] Fix double free load_image() didn't allocate PathName, don't have it free it. --- shim.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shim.c b/shim.c index 8fbc070..8c03915 100644 --- a/shim.c +++ b/shim.c @@ -976,8 +976,7 @@ error: FreePool(*data); *data = NULL; } - if (PathName) - FreePool(PathName); + if (fileinfo) FreePool(fileinfo); return efi_status; From 058c0368adf2de0369be090a0996785e55ce1477 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:31:14 -0400 Subject: [PATCH 25/26] Fix signature checking We could potentially find a valid signature and then fail to validate it due to not breaking out of the outer while loop. --- shim.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shim.c b/shim.c index 8c03915..816688e 100644 --- a/shim.c +++ b/shim.c @@ -237,6 +237,9 @@ static CHECK_STATUS check_db_cert_in_ram(EFI_SIGNATURE_LIST *CertList, Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize); } + if (IsFound) + break; + dbsize -= CertList->SignatureListSize; CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize); } From 201574d1be44ac8741294884ba26a126ae238013 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:39:31 -0400 Subject: [PATCH 26/26] Fix AuthenticodeVerify loop Cert needs to be modified inside the Index loop, not outside it. This is unlikely to ever trigger since there will typically only be one X509 certificate per EFI_SIGNATURE_LIST, but fix it anyway. --- shim.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shim.c b/shim.c index 816688e..c038d8e 100644 --- a/shim.c +++ b/shim.c @@ -232,9 +232,10 @@ static CHECK_STATUS check_db_cert_in_ram(EFI_SIGNATURE_LIST *CertList, hash, SHA256_DIGEST_SIZE); if (IsFound) break; + + Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize); } - Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) Cert + CertList->SignatureSize); } if (IsFound)