Provide stack smashing protection for mingw-clang builds

Currently with our mingw-w64/gcc builds we provide among other things stack smashing protection (SSP). We want to do the same for mingw-w64/clang builds.

https://bugzilla.mozilla.org/show_bug.cgi?id=1511073 is the upstream bug. https://bugzilla.mozilla.org/show_bug.cgi?id=1503589#c19 has a good summary to get us started (pointing e.g. to https://github.com/mstorsjo/llvm-mingw/blob/master/build-libssp.sh)

added GeorgKoppen201910 TorBrowserTeam201910R actualpoints::1.5 component::applications/tor browser owner::tbb-team points::1 priority::high resolution::fixed severity::normal status::closed tbb-9.0 tbb-rbm type::enhancement labels

Moving my tickets to Feb

Trac:
Keywords: GeorgKoppen201901 deleted, GeorgKoppen201902 added

Moving tickets to February.

Trac:
Keywords: TorBrowserTeam201901 deleted, TorBrowserTeam201902 added

Trac:
Priority: Medium to High

Moving my tickets to March.

Trac:
Keywords: TorBrowserTeam201902 deleted, TorBrowserTeam201903 added

Now for my keyword.

Trac:
Keywords: GeorgKoppen201902 deleted, GeorgKoppen201903 added

Moving tickets to April.

Trac:
Keywords: TorBrowserTeam201903 deleted, TorBrowserTeam201904 added

Moving my tickets for April

Trac:
Keywords: GeorgKoppen201903 deleted, GeorgKoppen201904 added

Moving tickets to May

Trac:
Keywords: TorBrowserTeam201904 deleted, TorBrowserTeam201905 added

Move my tickets.

Trac:
Keywords: GeorgKoppen201904 deleted, GeorgKoppen201905 added

Moving tickets to June

Trac:
Keywords: TorBrowserTeam201905 deleted, TorBrowserTeam201906 added

Moving my tickets to June

Trac:
Keywords: GeorgKoppen201905 deleted, GeorgKoppen201906 added

Moving my tickets to July.

Trac:
Keywords: GeorgKoppen201906 deleted, GeorgKoppen201907 added

Moving tickets to July

Trac:
Keywords: TorBrowserTeam201906 deleted, TorBrowserTeam201907 added

Moving more tickets to August

Trac:
Keywords: TorBrowserTeam201907 deleted, TorBrowserTeam201908 added

Move my tickets.

Trac:
Keywords: GeorgKoppen201907 deleted, GeorgKoppen201908 added

Trac:
Parent: #28238 (moved) to #30322 (moved)

Moving tickets to September

Trac:
Keywords: TorBrowserTeam201908 deleted, TorBrowserTeam201909 added

Trac:
Keywords: N/A deleted, tbb-9.0-must-alpha added

Trac:
Points: N/A to 1

https://bugzilla.mozilla.org/show_bug.cgi?id=1492225

I guess what we can do, instead of building libssp with build-libssp.sh in a separate step, is to just use the static libraries we build for mingw-w64 anyway.

FWIW, starting from GCC 8, there is --disable-libssp configure time option, because "On many targets library support is provided by the C library instead."

Replying to gk:

I guess what we can do, instead of building libssp with build-libssp.sh in a separate step, is to just use the static libraries we build for mingw-w64 anyway.

That approach is not working very well it seems. I used my bug_29013 branch for tor-browser and bug_29013_v2 for tor-browser-build.

First of all for some reason we need the *.dll as well in the Browser dir (I thought ssp got statically linked in) but copying it over results in crashes both on 32bit and 64bit versions during start-up.

More digging is needed here.

o_0 and libcxx?

Replying to gk:

Replying to gk:

I guess what we can do, instead of building libssp with build-libssp.sh in a separate step, is to just use the static libraries we build for mingw-w64 anyway.

That approach is not working very well it seems. I used my bug_29013 branch for tor-browser and bug_29013_v2 for tor-browser-build.

First of all for some reason we need the *.dll as well in the Browser dir (I thought ssp got statically linked in) but copying it over results in crashes both on 32bit and 64bit versions during start-up.

More digging is needed here.

Using the llvm-mingw approach results in the same problems. I guess I need to look closer in a debugger what's crashing here. Might help to solve #31546 (moved) first, though.

I might not get to that in time...

Trac:
Keywords: tbb-9.0-must-alpha deleted, tbb-9.0 added
Parent: #30322 (moved) to N/A

So, not adding libssp.dll.a solves actually both issues I had (the dynamically linking and the crashes). I might be tempted to pick this up to get it still into 9.0 or 9.5a1 if the former is too risky. :) (thanks again, Martin)

Trac:
Keywords: N/A deleted, TorBrowserTeam201910 added

Trac:
Keywords: TorBrowserTeam201909 deleted, N/A added

Trac:
Keywords: GeorgKoppen201908 deleted, GeorgKoppen201910 added

Replying to gk:

So, not adding libssp.dll.a solves actually both issues I had (the dynamically linking and the crashes). I might be tempted to pick this up to get it still into 9.0 or 9.5a1 if the former is too risky. :) (thanks again, Martin)

Here we are. bug_29013_v5 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_29013_v5&id=562fe8f1df0de8912c9eaf7ff8c3b4d989d4aa01) in my tor-browser-build repo and bug_29013_v2 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_29013_v2&id=cb5ccc9e3b50e9d37a4c7a34a0c81418df38adfe) in my tor-browser one.

The first patch is using our own mingw-w64 which we build anyway and is just copying the .a libs over. The tor-browser patch essentially backs out the special treatment of mingw-w64-clang.

Trac:
Actualpoints: N/A to 1.5
Status: new to needs_review
Keywords: TorBrowserTeam201910 deleted, TorBrowserTeam201910R added

FWIW: I am not really sure how to test that for Windows stack canaries are embedded in the binaries but I can see that the linker is at least loading libssp.a for ___stack_chk_fail during linking.

Is there any advantage of this approach vs. SSPStrong (via UCRT?) enabled by default in clang-cl?

Replying to cypherpunks:

Is there any advantage of this approach vs. SSPStrong (via UCRT?) enabled by default in clang-cl?

I don't know to be honest. I feel we are on the safer side following llvm-mingw here as that approach seems to be working. I'd be happy looking over a patch for your approach, though.

How does it work for e.g. libcxx if it is not enabled globally? llvm-mingw seems to be made for UCRT explicitly, so it can follow clang-cl's defaults and avoid dependency on GCC. Talk to Martin about it.

The two patches look good to me. I also checked that a 32bit and 64bit build with those two patches are still working for me on a Windows 7 VM.

I cherry-picked the tor-browser commit as 4b8a33af9610111d87dc5a901d06bcc20f1cc7b0, and merged the tor-browser-build commit with effde40cc5643080d45670d889a2fd81a6d39c67.

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

closed

changed time estimate to 8h

added 12h of time spent

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