sbsigntool fix (was Re: [PATCH] Fix PE/COFF checksum calculation)

Steve McIntyre steve at einval.com
Sat Jul 27 00:23:43 BST 2019


Hi James!

[ Adding Steve to CC for info too ]

Thanks for getting back to me.

On Fri, Jul 26, 2019 at 03:23:57PM -0700, James Bottomley wrote:
>On Thu, 2019-06-13 at 16:57 +0100, Steve McIntyre wrote:
>> On Thu, Jun 13, 2019 at 08:54:21AM -0700, James Bottomley wrote:
>> > On Thu, 2019-06-13 at 13:53 +0100, Steve McIntyre wrote:
>> > > Sharing with others too. No idea if James is having mail problems
>> > > or
>> > > something...
>> > 
>> > Sorry, no, sbsigntool has been somewhat low on my list of things to
>> > look after for a while (I was hoping after engine support it would
>> > just
>> > be complete).
>> > 
>> > Let me actually dust off the git tree and have a look at the
>> > problem.
>> 
>> ACK, thanks!
>
>OK, I looked at this but there's no way of checking or validating the
>change: the PECOFF checksum value is defined helpfully as 'whatever
>comes out of IMAGHELP.DLL'.  For EFI binaries this means it could be
>any random number and we never check it so originally we did just that
>(or more accurately assigned it to zero).  Steve Langasek later patched
>the tools to an algorithm he claimed he'd verified with IMAGHELP.DLL,
>in this commit:
>
>commit be1f3d8350c6d86fa5fd36bd22c94bf86e106dbb
>Author: Steve Langasek <steve.langasek at canonical.com>
>Date:   Wed Jan 27 11:06:02 2016 -0800
>
>    Update the PE checksum field using the somewhat-underdocumented
>    algorithm, so that we match the Microsoft implementation in our
>    signature generation.
> 
>You're saying what he did was wrong and thus shouldn't have matched the
>Microsoft tools.  How have you verified this?

I have *not* directly run anything against IMAGHELP.DLL, but what
prompted me to work on this was output from Microsoft themselves.

In the Debian shim-signed package build, we use sbattach to remove
then re-attach the Microsoft signature on the binary that they signed
for us, to validate 100% that there are no changes. This code was even
written by Steve Langasek! See

  https://salsa.debian.org/efi-team/shim-signed/blob/master/Makefile

for what we're doing.

We used to use sbsigntool 0.6 and this worked fine. It broke after we
updated to 0.9.2 in Debian - I could not get the checksums to match
again, so I worked through the code and found what looked like a
bug. Checking the log on that commit, I also see:

    [jejb: add endian to autogen.sh and fix for multi-sign]

and I'm thinking this maybe looks like a bug in the multi-sign
implementation. Can you (plural) shed some light on that maybe?

To explain for Steve's benefit too, my change at

  https://salsa.debian.org/efi-team/sbsigntool/blob/master/debian/patches/fix_checksum_calc.patch

made things interoperate again. We've since had another set of
MS-signed binaries come back, and the checksums also all worked on
that with no more changes. I think it's fair to say things are working
as they should with my change included.

-- 
Steve McIntyre, Cambridge, UK.                                steve at einval.com
  Mature Sporty Personal
  More Innovation More Adult
  A Man in Dandism
  Powered Midship Specialty




More information about the Efi mailing list