Skip to content
Snippets Groups Projects
New look: Off

First party isolation of Page Info

    • Closed (moved) Issue created by Arthur Edelstein @arthuredelstein

    First party isolation is not properly enforce in the Page Info window in ESR52. First reported in https://trac.torproject.org/projects/tor/ticket/21762?replyto=4#comment:4

    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items ...

    • Activity

    • Arthur Edelstein added TorBrowserTeam201705 component::applications/tor browser owner::arthuredelstein priority::high resolution::fixed severity::normal status::closed tbb-7.0-must tbb-linkability type::defect labels

      added TorBrowserTeam201705 component::applications/tor browser owner::arthuredelstein priority::high resolution::fixed severity::normal status::closed tbb-7.0-must tbb-linkability type::defect labels

    • A
      Arthur Edelstein @arthuredelstein ·
      Author

      Here's a patch for review: https://github.com/arthuredelstein/tor-browser/commit/22327

      Trac:
      Status: new to needs_review
      Keywords: tbb-7.0-must TorBrowserTeam201705 deleted, tbb-7.0-must TorBrowserTeam201705R added

    • Georg Koppen
      Georg Koppen @gk ·

      Hm, looking at that branch it seems we need 32287fdd336a9ed2bbd52e71595fcb7e61394dc5 as well? Or am I missing something?

      Trac:
      Status: needs_review to needs_information

    • Georg Koppen
      Georg Koppen @gk ·

      Replying to gk:

      Hm, looking at that branch it seems we need 32287fdd336a9ed2bbd52e71595fcb7e61394dc5 as well? Or am I missing something?

      It seems you are missing the

      +#include "nsSerializationHelper.h"

      in that backported patch? I somehow doubt it would compile without it looking at the code.

    • A
      Arthur Edelstein @arthuredelstein ·
      Author

      Replying to gk:

      Replying to gk:

      Hm, looking at that branch it seems we need 32287fdd336a9ed2bbd52e71595fcb7e61394dc5 as well? Or am I missing something?

      Yes, sorry, I should have also mentioned that I backported that patch.

      It seems you are missing the {{{ +#include "nsSerializationHelper.h" }}} in that backported patch? I somehow doubt it would compile without it looking at the code.

      You're right. The branch builds, but the #include line somehow migrated from Ehsan's patch onto my patch. I have put it back into his patch and I have rebased the two patches onto the latest tor-browser-52.1.1esr-7.0-1 branch. Here is the new result: https://github.com/arthuredelstein/tor-browser/commits/22327+1

      I rebuilt this new branch and ran the following manual test:

      1. add torbutton
      2. set extensions.torbutton.loglevel to 3
      3. open the Browser Console
      4. visit http://html5demo.com/video
      5. open Page Info
      6. observe "tor SOCKS" messages in the Browser Console

      When I ran this test, videos and images in the "media preview" box were correctly isolated to the content document's first party.

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Status: needs_information to needs_review

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Cc: N/A to mcs, brade

    • Mark Smith
      Mark Smith @mcs ·

      Kathy and I reviewed the changes and they look okay to us. But since we are not very familiar with the new way of doing isolation, we applied the patches and tried to run the revised code. Unfortunately, at least with our OSX debug build, invoking page info (Cmd+I) always causes an assertion failure:

      Assertion failure: pb == (doa.mPrivateBrowsingId > 0), at /.../tor-browser/netwerk/base/LoadContextInfo.cpp:147

      To make things more challenging, my lldb seems to have forgotten how to use symbols. I am not sure if we should be concerned about the assertion failure or not, but we will try again with an optimized build.

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Priority: Medium to High

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Owner: tbb-team to arthuredelstein
      Status: needs_review to assigned
      Cc: mcs, brade to mcs, brade, tbb-team

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Status: assigned to needs_review

    • Georg Koppen
      Georg Koppen @gk ·

      Here is the stacktrace in case it helps:

      Thread 1 "firefox" received signal SIGSEGV, Segmentation fault.
0x00007fffecc13b77 in mozilla::net::GetLoadContextInfo (aLoadContext=
    0x7fffbd7e3c00, aIsAnonymous=false)
    at /home/thomas/Arbeit/Tor/tor-browser/netwerk/base/LoadContextInfo.cpp:147
147	  MOZ_ASSERT(pb == (doa.mPrivateBrowsingId > 0));
(gdb bt
#0  0x00007fffecc13b77 in mozilla::net::GetLoadContextInfo (
    aLoadContext=0x7fffbd7e3c00, aIsAnonymous=false)
    at /home/thomas/Arbeit/Tor/tor-browser/netwerk/base/LoadContextInfo.cpp:147
#1  0x00007fffecc137fe in mozilla::net::LoadContextInfoFactory::FromLoadContext (
    this=0x7fffc938e500, aLoadContext=0x7fffbd7e3c00, aAnonymous=false, 
    _retval=0x7fffffffa988)
    at /home/thomas/Arbeit/Tor/tor-browser/netwerk/base/LoadContextInfo.cpp:100
#2  0x00007fffecb72740 in NS_InvokeByIndex (that=0x7fffc938e500, methodIndex=7, 
    paramCount=3, params=0x7fffffffa958)
    at /home/thomas/Arbeit/Tor/tor-browser/xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:182
#3  0x00007fffed9b6fd9 in CallMethodHelper::Invoke (this=0x7fffffffa910)
    at /home/thomas/Arbeit/Tor/tor-browser/js/xpconnect/src/XPCWrappedNative.cpp:2058
#4  0x00007fffed9b4c0e in CallMethodHelper::Call (this=0x7fffffffa910)
    at /home/thomas/Arbeit/Tor/tor-browser/js/xpconnect/src/XPCWrappedNative.cpp:1377
#5  0x00007fffed999d11 in XPCWrappedNative::CallMethod (ccx=..., 
    mode=XPCWrappedNative::CALL_METHOD)
    at /home/thomas/Arbeit/Tor/tor-browser/js/xpconnect/src/XPCWrappedNative.cpp:1344
