Opened 9 months ago

Closed 2 months ago

#32380 closed defect (wontfix)

Get current Tor Browser code ready for RLBox

Reported by: gk Owned by: gk
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-security, GeorgKoppen202002, TorBrowserTeam202004R
Cc: tbb-team Actual Points:
Parent ID: #32389 Points:
Reviewer: pospeselr Sponsor:

Description

RLBox is written in C++17. We likely hit build issues in an ESR 68 environment with a C++17 requirement. There is the option to adapt RLBox to C++11 code but we should not follow that path but backport build related patches instead.

https://bugzilla.mozilla.org/show_bug.cgi?id=1560664 is a ticket to start.

If that turns out to be too tricky to require C++17 for the whole codebase we can think about enabling it just for the part we want to start sandboxing.

Child Tickets

Change History (14)

comment:1 Changed 9 months ago by gk

Okay, with bug_30380_v3 (https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_32380_v3) I get the following build issue:

 1:06.10 /var/tmp/build/firefox-301a46905ed1/gfx/thebes/gfxFcPlatformFontList.cpp:495:18: error: value of type 'tainted_opaque<gr_face *, RLBoxThebes_T_Sbx>' (aka 'tainted_opaque<gr_face *, rlbox::rlbox_noop_sandbox>') is not contextually convertible to 'bool'
 1:06.10   if (mHBFace || mGrFace) {
 1:06.10                  ^~~~~~~
 1:06.10 /var/tmp/build/firefox-301a46905ed1/gfx/thebes/gfxFcPlatformFontList.cpp:495:15: error: invalid operands to binary expression ('hb_face_t *' and 'tainted_opaque<gr_face *, RLBoxThebes_T_Sbx>' (aka 'tainted_opaque<gr_face *, rlbox::rlbox_noop_sandbox>'))
 1:06.10   if (mHBFace || mGrFace) {
 1:06.10       ~~~~~~~ ^  ~~~~~~~
 1:06.10 /var/tmp/build/firefox-301a46905ed1/obj-x86_64-pc-linux-gnu/dist/include/mozilla/rlbox/rlbox.hpp:793:1: note: candidate template ignored: could not match 'tainted_base_impl' against 'tainted_opaque'
 1:06.11 BooleanBinaryOpWrappedRhs(||);
 1:06.11 ^
 1:06.11 /var/tmp/build/firefox-301a46905ed1/obj-x86_64-pc-linux-gnu/dist/include/mozilla/rlbox/rlbox.hpp:747:25: note: expanded from macro 'BooleanBinaryOpWrappedRhs'
 1:06.11   inline constexpr auto operator opSymbol(                                     \
 1:06.11                         ^
 1:06.11 /var/tmp/build/firefox-301a46905ed1/obj-x86_64-pc-linux-gnu/dist/include/mozilla/rlbox/rlbox.hpp:793:1: note: candidate template ignored: could not match 'tainted_base_impl' against 'tainted_opaque'
 1:06.11 /var/tmp/build/firefox-301a46905ed1/obj-x86_64-pc-linux-gnu/dist/include/mozilla/rlbox/rlbox.hpp:765:25: note: expanded from macro 'BooleanBinaryOpWrappedRhs'
 1:06.11   inline constexpr auto operator opSymbol(                                     \
 1:06.11                         ^
 1:06.86 2 errors generated.
 1:06.88 make[1]: *** [gfxFcPlatformFontList.o] Error 1

Our toolchain is fine in that it can compile the respective code from the canonical try push. It seems we have the problem that the patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1547063 did not land on ESR 68.

comment:2 Changed 8 months ago by gk

Keywords: GeorgKoppen201912 added; GeorgKoppen201911 removed

Moving my tickets to December.

comment:3 Changed 7 months ago by gk

Keywords: GeorgKoppen202001 added; GeorgKoppen201912 removed

No December anymore.

comment:4 Changed 6 months ago by gk

Keywords: GeorgKoppen202002 added; GeorgKoppen202001 removed

Move my tickets to Feb 2020.

comment:5 Changed 5 months ago by gk

Keywords: TorBrowserTeam202002R added
Status: newneeds_review

Okay, bug_32380_v11 (https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_32380_v11) has all the backports and changes we need for review. I took the following Mozilla bugs/changes into account as they landed on mozilla-central (oldest first, commit hashes refer to gecko-dev commits):

1566284 (31a26205a532f66c01e10fa6eb9dcc85ecb61165) x
1582009 (1e4332353e918b930027bcc7d918c4ff022b27cd) x (can skip)
1582205 (9e9bc3134700671fd761aa0a7e10d10e29727e5e) x
1571559 (bc51cbb9da1a3d6f651a229946417a234d6df4ac) x
1584011 (18f49450b79e6d9fa69412dddd873f9eaa6faf4d) x (can skip)
1575985 (bb7e97ff6a4801ed5762d39a52587a984077639c) x (part 2)
1584000 (286ed3b72c4c9e10930d99aad500c4cd2e206875) x
1582195 (0bc04914ab1429003703d5320e59471ce5a5a1ae) (okay to leave out)
1593844 (a1a8c1c228879b39e7dd2248a53efd8735a41c18) x
1576054 (c9a639872e389173f85035d27613dcf2d3369d62) x
1572619 (1f74a23938f50a0b08dec695c8045f508818f56d) x
1576051 (06e7d3e8d0c0463984890687b9b1b2f4fcd74424) x (can skip)
1597150 (eb4e9962a840aa278fc9c4bb69a8f94b26e3823b) x (can skip)
1582192 (5d35f602aa3ddec593864c54f807c7348af2f9cc) (okay to leave out)
1594552 (08c0c800638a26fc38208428c74f4fcf25335f05) x
1596475 (cbe14085a5a142ba4a36c1d87eea49ba5e6bb2e1) x
1599235 (149113d525b59f157d2fe4ab451e8fb3b0d60bea) (okay to leave out)
1599264 (938a5f9a2edba2a83c2b4236dba263031387ab10) (okay to leave out)
1596219 (0ab8b21981bb07c6d34a6c0d996a2a9a041913bd) (okay to leave out)
1599607 (93d3e01a912fb7b28ad8bbb88a8a177b22cef7de) (okay to leave out)
1594867 (d5eb7d0ea59bb655dca470d3e4aadc6bad76432e) x
1600697 (54c1252a7c58a339aa2cee42e30c924cac999439) (okay to leave out)
1600520 (8bc5f7c9dd7cdebdd5284212d9fa0b432c105642) x
1600664 (64c581817a18b8b6546f5c0a4196bca0a5739844) (okay to leave out)
1601407 (5aa8a29e6a70fd7bad5a01eda79c575a80e070a9) x
1601064 (9c9c54555743a9698e10db1f8aa31989d558c00d) (okay to leave out)
1601448 (6999ee5c427048194c7ddec134ac4939d81d3147) (okay to leave out)
1601753 (d43df94c5e93ac016b6a175fbaefe3a7cd7be2d7) x
1603657 (b90fd668217e7ad2acf390e8e1c53305ef05a142) x (probably can skip?)
1603658, 1604123 (7376850e9548266813756f51f581e46a2a34db15) x (can skip)
1566288 (c9ff62b7ef2dc90b621eba9a911601dfb2e6fd46) x
1569369 (bfd31f93d39ecf2bab68376abb10fe5b8f2c87c6) x
1576052 (a3f185ef1b2ed85f7b581456e33468c19487ef50) x
1605215 (eb3bb8c9566b63a8d3359f1c599dd7f2c31cf293) x
1605215 (6476df37aa9cb2f017a30135fa39882a36a2a4e3) x
1575985 (3201996fbadd40bf19dd6d0c4068b1ee304a02cb) x (part 1)
1605537 (6005490cacb8734a81d84ef4af60a7868da7dff6) x
1605537 (2408890a7d4a62bb2da50cc3956fbc4f10e6fad1) x
1605416 (de094fc617bcac170a699342cf06024af9dd17a0) x
1605944 (ff79c2e00a4f7c9a0fb759ba306a047b29fed57d) x (can skip)
1606625 (d98002172b0fa3d3a9dd7f458d39c385f84d91e8) x
1606739 (a95697631f69fdb1cd2ca8a5024a14099a6668eb) x
1606981 (c723d9bc1fe871c6a489d6aae35b6e33c1ed7a2f) x
1569370 (92e5beb60add0356928c7f82f6696cdeb57f4113) (okay to leave out)
1608326 (patch 1 + 2 + 3)                          (okay to leave out)

"x" denotes all the commits I included. "(okay to leave out)" is basically for those that change things in Mozilla's way of providing binaries like lucetc in their taskcluster setup, which we don't need (we build our stuff in tor-browser-build). "(can skip)" are excluded to reduce the pile of commits as much as possible by skipping intermediate repo updates as they landed on mozilla-central. We use the final one instead, so that we are landing on the right revisions without including the intermediary steps.

I needed to add two additional commits on top of those to fix two compilation bustages: one because of a bug in rlbox_lucet_sandbox uncovered by our toolchain and the other is a backport of a fix that landed in bug 1595617. The latter, however, does not apply cleanly as it is based on a series of patches which landed in bug 1547603, which we very likely don't want to backport. My testing suggests we are good, but we should double-check that or mabye just provide an alternative workaround instead of backporting the fix for bug 1595617.

Last edited 5 months ago by gk (previous) (diff)

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

Replying to gk:

the other is a backport of a fix that landed in bug 1595617. The latter, however, does not apply cleanly as it is based on a series of patches which landed in bug 1547603, which we very likely don't want to backport. My testing suggests we are good, but we should double-check that or mabye just provide an alternative workaround instead of backporting the fix for bug 1595617.

Turns out that was due to me not backporting the fix for 1566288 properly. Thanks to Shravan's help I can avoid backporting the fix for bug 1595617 and thus save one commit. bug_32380_v12 (https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_32380_v12) has the fixups.

comment:7 Changed 5 months ago by gk

Cc: tbb-team added
Owner: changed from tbb-team to gk
Status: needs_reviewassigned

comment:8 Changed 5 months ago by gk

Status: assignedneeds_review

comment:9 Changed 5 months ago by gk

I just pushed bug_32380_v13 which contains an additional patch (the one for bug 1607278), which I forgot and I think we should have.

Taking the list of commits from above it is placed between the ones for bug 1606625 and 1606739, like so:

1606625 (d98002172b0fa3d3a9dd7f458d39c385f84d91e8) x
1607278 (b0977482bc37d5c703bda5df5e72fd1e15b95a20) x
1606739 (a95697631f69fdb1cd2ca8a5024a14099a6668eb) x

comment:10 Changed 5 months ago by gk

Parent ID: #32379#32389

comment:11 Changed 5 months ago by pili

Keywords: TorBrowserTeam202003R added; TorBrowserTeam202002R removed

We are no longer in February moving reviews

comment:12 Changed 5 months ago by sysrqb

Reviewer: pospeselr

Adding pospeselr as a reviewer, for when he has some cycles for it.

comment:13 Changed 4 months ago by pili

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202003R removed

We are no longer in March

comment:14 Changed 2 months ago by gk

Resolution: wontfix
Status: needs_reviewclosed

It pains me to do this, but we won't get to that before switching to ESR 78. :(

Note: See TracTickets for help on using tickets.