Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
---+---
 Reporter:  arthuredelstein|  Owner:  tbb-team
 Type:  defect | Status:
   |  needs_revision
 Priority:  Medium |  Milestone:
Component:  Applications/Tor Browser   |Version:
 Severity:  Normal | Resolution:
 Keywords:  TorBrowserTeam201711, tbb-rbm  |  Actual Points:
Parent ID:  #21448 | Points:
 Reviewer: |Sponsor:
---+---

Comment (by tom):

 Replying to [comment:13 cypherpunks]:
 > What about `--icf=all` automatically? https://github.com/llvm/llvm-
 project/blob/d0f63f83e7c5c6fc11e964f848d1496234695182/lld/MinGW/Driver.cpp#L265

 Haven't heard of it; but https://clang.llvm.org/docs/UsersManual.html says
 that the arguements needed for ICF to work (-faddrsig) are ELF only...


 > > --forceinteg - not applicablt to clang/lld
 > What do you mean? Just disabled by default: https://github.com/llvm
 /llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1507

 Ahhah; I was wrong. So it looks like this sets
 IMAGE_DLLCHARACTERISTICS_FORCE_INTEGRITY which requires a file be signed
 before it's loaded.

 Frankly it seems kind of useless to me, an attacker who can modify the dll
 would invalidate the signature; but they could just strip the signature
 and the unset the flag. But if it cost nothing, I'd say sure, flip it: but
 I'm not sure which Tor Browser releases we Authenticode sign; which this
 would require.

 > > --no-seh - set by lld automatically ​https://reviews.llvm.org/D41252
 (but this would be good to confirm manually
 > What about `--safeseh` automatically? https://github.com/llvm/llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1617

 Oh good catch: on by default except for MinGW.  We should investigate why
 that is and if we can enable it.


 > > --tsaware - I'm not sure but I really hope that this is completely
 unneeded by now.
 > Because it is enabled and should be enabled by default, you mean?
 https://github.com/llvm/llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1513

 https://docs.microsoft.com/en-us/cpp/build/reference/tsaware-create-
 terminal-server-aware-application?view=vs-2019 "When an application is not
 Terminal Server aware (also known as a legacy application), Terminal
 Server makes certain modifications to the legacy application to make it
 work properly in a multiuser environment. For example, Terminal Server
 will create a virtual Windows folder, such that each user gets a Windows
 folder instead of getting the system's Windows directory. This gives users
 access to their own INI files. In addition, Terminal Server makes some
 adjustments to the registry for a legacy application. These modifications
 slow the loading of the legacy application on Terminal Server."

 I had hoped that all this nonsense was not needed/performed in Windows 10
 or at least the compiler set the flag automatically. The code makes it
 seem like it does not; but I can't find the flag in Firefox's code, which
 implies that it would not be setting it either...

 More investigation needed, specifically what Firefox sets and if this has
 any effect on Windows 7+

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
---+---
 Reporter:  arthuredelstein|  Owner:  tbb-team
 Type:  defect | Status:
   |  needs_revision
 Priority:  Medium |  Milestone:
Component:  Applications/Tor Browser   |Version:
 Severity:  Normal | Resolution:
 Keywords:  TorBrowserTeam201711, tbb-rbm  |  Actual Points:
Parent ID:  #21448 | Points:
 Reviewer: |Sponsor:
---+---

Comment (by cypherpunks):

 What about `--icf=all` automatically? https://github.com/llvm/llvm-
 project/blob/d0f63f83e7c5c6fc11e964f848d1496234695182/lld/MinGW/Driver.cpp#L265
 > --forceinteg - not applicablt to clang/lld
 What do you mean? Just disabled by default: https://github.com/llvm/llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1507
 > --no-seh - set by lld automatically ​https://reviews.llvm.org/D41252
 (but this would be good to confirm manually
 What about `--safeseh` automatically? https://github.com/llvm/llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1617
 > --tsaware - I'm not sure but I really hope that this is completely
 unneeded by now.
 Because it is enabled and should be enabled by default, you mean?
 https://github.com/llvm/llvm-
 project/blob/ee6fbebbaff5af0a0fbe58a0e33ef191340223ea/lld/COFF/Driver.cpp#L1513

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
---+---
 Reporter:  arthuredelstein|  Owner:  tbb-team
 Type:  defect | Status:
   |  needs_revision
 Priority:  Medium |  Milestone:
Component:  Applications/Tor Browser   |Version:
 Severity:  Normal | Resolution:
 Keywords:  TorBrowserTeam201711, tbb-rbm  |  Actual Points:
Parent ID:  #21448 | Points:
 Reviewer: |Sponsor:
---+---

Comment (by tom):

 Werror=format-security - enabled on non-Windows,
 https://bugzilla.mozilla.org/show_bug.cgi?id=1578619

 _FORTIFY_SOURCE - enabled but mingw-clang doesn't support it (yet. it's a
 WIP)

 -fno-delete-null-pointer-checks
 https://bugzilla.mozilla.org/show_bug.cgi?id=1578617

 -Wl,--enable-reloc-section - AFAICT unneeded for mingw-clang, since ASLR
 is working

 --image-base - AFAICT not needed on modern systems

 large-address-aware - enabled for mingw-clang

 --forceinteg - not applicablt to clang/lld

 --no-seh - set by lld automatically https://reviews.llvm.org/D41252 (but
 this would be good to confirm manually

 --tsaware - I'm not sure but I really hope that this is completely
 unneeded by now.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
---+---
 Reporter:  arthuredelstein|  Owner:  tbb-team
 Type:  defect | Status:
   |  needs_revision
 Priority:  Medium |  Milestone:
Component:  Applications/Tor Browser   |Version:
 Severity:  Normal | Resolution:
 Keywords:  TorBrowserTeam201711, tbb-rbm  |  Actual Points:
Parent ID:  #21448 | Points:
 Reviewer: |Sponsor:
---+---
Changes (by gk):

 * keywords:  TorBrowserTeam201711 => TorBrowserTeam201711, tbb-rbm


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201707  |  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+

Comment (by cypherpunks):

 Replying to [comment:1 arthuredelstein]:
 > Here's my patch for review. I would suggest we adding it after we have
 transitioned to rbm builds:
 > https://github.com/arthuredelstein/tor-browser-build/commit/23024
 Some kind of wrappers... whether they applied to all TBB parts?
 What to include (by order):
 $COMPILER_WARNINGS (like `-Werror=format-security`)
 $COMPILER_OPTIONS (like `-D_FORTIFY_SOURCE=2`)
 $COMPILER_OPTIMIZATIONS (like `-fno-delete-null-pointer-checks`)
 $LINKER_FLAGS
 `-Wl,--enable-reloc-section` - its name is awful (as awful as that this
 bug is still present). There should be no such flag as this is a part of
 `-Wl,--dynamicbase` by meaning. Firefox has import table, but TBB hasn't.
 There should be a better way to make the toolchain work properly, than a
 specific hack which can't get upstreamed.

 Other flags:
 `-Wl,--image-base,0x1000` to force relocations.
 `-Wl,--large-address-aware` is always set by the compiler driver (e.g.
 Cygwin gcc). MinGW too (usually yes)? If so, no need for #22477.
 `-Wl,--forceinteg` - Better than nothing. Code integrity checking, while
 no signatures.
 `-Wl,--no-seh` - that's the only part of SafeSEH GCC supports, see
 ticket:20322#comment:3. Upstream to https://dxr.mozilla.org/mozilla-
 esr52/source/old-configure.in#1218
 (`-Wl,--tsaware` - Terminal Server aware - is for upstream only.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201707  |  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+

Comment (by cypherpunks):

 Replying to [comment:1 arthuredelstein]:
 > However, a Windows Tor Browser built with this patch (using `-fstack-
 protector-all`) doesn't seem subjectively slower to me, so I would suggest
 trying this on the alpha, at least until we have a solution for `-fstack-
 protector-strong` on mingw-w64.
 Also you can copy https://dxr.mozilla.org/mozilla-esr52/source/old-
 configure.in#957 to `*-mingw*)` section to gain parity with Linux.

 Replying to [comment:4 gk]:
 > I tested `-fstack-protector-strong` on top of the latest `tor-browser-
 bundle` commit. And the compilation worked as expected. Is that a `tor-
 browser-build` issue? Or maybe the GCC version bump (tor 5.4.0) resolved
 this problem?
 tor 5.4.0 from 2540 :) Try with `--disable-auto-import` for fun :)
 > Regarding fortify source: Have you checked whether the `_chk` part is
 actually there after compiling with `-D_FORTIFY_SOURCE=2`? Because it does
 not seem to be the case. Doing a
 > {{{
 > i686-w64-mingw32-nm -C firefox.exe | grep strcpy
 > }}}
 > after compiling with the flags in your patch does only give ma a
 > {{{
 > 0041b3f4 I _imp__strcpy
 > 00413320 T strcpy
 > }}}
 > (Note: In order to check it the way I did you need to compile the
 browser part with `--disable-strip` and `--disable-install-strip`)
 >
 > Assuming I am not mistaken then the likely root cause of this problem is
 a GCC bug which the RedHat people are tracking in
 https://bugzilla.redhat.com/show_bug.cgi?id=1324759.
 This is https://bugzilla.mozilla.org/show_bug.cgi?id=1359908

 You also need something to:
 1. check your flags passed and applied properly
 2. check features compiled properly
 3. check features works properly

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201707  |  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+
Changes (by gk):

 * keywords:  TorBrowserTeam201707R => TorBrowserTeam201707
 * cc: boklm (added)
 * status:  needs_review => needs_revision