#6  0x00007fffed9a2884 in XPC_WN_CallMethod (cx=0x7fffdff65000, argc=2, 
    vp=0x7fffd9d2d098)
    at /home/thomas/Arbeit/Tor/tor-browser/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:999
#7  0x00007ffff2404ac6 in js::CallJSNative (cx=0x7fffdff65000, 
    native=0x7fffed9a257b <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/thomas/Arbeit/Tor/tor-browser/js/src/jscntxtinlines.h:239
#8  0x00007ffff23d0a3e in js::InternalCallOrConstruct (cx=0x7fffdff65000, 
    args=..., construct=js::NO_CONSTRUCT)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:459
#9  0x00007ffff23d0d82 in InternalCall (cx=0x7fffdff65000, args=...)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:504
#10 0x00007ffff23d0dac in js::CallFromStack (cx=0x7fffdff65000, args=...)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:510
#11 0x00007ffff23de3f9 in Interpret (cx=0x7fffdff65000, state=...)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:2922
#12 0x00007ffff23d068c in js::RunScript (cx=0x7fffdff65000, state=...)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:405
#13 0x00007ffff23d1b66 in js::ExecuteKernel (cx=0x7fffdff65000, script=..., 
    envChainArg=..., newTargetValue=..., evalInFrame=..., result=0x7fffffffc350)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:686
#14 0x00007ffff23d1e56 in js::Execute (cx=0x7fffdff65000, script=..., 
    envChainArg=..., rval=0x7fffffffc350)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/vm/Interpreter.cpp:719
