keepkey/keepkey-firmware

Checksum mismatch since v6.2.2

antonio-fr opened this issue ยท 9 comments

I tried to compiled the latest official fw v6.3.0 using a machine with docker.
I'm using the following script, to be sure to build a fresh new version each time.

version=v6.3.0 <- changed to the desired version
cd ~
rm -Rf keepkey-firmware/
git clone https://github.com/keepkey/keepkey-firmware.git
cd keepkey-firmware
git checkout $version
git submodule update --init --recursive
./scripts/build/docker/device/release.sh
# Compute hashes
sha256sum ./bin/firmware.keepkey.bin
tail -c +257 ./bin/firmware.keepkey.bin | sha256sum
wget https://github.com/keepkey/keepkey-firmware/releases/download/$version/firmware.keepkey.bin
tail -c +257 firmware.keepkey.bin | sha256sum

I get the following hashes :

6.3.0 โ“

1cc2f36eab7b71ba91d28d285210979e5a62363a4e4d777433ae268885daef4d
fac2060001d3d03aef65a3cef6805bb751b271313d20c3f178fb3b5341a53eda
3c63544ee6fa8c19f70071c563215e2010801381b34c385ec1cd92e1e203687c

6.2.2 โ“

a52431859b236b01291fc3f50dd560231be391ca4fc01ad7c9e243a2f7b70b30
597090aec0c426f14ecf88a5ada7d96f1699b410f372932511c84aa3e9bb7204
b1b22f06e81962c00ae8e807dc514641fd8d342f6953da0948a57027eebac71e

6.2.0 ๐Ÿ†—

c699183079b2cf3c581ffe040a2c924570daff7d4d7b506845dfc32b55c3d5b0
8c1be122c285f43745e3455633a70915518ce56cbf42571f07db8e8ba564735a
8c1be122c285f43745e3455633a70915518ce56cbf42571f07db8e8ba564735a

6.1.1 ๐Ÿ†—

76d822b8f050d44d1a361f1217634e3f614c5b6b031a8af0294d7ce650f263da
e3c85bc9ff45b89d0453ee0505dfc2febc669570535fc93bb10232a290fce81e
e3c85bc9ff45b89d0453ee0505dfc2febc669570535fc93bb10232a290fce81e

5.8.1 ๐Ÿ†—

697bee3b4688f06ac14a4ac250169b29b0a9a09929b445610b33807a7ce777d1
d4005014ee8c6fa93bb7518000a69a06cf2fbbc6c8b0d309f642e649c5aa0b4d
697bee3b4688f06ac14a4ac250169b29b0a9a09929b445610b33807a7ce777d1

There's no reproducibility since v6.2.2. What am I doing wrong ?

Also the check process changed in fw from hashing the whole bin result in v5.x to excluding signature header in v6.x, that's a minor issue, more like a README.md not updated.

On v6.3.0, I have:

$ tail -c +257 ./bin/firmware.keepkey.bin | shasum -a 256
3c63544ee6fa8c19f70071c563215e2010801381b34c385ec1cd92e1e203687c  -
$ tail -c +257 ./firmware.keepkey.bin | shasum -a 256
3c63544ee6fa8c19f70071c563215e2010801381b34c385ec1cd92e1e203687c  -

The hash I have written down in my notebook from when we signed it matches that of the firmware with each of the signing slots empty:

$ shasum -a 256 ./bin/firmware.keepkey.bin 
1a7ae2174f38bb2f3a0a25158af02a49aa1bcf5ff43783912cbda2be415efbb4  ./bin/firmware.keepkey.bin

So it's at least reproducible on my machine again, and @KeepKeyBrett was able to reproduce the same hash as was in my notebook, otherwise we would not have signed it.... so that's at least a small baseline of "not completely broken".

As for you not being able to reproduce that, I'd like to get to the bottom of that. For the pair of binaries you're finding that are different, what part(s) of the binary are different:

$ diff <(xxd ./bin/firmware.keepkey.bin) <(xxd ./firmware.keepkey.bin)

I'm hoping something sticks out, like perhaps part of the ETH tokens table or something, which had some changes in 7b4d953, e364ddf, 7894766, 6f0422f, 1935644, which is my best guess for something that'd have these symptoms, looking at git log v6.2.0..v6.2.2

