On Wed, 6 Oct 2021 at 16:15, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > On 10/6/21 10:02, Ilias Apalodimas wrote: > > On Wed, 6 Oct 2021 at 10:21, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> > >> > >> On 10/6/21 08:29, Ilias Apalodimas wrote: > >>> On Sun, Oct 03, 2021 at 11:23:19AM +0200, Heinrich Schuchardt wrote: > >>>> Simplify efi_sigstore_parse_sigdb() by using existing functions. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >>>> --- > >>>> v3: > >>>> Keep error handling in efi_sigstore_parse_sigdb() > >>>> v2: > >>>> remove a superfluous check > >>>> --- > >>>> lib/efi_loader/efi_signature.c | 11 ++--------- > >>>> 1 file changed, 2 insertions(+), 9 deletions(-) > >>>> > >>>> diff --git a/lib/efi_loader/efi_signature.c > >>>> b/lib/efi_loader/efi_signature.c > >>>> index bdd09881fc..97f6dfacd9 100644 > >>>> --- a/lib/efi_loader/efi_signature.c > >>>> +++ b/lib/efi_loader/efi_signature.c > >>>> @@ -746,18 +746,11 @@ struct efi_signature_store > >>>> *efi_sigstore_parse_sigdb(u16 *name) > >>>> efi_uintn_t db_size; > >>>> efi_status_t ret; > >>>> > >>>> - if (!u16_strcmp(name, L"PK") || !u16_strcmp(name, L"KEK")) { > >>>> - vendor = &efi_global_variable_guid; > >>>> - } else if (!u16_strcmp(name, L"db") || !u16_strcmp(name, L"dbx")) { > >>>> - vendor = &efi_guid_image_security_database; > >>>> - } else { > >>>> - EFI_PRINT("unknown signature database, %ls\n", name); > >>>> - return NULL; > >>>> - } > >>>> + vendor = efi_auth_var_get_guid(name); > >>> > >>> Should we return NULL if we get back the default guid? > >> > >> efi_sigstore_parse_sigdb() is only called with fixed values of 'name'. > >> So how should this occur? > > > > Bugs that slip through maybe? I generally prefer being more pedantic > > with security related code > > PK and KEK use efi_global_variable_guid and will be used as argument for > efi_sigstore_parse_sigdb(). Your proposed check would break the code.
Yea that's the reason I was asking about efi_auth_var_get_guid(). I would prefer if that returned NULL if the variable is not found instead of the default GUID Regards /Ilias > > Best regards > > Heinrich > > > > > Regards > > /Ilias > >> > >> Best regards > >> > >> Heinrich > >> > >>> > >>>> > >>>> /* retrieve variable data */ > >>>> db_size = 0; > >>>> - ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, > >>>> NULL)); > >>>> + ret = efi_get_variable_int(name, vendor, NULL, &db_size, NULL); > >>>> if (ret == EFI_NOT_FOUND) { > >>>> EFI_PRINT("variable, %ls, not found\n", name); > >>>> sigstore = calloc(sizeof(*sigstore), 1); > >>>> -- > >>>> 2.32.0 > >>>> > >>> > >>> Regards > >>> /Ilias > >>> >