#15 0x00007ffff213d788 in ExecuteScript (cx=0x7fffdff65000, scope=..., script=..., rval=0x7fffffffc350)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/jsapi.cpp:4350
#16 0x00007ffff213dd65 in JS::CloneAndExecuteScript (cx=0x7fffdff65000, 
    scriptArg=..., rval=...)
    at /home/thomas/Arbeit/Tor/tor-browser/js/src/jsapi.cpp:4413
#17 0x00007fffefdf03cc in mozilla::dom::XULDocument::ExecuteScript (
    this=0x7fffc413b800, aScript=0x7fffc8cfe290)
    at /home/thomas/Arbeit/Tor/tor-browser/dom/xul/XULDocument.cpp:3523
#18 0x00007fffefdefcbb in mozilla::dom::XULDocument::OnScriptCompileComplete (
    this=0x7fffc413b800, aScript=0x7fffc3b2d120, aStatus=nsresult::NS_OK)
    at /home/thomas/Arbeit/Tor/tor-browser/dom/xul/XULDocument.cpp:3410
#19 0x00007fffefe05212 in NotifyOffThreadScriptCompletedRunnable::Run (
    this=0x7fffbd999f40)
    at /home/thomas/Arbeit/Tor/tor-browser/dom/xul/nsXULElement.cpp:2797
#20 0x00007fffecb4432b in nsThread::ProcessNextEvent (this=0x7ffff6823600, 
    aMayWait=false, aResult=0x7fffffffc79f)
    at /home/thomas/Arbeit/Tor/tor-browser/xpcom/threads/nsThread.cpp:1216
#21 0x00007fffecbb1089 in NS_ProcessNextEvent (aThread=0x7ffff6823600, 
    aMayWait=false)
    at /home/thomas/Arbeit/Tor/tor-browser/xpcom/glue/nsThreadUtils.cpp:361
#22 0x00007fffed241843 in mozilla::ipc::MessagePump::Run (this=0x7ffff6958580, 
    aDelegate=0x7ffff6845c00)
    at /home/thomas/Arbeit/Tor/tor-browser/ipc/glue/MessagePump.cpp:96
#23 0x00007fffed1b1bfb in MessageLoop::RunInternal (this=0x7ffff6845c00)
    at /home/thomas/Arbeit/Tor/tor-browser/ipc/chromium/src/base/message_loop.cc:232
#24 0x00007fffed1b1b8e in MessageLoop::RunHandler (this=0x7ffff6845c00)
    at /home/thomas/Arbeit/Tor/tor-browser/ipc/chromium/src/base/message_loop.cc:225
#25 0x00007fffed1b1b67 in MessageLoop::Run (this=0x7ffff6845c00)
    at /home/thomas/Arbeit/Tor/tor-browser/ipc/chromium/src/base/message_loop.cc:205
#26 0x00007fffeff6494c in nsBaseAppShell::Run (this=0x7fffd989e5a0)
    at /home/thomas/Arbeit/Tor/tor-browser/widget/nsBaseAppShell.cpp:156
#27 0x00007ffff0c44175 in nsAppStartup::Run (this=0x7fffd986b450)
    at /home/thomas/Arbeit/Tor/tor-browser/toolkit/components/startup/nsAppStartup.cpp:283
#28 0x00007ffff0d1c50d in XREMain::XRE_mainRun (this=0x7fffffffcc60)
    at /home/thomas/Arbeit/Tor/tor-browser/toolkit/xre/nsAppRunner.cpp:5028
#29 0x00007ffff0d1cabd in XREMain::XRE_main (this=0x7fffffffcc60, argc=3, 
    argv=0x7fffffffe0a8, aAppData=0x7fffffffde80)
    at /home/thomas/Arbeit/Tor/tor-browser/toolkit/xre/nsAppRunner.cpp:5161
#30 0x00007ffff0d1cd36 in XRE_main (argc=3, argv=0x7fffffffe0a8, 
    aAppData=0x7fffffffde80, aFlags=0)
    at /home/thomas/Arbeit/Tor/tor-browser/toolkit/xre/nsAppRunner.cpp:5252
#31 0x000055555555c0a0 in do_main (argc=3, argv=0x7fffffffe0a8, 
    envp=0x7fffffffe0c8, xreDirectory=0x7ffff685ac00)
    at /home/thomas/Arbeit/Tor/tor-browser/browser/app/nsBrowserApp.cpp:282
#32 0x000055555555c386 in main (argc=3, argv=0x7fffffffe0a8, envp=0x7fffffffe0c8)
    at /home/thomas/Arbeit/Tor/tor-browser/browser/app/nsBrowserApp.cpp:415

      FWIW it happens with an optimized build as well.

      Trac:
      Keywords: tbb-7.0-must TorBrowserTeam201705R deleted, tbb-7.0-must TorBrowserTeam201705 added
      Status: needs_review to needs_revision

    • A
      Arthur Edelstein @arthuredelstein ·
      Author

      Replying to gk:

      Here is the stacktrace in case it helps: {{{ Thread 1 "firefox" received signal SIGSEGV, Segmentation fault. 0x00007fffecc13b77 in mozilla::net::GetLoadContextInfo (aLoadContext= 0x7fffbd7e3c00, aIsAnonymous=false) [...] }}}

      FWIW it happens with an optimized build as well.

      It turns out this assertion failure happens in a debug build when you open "Page Info" even without the patches in comment:4. I'm currently trying to track down the cause of the assertion by binary search. But my feeling is that the fix for this ticket shouldn't be delayed for what appears to be an unrelated issue.

    • A
      Arthur Edelstein @arthuredelstein ·
      Author

      I opened a ticket for the assert: #22462 (moved)

      Trac:
      Status: needs_revision to needs_review

    • Georg Koppen
      Georg Koppen @gk ·

      Replying to arthuredelstein:

      Replying to gk:

      Here is the stacktrace in case it helps: {{{ Thread 1 "firefox" received signal SIGSEGV, Segmentation fault. 0x00007fffecc13b77 in mozilla::net::GetLoadContextInfo (aLoadContext= 0x7fffbd7e3c00, aIsAnonymous=false) [...] }}}

      FWIW it happens with an optimized build as well.

      It turns out this assertion failure happens in a debug build when you open "Page Info" even without the patches in comment:4. I'm currently trying to track down the cause of the assertion by binary search. But my feeling is that the fix for this ticket shouldn't be delayed for what appears to be an unrelated issue.

      Good catch, I agree.

    • Georg Koppen
      Georg Koppen @gk ·

      Okay, I looked over it. Both the backport and the patch on top of it look good to me but I have to echo what mcs said that I am not very familiar with the new way of isolation either. That said, I compiled and tested the code and it works fine for me. Let it get into our next nightly build.

      Applied to tor-browser-52.1.0esr-7.0-2 (commits ef0f9d150d4210bed5b517763cf8872e7f729649 and af4e5af7cfa3cfd73ff53fd6ec1be4e97e43f630) and tor-browser-51.1.1esr-7.0-1 (commit 24862c15f6f719cd05a42d784ed7a7b7bb05c567 and 2d4d11e1e6bad5f384c409fa0391e97a9c202439).

      Thanks!

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

    • A
      Arthur Edelstein @arthuredelstein ·
      Author

      Trac:
      Keywords: tbb-7.0-must TorBrowserTeam201705 deleted, tbb-7.0-must tbb-first-party TorBrowserTeam201705 added

    • Georg Koppen
      Georg Koppen @gk ·

      Trac:
      Keywords: tbb-7.0-must tbb-first-party TorBrowserTeam201705 deleted, tbb-7.0-must tbb-linkability TorBrowserTeam201705 added

    • Trac closed

      closed

    • cypherpunks mentioned in issue #22451 (moved)

      mentioned in issue #22451 (moved)

    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    • Loading
    Please register or sign in to reply