Comment:

 I tested `-fstack-protector-strong` on top of the latest `tor-browser-
 bundle` commit. And the compilation worked as expected. Is that a `tor-
 browser-build` issue? Or maybe the GCC version bump (tor 5.4.0) resolved
 this problem?

 Regarding fortify source: Have you checked whether the `_chk` part is
 actually there after compiling with `-D_FORTIFY_SOURCE=2`? Because it does
 not seem to be the case. Doing a
 {{{
 i686-w64-mingw32-nm -C firefox.exe | grep strcpy
 }}}
 after compiling with the flags in your patch does only give ma a
 {{{
 0041b3f4 I _imp__strcpy
 00413320 T strcpy
 }}}
 (Note: In order to check it the way I did you need to compile the browser
 part with `--disable-strip` and `--disable-install-strip`)

 Assuming I am not mistaken then the likely root cause of this problem is a
 GCC bug which the RedHat people are tracking in
 https://bugzilla.redhat.com/show_bug.cgi?id=1324759.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+--
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201707R |  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+--
Changes (by arthuredelstein):

 * keywords:  TorBrowserTeam20170724R => TorBrowserTeam201707R


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+--
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam20170724R   |  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+--
Changes (by arthuredelstein):

 * keywords:   => TorBrowserTeam20170724R
 * status:  new => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #23024 [Applications/Tor Browser]: Flags to increase hardening on Windows

[Tor Bug Tracker & Wiki]

#23024: Flags to increase hardening on Windows
--+--
 Reporter:  arthuredelstein   |  Owner:  tbb-team
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #21448| Points:
 Reviewer:|Sponsor:
--+--

Comment (by arthuredelstein):

 Here's my patch for review. I would suggest we adding it after we have
 transitioned to rbm builds:
 https://github.com/arthuredelstein/tor-browser/commit/23024

 I tried building TBB with `-fstack-protector-strong` on mingw-w64 but so
 far ran into
 [https://sourceforge.net/p/mingw-w64/discussion/723798/thread/de524c41/#e4f9
 errors]. However, a Windows Tor Browser built with this patch (using
 `-fstack-protector-all`) doesn't seem subjectively slower to me, so I
 would suggest trying this on the alpha, at least until we have a solution
 for `-fstack-protector-strong` on mingw-w64.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs