Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#20195 closed defect (fixed)

HTTPS Everywhere's SSL Observatory code doesn't honor domain isolation.

Reported by: yawning Owned by: legind
Priority: High Milestone:
Component: HTTPS Everywhere/EFF-HTTPS Everywhere Version:
Severity: Major Keywords: tbb-linkability
Cc: gk, brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The HTTPs request made to check.torproject.org as part of startup doesn't use domain isolation at all.

How to reproduce:

  1. Monitor the SOCKS traffic (or circuit list).
  2. Start Tor Browser, get to the about:tor page.
  3. Gasp in horror.

Tested with 6.0.5.

Child Tickets

Attachments (1)

bug20195.pcapng (5.7 KB) - added by yawning 3 years ago.
Example pcap.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 3 years ago by gk

Component: Applications/TorbuttonApplications/Tor Browser
Keywords: tbb-torbutton added
Resolution: not a bug
Status: newclosed

That's because there is no URL bar domain to isolate the request to. There are a few internal ones which are put on the catch-all circuit which gets rotated every ten minutes. Thus, this seems not to be a bug to me.

comment:2 Changed 3 years ago by yawning

If the request used the catch-all circuit, I wouldn't have opened the bug. The catch-all circuit uses domain isolation (so the rotation works). This does not.

comment:3 Changed 3 years ago by yawning

Resolution: not a bug
Status: closedreopened

comment:4 Changed 3 years ago by gk

Keywords: tbb-linkability added

Okay, then I am confused. Could you provide a log explaining what happened and what should have happened instead?

comment:5 Changed 3 years ago by yawning

There is no log. The only reason I caught this was because I was dumping the SOCKS request bodies with my sandbox code.

What happens is, the internal check uses a connection to check.torproject.org to validate that tor is working. That request does not send a SOCKS username/password for isolation. If it were using domain isolation correctly, the catchall circuit (Username: ---unknown---) would be used.

The easiest way to reproduce this would probably be using a system tor instance and wireshark.

Changed 3 years ago by yawning

Attachment: bug20195.pcapng added

Example pcap.

comment:6 Changed 3 years ago by yawning

Interestingly enough this only happens with the release channel, and not the latest alpha.

So it's probably ok to eventually close as "fixed" as long as someone knows why the behavior changed, especially considering that it shouldn't affect most people.

comment:7 in reply to:  5 Changed 3 years ago by gk

Replying to yawning:

There is no log. The only reason I caught this was because I was dumping the SOCKS request bodies with my sandbox code.

You mean setting extensions.torbutton.loglevel to 3 and extensions.torbutton.logmethod to 0 does not show this request in your terminal while all the other ones are visible? Is that fixed in the alphas as well?

comment:8 Changed 3 years ago by gk

Owner: set to tbb-team
Status: reopenedassigned

comment:9 Changed 3 years ago by yawning

Ah there. Looks like the domain isolation code is getting called, but wireshark doesn't lie.
FWIW, I took the pcaps without my sandboxing stuff in play, and the behavior is consistent.

[09-22 08:31:02] Torbutton INFO: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozIThirdPartyUtil.getFirstPartyURIFromChannel]
[09-22 08:31:02] Torbutton INFO: tor SOCKS isolation catchall: https://check.torproject.org/?TorButton=true#0.99726695529027310.5190771246311907 via --unknown--:0
[09-22 08:31:02] Torbutton WARN: no SOCKS credentials found for current document.

comment:10 in reply to:  9 ; Changed 2 years ago by gk

Cc: gk added
Component: Applications/Tor BrowserHTTPS Everywhere/EFF-HTTPS Everywhere
Keywords: tbb-torbutton removed
Owner: changed from tbb-team to legind
Priority: MediumHigh
Severity: NormalMajor
Summary: torbutton-torCheckService doesn't honor domain isolation.HTTPS Everywhere's SSL Observatory code doesn't honor domain isolation.

Replying to yawning:

Ah there. Looks like the domain isolation code is getting called, but wireshark doesn't lie.
FWIW, I took the pcaps without my sandboxing stuff in play, and the behavior is consistent.

[09-22 08:31:02] Torbutton INFO: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozIThirdPartyUtil.getFirstPartyURIFromChannel]
[09-22 08:31:02] Torbutton INFO: tor SOCKS isolation catchall: https://check.torproject.org/?TorButton=true#0.99726695529027310.5190771246311907 via --unknown--:0
[09-22 08:31:02] Torbutton WARN: no SOCKS credentials found for current document.

Alright, so here is what is going on. First, do you see the weird floating point number thing appended to the # in the check.torproject.org URL? Torbutton does not do such things. It turns out this is part if the HTTPS-Everywhere SSL Observatory code where it checks whether Tor is available and to use (e.g. for submissions). As a sidenode: one does see the issue in the Tor Browser log as well without pcaps. It is visible there that the request does not go over the catch-all circuit but rather is put on one without any username/password isolation at all.

So, even if the request comes from HTTPS-Everywhere why is it not isolated like any other internal request? Looking at Necko logs shows that indeed things are wrong, already at the nsHTTPConnectionManager level:

-957356288[7fd9e2dabb60]: nsHttpConnectionMgr::OnMsgSpeculativeConnect [ci=.S....check.torproject.org:443 (socks:127.0.0.1:9150)[:]]

