From 9272bc5b847bf30f89d9bef49fcc5b04c780460e Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:41:52 -0400 Subject: 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. --- shim.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 4eab87ac..bffad137 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) -- cgit v1.2.3 From 9eaadb0d115eaf452f105398b300194939164664 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index bffad137..39ad9bba 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(); -- cgit v1.2.3 From 801d1b936be96f0d22fd5b91af973cafc1fcb68c Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 18 Oct 2012 17:43:53 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/MokManager.c b/MokManager.c index 88a08f72..18992f27 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); + + MokPW = LibGetVariableAndSize(L"MokPW", &shim_lock_guid, &MokPWSize); - enter_mok_menu(image_handle, MokNew, MokNewSize, MokSB, MokSBSize); + 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 39ad9bba..dbe5e849 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) { -- cgit v1.2.3 From 5f0a358b6349aa9bae3da562c928c920065afd17 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 23 Oct 2012 11:47:41 -0400 Subject: 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 (limited to 'shim.c') diff --git a/Makefile b/Makefile index bea0ebdb..af49678a 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 00000000..d19123c4 --- /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 dbe5e849..eb3cf521 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; -- cgit v1.2.3 From d5a2d9ea083f329f71a9cc5c3aac45b60fa60b93 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 23 Oct 2012 13:01:25 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index dbe5e849..9ff4b8be 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; + + return FALSE; +} - mokpw_status = uefi_call_wrapper(RT->GetVariable, 5, L"MokPW", - &shim_lock_guid, &attributes, - &size, (void *)&MokVar); +EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +{ + EFI_STATUS efi_status; - 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) { + 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) { -- cgit v1.2.3 From cbe214072bfa49f177e33f7e98f6e0431b4832eb Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 00:09:08 -0400 Subject: Miscellaneous small fixups Fixes for some small bugs noticed during review --- shim.c | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 0cd89b43..447cf876 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; } -- cgit v1.2.3 From 832e5161b5bf9bba3e46ee203d5a131fc8b087c8 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Wed, 24 Oct 2012 00:10:29 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 447cf876..2ba7e5af 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; } -- cgit v1.2.3 From 8b7685b212c2bbb2cd487a1171b07669dbe52a20 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Tue, 30 Oct 2012 10:35:36 -0400 Subject: Check the vendor blacklist correctly --- shim.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 0cd89b43..81e42314 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, -- cgit v1.2.3 From ed711b02ec18fecbf8b627b563e8cdfe1253170a Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 09:46:51 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/MokManager.c b/MokManager.c index eb5bb919..5802d274 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 2f7f8c2e..8fbc0708 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"); -- cgit v1.2.3 From 6f1616265333a7f9b611f7c7ea7f6e0a3054806d Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:12:20 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 8fbc0708..8c039159 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; -- cgit v1.2.3 From 058c0368adf2de0369be090a0996785e55ce1477 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:31:14 -0400 Subject: 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(+) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 8c039159..816688e1 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); } -- cgit v1.2.3 From 201574d1be44ac8741294884ba26a126ae238013 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 1 Nov 2012 10:39:31 -0400 Subject: 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(-) (limited to 'shim.c') diff --git a/shim.c b/shim.c index 816688e1..c038d8e1 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) -- cgit v1.2.3