From 17857eb8b55fa9864bfd71083d9291c74b0bab8e Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 26 Sep 2013 11:57:59 -0400 Subject: Port MokManager to Linux Foundation loader UI code This is the first stage of porting the MokManager UI to the UI code used by the Linux Foundation UEFI loader. --- lib/variables.c | 340 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 340 insertions(+) create mode 100644 lib/variables.c (limited to 'lib/variables.c') diff --git a/lib/variables.c b/lib/variables.c new file mode 100644 index 00000000..9db64809 --- /dev/null +++ b/lib/variables.c @@ -0,0 +1,340 @@ +/* + * Copyright 2012 + * + * see COPYING file + * + * Portions of this file are a direct cut and paste from Tianocore + * (http://tianocore.sf.net) + * + * SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c + * + * Copyright (c) 2011 - 2012, Intel Corporation. All rights reserved.
+ * This program and the accompanying materials + * are licensed and made available under the terms and conditions of the BSD License + * which accompanies this distribution. The full text of the license may be found + * at + * http://opensource.org/licenses/bsd-license.php + * + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + * + */ +#include +#include + +#include + +#include +#include +#include +#include +#include + +EFI_STATUS +variable_create_esl(void *cert, int cert_len, EFI_GUID *type, EFI_GUID *owner, + void **out, int *outlen) +{ + *outlen = cert_len + sizeof(EFI_SIGNATURE_LIST) + sizeof(EFI_GUID); + + *out = AllocateZeroPool(*outlen); + if (!*out) + return EFI_OUT_OF_RESOURCES; + + EFI_SIGNATURE_LIST *sl = *out; + + sl->SignatureHeaderSize = 0; + sl->SignatureType = *type; + sl->SignatureSize = cert_len + sizeof(EFI_GUID); + sl->SignatureListSize = *outlen; + + EFI_SIGNATURE_DATA *sd = *out + sizeof(EFI_SIGNATURE_LIST); + + if (owner) + sd->SignatureOwner = *owner; + + CopyMem(sd->SignatureData, cert, cert_len); + + return EFI_SUCCESS; +} + + +EFI_STATUS +CreateTimeBasedPayload ( + IN OUT UINTN *DataSize, + IN OUT UINT8 **Data + ) +{ + EFI_STATUS Status; + UINT8 *NewData; + UINT8 *Payload; + UINTN PayloadSize; + EFI_VARIABLE_AUTHENTICATION_2 *DescriptorData; + UINTN DescriptorSize; + EFI_TIME Time; + EFI_GUID efi_cert_type = EFI_CERT_TYPE_PKCS7_GUID; + + if (Data == NULL || DataSize == NULL) { + return EFI_INVALID_PARAMETER; + } + + // + // In Setup mode or Custom mode, the variable does not need to be signed but the + // parameters to the SetVariable() call still need to be prepared as authenticated + // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor without certificate + // data in it. + // + Payload = *Data; + PayloadSize = *DataSize; + + DescriptorSize = OFFSET_OF(EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData); + NewData = (UINT8*) AllocateZeroPool (DescriptorSize + PayloadSize); + if (NewData == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + if ((Payload != NULL) && (PayloadSize != 0)) { + CopyMem (NewData + DescriptorSize, Payload, PayloadSize); + } + + DescriptorData = (EFI_VARIABLE_AUTHENTICATION_2 *) (NewData); + + ZeroMem (&Time, sizeof (EFI_TIME)); + Status = uefi_call_wrapper(RT->GetTime,2, &Time, NULL); + if (EFI_ERROR (Status)) { + FreePool(NewData); + return Status; + } + Time.Pad1 = 0; + Time.Nanosecond = 0; + Time.TimeZone = 0; + Time.Daylight = 0; + Time.Pad2 = 0; + CopyMem (&DescriptorData->TimeStamp, &Time, sizeof (EFI_TIME)); + + DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData); + DescriptorData->AuthInfo.Hdr.wRevision = 0x0200; + DescriptorData->AuthInfo.Hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; + DescriptorData->AuthInfo.CertType = efi_cert_type; + + /* we're expecting an EFI signature list, so don't free the input since + * it might not be in a pool */ +#if 0 + if (Payload != NULL) { + FreePool(Payload); + } +#endif + + *DataSize = DescriptorSize + PayloadSize; + *Data = NewData; + return EFI_SUCCESS; +} + +EFI_STATUS +SetSecureVariable(CHAR16 *var, UINT8 *Data, UINTN len, EFI_GUID owner, + UINT32 options, int createtimebased) +{ + EFI_SIGNATURE_LIST *Cert; + UINTN DataSize; + EFI_STATUS efi_status; + + /* Microsoft request: Bugs in some UEFI platforms mean that PK or any + * other secure variable can be updated or deleted programmatically, + * so prevent */ + if (!variable_is_setupmode()) + return EFI_SECURITY_VIOLATION; + + if (createtimebased) { + int ds; + efi_status = variable_create_esl(Data, len, &X509_GUID, NULL, + (void **)&Cert, &ds); + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to create %s certificate %d\n", var, efi_status); + return efi_status; + } + + DataSize = ds; + } else { + /* we expect an efi signature list rather than creating it */ + Cert = (EFI_SIGNATURE_LIST *)Data; + DataSize = len; + } + efi_status = CreateTimeBasedPayload(&DataSize, (UINT8 **)&Cert); + if (efi_status != EFI_SUCCESS) { + Print(L"Failed to create time based payload %d\n", efi_status); + return efi_status; + } + + efi_status = uefi_call_wrapper(RT->SetVariable, 5, var, &owner, + EFI_VARIABLE_NON_VOLATILE + | EFI_VARIABLE_RUNTIME_ACCESS + | EFI_VARIABLE_BOOTSERVICE_ACCESS + | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS + | options, + DataSize, Cert); + + return efi_status; +} + +UINT64 +GetOSIndications(void) +{ + UINT64 indications; + UINTN DataSize = sizeof(indications); + EFI_STATUS efi_status; + + efi_status = uefi_call_wrapper(RT->GetVariable, 5, L"OsIndicationsSupported", &GV_GUID, NULL, &DataSize, &indications); + if (efi_status != EFI_SUCCESS) + return 0; + + return indications; +} + +EFI_STATUS +SETOSIndicationsAndReboot(UINT64 indications) +{ + UINTN DataSize = sizeof(indications); + EFI_STATUS efi_status; + + efi_status = uefi_call_wrapper(RT->SetVariable, 5, L"OsIndications", + &GV_GUID, + EFI_VARIABLE_NON_VOLATILE + | EFI_VARIABLE_RUNTIME_ACCESS + | EFI_VARIABLE_BOOTSERVICE_ACCESS, + DataSize, &indications); + + if (efi_status != EFI_SUCCESS) + return efi_status; + + uefi_call_wrapper(RT->ResetSystem, 4, EfiResetWarm, EFI_SUCCESS, 0, NULL); + /* does not return */ + + return EFI_SUCCESS; +} + +EFI_STATUS +get_variable_attr(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner, + UINT32 *attributes) +{ + EFI_STATUS efi_status; + + *len = 0; + + efi_status = uefi_call_wrapper(RT->GetVariable, 5, var, &owner, + NULL, len, NULL); + if (efi_status != EFI_BUFFER_TOO_SMALL) + return efi_status; + + *data = AllocateZeroPool(*len); + if (!data) + return EFI_OUT_OF_RESOURCES; + + efi_status = uefi_call_wrapper(RT->GetVariable, 5, var, &owner, + attributes, len, *data); + + if (efi_status != EFI_SUCCESS) { + FreePool(*data); + *data = NULL; + } + return efi_status; +} + +EFI_STATUS +get_variable(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner) +{ + return get_variable_attr(var, data, len, owner, NULL); +} + +EFI_STATUS +find_in_esl(UINT8 *Data, UINTN DataSize, UINT8 *key, UINTN keylen) +{ + EFI_SIGNATURE_LIST *CertList; + + certlist_for_each_certentry(CertList, Data, DataSize, DataSize) { + if (CertList->SignatureSize != keylen + sizeof(EFI_GUID)) + continue; + EFI_SIGNATURE_DATA *Cert; + + certentry_for_each_cert(Cert, CertList) + if (CompareMem (Cert->SignatureData, key, keylen) == 0) + return EFI_SUCCESS; + } + return EFI_NOT_FOUND; +} + +EFI_STATUS +find_in_variable_esl(CHAR16* var, EFI_GUID owner, UINT8 *key, UINTN keylen) +{ + UINTN DataSize; + UINT8 *Data; + EFI_STATUS status; + + status = get_variable(var, &Data, &DataSize, owner); + if (status != EFI_SUCCESS) + return status; + + status = find_in_esl(Data, DataSize, key, keylen); + + FreePool(Data); + + return status; +} + +int +variable_is_setupmode(void) +{ + /* set to 1 because we return true if SetupMode doesn't exist */ + UINT8 SetupMode = 1; + UINTN DataSize = sizeof(SetupMode); + + uefi_call_wrapper(RT->GetVariable, 5, L"SetupMode", &GV_GUID, NULL, + &DataSize, &SetupMode); + + return SetupMode; +} + +int +variable_is_secureboot(void) +{ + /* return false if variable doesn't exist */ + UINT8 SecureBoot = 0; + UINTN DataSize; + + DataSize = sizeof(SecureBoot); + uefi_call_wrapper(RT->GetVariable, 5, L"SecureBoot", &GV_GUID, NULL, + &DataSize, &SecureBoot); + + return SecureBoot; +} + +EFI_STATUS +variable_enroll_hash(CHAR16 *var, EFI_GUID owner, + UINT8 hash[SHA256_DIGEST_SIZE]) +{ + EFI_STATUS status; + + if (find_in_variable_esl(var, owner, hash, SHA256_DIGEST_SIZE) + == EFI_SUCCESS) + /* hash already present */ + return EFI_ALREADY_STARTED; + + UINT8 sig[sizeof(EFI_SIGNATURE_LIST) + sizeof(EFI_SIGNATURE_DATA) - 1 + SHA256_DIGEST_SIZE]; + EFI_SIGNATURE_LIST *l = (void *)sig; + EFI_SIGNATURE_DATA *d = (void *)sig + sizeof(EFI_SIGNATURE_LIST); + SetMem(sig, 0, sizeof(sig)); + l->SignatureType = EFI_CERT_SHA256_GUID; + l->SignatureListSize = sizeof(sig); + l->SignatureSize = 16 +32; /* UEFI defined */ + CopyMem(&d->SignatureData, hash, SHA256_DIGEST_SIZE); + d->SignatureOwner = MOK_OWNER; + + if (CompareGuid(&owner, &SIG_DB) == 0) + status = SetSecureVariable(var, sig, sizeof(sig), owner, + EFI_VARIABLE_APPEND_WRITE, 0); + else + status = uefi_call_wrapper(RT->SetVariable, 5, var, &owner, + EFI_VARIABLE_NON_VOLATILE + | EFI_VARIABLE_BOOTSERVICE_ACCESS + | EFI_VARIABLE_APPEND_WRITE, + sizeof(sig), sig); + return status; +} -- cgit v1.2.3 From 7f0208a0f93ac83635e1d5971387e5fbfdaaf734 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Thu, 26 Sep 2013 11:58:02 -0400 Subject: Merge variable retrieving functions --- MokManager.c | 34 +++-------------------- include/variables.h | 2 +- lib/Makefile | 2 +- lib/variables.c | 1 - shim.c | 78 +++++++++++++++++------------------------------------ 5 files changed, 31 insertions(+), 86 deletions(-) (limited to 'lib/variables.c') diff --git a/MokManager.c b/MokManager.c index b01c65ec..805017b6 100644 --- a/MokManager.c +++ b/MokManager.c @@ -9,6 +9,7 @@ #include "guid.h" #include "console.h" +#include "variables.h" #include "simple_file.h" #include "efiauthenticated.h" @@ -50,32 +51,6 @@ typedef struct { CHAR16 Password[SB_PASSWORD_LEN]; } __attribute__ ((packed)) MokSBvar; -static EFI_STATUS get_variable (CHAR16 *name, EFI_GUID guid, UINT32 *attributes, - UINTN *size, void **buffer) -{ - EFI_STATUS efi_status; - char allocate = !(*size); - - efi_status = uefi_call_wrapper(RT->GetVariable, 5, name, &guid, - attributes, size, buffer); - - if (efi_status != EFI_BUFFER_TOO_SMALL || !allocate) { - return efi_status; - } - - *buffer = AllocatePool(*size); - - if (!*buffer) { - console_notify(L"Unable to allocate variable buffer"); - return EFI_OUT_OF_RESOURCES; - } - - efi_status = uefi_call_wrapper(RT->GetVariable, 5, name, &guid, - attributes, size, *buffer); - - return efi_status; -} - static EFI_STATUS get_sha1sum (void *Data, int DataSize, UINT8 *hash) { EFI_STATUS status; @@ -904,7 +879,7 @@ static EFI_STATUS delete_keys (void *MokDel, UINTN MokDelSize) UINT8 auth[PASSWORD_CRYPT_SIZE]; UINTN auth_size = PASSWORD_CRYPT_SIZE; UINT32 attributes; - void *MokListData = NULL; + UINT8 *MokListData = NULL; UINTN MokListDataSize = 0; MokListNode *mok, *del_key; INTN mok_num, del_num; @@ -929,9 +904,8 @@ static EFI_STATUS delete_keys (void *MokDel, UINTN MokDelSize) if (efi_status != EFI_SUCCESS) return EFI_ACCESS_DENIED; - efi_status = get_variable(L"MokList", shim_lock_guid, &attributes, - &MokListDataSize, &MokListData); - + efi_status = get_variable_attr (L"MokList", &MokListData, &MokListDataSize, + shim_lock_guid, &attributes); if (attributes & EFI_VARIABLE_RUNTIME_ACCESS) { console_alertbox((CHAR16 *[]){L"MokList is compromised!", L"Erase all keys in MokList!", diff --git a/include/variables.h b/include/variables.h index c171bd53..b207dbf3 100644 --- a/include/variables.h +++ b/include/variables.h @@ -1,6 +1,6 @@ #include -#include /* for SHA256_DIGEST_SIZE */ +#include /* for SHA256_DIGEST_SIZE */ #define certlist_for_each_certentry(cl, cl_init, s, s_init) \ for (cl = (EFI_SIGNATURE_LIST *)(cl_init), s = (s_init); \ diff --git a/lib/Makefile b/lib/Makefile index 43907005..e85c1fd5 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -1,6 +1,6 @@ TARGET = lib.a -LIBFILES = simple_file.o guid.o console.o execute.o configtable.o shell.o +LIBFILES = simple_file.o guid.o console.o execute.o configtable.o shell.o variables.o ARCH = $(shell uname -m | sed s,i[3456789]86,ia32,) diff --git a/lib/variables.c b/lib/variables.c index 9db64809..81bd34db 100644 --- a/lib/variables.c +++ b/lib/variables.c @@ -27,7 +27,6 @@ #include #include #include -#include #include EFI_STATUS diff --git a/shim.c b/shim.c index 9ffc94a6..c2d54c44 100644 --- a/shim.c +++ b/shim.c @@ -43,6 +43,7 @@ #include "ucs2.h" #include "guid.h" +#include "variables.h" #include "efiauthenticated.h" #define FALLBACK L"\\fallback.efi" @@ -81,32 +82,6 @@ typedef struct { UINT8 *Mok; } MokListNode; -static EFI_STATUS get_variable (CHAR16 *name, EFI_GUID guid, UINT32 *attributes, - UINTN *size, void **buffer) -{ - EFI_STATUS efi_status; - char allocate = !(*size); - - efi_status = uefi_call_wrapper(RT->GetVariable, 5, name, &guid, - attributes, size, buffer); - - if (efi_status != EFI_BUFFER_TOO_SMALL || !allocate) { - return efi_status; - } - - *buffer = AllocatePool(*size); - - if (!*buffer) { - Print(L"Unable to allocate variable buffer\n"); - return EFI_OUT_OF_RESOURCES; - } - - efi_status = uefi_call_wrapper(RT->GetVariable, 5, name, &guid, - attributes, size, *buffer); - - return efi_status; -} - /* * Perform basic bounds checking of the intra-image pointers */ @@ -270,15 +245,14 @@ static CHECK_STATUS check_db_cert(CHAR16 *dbname, EFI_GUID guid, EFI_STATUS efi_status; EFI_SIGNATURE_LIST *CertList; UINTN dbsize = 0; - UINT32 attributes; - void *db; + UINT8 *db; - efi_status = get_variable(dbname, guid, &attributes, &dbsize, &db); + efi_status = get_variable(dbname, &db, &dbsize, guid); if (efi_status != EFI_SUCCESS) return VAR_NOT_FOUND; - CertList = db; + CertList = (EFI_SIGNATURE_LIST *)db; rc = check_db_cert_in_ram(CertList, dbsize, data, hash); @@ -336,17 +310,16 @@ static CHECK_STATUS check_db_hash(CHAR16 *dbname, EFI_GUID guid, UINT8 *data, { EFI_STATUS efi_status; EFI_SIGNATURE_LIST *CertList; - UINT32 attributes; UINTN dbsize = 0; - void *db; + UINT8 *db; - efi_status = get_variable(dbname, guid, &attributes, &dbsize, &db); + efi_status = get_variable(dbname, &db, &dbsize, guid); if (efi_status != EFI_SUCCESS) { return VAR_NOT_FOUND; } - CertList = db; + CertList = (EFI_SIGNATURE_LIST *)db; CHECK_STATUS rc = check_db_hash_in_ram(CertList, dbsize, data, SignatureSize, CertType); @@ -423,15 +396,16 @@ static BOOLEAN secure_mode (void) { EFI_STATUS status; EFI_GUID global_var = EFI_GLOBAL_VARIABLE; - UINTN charsize = sizeof(char); + UINTN len; + UINT8 *Data; UINT8 sb, setupmode; - UINT32 attributes; if (insecure_mode) return FALSE; - status = get_variable(L"SecureBoot", global_var, &attributes, &charsize, - (void *)&sb); + status = get_variable(L"SecureBoot", &Data, &len, global_var); + sb = *Data; + FreePool(Data); /* FIXME - more paranoia here? */ if (status != EFI_SUCCESS || sb != 1) { @@ -440,8 +414,9 @@ static BOOLEAN secure_mode (void) return FALSE; } - status = get_variable(L"SetupMode", global_var, &attributes, &charsize, - (void *)&setupmode); + status = get_variable(L"SetupMode", &Data, &len, global_var); + setupmode = *Data; + FreePool(Data); if (status == EFI_SUCCESS && setupmode == 1) { if (verbose) @@ -629,12 +604,12 @@ done: static EFI_STATUS verify_mok (void) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; EFI_STATUS status = EFI_SUCCESS; - void *MokListData = NULL; + UINT8 *MokListData = NULL; UINTN MokListDataSize = 0; UINT32 attributes; - status = get_variable(L"MokList", shim_lock_guid, &attributes, - &MokListDataSize, &MokListData); + status = get_variable_attr(L"MokList", &MokListData, &MokListDataSize, + shim_lock_guid, &attributes); if (attributes & EFI_VARIABLE_RUNTIME_ACCESS) { Print(L"MokList is compromised!\nErase all keys in MokList!\n"); @@ -1325,12 +1300,10 @@ EFI_STATUS mirror_mok_list() { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; EFI_STATUS efi_status; - UINT32 attributes; - void *Data = NULL; + UINT8 *Data = NULL; UINTN DataSize = 0; - efi_status = get_variable(L"MokList", shim_lock_guid, &attributes, - &DataSize, &Data); + efi_status = get_variable(L"MokList", &Data, &DataSize, shim_lock_guid); if (efi_status != EFI_SUCCESS) { goto done; @@ -1400,12 +1373,12 @@ static EFI_STATUS check_mok_sb (void) { EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; EFI_STATUS status = EFI_SUCCESS; - void *MokSBState = NULL; + UINT8 *MokSBState = NULL; UINTN MokSBStateSize = 0; UINT32 attributes; - status = get_variable(L"MokSBState", shim_lock_guid, &attributes, - &MokSBStateSize, &MokSBState); + status = get_variable_attr(L"MokSBState", &MokSBState, &MokSBStateSize, + shim_lock_guid, &attributes); if (status != EFI_SUCCESS) return EFI_ACCESS_DENIED; @@ -1517,7 +1490,6 @@ EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *passed_systab) EFI_STATUS efi_status; UINT8 verbose_check; UINTN verbose_check_size; - UINT32 attributes; EFI_GUID global_var = EFI_GLOBAL_VARIABLE; /* @@ -1536,8 +1508,8 @@ EFI_STATUS efi_main (EFI_HANDLE image_handle, EFI_SYSTEM_TABLE *passed_systab) InitializeLib(image_handle, systab); verbose_check_size = 1; - efi_status = get_variable(L"SHIM_VERBOSE", global_var, &attributes, - &verbose_check_size, (void *)&verbose_check); + efi_status = get_variable(L"SHIM_VERBOSE", (void *)&verbose_check, + &verbose_check_size, global_var); if (!EFI_ERROR(efi_status)) verbose = verbose_check; -- cgit v1.2.3 From 17621118315466dc878cf468d8c15ffadcb50482 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 21 Nov 2013 11:48:24 -0500 Subject: Error check the right thing in get_variable_attr() when allocating. Signed-off-by: Peter Jones --- lib/variables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/variables.c') diff --git a/lib/variables.c b/lib/variables.c index 81bd34db..3a9735e6 100644 --- a/lib/variables.c +++ b/lib/variables.c @@ -224,7 +224,7 @@ get_variable_attr(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner, return efi_status; *data = AllocateZeroPool(*len); - if (!data) + if (!*data) return EFI_OUT_OF_RESOURCES; efi_status = uefi_call_wrapper(RT->GetVariable, 5, var, &owner, -- cgit v1.2.3 From 7a72592b75879542e9ebd808868f83a78bdfbbc6 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin Date: Wed, 25 Jun 2014 10:55:12 -0400 Subject: Check the secure variables with the lib functions There are functions defined in lib to check the secure variables. Use the functions to shun the duplicate code. Signed-off-by: Gary Ching-Pang Lin Conflicts: shim.c --- lib/variables.c | 14 ++++++++++---- shim.c | 32 ++------------------------------ 2 files changed, 12 insertions(+), 34 deletions(-) (limited to 'lib/variables.c') diff --git a/lib/variables.c b/lib/variables.c index 3a9735e6..4c64d7e4 100644 --- a/lib/variables.c +++ b/lib/variables.c @@ -284,9 +284,12 @@ variable_is_setupmode(void) /* set to 1 because we return true if SetupMode doesn't exist */ UINT8 SetupMode = 1; UINTN DataSize = sizeof(SetupMode); + EFI_STATUS status; - uefi_call_wrapper(RT->GetVariable, 5, L"SetupMode", &GV_GUID, NULL, - &DataSize, &SetupMode); + status = uefi_call_wrapper(RT->GetVariable, 5, L"SetupMode", &GV_GUID, NULL, + &DataSize, &SetupMode); + if (EFI_ERROR(status)) + return 1; return SetupMode; } @@ -297,10 +300,13 @@ variable_is_secureboot(void) /* return false if variable doesn't exist */ UINT8 SecureBoot = 0; UINTN DataSize; + EFI_STATUS status; DataSize = sizeof(SecureBoot); - uefi_call_wrapper(RT->GetVariable, 5, L"SecureBoot", &GV_GUID, NULL, - &DataSize, &SecureBoot); + status = uefi_call_wrapper(RT->GetVariable, 5, L"SecureBoot", &GV_GUID, NULL, + &DataSize, &SecureBoot); + if (EFI_ERROR(status)) + return 0; return SecureBoot; } diff --git a/shim.c b/shim.c index 210e778a..14fb601c 100644 --- a/shim.c +++ b/shim.c @@ -475,44 +475,16 @@ static EFI_STATUS check_whitelist (WIN_CERTIFICATE_EFI_PKCS *cert, static BOOLEAN secure_mode (void) { - EFI_STATUS status; - EFI_GUID global_var = EFI_GLOBAL_VARIABLE; - UINTN len; - UINT8 *Data; - UINT8 sb, setupmode; - if (user_insecure_mode) return FALSE; - status = get_variable(L"SecureBoot", &Data, &len, global_var); - if (status != EFI_SUCCESS) { + if (variable_is_secureboot() != 1) { if (verbose && !in_protocol) console_notify(L"Secure boot not enabled"); return FALSE; } - sb = *Data; - FreePool(Data); - - if (sb != 1) { - if (verbose && !in_protocol) - console_notify(L"Secure boot not enabled"); - return FALSE; - } - - /* If we /do/ have "SecureBoot", but /don't/ have "SetupMode", - * then the implementation is bad, but we assume that secure boot is - * enabled according to the status of "SecureBoot". If we have both - * of them, then "SetupMode" may tell us additional data, and we need - * to consider it. - */ - status = get_variable(L"SetupMode", &Data, &len, global_var); - if (status != EFI_SUCCESS) - return TRUE; - - setupmode = *Data; - FreePool(Data); - if (setupmode == 1) { + if (variable_is_setupmode() == 1) { if (verbose && !in_protocol) console_notify(L"Platform is in setup mode"); return FALSE; -- cgit v1.2.3 From 9ea3d9b401ed73ae95b60e6b566f9293af3ac4d7 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 25 Jun 2014 10:55:56 -0400 Subject: Make sure we default to assuming we're locked down. If "SecureBoot" exists but "SetupMode" does not, assume "SetupMode" says we're not in Setup Mode. Signed-off-by: Peter Jones --- include/variables.h | 2 +- lib/variables.c | 8 ++++---- shim.c | 8 +++++++- 3 files changed, 12 insertions(+), 6 deletions(-) (limited to 'lib/variables.c') diff --git a/include/variables.h b/include/variables.h index b207dbf3..deed269c 100644 --- a/include/variables.h +++ b/include/variables.h @@ -50,7 +50,7 @@ SETOSIndicationsAndReboot(UINT64 indications); int variable_is_secureboot(void); int -variable_is_setupmode(void); +variable_is_setupmode(int default_return); EFI_STATUS variable_enroll_hash(CHAR16 *var, EFI_GUID owner, UINT8 hash[SHA256_DIGEST_SIZE]); diff --git a/lib/variables.c b/lib/variables.c index 4c64d7e4..59d7d054 100644 --- a/lib/variables.c +++ b/lib/variables.c @@ -139,7 +139,7 @@ SetSecureVariable(CHAR16 *var, UINT8 *Data, UINTN len, EFI_GUID owner, /* Microsoft request: Bugs in some UEFI platforms mean that PK or any * other secure variable can be updated or deleted programmatically, * so prevent */ - if (!variable_is_setupmode()) + if (!variable_is_setupmode(1)) return EFI_SECURITY_VIOLATION; if (createtimebased) { @@ -279,17 +279,17 @@ find_in_variable_esl(CHAR16* var, EFI_GUID owner, UINT8 *key, UINTN keylen) } int -variable_is_setupmode(void) +variable_is_setupmode(int default_return) { /* set to 1 because we return true if SetupMode doesn't exist */ - UINT8 SetupMode = 1; + UINT8 SetupMode = default_return; UINTN DataSize = sizeof(SetupMode); EFI_STATUS status; status = uefi_call_wrapper(RT->GetVariable, 5, L"SetupMode", &GV_GUID, NULL, &DataSize, &SetupMode); if (EFI_ERROR(status)) - return 1; + return default_return; return SetupMode; } diff --git a/shim.c b/shim.c index 14fb601c..fe73ec1a 100644 --- a/shim.c +++ b/shim.c @@ -484,7 +484,13 @@ static BOOLEAN secure_mode (void) return FALSE; } - if (variable_is_setupmode() == 1) { + /* If we /do/ have "SecureBoot", but /don't/ have "SetupMode", + * then the implementation is bad, but we assume that secure boot is + * enabled according to the status of "SecureBoot". If we have both + * of them, then "SetupMode" may tell us additional data, and we need + * to consider it. + */ + if (variable_is_setupmode(0) == 1) { if (verbose && !in_protocol) console_notify(L"Platform is in setup mode"); return FALSE; -- cgit v1.2.3