Opened 5 years ago

Last modified 6 months ago

#12387 assigned defect

(Some) Pluggable Transport binaries are not stripped

Reported by: gk Owned by: mikeperry
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-testcase, tbb-rbm
Cc: asn, yawning Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Mike mentioned the other day that (some) Pluggable Transport binaries are not stripped. We should make sure that is the case to make the TBBs not larger as needed.

Child Tickets

TicketStatusOwnerSummaryComponent
#12649closederinnOmit symbol tables and debugging from golang binariesApplications/Tor bundles/installation

Change History (30)

comment:1 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201406 added

comment:2 Changed 5 years ago by boklm

Keywords: tbb-testcase added

comment:3 Changed 5 years ago by gk

Keywords: TorBrowserTeam201407 added; TorBrowserTeam201406 removed

comment:4 Changed 5 years ago by dcf

Specifically for Go programs, "it isn't supported, isn't tested and is known to produce broken executables." I don't know exactly what the problem is, but we should check it out before trying to strip Go programs. I heard that Yawning has been stripping obfs4 without ill effects.

comment:5 in reply to:  4 ; Changed 5 years ago by dcf

Replying to dcf:

Specifically for Go programs, "it isn't supported, isn't tested and is known to produce broken executables." I don't know exactly what the problem is, but we should check it out before trying to strip Go programs. I heard that Yawning has been stripping obfs4 without ill effects.

According to the lantern-devel mailing list, "Compiling Go binaries with the -w linker flag makes binaries quite a bit smaller." -w is "omit the DWARF symbol table." Additionally -s ("omit the symbol table and debug information") looks useful among the ld options.

comment:6 in reply to:  5 ; Changed 5 years ago by yawning

Replying to dcf:

According to the lantern-devel mailing list, "Compiling Go binaries with the -w linker flag makes binaries quite a bit smaller." -w is "omit the DWARF symbol table." Additionally -s ("omit the symbol table and debug information") looks useful among the ld options.

Per the Debian bug:

We believe that the bug you reported is fixed in the latest version of golang, which is due to be installed in the Debian FTP archive.

I'm not sure how relevant this problem is, because:

  • We don't build ARM binaries (except for Orbot and eventually Firefox OS, where we can change the build procedure)
  • We use Go 1.2.x last I checked.

I've only been stripping binaries for amd64, but I have had none of the issues described at all (probably because I use Go 1.3 on amd64 instead of ARM). As an additional point of reference even if we decide against stripping, moving to Go 1.3 will omit the plan 9 symbol table resulting in a substantial binary size reduction (in addition to other runtime bugfixes), so it is probably worth considering.

comment:7 in reply to:  5 Changed 5 years ago by dcf

Replying to dcf:

Replying to dcf:

Specifically for Go programs, "it isn't supported, isn't tested and is known to produce broken executables." I don't know exactly what the problem is, but we should check it out before trying to strip Go programs. I heard that Yawning has been stripping obfs4 without ill effects.

According to the lantern-devel mailing list, "Compiling Go binaries with the -w linker flag makes binaries quite a bit smaller." -w is "omit the DWARF symbol table." Additionally -s ("omit the symbol table and debug information") looks useful among the ld options.

Here are tests using go1.3 on linux64. Summary: -s implies -w; and -s saves a lot, as much as strip does.

go build
6.1M meek-client
2.8M meek-client-torbrowser

go build -ldflags '-w'
4.7M meek-client
2.2M meek-client-torbrowser

go build -ldflags '-s'
4.4M meek-client
2.0M meek-client-torbrowser

go build -ldflags '-s -w'
4.4M meek-client
2.0M meek-client-torbrowser

go build && strip
4.4M meek-client
2.0M meek-client-torbrowser

It looks like we only have to use -s, because -s mostly implies -w. In the linker, the primary functions that check the -w option...

https://code.google.com/p/go/source/browse/src/cmd/ld/dwarf.c?name=6bdbf9086c00#2163

