Skip to content

Commit 8b3f5ce

Browse files
THS-onTHS-on
committed
better error message when shim self validation fails
This exports the component name and found and expected SBAT level from verify_sbat_section for us to display to the user why validation failed. Further we now differentiate between errors regarding .sbat section related errors and SBAT revocations. Signed-off-by: Thore Sommer <[email protected]>
1 parent 0287c6b commit 8b3f5ce

File tree

6 files changed

+90
-23
lines changed

6 files changed

+90
-23
lines changed

‎include/pe.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ EFI_STATUS verify_image(void *data, unsigned int datasize,
1919
PE_COFF_LOADER_IMAGE_CONTEXT *context);
2020

2121
EFI_STATUS
22-
verify_sbat_section(char *SBATBase, size_t SBATSize);
22+
verify_sbat_section(char *SBATBase, size_t SBATSize, UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found, CHAR8 **sbat_component_name);
2323

2424
EFI_STATUS
2525
get_section_vma (UINTN section_num,

‎include/sbat.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,14 @@ parse_sbat_section(char *section_base, size_t section_size, size_t *n,
7777
struct sbat_section_entry ***entriesp);
7878
void cleanup_sbat_section_entries(size_t n, struct sbat_section_entry **entries);
7979

80-
EFI_STATUS verify_sbat(size_t n, struct sbat_section_entry **entries);
80+
EFI_STATUS verify_sbat(size_t n, struct sbat_section_entry **entries, UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found, CHAR8 **sbat_component_name);
8181

8282
#ifdef SHIM_UNIT_TEST
8383
EFI_STATUS parse_sbat_var_data(list_t *entries, UINT8 *data, UINTN datasize);
84-
EFI_STATUS verify_sbat_helper(list_t *sbat_var, size_t n,
85-
struct sbat_section_entry **entries);
84+
EFI_STATUS verify_sbat_helper(list_t *local_sbat_var, size_t n,
85+
struct sbat_section_entry **entries,
86+
UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found,
87+
CHAR8 **sbat_component_name);
8688
#endif /* !SHIM_UNIT_TEST */
8789
#endif /* !SBAT_H_ */
8890
// vim:fenc=utf-8:tw=75:noet

‎pe.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ generate_hash(char *data, unsigned int datasize,
333333
}
334334

335335
EFI_STATUS
336-
verify_sbat_section(char *SBATBase, size_t SBATSize)
336+
verify_sbat_section(char *SBATBase, size_t SBATSize, UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found, CHAR8 **sbat_component_name)
337337
{
338338
unsigned int i;
339339
EFI_STATUS efi_status;
@@ -385,7 +385,7 @@ verify_sbat_section(char *SBATBase, size_t SBATSize)
385385
entries[i]->vendor_url);
386386
}
387387

388-
efi_status = verify_sbat(n, entries);
388+
efi_status = verify_sbat(n, entries, sbat_gen_expected, sbat_gen_found, sbat_component_name);
389389

390390
cleanup_sbat_section_entries(n, entries);
391391

‎sbat.c

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,8 @@ cleanup_sbat_section_entries(size_t n, struct sbat_section_entry **entries)
131131
}
132132

133133
EFI_STATUS
134-
verify_single_entry(struct sbat_section_entry *entry, struct sbat_var_entry *sbat_var_entry, bool *found)
134+
verify_single_entry(struct sbat_section_entry *entry, struct sbat_var_entry *sbat_var_entry, bool *found, UINT16 *sbat_gen, UINT16 *sbat_var_gen)
135135
{
136-
UINT16 sbat_gen, sbat_var_gen;
137-
138136
if (strcmp((const char *)entry->component_name, (const char *)sbat_var_entry->component_name) == 0) {
139137
dprint(L"component %a has a matching SBAT variable entry, verifying\n",
140138
entry->component_name);
@@ -144,12 +142,12 @@ verify_single_entry(struct sbat_section_entry *entry, struct sbat_var_entry *sba
144142
* atoi returns zero for failed conversion, so essentially
145143
* badly parsed component_generation will be treated as zero
146144
*/
147-
sbat_gen = atoi((const char *)entry->component_generation);
148-
sbat_var_gen = atoi((const char *)sbat_var_entry->component_generation);
145+
*sbat_gen = atoi((const char *)entry->component_generation);
146+
*sbat_var_gen = atoi((const char *)sbat_var_entry->component_generation);
149147

150-
if (sbat_gen < sbat_var_gen) {
148+
if (*sbat_gen < *sbat_var_gen) {
151149
dprint(L"component %a, generation %d, was revoked by %s variable\n",
152-
entry->component_name, sbat_gen, SBAT_VAR_NAME);
150+
entry->component_name, *sbat_gen, SBAT_VAR_NAME);
153151
LogError(L"image did not pass SBAT verification\n");
154152
return EFI_SECURITY_VIOLATION;
155153
}
@@ -177,9 +175,10 @@ cleanup_sbat_var(list_t *entries)
177175
}
178176

179177
EFI_STATUS
180-
verify_sbat_helper(list_t *local_sbat_var, size_t n, struct sbat_section_entry **entries)
178+
verify_sbat_helper(list_t *local_sbat_var, size_t n, struct sbat_section_entry **entries, UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found, CHAR8 **sbat_component_name)
181179
{
182180
unsigned int i;
181+
UINTN component_name_size;
183182
list_t *pos = NULL;
184183
EFI_STATUS efi_status = EFI_SUCCESS;
185184
struct sbat_var_entry *sbat_var_entry;
@@ -192,26 +191,39 @@ verify_sbat_helper(list_t *local_sbat_var, size_t n, struct sbat_section_entry *
192191
for (i = 0; i < n; i++) {
193192
list_for_each(pos, local_sbat_var) {
194193
bool found = false;
194+
*sbat_gen_expected = 0;
195+
*sbat_gen_found = 0;
195196
sbat_var_entry = list_entry(pos, struct sbat_var_entry, list);
196-
efi_status = verify_single_entry(entries[i], sbat_var_entry, &found);
197-
if (EFI_ERROR(efi_status))
197+
efi_status = verify_single_entry(entries[i], sbat_var_entry, &found, sbat_gen_found, sbat_gen_expected);
198+
if (EFI_ERROR(efi_status)) {
199+
if (found && efi_status == EFI_SECURITY_VIOLATION) {
200+
component_name_size = strlen((const char *)entries[i]->component_name);
201+
*sbat_component_name = AllocatePool(component_name_size + 1);
202+
if (!*sbat_component_name) {
203+
efi_status = EFI_OUT_OF_RESOURCES;
204+
goto err;
205+
}
206+
memcpy(*sbat_component_name, (const char *)entries[i]->component_name, component_name_size + 1);
207+
}
198208
goto out;
209+
}
199210
if (found)
200211
break;
201212
}
202213
}
203214

204215
out:
205216
dprint(L"finished verifying SBAT data: %r\n", efi_status);
217+
err:
206218
return efi_status;
207219
}
208220

209221
EFI_STATUS
210-
verify_sbat(size_t n, struct sbat_section_entry **entries)
222+
verify_sbat(size_t n, struct sbat_section_entry **entries, UINT16 *sbat_gen_expected, UINT16 *sbat_gen_found, CHAR8 **sbat_component_name)
211223
{
212224
EFI_STATUS efi_status;
213225

214-
efi_status = verify_sbat_helper(&sbat_var, n, entries);
226+
efi_status = verify_sbat_helper(&sbat_var, n, entries, sbat_gen_expected, sbat_gen_found, sbat_component_name);
215227
return efi_status;
216228
}
217229

@@ -559,9 +571,12 @@ set_sbat_uefi_variable(char *sbat_var_automatic, char *sbat_var_latest)
559571
}
560572

561573
#ifndef SHIM_UNIT_TEST
574+
UINT16 sbat_gen_expected = 0;
575+
UINT16 sbat_gen_found = 0;
576+
CHAR8 *sbat_component_name = NULL;
562577
char *sbat_start = (char *)&_sbat;
563578
char *sbat_end = (char *)&_esbat;
564-
efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1);
579+
efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1, &sbat_gen_expected, &sbat_gen_found, &sbat_component_name);
565580
if (EFI_ERROR(efi_status)) {
566581
CHAR16 *title = L"New SbatLevel would self-revoke current shim. Not applied";
567582
CHAR16 *message = L"Press any key to continue";

‎shim.c

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,10 @@ verify_buffer_sbat (char *data, int datasize,
712712
{
713713
int i;
714714
EFI_IMAGE_SECTION_HEADER *Section;
715+
EFI_STATUS efi_status;
716+
UINT16 sbat_gen_expected = 0;
717+
UINT16 sbat_gen_found = 0;
718+
CHAR8 *sbat_component_name = NULL;
715719
char *SBATBase = NULL;
716720
size_t SBATSize = 0;
717721

@@ -759,7 +763,11 @@ verify_buffer_sbat (char *data, int datasize,
759763
}
760764
}
761765

762-
return verify_sbat_section(SBATBase, SBATSize);
766+
efi_status = verify_sbat_section(SBATBase, SBATSize, &sbat_gen_expected, &sbat_gen_found, &sbat_component_name);
767+
if (sbat_component_name) {
768+
FreePool(sbat_component_name);
769+
}
770+
return efi_status;
763771
}
764772

765773
/*
@@ -1841,6 +1849,9 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
18411849
{
18421850
EFI_STATUS efi_status;
18431851
EFI_HANDLE image_handle;
1852+
UINT16 sbat_gen_expected = 0;
1853+
UINT16 sbat_gen_found = 0;
1854+
CHAR8 *sbat_component_name = NULL;
18441855

18451856
verification_method = VERIFIED_BY_NOTHING;
18461857

@@ -1927,7 +1938,7 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
19271938
goto die;
19281939
}
19291940

1930-
efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1);
1941+
efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1, &sbat_gen_expected, &sbat_gen_found, &sbat_component_name);
19311942
if (EFI_ERROR(efi_status)) {
19321943
perror(L"Verifiying shim SBAT data failed: %r\n",
19331944
efi_status);
@@ -1966,6 +1977,23 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
19661977
die:
19671978
console_print(L"Something has gone seriously wrong: %s: %r\n",
19681979
msgs[msg], efi_status);
1980+
1981+
/*
1982+
* Provide additional information when the SBAT self check fails
1983+
*/
1984+
if ( msg == SBAT_SELF_CHECK ) {
1985+
if (sbat_gen_expected == 0 && sbat_gen_found == 0 && sbat_component_name == NULL) {
1986+
// In this case something related to the .sbat section is wrong.
1987+
console_print(L"\n\nSomething went wrong validating SBAT. Either:\n"
1988+
"- The shim has no .sbat section or is corrupted\n"
1989+
"- Something went wrong internally");
1990+
} else {
1991+
console_print(L"\n\nYour shim has been revoked via SBAT. "
1992+
"Please update to a newer shim version.\n"
1993+
"For component \"%a\" expected at least level: %u. Current shim has: %u.\n",
1994+
sbat_component_name, sbat_gen_expected, sbat_gen_found);
1995+
}
1996+
}
19691997
#if defined(ENABLE_SHIM_DEVEL)
19701998
devel_egress(COLD_RESET);
19711999
#else
@@ -1974,6 +2002,9 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab)
19742002
0, NULL);
19752003
#endif
19762004
}
2005+
if (sbat_component_name) {
2006+
FreePool(sbat_component_name);
2007+
}
19772008

