| Age | Commit message (Collapse) | Author |
|---|
|
Fix a couple of small off-by-one errors in the SBAT variable initial
setup and validation path.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes it so we can tell what it's actually doing and why.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes it so that if you build with ENABLE_SHIM_DEVEL, the SBAT we
use is named SBAT_DEVEL instead of SBAT, and it's expected to have
EFI_VARIABLE_RUNTIME_ACCESS set.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
A few cleanups:
- Ensure that the data we get from get_variable() is at least big enough
to actually be an SBAT variable
- Only try to delete if the variable is actually set
- Don't set the variable again if deleting it failed
- We don't actually need to get the size of the variable, allocate,
and then get the variable; get_variable() does the allocation for us.
- Don't compare the variable data when get_variable() failed
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This re-structures our includes so we can be sure everything is always
including all the system headers in a uniform, predictable way.
Temporarily it also adds a bunch of junk at all the places we use
variadic functions to specifically pick either the MS (cdecl) or ELF
ABIs.
I'm not 100% sure that's all correct (see later patch) but it's enough
to allow this to build.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Fix a typo that inverted the check that validates that a CSV
column is not empty.
Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
|
|
This commit fixes the following gcc warnings for casting.
sbat.c: In function 'verify_single_entry':
sbat.c:157:13: error: pointer targets in passing argument 1 of 'strcmp' differ in signedness [-Werror=pointer-sign]
if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) {
^~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
int strcmp (const char *, const char *);
^~~~~~
sbat.c:157:36: error: pointer targets in passing argument 2 of 'strcmp' differ in signedness [-Werror=pointer-sign]
if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) {
^~~~~~~~~~~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
int strcmp (const char *, const char *);
^~~~~~
sbat.c:165:19: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign]
sbat_gen = atoi(entry->component_generation);
^
Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi'
#define atoi(nptr) AsciiStrDecimalToUintn(nptr)
^~~~
In file included from Cryptlib/Include/OpenSslSupport.h:21:0,
from Cryptlib/Include/string.h:15,
from sbat.c:7:
Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
UINTN AsciiStrDecimalToUintn(const char *String);
^~~~~~~~~~~~~~~~~~~~~~
In file included from Cryptlib/Include/string.h:15:0,
from sbat.c:7:
sbat.c:166:23: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign]
sbat_var_gen = atoi(sbat_var_entry->component_generation);
^
Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi'
#define atoi(nptr) AsciiStrDecimalToUintn(nptr)
^~~~
In file included from Cryptlib/Include/OpenSslSupport.h:21:0,
from Cryptlib/Include/string.h:15,
from sbat.c:7:
Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}'
UINTN AsciiStrDecimalToUintn(const char *String);
^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [<builtin>: sbat.o] Error 1
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes our SBAT variable parser use the generic CSV parser, and also
changes its API slightly to produce a more testable intermediate
interface.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes the .sbat section parser use parse_csv_data(). It also
re-names a couple of the structs, because they were still too easy to
get lost in.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves is_utf8_bom() to str.h, and also adds two #defines, UTF8_BOM
and UTF8_BOM_SIZE.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
"gcc -fanalyzer" found two NULL pointer checks we're missing in sbat.c:
include/str.h: In function ‘get_sbat_field.part.0’:
sbat.c:20:14: error: dereference of NULL ‘offset’ [CWE-476] [-Werror=analyzer-null-dereference]
20 | if (!*offset)
and
include/str.h: In function ‘parse_sbat’:
sbat.c:140:27: error: dereference of NULL ‘current’ [CWE-476] [-Werror=analyzer-null-dereference]
140 | } while (entry && *current != '\0');
Both are simple, and this patch fixes them.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The parse_sbat() function is currently removing the last character of the
passed buffer, which will usually be a null-terminated string to parse.
There's no reason to do this and just take the whole size as specified by
the caller.
Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
On a typical boot we validate at least two binaries; parsing the SBAT
EFI variable each time, when it should not be changing, is not worth the
effort.
This patch moves the parsing out to some setup code, instead of doing it
during the verification stage.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Per Peter Jones suggestion, we will be flexible in what data we expect
while parsing the variable. Three fields are mandatory:
component_generation, component_name_size, component_name
However we also support adding comments and additional information to be
added after component name, with ',' as a separator. Those information
will be ignored and not used for verification purposes.
So:
grub,1
and
grub,1,wow,this,is,my,comment
will provide exactly same set of data for verification.
[0]: https://github.com/rhboot/shim/blob/main/SBAT.md
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The struct sbat isn't doing anything and only has two fields so let's pass
pass those two to the functions directly instead of storing it in a struct.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Add parameter checking to parse_sbat().
Set end pointer to be sbat_base + sbat_size - 1. We directly
dereference the end pointer but this is technically outside of
our sbat_base buffer range.
Remove current and end while loops that account for extra CRLF
or LF characters before and after the .sbat section. We will
rely on automated tooling to verify the .sbat section is sane.
Remove the overwriting of *(end - 1) with '\0'. This behavior
causes a segfault in the unit test. parse_sbat_entry() expects
a very specific pattern "_,_,_,_,_,_\n" for every entry and uses
strchrnul() to process each individual field. When *(end - 1)='\0'
is present, it short-circuits the final \n and causes the final
get_sbat_field() to return NULL, thereby setting current = NULL.
Eventually parse_sbat attempts to access current in the do-while
condition and the segfault happens.
Signed-off-by: Chris Co <chrco@microsoft.com>
|
|
It's a left over from an early implementation that was never cleaned.
Reported-by: Christopher Co <christopher.co@microsoft.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Parse the SBAT [0] Version-Based Revocation Metadata that's contained in a
.sbat data section of the loaded PE binary. This information is used along
with data in a SBAT variable to determine if a EFI binary has been revoked.
[0]: https://github.com/rhboot/shim/blob/sbat/SBAT.md
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The Secure Boot Advanced Targeting (SBAT) [0] is a Generation Number Based
Revocation mechanism that is meant to replace the DBX revocation file list.
Binaries must contain a .sbat data section that has a set entries, each of
them consisting of UTF-8 strings as comma separated values. Allow to embed
this information into the fwupd EFI binary at build time.
The SBAT metadata must contain at least two entries. One that defines the
SBAT version used and another one that defines the component generation.
This patch adds a sbat.csv that contains these two entries and downstream
users can override if additional entries are needed due changes that make
them diverge from upstream code and potentially add other vulnerabilities.
The same SBAT metadata is added to the fallback and MOK manager binaries
because these are built from the same shim source. These need to have SBAT
metadata as well to be booted if a .sbat section is mandatory.
[0]: https://github.com/rhboot/shim/blob/sbat/SBAT.md
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