Before and after the colon in the brackets should be the respective username and password.

Looking closer at the SSL Observatory code shows it is bypassing our proxy filter respoonsible for domain isolation in case the CSRF nonce is found in the path:

    if (isSubmission || testingForTor) {
      if (aURI.path.search(this.csrf_nonce+"$") != -1) {

        this.log(INFO, "Got observatory url + nonce: "+aURI.spec);
        var proxy_settings = null;
        var proxy = null;

        // Send it through tor by creating an nsIProxy instance
        // for the torbutton proxy settings.
        try {
          proxy_settings = this.getProxySettings(testingForTor);
          proxy = this.pps.newProxyInfo(
            proxy_settings.type,
            proxy_settings.host,
            proxy_settings.port,
            Ci.nsIProxyInfo.TRANSPARENT_PROXY_RESOLVES_HOST,
            0xFFFFFFFF, null);
        } catch(e) {
          this.log(WARN, "Error specifying proxy for observatory: "+e);
        }

        this.log(INFO, "Specifying proxy: "+proxy);

        // TODO: Use new identity or socks u/p to ensure we get a unique
        // tor circuit for this request
        return proxy;
      }
    }

FWIW: the reason you thought this was fixed in the alpha was due to HTTPS-Everywhere 5.2.4 that was the latest version at that time. In that version SSL Observatory code was broken due to #19996. This got fixed in 5.2.5 which makes this issue visible in the alphas again.

Last edited 2 years ago by gk (previous) (diff)

comment:11 in reply to:  10 ; Changed 2 years ago by yawning

Replying to gk:

Alright, so here is what is going on. First, do you see the weird floating point number thing appended to the # in the check.torproject.org URL? Torbutton does not do such things. It turns out this is part if the HTTPS-Everywhere SSL Observatory code where it checks whether Tor is available and to use (e.g. for submissions). As a sidenode: one does see the issue in the Tor Browser log as well without pcaps. It is visible there that the request does not go over the catch-all circuit but rather is put on one without any username/password isolation at all.

Nice catch.

Is there a ticket for "SSL Observatory makes at least one network request on startup to check proxy settings, even if it's disabled"? If "Use the Observatory?" isn't checked, this request shouldn't be made at all, but as it stands absolutely everyone (with working SSL-Observatory) is hitting this bug.

comment:12 Changed 2 years ago by mcs

Cc: brade mcs added

comment:13 in reply to:  11 Changed 2 years ago by gk

Replying to yawning:

Is there a ticket for "SSL Observatory makes at least one network request on startup to check proxy settings, even if it's disabled"? If "Use the Observatory?" isn't checked, this request shouldn't be made at all, but as it stands absolutely everyone (with working SSL-Observatory) is hitting this bug.

Not yet, but I guess a good solution for this one would solve that problem as well.

So the following things could be done:

If you want to check whether Tor is enabled check for an existing Torbutton component. No request getting sent to check.torproject.org is necessary in this scenario. And if such a component is found let Tor Browser handle the traffic (i.e. don't mess with proxy settings) as Torbutton alone should not be functional anymore (i.e. you can be sure the user has a Tor Browser).

That would be sufficient for us. But what if you don't find an existing Torbutton component? Still, I think, there should not be any check if the SSL Observatory is disabled. Not sure, though, if you want to support Firefox users that have a tor running somewhere but are not using Tor Browser.

comment:14 in reply to:  10 Changed 2 years ago by bugzilla

Replying to gk:

[09-22 08:31:02] Torbutton WARN: no SOCKS credentials found for current document.

TBB has a lot of places with this warning, e.g. while fetching RecommendedTBBVersions, so what?

Alright, so here is what is going on. First, do you see the weird floating point number thing appended to the # in the check.torproject.org URL?

FP with two dots? He-he.

Torbutton does not do such things.

But it looked like yours :)

It is visible there that the request does not go over the catch-all circuit but rather is put on one without any username/password isolation at all.

If getinfo circuit-status doesn't lie, the request does go over the catch-all circuit, even though without any username/password isolation at all.

This is another one recent crap from HTTPSE: it look like it was developed as a virus or without security audit at all. Is it suitable for TBB?

(Also it is doing 3 requests in a row to check.torproject.org, on NEWNYM too.)

comment:15 Changed 2 years ago by legind

The suggestion in https://trac.torproject.org/projects/tor/ticket/20195#comment:13 has been implemented in https://github.com/EFForg/https-everywhere/pull/7342:

This resolves the issue in https://trac.torproject.org/projects/tor/ticket/20195 where the SSL Observatory proxy checking code and submissions were bypassing domain isolation. That code was a relic from the TorButton days.

Now, check.torproject.org is no longer accessed when we're using Tor Browser, we assume successful Tor access. In this case, we let TB transparently proxy for us, instead of accessing the Tor Browser proxy settings directly.

This can be tested within HTTPS Everywhere by running:

test/tor-browser.sh PATH_TO_TOR_ARCHIVE

I'll close this once the fix is merged on our side.

comment:16 Changed 2 years ago by legind

Resolution: fixed
Status: assignedclosed

comment:17 Changed 2 years ago by bugzilla

And what to do with all other Torbutton WARN: no SOCKS credentials found for current document.?

Note: See TracTickets for help on using tickets.