19782009
/*
19792010
* This variable is supposed to be set by second stages, so ensure it is

‎test-sbat.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,10 +468,19 @@ test_verify_sbat_null_sbat_section(void)
468468
status = parse_sbat_var_data(&test_sbat_var, sbat_var_data, sizeof(sbat_var_data));
469469
assert_equal_goto(status, EFI_SUCCESS, err, "got %#x expected %#x\n");
470470

471-
status = verify_sbat_helper(&test_sbat_var, n, entries);
471+
UINT16 sbat_gen_expected = 0;
472+
UINT16 sbat_gen_found = 0;
473+
CHAR8 *sbat_component_name = NULL;
474+
status = verify_sbat_helper(&test_sbat_var, n, entries, &sbat_gen_expected, &sbat_gen_found, &sbat_component_name);
475+
assert_equal_goto(sbat_component_name, NULL, err, "got %#x expected %#x\n");
476+
assert_equal_goto(sbat_gen_expected, 0, err, "got %#x expected %#x\n");
477+
assert_equal_goto(sbat_gen_found, 0, err, "got %#x expected %#x\n");
472478
assert_equal_goto(status, EFI_SUCCESS, err, "got %#x expected %#x\n");
473479
rc = 0;
474480
err:
481+
if (sbat_component_name) {
482+
free(sbat_component_name);
483+
}
475484
cleanup_sbat_var(&test_sbat_var);
476485

477486
return rc;
@@ -973,12 +982,22 @@ test_parse_and_verify(void)
973982
if (status != EFI_SUCCESS || list_empty(&sbat_var))
974983
return -1;
975984

976-
status = verify_sbat(n_section_entries, section_entries);
985+
UINT16 sbat_gen_expected = 0;
986+
UINT16 sbat_gen_found = 0;
987+
CHAR8 *sbat_component_name = NULL;
988+
status = verify_sbat(n_section_entries, section_entries, &sbat_gen_expected, &sbat_gen_found, &sbat_component_name);
989+
assert_nonzero_goto(sbat_component_name, err, "not expected component name to be NULL");
990+
assert_goto(strcmp(sbat_component_name, "test1") == 0, err, "expected 'test1' got '%s'", sbat_component_name);
991+
assert_equal_goto(sbat_gen_expected, 5, err, "expected %#x got %#x\n");
992+
assert_equal_goto(sbat_gen_found, 1, err, "expected %#x got %#x\n");
977993
assert_equal_goto(status, EFI_SECURITY_VIOLATION, err, "expected %#x got %#x\n");
978994

979995
rc = 0;
980996
err:
981997
cleanup_sbat_section_entries(n_section_entries, section_entries);
998+
if (sbat_component_name) {
999+
free(sbat_component_name);
1000+
}
9821001
cleanup_sbat_var(&sbat_var);
9831002

9841003
return rc;

0 commit comments

Comments
 (0)