Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20352 closed task (fixed)

Integrate sandboxed Tor Browser into our gitian build system

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-gitian, tbb-sandboxing, TorBrowserTeam201612R, GeorgKoppen201612
Cc: boklm, adrelanos, yawning Actual Points:
Parent ID: #19750 Points:
Reviewer: Sponsor:

Description

We should write a descriptor for building the sandbox code inside our Gitian environment.

Child Tickets

Attachments (2)

Change History (35)

comment:1 Changed 3 years ago by gk

Keywords: GeorgKoppen201611 added; GeorgKoppen201610 removed

Moving my tickets to November.

comment:2 Changed 3 years ago by gk

Keywords: TorBrowserTeam201611 added; TorBrowserTeam201610 removed

Moving tickets over to November.

comment:3 Changed 3 years ago by yawning

These are the relevant versions of the runtime dependencies I need that ship with Debian stable, which is probably the oldest set of packages that are "reasonable" to assume installed.

  • libx11-dev (The calls I use have always been there, and always will).
  • Gtk+ 3.14 - Build assumes this, see the Makefile.
  • libseccomp2 (2.1.1, 2.2.3 in backports).

~The libseccomp bindings I use determine the version of the library at compile time, so if we build against 2.1.1, we will be stuck with the features supported by it. This degrades the effectiveness of the seccomp filters I use somewhat because conditional rules do not work correctly prior to 2.2.1.~ (Edit: This is probably fine, upon close examination, needs testing.)

