[tor-bugs] #8786 [Tor]: Add extra-info line that tracks the number of consensus downloads of each pluggable transports
Tor Bug Tracker & Wiki
blackhole at torproject.org
Wed Feb 24 11:39:51 UTC 2016
#8786: Add extra-info line that tracks the number of consensus downloads of each
pluggable transports
-------------------------------------------------+-------------------------
Reporter: asn | Owner: karsten
Type: enhancement | Status:
Priority: Low | needs_revision
Component: Tor | Milestone: Tor:
Severity: Normal | 0.2.8.x-final
Keywords: pt, tor-bridge, flashproxy, | Version:
026-triaged-1, 026-deferrable, | Resolution:
027-triaged-1-out | Actual Points:
Parent ID: | Points:
Sponsor: |
-------------------------------------------------+-------------------------
Comment (by karsten):
nickm, thanks a lot for the review! I worked on those suggestion and
pushed them to the same branch. But let me start with another fix that I
started working on before reading your review:
'''tl;wr:''' My branch (commit cda77b4) is broken, though only in a way
that it logs something inaccurate, not in a harmful way. I believe that
neither the bridges' configurations nor the extended OR port
implementation are to blame. I wrote a fix for this.
After adding extensive logging to all code having to do with dirreq_id and
letting it run for hours, I found the issue with my branch (commit
cda77b4). Here's what happened:
- The client sends a BEGIN_DIR cell on its circuit to the bridge.
- The bridge assigns dirreq_id 9779 to the circuit and the circuit's
p_chan, opens an edge connection and a dir connection, and assigns the
same dirreq_id 9779 to both new connections. It then sends back a cell
(CONNECTED?) to the client and waits for the client to send its GET
request via that circuit.
- The client sends another BEGIN_DIR cell on the same circuit.
- The bridge assigns dirreq_id 9780 to the circuit and its p_chan,
'''overwriting previous value 9779'''. It also creates a new edge
connection and a new dir connection and sets their dirreq_id to 9780, but
that's irrelevant here.
- The client sends the GET request to the dir connection with dirreq_id
9779, which is a request for a consensus.
- The bridge attempts to find circuit with dirreq_id 9779 to find out the
transport name of its p_chan, but there is no circuit with that dirreq_id
anymore, because it was overwritten with 9780 above. The bridge falls
back to counting this request as coming in via the <OR> transport, which
is wrong. It should probably count it as <??>, but really we should fix
this bug and count it as obfs3 in this case.
- The client sends a second GET request for the bridge's server
descriptor to the dir connection with dirreq_id 9780.
I'm also pasting the logs below:
{{{
Feb 22 14:34:48.463 [notice] [...]
Feb 22 14:34:48.673 [notice] XXX8786 Assigning new dirreq_id 9779 to
circuit with p_circ_id 3181807694 and non-zero dirreq_id 9776 and to
p_chan with global_identifier 5 and previous dirreq_id 9776.
Feb 22 14:34:48.673 [notice] XXX8786 Assigning dirreq_id 9779 to new edge
connection.
Feb 22 14:34:48.674 [notice] XXX8786 Assigning dirreq_id 9779 to new dir
connection.
Feb 22 14:34:48.674 [notice] XXX8786 Assigning new dirreq_id 9780 to
circuit with p_circ_id 3181807694 and non-zero dirreq_id 9779 and to
p_chan with global_identifier 5 and previous dirreq_id 9779.
Feb 22 14:34:48.674 [notice] XXX8786 Assigning dirreq_id 9780 to new edge
connection.
Feb 22 14:34:48.674 [notice] XXX8786 Assigning dirreq_id 9780 to new dir
connection.
Feb 22 14:34:48.675 [notice] XXX8786 Request for /tor/status-vote/current
/consensus-microdesc/1AFF35+2C2591+E34739.
Feb 22 14:34:48.675 [notice] XXX8786 Counting directory request for /tor
/status-vote/current/consensus-microdesc/1AFF35+2C2591+E34739 with
dirreq_id 9779 as <OR>, because we didn't find a circuit with matching
dirreq_id.
Feb 22 14:34:48.676 [notice] XXX8786 Request for /tor/server/authority.
Feb 22 14:34:53.461 [notice] [...]
}}}
The bug here is that the dirreq_id-assigning code does not take concurrent
directory requests over the same circuit into account. We can expect that
the stats based on dirreq_id, in particular "dirreq-v3-tunneled-dl" lines,
are partially broken, too. Fortunately, we're not using those lines for
anything critical. That's the bad news.
The good news is that I wrote a fix for counting requests by transport.
Please see commit a743f3a in the updated branch task-8786 in my public
repository. The commit message says what was fixed. Pasting here, though
it overlaps with the description above:
{{{
The previous approach to look up the transport name of a directory
request by going through the list of circuits and finding the circuit
with same dirreq_id was broken. That dirreq_id may already have been
overwritten by a newly arriving BEGIN_DIR cell before the HTTP GET
request that we're about to include in the stats arrives and gets
parsed. This issue also affects "dirreq-v3-tunneled-dl" lines, but
fixing those is independent of implementing this new feature.
The new approach is to store the transport name in the locally created
dir connection when processing the BEGIN_DIR cell. That way it cannot
be changed by a subsequent BEGIN_DIR cell on the same circuit. The
downside is a new char* in dir_connection_t, the upside is that we can
avoid iterating over all circuits.
}}}
nickm, I also tried to address your suggestions in subsequent commit
1508787.
I would like to get this branch tested on asn's and mrphs' bridges and on
an IPv6 bridge before getting it merged. It also contains a temp commit
for testing that '''must''' be reverted before merging! Leaving this
ticket at needs_revision until that is all done. Deferring until 0.2.9 is
fine by me.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8786#comment:22>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list