[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Please review changes for the Debian shim -7 upload



On Mon, May 6, 2019 at 6:25 AM Steve McIntyre <steve@einval.com> wrote:
>
> Hey folks,
>
> I think I'm about ready to upload a new shim, with a number of
> changes., It would be lovely to get more eyes on these, as I'm hoping
> this will be our last upload before buster and I want to get this
> signed.
>
> See https://salsa.debian.org/efi-team/shim/commits/master
>
> Changes for review:
>
> 549f650 (93sam/hack, hack) Add more hashes that we want to blacklist
> 88a7a65 Add initial file with test checksums for the dbx list
> 6cf246a Generate a vendor dbx file at build time
> e17b0af Build using gcc-7
> 315e876 Fix OBJ_create() to tolerate a NULL sn and ln
> 878d860 VLogError(): Avoid NULL pointer dereferences in (V)Sprint calls
>
> In particular, please check my logic in the dbx file creation. I've
> tested this using a local VM set up with secure boot enabldd and test
> keys in the firmware, and it looked to work ok.
>
> Please review/test as soon as you can - I don't want this to be
> blocking the Buster release.

Nice work Steve! I don't have any feedback on the overall logic, but I
did have some take-or-leave suggestions for the packaging, mostly
around making the dbx list generation more robust. See:
  https://salsa.debian.org/dannf/shim/tree/dbx-hash-cleanup

The "set -e" thing is the only one I'd strongly push for.

One other thing - is there is a way to sanity check dbx.esl from the
command line? If so, I'd suggest adding that test to the $(DBX_LIST)
target, so that a misbehaving efisiglist fails the build. That should
also avoid the need to version the build-dep on pesign.

  -dann


Reply to: