Skip to content

Commit

Permalink
Also use a config table to mirror mok variables.
Browse files Browse the repository at this point in the history 
Everything was going just fine until I made a vendor_db with 17kB of
sha256 sums in it.  And then the same source tree that had worked fine
without that threw errors and failed all over the place.  I wrote some
code to diagnose the problem, and of course it was a failure in
mirroring MokList to MokListRT.

As Patrick noted in 741c61a, some systems have obnoxiously low
amounts of variable storage available:

mok.c:550:import_mok_state() BS+RT variable info:
		     MaximumVariableStorageSize:0x000000000000DFE4
		     RemainingVariableStorageSize:0x000000000000D21C
		     MaximumVariableSize:0x0000000000001FC4

The most annoying part is that on at least this edk2 build,
SetVariable() /does actually appear to set the variable/, but it returns
EFI_INVALID_PARAMETER.  I'm not planning on relying on that behavior.

So... yeah, the largest *volatile* (i.e. RAM only) variable this edk2
build will let you create is less than two pages.  It's only got 7.9G
free, so I guess it's feeling like space is a little tight.

We're also not quite preserving that return code well enough for his
workaround to work.

New plan.  We try to create variables the normal way, but we don't
consider not having enough space to be fatal.  In that case, we create
an EFI_SECURITY_LIST with one sha256sum in it, with a value of all 0,
and try to add that so we're sure there's /something/ there that's
innocuous.  On systems where the first SetVariable() /
QueryVariableInfo() lied to us, the correct variable should be there,
otherwise the one with the zero-hash will be.

We then also build a config table to hold this info and install that.

The config table is a packed array of this struct:

struct mok_variable_config_entry {
       CHAR8 name[256];
       UINT64 data_size;
       UINT8 data[];
};

There will be N+1 entries, and the last entry is all 0 for name and
data_size.  The total allocation size will always be a multiple of 4096.
In the typical RHEL 7.9 case that means it'll be around 5 pages.

It's installed with this guid:

c451ed2b-9694-45d3-baba-ed9f8988a389

Anything that can go wrong will.

Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: not yet, I don't want people to read this before Wednesday.
Signed-off-by: Peter Jones <pjones@redhat.com>
  • Loading branch information
@vathpela
vathpela committed Jul 26, 2020
1 parent fc4368f commit fecc2df
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 14 deletions.
2 changes: 2 additions & 0 deletions include/guid.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ extern EFI_GUID SECURITY_PROTOCOL_GUID;
extern EFI_GUID SECURITY2_PROTOCOL_GUID;
extern EFI_GUID SHIM_LOCK_GUID;

extern EFI_GUID MOK_VARIABLE_STORE;

