Opened 4 months ago

Last modified 2 weeks ago

#29185 new defect

8.5a7-build3 Windows .exe files are not reproducibly built

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: GeorgKoppen201903, tbb-8.5, TorBrowserTeam201905
Cc: boklm, pospeselr Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

boklm and I got not matching Windows .exe files when building with tbb-8.5a7-build3. We suspect #28546, especially as the contents _are_ matching (extracting and comparing the fresh bundle shows just a different lnk file, which is to be expected I think)

Child Tickets

Change History (24)

comment:1 Changed 4 months ago by gk

Here is the diff generated with xxd between

https://people.torproject.org/~gk/testbuilds/torbrowser-install-win64-8.5a7_tr.exe

and

https://people.torproject.org/~boklm/builds/8.5a7-build3/torbrowser-install-win64-8.5a7_tr.exe

--- /dev/fd/63	2019-01-27 16:59:57.939420883 +0100
+++ /dev/fd/62	2019-01-27 16:59:57.939420883 +0100
@@ -11,7 +11,7 @@
 000000a0: 00ae 0000 00ec 0100 9d3c 0000 0010 0000  .........<......
 000000b0: 0000 4000 0000 0000 0010 0000 0002 0000  ..@.............
 000000c0: 0400 0000 0600 0000 0500 0200 0000 0000  ................
-000000d0: 0080 0700 0004 0000 b940 6d03 0200 6085  .........@m...`.
+000000d0: 0080 0700 0004 0000 560b 6e03 0200 6085  ........V.n...`.
 000000e0: 0000 2000 0000 0000 0010 0000 0000 0000  .. .............
 000000f0: 0000 1000 0000 0000 0010 0000 0000 0000  ................
 00000100: 0000 0000 1000 0000 0000 0000 0000 0000  ................
