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

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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 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