(Someone should double check that sandboxed-tor-browser built against ancient libs works even if more modern versions are installed, even if it isn't as good as it can be.)

As far as building goes, since I use cgo, Go prior to 1.6.x would probably end badly.

Last edited 3 years ago by yawning (previous) (diff)

comment:4 in reply to:  3 Changed 3 years ago by gk

Replying to yawning:

These are the relevant versions of the runtime dependencies I need that ship with Debian stable, which is probably the oldest set of packages that are "reasonable" to assume installed.

  • libx11-dev (The calls I use have always been there, and always will).
  • Gtk+ 3.14 - Build assumes this, see the Makefile.
  • libseccomp2 (2.1.1, 2.2.3 in backports).

~The libseccomp bindings I use determine the version of the library at compile time, so if we build against 2.1.1, we will be stuck with the features supported by it. This degrades the effectiveness of the seccomp filters I use somewhat because conditional rules do not work correctly prior to 2.2.1.~ (Edit: This is probably fine, upon close examination, needs testing.)

Good!

(Someone should double check that sandboxed-tor-browser built against ancient libs works even if more modern versions are installed, even if it isn't as good as it can be.)

As far as building goes, since I use cgo, Go prior to 1.6.x would probably end badly.

That's not an issue as we need Go 1.7 anyway for other stuff.

comment:5 Changed 3 years ago by gk

The compiler does not like your stub code it seems:

gcc -shared -pthread -Os -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-all -Wstack-protector --param ssp-buffer-size=1 -fPIC -Wall -Werror -Wextra -Wl,-z,relro,-z,now src/tbb_stub/tbb_stub.c -o data/tbb_stub.so
src/tbb_stub/tbb_stub.c: In function 'stub_init':
src/tbb_stub/tbb_stub.c:197:3: error: implicit declaration of function 'secure_getenv' [-Werror=implicit-function-declaration]
src/tbb_stub/tbb_stub.c:197:22: error: initialization makes pointer from integer without a cast [-Werror]
src/tbb_stub/tbb_stub.c:198:24: error: initialization makes pointer from integer without a cast [-Werror]
cc1: all warnings being treated as errors
make: *** [tbb_stub] Error 1

comment:6 in reply to:  3 ; Changed 3 years ago by gk

Replying to yawning:

These are the relevant versions of the runtime dependencies I need that ship with Debian stable, which is probably the oldest set of packages that are "reasonable" to assume installed.

  • libx11-dev (The calls I use have always been there, and always will).
  • Gtk+ 3.14 - Build assumes this, see the Makefile.

Hm. That's a bummer as we don't have this in Debian Wheezy which we use for our Linux builds. I wonder what to do about that as I guess compiling Gtk 3.14 for use on Wheezy is non-trivial...

comment:7 in reply to:  6 Changed 3 years ago by gk

Replying to gk:

Replying to yawning:

These are the relevant versions of the runtime dependencies I need that ship with Debian stable, which is probably the oldest set of packages that are "reasonable" to assume installed.

  • libx11-dev (The calls I use have always been there, and always will).
  • Gtk+ 3.14 - Build assumes this, see the Makefile.

Hm. That's a bummer as we don't have this in Debian Wheezy which we use for our Linux builds. I wonder what to do about that as I guess compiling Gtk 3.14 for use on Wheezy is non-trivial...

Maybe we could try building the sandboxed thing on Debian Jessie instead? It is pretty much self-contained and using a Go built on Wheezy should be not a problem. Plus I somehow doubt there is a pre-Jessie system that has bubblewrap etc.

comment:8 in reply to:  5 Changed 3 years ago by yawning

Replying to gk:

The compiler does not like your stub code it seems:

gcc -shared -pthread -Os -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-all -Wstack-protector --param ssp-buffer-size=1 -fPIC -Wall -Werror -Wextra -Wl,-z,relro,-z,now src/tbb_stub/tbb_stub.c -o data/tbb_stub.so
src/tbb_stub/tbb_stub.c: In function 'stub_init':
src/tbb_stub/tbb_stub.c:197:3: error: implicit declaration of function 'secure_getenv' [-Werror=implicit-function-declaration]
src/tbb_stub/tbb_stub.c:197:22: error: initialization makes pointer from integer without a cast [-Werror]
src/tbb_stub/tbb_stub.c:198:24: error: initialization makes pointer from integer without a cast [-Werror]
cc1: all warnings being treated as errors
make: *** [tbb_stub] Error 1

As discussed on irc:
secure_getenv() first appeared in glibc 2.17.

I could just as easily use getenv(), or pull in the secure_getenv() symbol via dlsym(), but I think that it is reasonable to use calls that have been in glibc since December 2012.

comment:9 Changed 3 years ago by yawning

As an update, the libseccomp dependency (both at compile and runtime) is now x86 (32 bit) only. Not only do I get to shave off a dependency, but the code now uses the same seccomp rule parser/compiler that subgraph does, so it will be easier to keep the Tor Browser whitelist in sync.

comment:10 Changed 3 years ago by gk

After switching to Debian Jessie this is building fine for x86 and x86_64. A complication that turned up is gb not caring about deterministic builds:

-00b58fb0: 702e 676f 0000 2f74 6d70 2f67 6234 3537  p.go../tmp/gb457
-00b58fc0: 3938 3136 3238 2f67 6974 6875 622e 636f  981628/github.co
+00b58fb0: 702e 676f 0000 2f74 6d70 2f67 6231 3538  p.go../tmp/gb158
+00b58fc0: 3532 3636 3333 2f67 6974 6875 622e 636f  526633/github.co

But that's not the only issue it seems: :(

-000001b0: 0300 0000 474e 5500 c50f b1fd 318e 7ecb  ....GNU.....1.~.
-000001c0: a61b 168e a2ec 32b6 0391 4dfb 0400 0000  ......2...M.....
-000001d0: 1400 0000 0300 0000 474e 5500 da00 559f  ........GNU...U.
-000001e0: d622 3c47 638e 0f4f 4ad1 3d2d 7e93 91e8  ."<Gc..OJ.=-~...
+000001b0: 0300 0000 474e 5500 0e56 fb68 814e 9f95  ....GNU..V.h.N..
+000001c0: b667 fc03 2349 a2d9 b0e8 922a 0400 0000  .g..#I.....*....
+000001d0: 1400 0000 0300 0000 474e 5500 6ece 2446  ........GNU.n.$F
+000001e0: 2790 d47f cfae 4b3b 3066 f673 b22b 146c  '.....K;0f.s.+.l

comment:11 Changed 3 years ago by gk

Cc: boklm added
Keywords: TorBrowserTeam201611R added; TorBrowserTeam201611 removed
Status: newneeds_review

I have bug_20352_v4 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_20352_v4 commits 03852126e8abb782c850dc9230eaf98e81851366 and b7f4ab23fe3e508fee6eab106632679992ca4688) which is up for review (while I look at the reproducibility issues). boklm: Could you please have a look and test it on your machine(s)? One thing I am particularly interested in is your report about whether killing the old qemu process (from the previous build step) still works as it used to. It turns out on my machine I have to wait about an hour [sic!] until the process gets killed. If that is not just my machine I'd be happy to hear about possible workarounds or fixes.

comment:13 in reply to:  12 ; Changed 3 years ago by mcs

Replying to gk:

Preliminary builds for testing can be found at:
...

One small data point: on an up-to-date Ubuntu stable (16.10) system, I saw this error message:

Failed to launch Tor Browser: sandbox: bubblewrap appears to be older than 0.1.3;, you MUST upgrade.

I installed a newer bubblewrap and was able to get up and running. I did not do a lot of testing, but alpha and release both seemed to work.

comment:14 in reply to:  13 Changed 3 years ago by yawning

Replying to mcs:

One small data point: on an up-to-date Ubuntu stable (16.10) system, I saw this error message:

Failed to launch Tor Browser: sandbox: bubblewrap appears to be older than 0.1.3;, you MUST upgrade.

Yeah. That's intentional. Ubuntu has 0.1.2 (unpatched) which has a rather large security issue. I filed a bug on launchpad that's currently being ignored, but past that I am opting for caution rather than compatibility on this one, especially since I can't easily disambiguate the unpatched vs patched versions of 0.1.2.

Sorry. :(

I've tested this on, or have had victims test on:

  • Arch Linux (amd64)
  • Debian stable (amd64/x86)
  • Fedora 24/25 (amd64)
  • OpenSuse Tumbleweed (amd64)

All of those have up to date packages available (and in the case of Fedora 25, it's preinstalled, giving the best out of the box experience).

comment:15 in reply to:  10 Changed 3 years ago by gk

Replying to gk:

After switching to Debian Jessie this is building fine for x86 and x86_64. A complication that turned up is gb not caring about deterministic builds:

I moved that out into #20758.

comment:16 in reply to:  11 ; Changed 3 years ago by boklm

Replying to gk:

I have bug_20352_v4 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_20352_v4 commits 03852126e8abb782c850dc9230eaf98e81851366 and b7f4ab23fe3e508fee6eab106632679992ca4688) which is up for review (while I look at the reproducibility issues). boklm: Could you please have a look and test it on your machine(s)? One thing I am particularly interested in is your report about whether killing the old qemu process (from the previous build step) still works as it used to. It turns out on my machine I have to wait about an hour [sic!] until the process gets killed. If that is not just my machine I'd be happy to hear about possible workarounds or fixes.

I tried building that on my machine, but it gets stuck while creating the VMs:

Formatting 'target-jessie-amd64.qcow2', fmt=qcow2 size=17179869184 backing_file='base-jessie-amd64.qcow2' encryption=off cluster_size=65536 lazy_refcounts=off

I can see this qemu process running:

qemu-system-i386 -enable-kvm -m 2000 -smp 2 -drive file=target-jessie-i386.qcow2,cache=writeback,if=virtio -net nic,model=virtio -net user,hostfwd=tcp:127.0.0.1:2223-:22 -vnc 127.0.0.1:16

After stopping the build, killing qemu, and restarting it, it gets stuck again at the same point.

comment:17 in reply to:  16 ; Changed 3 years ago by gk

Replying to boklm:

Replying to gk:

I have bug_20352_v4 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_20352_v4 commits 03852126e8abb782c850dc9230eaf98e81851366 and b7f4ab23fe3e508fee6eab106632679992ca4688) which is up for review (while I look at the reproducibility issues). boklm: Could you please have a look and test it on your machine(s)? One thing I am particularly interested in is your report about whether killing the old qemu process (from the previous build step) still works as it used to. It turns out on my machine I have to wait about an hour [sic!] until the process gets killed. If that is not just my machine I'd be happy to hear about possible workarounds or fixes.

I tried building that on my machine, but it gets stuck while creating the VMs:

Formatting 'target-jessie-amd64.qcow2', fmt=qcow2 size=17179869184 backing_file='base-jessie-amd64.qcow2' encryption=off cluster_size=65536 lazy_refcounts=off

I can see this qemu process running:

qemu-system-i386 -enable-kvm -m 2000 -smp 2 -drive file=target-jessie-i386.qcow2,cache=writeback,if=virtio -net nic,model=virtio -net user,hostfwd=tcp:127.0.0.1:2223-:22 -vnc 127.0.0.1:16

After stopping the build, killing qemu, and restarting it, it gets stuck again at the same point.

That's weird because after the first try you should have a base-jessie-i386.qcow2 and during the second one (after killing qemu) you would create the 64bit one. Running ./make-vms.sh again now should indicate that all images are created. Maybe you are seeing something different like the build is going trying to kill the qemu process dealing with jessie 64bit?

Either way it seems you hit the same problem as I: the images are not shutting down properly. Any idea how to cope with that? Maybe we kill them right away in stop-target until we have a proper fix at least?

comment:18 in reply to:  17 Changed 3 years ago by boklm

Replying to gk:

That's weird because after the first try you should have a base-jessie-i386.qcow2 and during the second one (after killing qemu) you would create the 64bit one. Running ./make-vms.sh again now should indicate that all images are created. Maybe you are seeing something different like the build is going trying to kill the qemu process dealing with jessie 64bit?

After the first try, I cleaned all VM images before relaunching. I was also wondering if the vmclean Makefile rule should be updated to also clean gitian-builder/target-jessie-* files, so I did it manually.

On the third try, without cleaning VM images this time, it now started the building part.

Either way it seems you hit the same problem as I: the images are not shutting down properly. Any idea how to cope with that? Maybe we kill them right away in stop-target until we have a proper fix at least?

I will investigate this.

comment:19 Changed 3 years ago by boklm

I attached a patch for builders/gitian-builder.git which updates libexec/stop-target to use poweroff instead of halt.

It seems that in previous versions of Debian/Ubuntu, halt and poweroff were equivalent, but it is no longer the case in Debian Jessie (maybe after the switch to systemd) where halt shuts down all services but does not turn off power (which results in the qemu process not exiting). Using the poweroff command instead of halt should fix that.

comment:20 Changed 3 years ago by boklm

I also opened a pull request on upstream gitian-builder:
https://github.com/devrandom/gitian-builder/pull/141

comment:21 in reply to:  19 Changed 3 years ago by gk

Replying to boklm:

I attached a patch for builders/gitian-builder.git which updates libexec/stop-target to use poweroff instead of halt.

It seems that in previous versions of Debian/Ubuntu, halt and poweroff were equivalent, but it is no longer the case in Debian Jessie (maybe after the switch to systemd) where halt shuts down all services but does not turn off power (which results in the qemu process not exiting). Using the poweroff command instead of halt should fix that.

Thanks! Works for me and is now commit 0557666cb7d3bf474cb36d5d755438b06af86b7d on tor-browser-builder-4.

comment:23 Changed 3 years ago by adrelanos

Cc: adrelanos added

comment:24 Changed 3 years ago by gk

Keywords: TorBrowserTeam201612R added; TorBrowserTeam201611R removed

Moving our review tickets to December.

comment:25 Changed 3 years ago by yawning

So, I just pushed a huge change to go back to libseccomp2 for all my seccomp needs, primarily so I can filter arguments on x86. This *shouldn't* change anything since we pull it down as a dependency anyway, but if anything breaks there, that would be why.

Debian jessie users will need to fetch libseccomp2 from backports to run the thing in addition to bubblewrap, because the library flat out refuses to filter by conditional on anything pre 2.2.1, but, filtering arguments is kind of important.

comment:26 Changed 3 years ago by yawning

Ugh. Old libseccomp doesn't let me distinguish the version of the shared library at runtime (the version was a bunch of #defines). So the build's broken right now, because libseccomp packages from backports are needed to get something that will function at all, and even then the packages *may* misbehave on jessie (amd64) systems.

This is the upstream issue that's fixed in newer releases: https://github.com/seccomp/libseccomp/commit/b43a7dde03f96ce6a291eb58f620c5d2b7700b51

I'm not sure what the best solution here is out of:

  1. Kludge out the version check and pray that we don't hit the edge case.
  2. Ship pre-generated bpf.
    1. Generated at compile time (adds another step to the build process, the gitian descriptor needs to be modified to pull in backports packages, but the user does not need ibseccomp at all).
    2. Generated by me, embedded in the tree as static assets. The build system won't need libesccomp at all, and neither will the user.
  3. Drop 32 bit intel support and go back to using gosecco.

I'm personally leaning toward option 2a, with a plan to fall back to 2b, since the work involved is comparable.

comment:27 Changed 3 years ago by yawning

Cc: yawning added

I'm personally leaning toward option 2a

Did that. The gitian env needs new libseccomp, because it will compile the bpf filters when you run make. The one in backports is sufficiently recent, newer doesn't hurt.

The build will fail if the library is too old.

Sorry for the extra work, but the seccomp stuff on 32 bit intel is a lot better now.

comment:28 in reply to:  27 ; Changed 3 years ago by gk

Replying to yawning:

I'm personally leaning toward option 2a

Did that. The gitian env needs new libseccomp, because it will compile the bpf filters when you run make. The one in backports is sufficiently recent, newer doesn't hurt.

The build will fail if the library is too old.

Sorry for the extra work, but the seccomp stuff on 32 bit intel is a lot better now.

Okay. boklm: I have bug_20352 (https://gitweb.torproject.org/user/gk/gitian-builder.git/commit/?h=bug_20352&id=eab8568ddcb3c4ea8b7b1d8b6321946f68b2f10e) in my gitian-builder repo enabling backports. It seems that might not be enough, though, as one has to request using backports for a package explicitely (IIRC). I try to test that later but my build/test machine is currently occupied with other high prio stuff. If you could give that a whirl that would be great.

comment:29 in reply to:  28 Changed 3 years ago by boklm

Replying to gk:

Okay. boklm: I have bug_20352 (https://gitweb.torproject.org/user/gk/gitian-builder.git/commit/?h=bug_20352&id=eab8568ddcb3c4ea8b7b1d8b6321946f68b2f10e) in my gitian-builder repo enabling backports. It seems that might not be enough, though, as one has to request using backports for a package explicitely (IIRC). I try to test that later but my build/test machine is currently occupied with other high prio stuff. If you could give that a whirl that would be great.

I added a patch for gitian-builder.git, adding support for a backports_packages option in the descriptiors (in addition to your patch enabling the repository):
https://trac.torproject.org/projects/tor/attachment/ticket/20352/0001-Bug-20352-add-option-to-install-backports-packages.patch

After making this change to gitian-sandbox.yml, the libseccomp-dev and libseccomp2 packages are now installed from backports:

diff --git a/gitian/descriptors/linux/gitian-sandbox.yml b/gitian/descriptors/linux/gitian-sandbox.yml
index 70b9902..71910c2 100644
--- a/gitian/descriptors/linux/gitian-sandbox.yml
+++ b/gitian/descriptors/linux/gitian-sandbox.yml
@@ -13,9 +13,10 @@ packages:
 # Needed for the sandboxing code
 - "libx11-dev"
 - "pkg-config"
-- "libseccomp-dev"
-- "libseccomp2"
 - "libgtk-3-dev"
+backports_packages:
+  - "libseccomp-dev"
+  - "libseccomp2"
 reference_datetime: "2000-01-01 00:00:00"
 remotes:
 - "url": "https://github.com/pkg/error"

comment:30 Changed 3 years ago by gk

Keywords: GeorgKoppen201612 added; GeorgKoppen201611 removed

Moving my tickets

comment:31 Changed 3 years ago by boklm

The changes in branch bug_20352_v4 look good to me.

comment:32 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

I added the gitian-builder patches to tor-browser-builder-4 as commits afe8247f9fdf561b3e0d7701d2f49ef3bd5e6cb4 and dd26189595159bb8f50185a754dd4d0bc61b5787. I folded the changes you mentioned in comment:29 into my bug_20352_v4 and applied the result to tor-browser-bundle master as commit 522cee6cb9c83be66cc2a6b5bbfefdb3c2bc3217. Thanks!

comment:33 in reply to:  32 Changed 3 years ago by gk

Replying to gk:

I added the gitian-builder patches to tor-browser-builder-4 as commits afe8247f9fdf561b3e0d7701d2f49ef3bd5e6cb4 and dd26189595159bb8f50185a754dd4d0bc61b5787. I folded the changes you mentioned in comment:29 into my bug_20352_v4 and applied the result to tor-browser-bundle master as commit 522cee6cb9c83be66cc2a6b5bbfefdb3c2bc3217. Thanks!

I forgot to push another commit from bug_20352_v4 which I fixed on master (commit af596cb55dbcd300c989d02be7fffd1a16a7fb8a).

Note: See TracTickets for help on using tickets.