#endif /* SHIM_GUID_H */
2 changes: 2 additions & 0 deletions lib/guid.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ EFI_GUID EFI_SECURE_BOOT_DB_GUID = { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc,
EFI_GUID EFI_SIMPLE_FILE_SYSTEM_GUID = SIMPLE_FILE_SYSTEM_PROTOCOL;
EFI_GUID SECURITY_PROTOCOL_GUID = { 0xA46423E3, 0x4617, 0x49f1, {0xB9, 0xFF, 0xD1, 0xBF, 0xA9, 0x11, 0x58, 0x39 } };
EFI_GUID SECURITY2_PROTOCOL_GUID = { 0x94ab2f58, 0x1438, 0x4ef1, {0x91, 0x52, 0x18, 0x94, 0x1a, 0x3a, 0x0e, 0x68 } };

EFI_GUID SHIM_LOCK_GUID = {0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } };
EFI_GUID MOK_VARIABLE_STORE = {0xc451ed2b, 0x9694, 0x45d3, {0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89} };
150 changes: 136 additions & 14 deletions mok.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ struct mok_state_variable {
CHAR16 *name;
char *name8;
CHAR16 *rtname;
char *rtname8;
EFI_GUID *guid;

UINT8 *data;
Expand Down Expand Up @@ -121,6 +122,7 @@ struct mok_state_variable mok_state_variables[] = {
{.name = L"MokList",
.name8 = "MokList",
.rtname = L"MokListRT",
.rtname8 = "MokListRT",
.guid = &SHIM_LOCK_GUID,
.yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_NON_VOLATILE,
Expand All @@ -133,12 +135,14 @@ struct mok_state_variable mok_state_variables[] = {
.build_cert_size = &build_cert_size,
#endif /* defined(ENABLE_SHIM_CERT) */
.flags = MOK_MIRROR_KEYDB |
MOK_MIRROR_DELETE_FIRST |
MOK_VARIABLE_LOG,
.pcr = 14,
},
{.name = L"MokListX",
.name8 = "MokListX",
.rtname = L"MokListXRT",
.rtname8 = "MokListXRT",
.guid = &SHIM_LOCK_GUID,
.yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_NON_VOLATILE,
Expand All @@ -147,12 +151,14 @@ struct mok_state_variable mok_state_variables[] = {
.addend = &vendor_deauthorized,
.addend_size = &vendor_deauthorized_size,
.flags = MOK_MIRROR_KEYDB |
MOK_MIRROR_DELETE_FIRST |
MOK_VARIABLE_LOG,
.pcr = 14,
},
{.name = L"MokSBState",
.name8 = "MokSBState",
.rtname = L"MokSBStateRT",
.rtname8 = "MokSBStateRT",
.guid = &SHIM_LOCK_GUID,
.yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_NON_VOLATILE,
Expand All @@ -166,6 +172,7 @@ struct mok_state_variable mok_state_variables[] = {
{.name = L"MokDBState",
.name8 = "MokDBState",
.rtname = L"MokIgnoreDB",
.rtname8 = "MokIgnoreDB",
.guid = &SHIM_LOCK_GUID,
.yes_attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_NON_VOLATILE,
Expand Down Expand Up @@ -204,6 +211,7 @@ mirror_one_mok_variable(struct mok_state_variable *v)
* we're always mirroring the original data, whether this is an efi
* security database or not
*/
dprint(L"v->name:\"%s\" v->rtname:\"%s\"\n", v->name, v->rtname);
dprint(L"v->data_size:%lu v->data:0x%08llx\n", v->data_size, v->data);
dprint(L"FullDataSize:%lu FullData:0x%08llx\n", FullDataSize, FullData);
if (v->data_size) {
Expand Down Expand Up @@ -299,6 +307,7 @@ mirror_one_mok_variable(struct mok_state_variable *v)
}
}


/*
* Now we have the full size
*/
Expand Down Expand Up @@ -417,28 +426,72 @@ mirror_one_mok_variable(struct mok_state_variable *v)
FullDataSize, FullData, p, p-(uintptr_t)FullData);
}

dprint(L"FullDataSize:%lu FullData:0x%08llx p:0x%08llx pos:%lld\n",
dprint(L"FullDataSize:%lu FullData:0x%016llx p:0x%016llx pos:%lld\n",
FullDataSize, FullData, p, p-(uintptr_t)FullData);
if (FullDataSize) {
dprint(L"Setting %s with %lu bytes of data\n",
v->rtname, FullDataSize);
uint32_t attrs = EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
uint64_t max_storage_sz = 0;
uint64_t remaining_sz = 0;
uint64_t max_var_sz = 0;
UINT8 *tmp = NULL;
UINTN tmpsz = 0;

efi_status = gRT->QueryVariableInfo(attrs, &max_storage_sz,
&remaining_sz, &max_var_sz);
if (EFI_ERROR(efi_status)) {
perror(L"Could not get variable storage info: %r\n", efi_status);
return efi_status;
}
dprint(L"calling SetVariable(\"%s\", 0x%016llx, 0x%08lx, %lu, 0x%016llx)\n",
v->rtname, v->guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS
| EFI_VARIABLE_RUNTIME_ACCESS,
FullDataSize, FullData);
efi_status = gRT->SetVariable(v->rtname, v->guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS,
EFI_VARIABLE_BOOTSERVICE_ACCESS
| EFI_VARIABLE_RUNTIME_ACCESS,
FullDataSize, FullData);
if (EFI_ERROR(efi_status)) {
perror(L"Failed to set %s: %r\n",
v->rtname, efi_status);
if (efi_status == EFI_INVALID_PARAMETER && max_var_sz < FullDataSize) {
/*
* In this case we're going to try to create a
* dummy variable so that there's one there. It
* may or may not work, because on some firmware
* builds when the SetVariable call above fails it
* does actually set the variable(!), so aside from
* not using the allocation if it doesn't work, we
* don't care about failures here.
*/
console_print(L"WARNING: Maximum volatile variable size is %lu.\n", max_var_sz);
console_print(L"WARNING: Cannot set %s (%lu bytes)\n", v->rtname, FullDataSize);
perror(L"Failed to set %s: %r\n", v->rtname, efi_status);
efi_status = variable_create_esl(
null_sha256, sizeof(null_sha256),
&EFI_CERT_SHA256_GUID, &SHIM_LOCK_GUID,
&tmp, &tmpsz);
/*
* from here we don't really care if it works or
* doens't.
*/
if (!EFI_ERROR(efi_status) && tmp && tmpsz) {
gRT->SetVariable(v->rtname, v->guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS
| EFI_VARIABLE_RUNTIME_ACCESS,
tmpsz, tmp);
FreePool(tmp);
}
efi_status = EFI_INVALID_PARAMETER;
} else if (EFI_ERROR(efi_status)) {
perror(L"Failed to set %s: %r\n", v->rtname, efi_status);
}
}
if (v->data && v->data_size) {
if (v->data && v->data_size && v->data != FullData) {
FreePool(v->data);
v->data = NULL;
v->data_size = 0;
}
if (FullData && FullDataSize) {
FreePool(FullData);
}
v->data = FullData;
v->data_size = FullDataSize;
dprint(L"returning %r\n", efi_status);
return efi_status;
}
Expand All @@ -454,8 +507,11 @@ maybe_mirror_one_mok_variable(struct mok_state_variable *v, EFI_STATUS ret)
BOOLEAN present = FALSE;

if (v->rtname) {
if (v->flags & MOK_MIRROR_DELETE_FIRST)
LibDeleteVariable(v->rtname, v->guid);
if (v->flags & MOK_MIRROR_DELETE_FIRST) {
dprint(L"deleting \"%s\"\n", v->rtname);
efi_status = LibDeleteVariable(v->rtname, v->guid);
dprint(L"LibDeleteVariable(\"%s\",...) => %r\n", v->rtname, efi_status);
}

efi_status = mirror_one_mok_variable(v);
if (EFI_ERROR(efi_status)) {
Expand Down Expand Up @@ -505,6 +561,12 @@ maybe_mirror_one_mok_variable(struct mok_state_variable *v, EFI_STATUS ret)
return ret;
}

struct mok_variable_config_entry {
CHAR8 name[256];
UINT64 data_size;
UINT8 data[];
};

/*
* Verify our non-volatile MoK state. This checks the variables above
* accessable and have valid attributes. If they don't, it removes
Expand All @@ -527,6 +589,11 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle)
user_insecure_mode = 0;
ignore_db = 0;

UINT64 config_sz = 0;
UINT8 *config_table = NULL;
size_t npages = 0;
struct mok_variable_config_entry config_template;

dprint(L"importing mok state\n");
for (i = 0; mok_state_variables[i].name != NULL; i++) {
struct mok_state_variable *v = &mok_state_variables[i];
Expand Down Expand Up @@ -579,6 +646,61 @@ EFI_STATUS import_mok_state(EFI_HANDLE image_handle)
}

ret = maybe_mirror_one_mok_variable(v, ret);
if (v->data && v->data_size) {
config_sz += v->data_size;
config_sz += sizeof(config_template);
}
}

/*
* Alright, so we're going to copy these to a config table. The
* table is a packed array of N+1 struct mok_variable_config_entry
* items, with the last item having all zero's in name and
* data_size.
*/
if (config_sz) {
config_sz += sizeof(config_template);
npages = ALIGN_VALUE(config_sz, PAGE_SIZE) >> EFI_PAGE_SHIFT;
config_table = NULL;
efi_status = gBS->AllocatePages(AllocateAnyPages,
EfiRuntimeServicesData,
npages,
(EFI_PHYSICAL_ADDRESS *)&config_table);
if (EFI_ERROR(efi_status) || !config_table) {
console_print(L"Allocating %lu pages for mok config table failed: %r\n",
npages, efi_status);
if (ret != EFI_SECURITY_VIOLATION)
ret = efi_status;
config_table = NULL;
} else {
ZeroMem(config_table, npages << EFI_PAGE_SHIFT);
}
}

UINT8 *p = (UINT8 *)config_table;
for (i = 0; p && mok_state_variables[i].name != NULL; i++) {
struct mok_state_variable *v = &mok_state_variables[i];

ZeroMem(&config_template, sizeof(config_template));
strncpya(config_template.name, (CHAR8 *)v->rtname8, 255);
config_template.name[255] = '\0';

config_template.data_size = v->data_size;

CopyMem(p, &config_template, sizeof(config_template));
p += sizeof(config_template);
CopyMem(p, v->data, v->data_size);
p += v->data_size;
}
if (p) {
ZeroMem(&config_template, sizeof(config_template));
CopyMem(p, &config_template, sizeof(config_template));

efi_status = gBS->InstallConfigurationTable(&MOK_VARIABLE_STORE,
config_table);
if (EFI_ERROR(efi_status)) {
console_print(L"Couldn't install MoK configuration table\n");
}
}

/*
Expand Down

0 comments on commit fecc2df

Please sign in to comment.