I just asked a co-worker to build it, and he got yet another hash, different from both yours and mine.

Andddd it is the ETH tokens table that's different. They're in a completely different order. Sigh.

FAOD, here's the contents of the build artifact that's causing the issue: https://gist.github.com/keepkeyjon/5b66d06cb5d2a80a2d36e544a37ea286 (which you can also find in the binaries.tar.bz2 I uploaded with the release). If you drop that file in, and prevent cmake from generating it (see lib/firmware/CMakeLists.txt), you should see the same hashes as in the signed release.

I'm not sure yet why the host OS has an influence over this. I'll cook up a python-keepkey patch to fix this going forward.

Good catch and quick fix! Indeed 'for x in z' are no guarantee to be reproducible. Will be better for the future, else maybe one day you would have not the same hash as the other developer and wonder why.

Can you provide a more detailed about how to exclude rebuild of ethereum_tokens.def in lib/firmware/CMakeLists.txt ? So I will confirm the 6.2.2 and 6.3.0 firmware.

Also, you can consider updating the README of the fw here, so that it explains to compare the hash of a given tagged build from byte num 257, not from the beginning. I think that's a change since fw v6.x.

Can you provide a more detailed about how to exclude rebuild of ethereum_tokens.def in lib/firmware/CMakeLists.txt ? So I will confirm the 6.2.2 and 6.3.0 firmware.

Something like:

diff --git a/lib/firmware/CMakeLists.txt b/lib/firmware/CMakeLists.txt
index d8d02117..e5cf1fe5 100644
--- a/lib/firmware/CMakeLists.txt
+++ b/lib/firmware/CMakeLists.txt
@@ -37,10 +37,4 @@ include_directories(
 add_library(kkfirmware ${sources})
 add_dependencies(kkfirmware kktransport.pb trezorcrypto qrcodegenerator ethereum_tokens.def)
 
-set(ETHEREUM_TOKENS ${CMAKE_BINARY_DIR}/include/keepkey/firmware/ethereum_tokens)
-
-add_custom_target(ethereum_tokens.def
-  COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_BINARY_DIR}/include/keepkey/firmware
-  COMMAND python ${CMAKE_SOURCE_DIR}/deps/python-keepkey/keepkeylib/eth/ethereum_tokens.py ${ETHEREUM_TOKENS}.def)
-
 add_library(kkfirmware.keepkey variant/keepkey/resources.c)

and drop ethereum_tokens.def into lib/firmware.

Also, you can consider updating the README of the fw here, so that it explains to compare the hash of a given tagged build from byte num 257, not from the beginning. I think that's a change since fw v6.x.

Sure, happy to!

Great work!

You just forget to tell to delete "ethereum_tokens.def" in lib/firmware/CMakeLists.txt on line 38. ๐Ÿ˜‰

Here's what I did for v6.3.0 :

version=v6.3.0
cd ~
rm -Rf keepkey-firmware/
git clone https://github.com/keepkey/keepkey-firmware.git
cd keepkey-firmware
git checkout $version
git submodule update --init --recursive
wget -O include/keepkey/firmware/ethereum_tokens.def https://gist.githubusercontent.com/keepkeyjon/5b66d06cb5d2a80a2d36e544a37ea286/raw/ethereum_tokens.def
sed -i -e '40,44d;' lib/firmware/CMakeLists.txt
sed -i -e 's/\<ethereum_tokens.def\>//g' lib/firmware/CMakeLists.txt
./scripts/build/docker/device/release.sh
wget https://github.com/keepkey/keepkey-firmware/releases/download/$version/firmware.keepkey.bin
sha256sum ./bin/firmware.keepkey.bin
tail -c +257 ./bin/firmware.keepkey.bin | sha256sum
tail -c +257 firmware.keepkey.bin | sha256sum
1a7ae2174f38bb2f3a0a25158af02a49aa1bcf5ff43783912cbda2be415efbb4
3c63544ee6fa8c19f70071c563215e2010801381b34c385ec1cd92e1e203687c
3c63544ee6fa8c19f70071c563215e2010801381b34c385ec1cd92e1e203687c

๐Ÿ†—

For v6.2.2, the shitcoin tokens list might be different. But I guess the same kind of recipe should work.