Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mirrored MOK variables could be accidentally deleted #386

Closed
lcp opened this issue Jun 30, 2021 · 0 comments
Closed

Mirrored MOK variables could be accidentally deleted #386

lcp opened this issue Jun 30, 2021 · 0 comments

Comments

@lcp
Copy link
Collaborator

lcp commented Jun 30, 2021

Per the comment of MOK_MIRROR_DELETE_FIRST, any MOK variable with the flag would delete the existing mirrored variable first.

shim/mok.c

Line 127 in 9f973e4

* MOK_MIRROR_DELETE_FIRST delete any existing variable first

The code to mirror a MOK variable is in mirror_mok_db():

shim/mok.c

Lines 373 to 379 in 9f973e4

if (FullDataSize <= max_var_sz) {
if (only_first)
efi_status = SetVariable(name, guid, attrs,
FullDataSize, FullData);
return efi_status;
}

It writes the variable only when only_first is true.

However, in maybe_mirror_one_mok_variable(),

shim/mok.c

Lines 866 to 870 in 9f973e4

if (!only_first && (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);
}

LibDeleteVariable() is called to delete the mirrored MOK variable when only_first is false.

The logic looks strange to me since it tries to delete the mirrored MOK variable we just created. The problem doesn't show in QEMU/OVMF VMs because LibDeleteVariable() deletes the variables with BS+RT+NV but we create the mirrored variable with BS+RT, so OVMF just returns "EFI_INVALID_PARAMETER". However, for the VMWare VMs, the firmware doesn't check the attributes and just deletes the variable matching the given name+guid, so those RT variables never show in OS.

lcp added a commit to lcp/shim that referenced this issue Jul 1, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Jul 22, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Aug 3, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Aug 6, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Aug 16, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Aug 17, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Sep 3, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
vathpela pushed a commit to vathpela/mallory that referenced this issue Sep 7, 2021
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.

This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.

Fixes: rhboot#386

Signed-off-by: Gary Lin <glin@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant