-
Notifications
You must be signed in to change notification settings - Fork 305
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
Make SBAT variable payload introspectable #483
Conversation
dd2877f
to
4da89d0
Compare
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor stuff inline.
sbat_var.S
Outdated
|
||
# include "include/sbat_var_defs.h" | ||
|
||
#if defined(ENABLE_SHIM_DEVEL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better to have this stuff in sbat_ver_defs.h?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've put it there instead.
sbat_var.S
Outdated
@@ -0,0 +1,46 @@ | |||
// SPDX-License-Identifier: BSD-2-Clause-Patent | |||
|
|||
# include "include/sbat_var_defs.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace between the '#' and "include".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now.
sbat_var.S
Outdated
SBAT_VAR_LATEST_REVOCATIONS | ||
#endif /* ENABLE_SHIM_DEVEL */ | ||
|
||
.section .sbatlevel, "a", %progbits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation from here down seems wonky. So far for asm files (i.e., cert.S) we've used hard tabs only. Some of these lines are using tabs, some spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed this to just use tabs now.
(Also, this needs rebased at some point - conflicts in sbat.h.) |
Given a set of EFI variables and boot assets, it should be possible to compute what the value of PCR 7 will be on the next boot. As shim manages the contents of the SbatLevel variable and this is measured to PCR 7, export the payloads that shim contains in a new COFF section (.sbatlevel) so that it can be introspected by code outside of shim. The new section works a bit like .vendor_cert - it contains a header and then the payload. In this case, the header contains no size fields because the strings are NULL terminated. Shim uses this new section internally in set_sbat_uefi_variable. The .sbatlevel section starts with a 4 byte version field which is not used by shim but may be useful for external auditors if the format of the section contents change in the future. Signed-off-by: Chris Coulson <[email protected]>
4da89d0
to
6ff23a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, no further objections.
This is for after 15.6, but we'll probably add something like this to
our next shim in Ubuntu, so I'd at least like to agree on the correct
way to do this.
Given a set of EFI variables and boot assets, it should be possible
to compute what the value of PCR 7 will be on the next boot.
As shim manages the contents of the SbatLevel variable and this is
measured to PCR 7, export the payloads that shim contains in a new
COFF section (.sbatlevel) so that it can be introspected by code
outside of shim.
The new section works a bit like .vendor_cert - it contains a header
and then the payload. In this case, the header contains no size fields
because the strings are NULL terminated. Shim uses this new section
internally in set_sbat_uefi_variable.
The .sbatlevel section starts with a 4 byte version field which is
not used by shim but may be useful for external auditors if the
format of the section contents change in the future.