@@ -12938,7 +12938,7 @@
 00032890: 2000 a825 0000 0400 4040 0000 0100 2000   ..%....@@.... .
 000328a0: 2842 0000 0300 8080 0000 0100 2000 2808  (B.......... .(.
 000328b0: 0100 0100 0000 0000 0100 2000 174f 0000  .......... ..O..
-000328c0: 0200 6d02 0000 0000 0400 0000 3030 0000  ..m.........00..
+000328c0: 0200 8200 0000 0000 0100 0000 2020 0000  ............  ..
 000328d0: 3c3f 786d 6c20 7665 7273 696f 6e3d 2231  <?xml version="1
 000328e0: 2e30 2220 656e 636f 6469 6e67 3d22 5554  .0" encoding="UT
 000328f0: 462d 3822 2073 7461 6e64 616c 6f6e 653d  F-8" standalone=
@@ -3592440,4 +3592440,4 @@
 036d0f70: 8cb5 4d1c a956 fb08 6c2c 76f6 ad15 0e55  ..M..V..l,v....U
 036d0f80: 1b09 a7ab da22 17a9 43b3 70e8 5ae7 e2d7  ....."..C.p.Z...
 036d0f90: 565f 7ac0 194a be01 d7ab 5e61 145d 0cbd  V_z..J....^a.]..
-036d0fa0: 22e0 9bff c9f5 3e3e 4574 fb47 0000 0000  ".....>>Et.G....
+036d0fa0: 22e0 9bff c9f5 3e3e e74d f54a 0000 0000  ".....>>.M.J....

comment:2 Changed 4 months ago by langpack

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis /DCHANNEL=[% c("var/channel") %] torbrowser.nsi

comment:3 in reply to:  2 ; Changed 4 months ago by gk

Replying to langpack:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis /DCHANNEL=[% c("var/channel") %] torbrowser.nsi

That just breaks the build with Can't open script "/DCHANNEL=alpha".

comment:4 Changed 4 months ago by gk

Okay, backing out the patch for #28546 does indeed solve the problem for me, fun.

comment:5 in reply to:  3 ; Changed 4 months ago by langpack

Replying to gk:

Replying to langpack:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis /DCHANNEL=[% c("var/channel") %] torbrowser.nsi

That just breaks the build with Can't open script "/DCHANNEL=alpha".

Oh, it seems https://sourceforge.net/p/nsis/bugs/568/ doesn't work, because this is a some kind of cross-nsis like https://github.com/inponomarev/nsis/blob/master/tasks/main.yml
But, please, use the latest security update (3.04) with fixed https://sourceforge.net/p/nsis/bugs/1204/

comment:6 in reply to:  5 ; Changed 4 months ago by gk

Replying to langpack:

Replying to gk:

Replying to langpack:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis /DCHANNEL=[% c("var/channel") %] torbrowser.nsi

That just breaks the build with Can't open script "/DCHANNEL=alpha".

Oh, it seems https://sourceforge.net/p/nsis/bugs/568/ doesn't work, because this is a some kind of cross-nsis like https://github.com/inponomarev/nsis/blob/master/tasks/main.yml
But, please, use the latest security update (3.04) with fixed https://sourceforge.net/p/nsis/bugs/1204/

Yes, we should do that and we have #29187 for that. However, switching to it does not solve this bug.

comment:7 in reply to:  6 ; Changed 4 months ago by langpack

Replying to gk:

Replying to langpack:

Replying to gk:

Replying to langpack:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis /DCHANNEL=[% c("var/channel") %] torbrowser.nsi

That just breaks the build with Can't open script "/DCHANNEL=alpha".

Oh, it seems https://sourceforge.net/p/nsis/bugs/568/ doesn't work, because this is a some kind of cross-nsis like https://github.com/inponomarev/nsis/blob/master/tasks/main.yml
But, please, use the latest security update (3.04) with fixed https://sourceforge.net/p/nsis/bugs/1204/

Yes, we should do that and we have #29187 for that. However, switching to it does not solve this bug.

Will #29187 be included in 8.5a7? Should it?
As a quick possible solution:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis -DCHANNEL="[% c("var/channel") %]" torbrowser.nsi

?

comment:8 in reply to:  7 Changed 4 months ago by boklm

Replying to langpack:

As a quick possible solution:

- makensis -DCHANNEL=[% c("var/channel") %] torbrowser.nsi
+ makensis -DCHANNEL="[% c("var/channel") %]" torbrowser.nsi

Since var/channel does not contain any space, $ or special characters, I don't seen any difference between those two lines.

comment:9 Changed 4 months ago by gk

Keywords: GeorgKoppen201901 added
Priority: ImmediateVery High

Backing out the fix for #28546 "helped", but we should investigate and fix this properly asap.

comment:10 Changed 4 months ago by cypherpunks2

torbrowser.ico 23.4 KB
torbrowser-alpha.ico 117 KB

comment:11 Changed 4 months ago by cypherpunks2

    IMAGE_RESOURCE_DATA_ENTRY rDataE = {0,};
    rDataE.CodePage = ConvertEndianness(cRDataE->GetCodePage());
    rDataE.Size = ConvertEndianness(cRDataE->GetSize());

    CopyMemory(seeker, &rDataE, sizeof(IMAGE_RESOURCE_DATA_ENTRY));
typedef struct _IMAGE_RESOURCE_DATA_ENTRY {
  DWORD OffsetToData;
  DWORD Size;
  DWORD CodePage;
  DWORD Reserved;
} IMAGE_RESOURCE_DATA_ENTRY,*PIMAGE_RESOURCE_DATA_ENTRY;

Reserved undefined?

comment:12 Changed 4 months ago by cypherpunks2

Reserved undefined?

Nope.

comment:13 in reply to:  10 Changed 4 months ago by gk

Replying to cypherpunks2:

torbrowser.ico 23.4 KB
torbrowser-alpha.ico 117 KB

Yeah, and just using torbrowser.ico instead of the torbrowser-alpha.ico "fixes" the issue.

comment:14 Changed 4 months ago by cypherpunks2

No, I don't want to play this game. cpunks account removed. cpunks3 owned. Sorry.

Last edited 4 months ago by cypherpunks2 (previous) (diff)

comment:15 Changed 4 months ago by cypherpunks2

static LPBYTE generate_icon_group(IconGroup icon, IconPairs order, bool first)
{
  LPBYTE group = new BYTE[
    sizeof(IconGroupHeader) // header
    + order.size() * SIZEOF_RSRC_ICON_GROUP_ENTRY // entries
  ];

  IconGroupHeader* header = (IconGroupHeader*) group;

  header->wReserved = 0;
  header->wIsIcon   = FIX_ENDIAN_INT16(1);
  header->wCount    = FIX_ENDIAN_INT16((WORD)icon.size());

  order = sort_pairs(order, first);

  for (IconGroup::size_type i = 0; i < icon.size(); i++)
  {
    RsrcIconGroupEntry* entry = (RsrcIconGroupEntry*)
      &group[sizeof(IconGroupHeader) + SIZEOF_RSRC_ICON_GROUP_ENTRY * i];
    unsigned index = first ? order[i].index1 : order[i].index2;

    memcpy(&entry->header, &icon[index].meta, sizeof(IconGroupEntry));
    entry->wRsrcId = FIX_ENDIAN_INT16(order[i].size_index + 1);
  }

  return group;
}
#define SIZEOF_RSRC_ICON_GROUP_ENTRY 14
typedef struct
{
  BYTE bWidth;
  BYTE bHeight;
  BYTE bPaletteEntries;
  BYTE bReserved;
  WORD wPlanes;
  WORD wBitsPerPixel;
  DWORD dwRawSize;
} IconGroupEntry;

Why?

order.size() == icon.size()? How?

ZeroMemory for group need.

Version 0, edited 4 months ago by cypherpunks2 (next)

comment:16 Changed 4 months ago by cypherpunks2

Simple fix. Unicorn!

 ;Interface Configuration
 
   !define MUI_ICON "${CHANNEL_ICON}"
+  !define MUI_UNICON "${CHANNEL_ICON}"
   !define MUI_ABORTWARNING
 
 ;--------------------------------

comment:17 Changed 4 months ago by cypherpunks2

0x0 pixels icons from new ICOs makes unicorn sad too.

comment:18 Changed 4 months ago by gk

Keywords: GeorgKoppen201902 added; GeorgKoppen201901 removed

Moving my tickets to Feb

comment:19 Changed 4 months ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201901 removed

Moving tickets to February.

comment:20 Changed 3 months ago by gk

Keywords: TorBrowserTeam201903 added; TorBrowserTeam201902 removed

Moving my tickets to March.

comment:21 Changed 3 months ago by gk

Keywords: GeorgKoppen201903 added; GeorgKoppen201902 removed

Now for my keyword.

comment:22 Changed 3 months ago by gk

Keywords: tbb-8.5 added

Tickets on our radar for 8.5

comment:23 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201904 added; TorBrowserTeam201903 removed

Moving tickets to April.

comment:24 Changed 2 weeks ago by gk

Keywords: TorBrowserTeam201905 added; TorBrowserTeam201904 removed

Moving tickets to May

Note: See TracTickets for help on using tickets.