Re: [PATCH v6 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface

Ilias Apalodimas
2022-11-07T13:38:31Z

Replying to myself here for a clarification on sign-efi-sig-list

On Mon, 7 Nov 2022 at 15:27, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Kojima-san
>
> [...]
>
> > > > + }
> > > > +
> > > > + if (!file_have_auth_header(buf, size)) {
> > >
> > > Can you explain why we need this? I would expect the user to prepare an
> > > .esl file with ./tools/efivar.py
> >
> > This is for the case that the user selects the .auth file
> > signed by 'sign-efi-sig-list' tool.
>
> Right that's what I imagined. So we are trying to make sure the '-t'
> option from sign-efi-sig-list is the user didn't since it's now mandatory
> on the spec, right?

I remembered sign-efi-sig-list wrong, if -t is not specified the
system time is added

Cheers
/Ilias

>
> I get what you are trying to do here. You basically want to make sure the
> user will be allowed to enroll the keys in random order. IOW if the user
> first enrolls a PK, the KEK, DB and DBX must be authenticated variables.
> But if he started by enrolling DB(x) he can use with the .esl file
> right ?(at least until PK is registered)
>
> I don't think this is a bad idea, but I'd prefer being more pedantic here.
> I think we are better off *always* expecting .auth files and leave the decision
> of accepting a timestamped authenticated variable or not to the core UEFI
> subsystem, instead of shoehorning a timestamp.
>
> Heirich, thoughts?
>
> Thanks
> /Ilias
> >
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > > + struct efi_signature_store *sigstore;
> > > > + char *tmp_buf;
> > > > +
> > > > + /* Check if the file is valid EFI Signature List(s) */
> > > > + tmp_buf = calloc(1, size);
> > > > + if (!tmp_buf) {
> > > > + ret = EFI_OUT_OF_RESOURCES;
> > > > + goto out;
> > > > + }
> > > > + memcpy(tmp_buf, buf, size);
> > > > + /* tmp_buf is freed in efi_build_signature_store() */
> > > > + sigstore = efi_build_signature_store(tmp_buf, size);
> > > > + if (!sigstore) {
> > > > + eficonfig_print_msg("ERROR! Invalid file format.");
> > > > + ret = EFI_INVALID_PARAMETER;
> > > > + goto out;
> > > > + }
> > > > + efi_sigstore_free(sigstore);
> > > > +
> > > > + ret = create_time_based_payload(buf, &new_db, &size);
> > > > + if (ret != EFI_SUCCESS) {
> > > > + eficonfig_print_msg("ERROR! Failed to create payload with timestamp.");
> > > > + goto out;
> > > > + }
> > > > +
> > > > + free(buf);
> > > > + buf = new_db;
> > > > + }
> > > > +
> > > > + attr = EFI_VARIABLE_NON_VOLATILE |
> > > > + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > > > + EFI_VARIABLE_RUNTIME_ACCESS |
> > > > + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> > > > +
> > > [...]
> > >
> > > Thanks
> > > /Ilias