Opened 13 months ago

Closed 9 months ago

#18762 closed defect (worksforme)

implement first-party isolation for OCSP generated by speculative connect

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-linkability
Cc: gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

#13670.2 has a TODO that maybe needs to be implemented. Although is it possible that we have speculative connect disabled?

Child Tickets

Change History (5)

comment:1 follow-up: Changed 13 months ago by gk

  • Cc gk added
  • Keywords tbb-linkability added

I don't think we have this disabled.

comment:2 in reply to: ↑ 1 Changed 13 months ago by mcs

Replying to gk:

I don't think we have this disabled.

It seems like we talked about this before, but I cannot remember for sure. Why have we not disabled speculative connect? I guess it is a privacy vs. performance tradeoff.

comment:3 follow-up: Changed 10 months ago by gk

I was looking a bit closer at a thing which was nagging me while doing the review for #16998. There is

    // Check for proxy information. If there is a proxy configured then a
    // speculative connect should not be performed because the potential
    // reward is slim with tcp peers closely located to the browser.

and this piece of code in nsIOService.cpp:

NS_IMETHODIMP
IOServiceProxyCallback::OnProxyAvailable(nsICancelable *request, nsIChannel *channel,
                                         nsIProxyInfo *pi, nsresult status)
{
    // Checking proxy status for speculative connect
    nsAutoCString type;
    if (NS_SUCCEEDED(status) && pi &&
        NS_SUCCEEDED(pi->GetType(type)) &&
        !type.EqualsLiteral("direct")) {
        // proxies dont do speculative connect
        return NS_OK;
    }

And it seems to me we hit this code path with Tor Browser. Retesting #16324 by looking at tcpdump output confirms my suspicion as well: there is no network activity visible even if Torbutton claims isolation is happening.

So, it seems to me that at least this ticket and #16324 can be closed. I am not sure yet what this means for #16998. I guess, we should not have been worried by it because there is no speculative connect happening anyway?

comment:4 in reply to: ↑ 3 Changed 9 months ago by arthuredelstein

Replying to gk:

I was looking a bit closer at a thing which was nagging me while doing the review for #16998. There is

    // Check for proxy information. If there is a proxy configured then a
    // speculative connect should not be performed because the potential
    // reward is slim with tcp peers closely located to the browser.

and this piece of code in nsIOService.cpp:

NS_IMETHODIMP
IOServiceProxyCallback::OnProxyAvailable(nsICancelable *request, nsIChannel *channel,
                                         nsIProxyInfo *pi, nsresult status)
{
    // Checking proxy status for speculative connect
    nsAutoCString type;
    if (NS_SUCCEEDED(status) && pi &&
        NS_SUCCEEDED(pi->GetType(type)) &&
        !type.EqualsLiteral("direct")) {
        // proxies dont do speculative connect
        return NS_OK;
    }

And it seems to me we hit this code path with Tor Browser. Retesting #16324 by looking at tcpdump output confirms my suspicion as well: there is no network activity visible even if Torbutton claims isolation is happening.

So, it seems to me that at least this ticket and #16324 can be closed. I am not sure yet what this means for #16998. I guess, we should not have been worried by it because there is no speculative connect happening anyway?

I watch for STREAM events in the browser console and I can confirm that the speculative connects don't seem to be causing any network activity. So I agree that this ticket and #16324 can be closed. However, I did notice that under some special situations, a favicon is displayed in the searchbar popup which causes a connection over the catchall circuit; see #19741.

comment:5 Changed 9 months ago by gk

  • Resolution set to worksforme
  • Status changed from new to closed

Setting this to WORKSFORME. However, this still needs to get done on Mozilla's side when upstreaming our patches.

Note: See TracTickets for help on using tickets.