Removing signature on Tor Browser .exe should result in SHA256 value listed in sha256sums.txt
Since #3861 (closed) landed we do the authenticode code-signing for Windows Tor Browser .exe files. In order to compare these files with the ones we built deterministically we need to strip its signature. It turns out that this works using e.g. osslsigncode but (surprise!) one does not get the same SHA256 sum back.
Activity
Useful analysis done in #3861 (closed):
"Neither osslsigncode nor disitool get a .exe that matches the original, unsigned .exe (which got signed by signtool as a stopgap until we get our authenticode signing on Linux going) after I stripped the signature. The diff boils down to:
--- /dev/fd/63 2015-03-31 16:45:12.639747705 +0200 +++ /dev/fd/62 2015-03-31 16:45:12.647747512 +0200 @@ -11,7 +11,7 @@ 00000a0: 0050 0000 00ac 0100 2743 0000 0010 0000 .P......'C...... 00000b0: 00a0 0000 0000 4000 0010 0000 0002 0000 ......@......... 00000c0: 0400 0000 0600 0000 0400 0000 0000 0000 ................ -00000d0: 0060 0400 0004 0000 41b6 0100 0200 0080 .`......A....... +00000d0: 0060 0400 0004 0000 94b6 2202 0200 0080 .`........"..... 00000e0: 0000 2000 0010 0000 0000 1000 0010 0000 .. ............. 00000f0: 0000 0000 1000 0000 0000 0000 0000 0000 ................ 0000100: 0090 0200 0413 0000 00c0 0300 a894 0000 ................ @@ -2236612,4 +2236612,4 @@ 2220c30: 7d7b 4d5b 0d90 1b6d cff0 0563 5fb0 5f4a }{M[...m...c_._J 2220c40: 950a 1208 9218 b015 49a0 05f9 db75 391f ........I....u9. 2220c50: 855e 2cec 1272 e4ff ffb1 edbc b0b6 d819 .^,..r.......... -2220c60: f9 . +2220c60: f900 0000 0000 0000 ........"
Trac:
Description: Since #3861 (closed) landed we do the authenticode code-signing for Windows Tor Browser .exe files. In order to compare these files with the ones we built deterministically we need to strip its signature. It turns out that this works using e.g. osslsigncode but (surprise!) one does not get the same SHA256 sum back.to
Since #3861 (closed) landed we do the authenticode code-signing for Windows Tor Browser .exe files. In order to compare these files with the ones we built deterministically we need to strip its signature. It turns out that this works using e.g.
osslsigncodebut (surprise!) one does not get the same SHA256 sum back.-
Another good idea a bit deeper buried:
"NSIS generates unaligned files. You can to test with exe from bundle, it should to matches. NSIS generates totally wrong PE files, it just appends compressed stuff to end of valid and aligned PE file without changing anything to PE-header, like malware."
-
Padding is not the only issue. If you take torbrowser-install-4.5a5_ko.exe before the authenticode signing, sign it and strip the signature you get still a broken PE-Header: {{{ --- /dev/fd/63 2015-04-01 14:37:37.594384310 +0000 +++ /dev/fd/62 2015-04-01 14:37:37.590384415 +0000 @@ -11,7 +11,7 @@ 00000a0: 0050 0000 00ac 0100 2743 0000 0010 0000 .P......'C...... 00000b0: 00a0 0000 0000 4000 0010 0000 0002 0000 ......@......... 00000c0: 0400 0000 0600 0000 0400 0000 0000 0000 ................ -00000d0: 0060 0400 0004 0000 41b6 0100 0200 0080 .
......A....... +00000d0: 0060 0400 0004 0000 94d0 2702 0200 0080 .........'..... 00000e0: 0000 2000 0010 0000 0000 1000 0010 0000 .. ............. 00000f0: 0000 0000 1000 0000 0000 0000 0000 0000 ................ 0000100: 0090 0200 0413 0000 00c0 0300 a894 0000 ................ Because PE-checksum (it's at 0xd8 offset) depends size of file too, but NSIS don't care to include stuff to PE-checksum it appends to end of file (it could to do zeroing of PE-checksum then, but not to leave broken PE-file with wrong checksum). osslsigncode removing signature and recalculates valid PE-checksum for whole file (includes len, changed after padding).
NSIS and checksum:
// No check sum support yet... DWORD* pdwCheckSum = GetMemberFromOptionalHeader(m_ntHeaders->OptionalHeader, CheckSum); if (*pdwCheckSum) { // clear checksum (should be [re]calculated after all changes done) pdwCheckSum = 0; //throw runtime_error("CResourceEditor doesn't yet support check sum"); }Found only one place where it tries to change checksum, where is "[re]calculated after all changes done" then? It's not even right code then, it clears pointer to DWORD with checksum (why?). If to truncate stuff they appends to PE-file then checksum value from generated exe is not random for sure but not real one again. Have no resources to compile it either for tests.
I think in order to solve this bug we should indeed patch NSIS as the other tools are behaving properly it seems.
We could to fix it by kludges too, it's faster at least, like this:
cp torbrowser-install.exe moditexe dd if=/dev/zero bs=1 count=$((8-$(stat -c%s moditexe)%8)) >> moditexe echo "import pefile; pef = pefile.PE('moditexe', fast_load=True); " > chksumfix.py echo "pef.OPTIONAL_HEADER.CheckSum = pef.generate_checksum(); " >> chksumfix.py echo "pef.write(filename='moditexe2')" >> chksumfix.py python ./chksumfix.py rm torbrowser-install.exe rm moditexe mv moditexe2 torbrowser-install.exe(req python-pefile pkg) But test shows it spends too many seconds while trying to calculate and to write so many bytes, probably local problems, however.
dd if=/dev/zero bs=1 count=
((8-(stat -c%s moditexe)%0.1.1.x-final)) >> moditexe Alignment need to be implemented in the same python script, dd just for poc purpose.-
Replying to gk:
Thanks for looking into it. I think in order to solve this bug we should indeed patch NSIS as the other tools are behaving properly it seems. But, boy, this will be a fun task. I'll start with trying to get NSIS compiled (in our gitian environment).
On a second thought I think this is a lot of work for little gain especially as we might get away with the workaround mentioned in comment:10 (at least in the short/medium term). Let's try that one instead first.
-
I have a fix for this which is working. I'll post a proper branch after incorporating everything into our gitian framework and give it a last round of testing. I think it should make it into 4.5 as we start doing the windows signing there too and it is actually only one additional rebundling if we don't want to wait for it before we start building (which is fine to me). The risk that it'll break things after testing is pretty low I think.
Trac:
Status: new to assigned
Keywords: N/A deleted, tbb-4.5-alpha added -
bug_15539_v4 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_15539_v4&id=e487b6ffca7b023442fd8820eed8345a20310fde) is up for review. It still gives the old .exe back after stripping off the signature with
osslsigncode. For testing purposes I made a bunch of nightly builds and I got always the same SHA256 sum. Thus,pefiledoes not seem to introduce new reproducibility issues either.Trac:
Status: assigned to needs_review mentioned in issue #15598 (moved)
mentioned in issue #20062 (closed)
mentioned in issue #20254 (moved)
