Deal with W^X backport bustage in upcoming ESR release

https://hg.mozilla.org/releases/mozilla-esr45/rev/347c10e4d6d1 backports a security bug fix but is removing the nonWritableJitCode option we use for our W^X patch backport. It might be okay to add that part back as it seems the removal is unrelated. Jan de Mooij writes:

I also removed the nonWritableJitCode option as ESR45 predates W^X and nobody is going to enable that option there.

We should be sure about that though and make sure as well that our backported patches are still working and not causing unpredictable issues. If the risk is too high we need to back out the security improvements, alas.

added TorBrowserTeam201703R component::applications/tor browser owner::arthuredelstein priority::very high resolution::fixed severity::critical status::closed type::task labels

Arthur: you backported the patches back then? Could you take a look at that one?

Trac:
Description: https://hg.mozilla.org/releases/mozilla-esr45/rev/347c10e4d6d1 backports a security bug fix but is removing the nonWritableJitCode option we use for our W^X patch backport. It might be okay to add that part back as it seems the removal is unrelated. Jan de Mooij writes:

I also removed the nonWritableJitCode option as ESR45 predates W^X and nobody is going to enable that option there.

We should be sure about that though and make sure as well that our backported patches are still working and not causing unpredictable issues. If the risk is too high we need to back out the security improvements, alas.

to

https://hg.mozilla.org/releases/mozilla-esr45/rev/347c10e4d6d1 backports a security bug fix but is removing the nonWritableJitCode option we use for our W^X patch backport. It might be okay to add that part back as it seems the removal is unrelated. Jan de Mooij writes:

I also removed the nonWritableJitCode option as ESR45 predates W^X and nobody is going to enable that option there.

We should be sure about that though and make sure as well that our backported patches are still working and not causing unpredictable issues. If the risk is too high we need to back out the security improvements, alas.

Trac:
Owner: tbb-team to arthuredelstein
Status: new to accepted

I've heard nothing back and needed to back the backported patches out for now to get a first build for 6.5.1 going. I think we could rebuild the firefox part if we have updated patches in time, though.

I believe I have finally got it working:

https://github.com/arthuredelstein/tor-browser/commit/21514+4

Thanks to Jan de Mooij for looking it over and helpful advice.

So far I have tested on Linux, but this will need testing on Windows and Mac at least to show that it doesn't cause a crash. I am reasonably confident the code covers all three platforms.

Jan also suggested we could examine the memory maps with and without the patch, using gdb or /proc/pid/maps on Linux and VMMap on Windows. He points out that without the patch we should hopefully observe multiple rwx regions.

Trac:
Status: accepted to needs_review

There are two more backported patches related to the JIT code. The first, 9979ff809363114d1435846a9331c7bcd2490a7c (Bug 1234246), I simply cherry-picked from our previous 45.7 branch. The second, 9c35223e6da4a29f48e1fba8b405aa92525d4bc2 (Bug 1238694) needed to be modified a little to be compatible with the security bug fix in 52419a78cd379b7b253d405c2ec65421210f10cb.

​https://github.com/arthuredelstein/tor-browser/commits/21514+5

Trac:
Cc: arthuredelstein to arthuredelstein, brade, mcs
Keywords: TorBrowserTeam201702 deleted, TorBrowserTeam201703R added

I uploaded test bundles built with Arthur's patches applied:

https://people.torproject.org/~gk/testbuilds/TorBrowser-tbb-nightly-osx64_21514_ALL.dmg https://people.torproject.org/~gk/testbuilds/TorBrowser-tbb-nightly-osx64_21514_ALL.dmg.asc

https://people.torproject.org/~gk/testbuilds/tor-browser-linux32-tbb-nightly_21514_ALL.tar.xz https://people.torproject.org/~gk/testbuilds/tor-browser-linux32-tbb-nightly_21514_ALL.tar.xz.asc

https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-tbb-nightly_21514_ALL.tar.xz.asc https://people.torproject.org/~gk/testbuilds/tor-browser-linux64-tbb-nightly_21514_ALL.tar.xz.asc

https://people.torproject.org/~gk/testbuilds/torbrowser-install-tbb-nightly_21524_ALL.exe https://people.torproject.org/~gk/testbuilds/torbrowser-install-tbb-nightly_21524_ALL.exe.asc

Kathy and I reviewed the patches and they look okay to us. We also ran gk's test bundle on OSX and it seemed to work (we did notice that Unix domain sockets did not work for SOCKS, but that seems like a different problem... maybe a missing patch?)

We could not figure out how to use lldb to examine the memory protection level of the running process (and find the JS-related pages). Maybe if someone can describe how to do that with gdb on Linux we can do something similar on OSX.

Replying to mcs:

Kathy and I reviewed the patches and they look okay to us. We also ran gk's test bundle on OSX and it seemed to work (we did notice that Unix domain sockets did not work for SOCKS, but that seems like a different problem... maybe a missing patch?)

Yes, sorry. I just rebuilt the browser part of nightly builds I had around which contained torbutton master, while the browser part was based on 45.8.0esr-6.5. So, it needs probably a stable torbutton.

I tested my own build of the 21514+5 branch on Linux. Using

cat /proc/$PID/maps | grep rwx

I confirmed that there were no rwx pages.

I also tried reverting the W^X patch and then the same command showed several "rwxp" pages. So it looks like it is working as expected.

I'm not sure yet how to do the same thing on OS X, but the vmmap command line tool looks promising. Windows has VMMap (https://technet.microsoft.com/en-us/sysinternals/vmmap.aspx), which I am going to try next.

I used VMMap on Windows to examine the memory protections of gk's build from comment:8. Good news and bad news. The bad news is there was one 4k page with RWX protection. The good news is that Jan helped me confirm that this block was far from (and therefore not part of) the 128 MB block allocated for the JIT. All the memory in the JIT block was RX or "Reserved".

So I think my JIT W^X patch is also working correctly on Windows. But it would be good to track down the cause of the RWX block. I opened #21617 (moved) to look at this problem.

I used the vmmap command on a macOS Sierra (10.12.3) system to confirm that there are no rwx segments when the patches are present. Without them, I see several such segments (as I do with Firefox ESR 45.x). So I think this is working on OSX as well.

Trac:
Username: tokotoko
Cc: arthuredelstein, brade, mcs to arthuredelstein, brade, mcs, fdsfgs@krutt.org

Alright, thanks for the manual testing. I decided to include the new patch into the upcoming stable series as

1. we retain W^X JIT support that way
2. the patch looks good to all of us
3. the patch looks good to Jan de Mooij
4. the patch contains in large parts code that is in mozilla-central
5. we verified manually that it works on all three platforms
6. we did not encounter any crashes (which would be pretty easy to trigger (at least I'd assume so))

I kept the other two backported patches as well. Those three patches are on tor-browser-45.8.0esr-6.5-2 now as commit ad729fa2597d44b1d128a66d07b89e849e44cb80, a86798d665931a7250d44366faaad812a36d38b2, and f01f8bf4c84420e8f3538a4b9a978a7a2383a8c6.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue #21617 (moved)

moved to tpo/applications/tor-browser#21514 (closed)

mentioned in issue tpo/applications/tor-browser#21617 (closed)