void
dwarfaddshstrings(LSym *shstrtab)
{
        if(debug['w'])  // disable dwarf
                return;

https://code.google.com/p/go/source/browse/src/cmd/ld/dwarf.c?name=6bdbf9086c00#2344

void
dwarfaddmachoheaders(void)
{
        MachoSect *msect;
        MachoSeg *ms;
        vlong fakestart;
        int nsect;

        if(debug['w'])  // disable dwarf
                return;

https://code.google.com/p/go/source/browse/src/cmd/ld/dwarf.c?name=6bdbf9086c00#2435

void
dwarfaddpeheaders(void)
{
        if(debug['w'])  // disable dwarf
                return;

...are also gated on the outside by -s. There are a few other places where -w is checked that aren't so gated, but it appears they don't matter much for file size.

https://code.google.com/p/go/source/browse/src/cmd/ld/elf.c?name=6bdbf9086c00#963

      if(!debug['s']) {
              addstring(shstrtab, ".symtab");
              addstring(shstrtab, ".strtab");
              dwarfaddshstrings(shstrtab);
      }

https://code.google.com/p/go/source/browse/src/cmd/ld/macho.c?name=6bdbf9086c00#478

      if(!debug['s'] && linkmode != LinkExternal)
              dwarfaddmachoheaders();

https://code.google.com/p/go/source/browse/src/cmd/ld/pe.c?name=6bdbf9086c00#639

      if(!debug['s'])
              dwarfaddpeheaders();
Last edited 5 years ago by dcf (previous) (diff)

comment:8 in reply to:  6 Changed 5 years ago by dcf

Replying to yawning:

Per the Debian bug:

We believe that the bug you reported is fixed in the latest version of golang, which is due to be installed in the Debian FTP archive.

"We believe the bug is fixed" means that Debian stopped stripping golang executables, not that it became safe to strip them. (Check the "Wed, 17 Jul 2013 19:15:18 +0200" entry in the Debian changelog.)

I keep running into comments by Dave Cheney saying "don't run strip on your executables." According to comment:7, -ldflags '-s' works just as well anyway.

I'm going to work on a patch to use the -s option and see how it affects bundle sizes.

Last edited 5 years ago by dcf (previous) (diff)

comment:9 Changed 5 years ago by dcf

I made #12649 as a child ticket for golang binaries.

comment:10 Changed 5 years ago by erinn

Keywords: needs-triage added

comment:11 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201408 added; TorBrowserTeam201407 removed

comment:12 Changed 5 years ago by mikeperry

Owner: changed from erinn to dcf
Status: newassigned

FWIW, this also applies to the compiled .so/dylib/dll files in the python transports. Can we safely strip those?

comment:13 Changed 5 years ago by gk

Keywords: needs-triage removed

comment:14 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201410 added; TorBrowserTeam201408 removed

comment:15 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201410Easy added; TorBrowserTeam201410 removed

comment:16 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201411 added

comment:17 Changed 4 years ago by mikeperry

Keywords: added; TorBrowserTeam201410Easy removed

comment:18 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201412 added; TorBrowserTeam201411 removed

comment:19 Changed 4 years ago by mikeperry

To find the current culprits, cd into a Tor Browser directory and run:

find Browser/TorBrowser/Tor/ -type f -executable -exec file {} \;  | grep "not stripped" | less

comment:20 in reply to:  19 Changed 4 years ago by dcf

Replying to mikeperry:

To find the current culprits, cd into a Tor Browser directory and run:

find Browser/TorBrowser/Tor/ -type f -executable -exec file {} \;  | grep "not stripped" | less

...which in https://archive.torproject.org/tor-package-archive/torbrowser/4.0.2/tor-browser-linux64-4.0.2_en-US.tar.xz gives:

$ find . -type f -executable -exec file {} \; | grep "not stripped" | sed -e 's/: .*//'
./libgmpxx.so.4.3.3
./libgmp.so.10.1.3
./libgmpxx.so
./libgmpxx.so.4
./libgmp.so
./PluggableTransports/M2Crypto/__m2crypto.so
./PluggableTransports/fte/cDFA.so
./PluggableTransports/zope/interface/_zope_interface_coptimizations.so
./PluggableTransports/twisted/runner/portmap.so
./PluggableTransports/twisted/test/raiser.so
./PluggableTransports/twisted/python/_initgroups.so
./PluggableTransports/twisted/python/sendmsg.so
./PluggableTransports/Crypto/Util/strxor.so
./PluggableTransports/Crypto/Util/_counter.so
./PluggableTransports/Crypto/Hash/_RIPEMD160.so
./PluggableTransports/Crypto/Hash/_SHA256.so
./PluggableTransports/Crypto/Hash/_SHA512.so
./PluggableTransports/Crypto/Hash/_MD2.so
./PluggableTransports/Crypto/Hash/_SHA384.so
./PluggableTransports/Crypto/Hash/_SHA224.so
./PluggableTransports/Crypto/Hash/_MD4.so
./PluggableTransports/Crypto/Cipher/_ARC4.so
./PluggableTransports/Crypto/Cipher/_CAST.so
./PluggableTransports/Crypto/Cipher/_DES.so
./PluggableTransports/Crypto/Cipher/_Blowfish.so
./PluggableTransports/Crypto/Cipher/_ARC2.so
./PluggableTransports/Crypto/Cipher/_AES.so
./PluggableTransports/Crypto/Cipher/_DES3.so
./PluggableTransports/Crypto/Cipher/_XOR.so
./libgmp.so.10

The files have size 4.0 MB unstripped, and 2.5 MB stripped.

$ FILES="$(find . -type f -executable -exec file {} \; | grep "not stripped" | sed -e 's/: .*//')"
$ du -h --total $FILES | tail -n 1
4.0M    total
$ strip $FILES
$ du -h --total $FILES | tail -n 1
2.5M    total

comment:21 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201501 added; TorBrowserTeam201412 removed

comment:22 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201502 MikePerry201502 added; TorBrowserTeam201501 removed

comment:23 Changed 4 years ago by mikeperry

Owner: changed from dcf to mikeperry

comment:24 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201503 added; TorBrowserTeam201502 removed

comment:25 Changed 4 years ago by mikeperry

Keywords: MikePerry201504 TorBrowserTeam201504 tbb-4.5-alpha added; MikePerry201502 TorBrowserTeam201503 removed

comment:26 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201505 added; TorBrowserTeam201504 tbb-4.5-alpha removed

Actually, the go and python binding stuff makes me nervous here. We shouldn't risk this on 4.5-stable, especially if I'm the one doing it. We should reconsider this at some point during the ff38 rebase.

comment:27 in reply to:  26 Changed 4 years ago by dcf

Replying to mikeperry:

Actually, the go and python binding stuff makes me nervous here. We shouldn't risk this on 4.5-stable, especially if I'm the one doing it. We should reconsider this at some point during the ff38 rebase.

The go stuff is done and merged since a long time ago: #12649.

comment:28 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201505 removed

comment:29 Changed 17 months ago by teor

Severity: Normal

Set all open tickets without a severity to "Normal"

comment:30 Changed 6 months ago by gk

Component: Applications/Tor bundles/installationApplications/Tor Browser
Keywords: tbb-rbm added; MikePerry201504 removed
Note: See TracTickets for help on using tickets.