[tor-bugs] #25573 [Core Tor/Tor]: Track half-closed stream IDs

[Tor Bug Tracker & Wiki]

#25573: Track half-closed stream IDs
------------------------------------------------+--------------------------
 Reporter:  mikeperry                           |          Owner:  (none)
     Type:  defect                              |         Status:
                                                |  needs_revision
 Priority:  Medium                              |      Milestone:  Tor:
                                                |  0.3.4.x-final
Component:  Core Tor/Tor                        |        Version:
 Severity:  Normal                              |     Resolution:
 Keywords:  guard-discovery-stats 034-backport  |  Actual Points:
Parent ID:  #25574                              |         Points:
 Reviewer:  teor                                |        Sponsor:
                                                |  SponsorV-can
------------------------------------------------+--------------------------

Comment (by mikeperry):

 Replying to [comment:15 teor]:
 > The branch looks good now.
 >
 > Here are my remaining minor concerns:
 > * Please make check-spaces

 Fixed, sorry about that.

 > * Do you test what happens when stream ids wrap? Do you think that's
 important now with all the other changes?

 There are a couple of different types of "wrapping". One is if we pick the
 initial stream_id for a circuit near UINT16_MAX, and that wraps around.
 This wrap will happen in get_unique_stream_id_by_circ(), which I don't
 currently test. I can easily test this, and I will in a fixup. (I also
 just realized I forgot to switch to bsearch in
 get_unique_stream_id_by_circ()).

 The other way that they can "wrap" is if more than 65536 streams total are
 in the opened list and/or the half-closed list. In this case, by
 definition all stream IDs will be taken, so get_unique_stream_id_by_circ()
 should fail upon new stream creation. I do not test the consequences of
 this failure (there are 3 places in the codebase that check for it), but
 failure means that the circuit is marked unusable for new streams, and the
 stream is retried. I can test get_unique_stream_id_by_circ() with a full
 half-closed array and ensure it fails in this case. Going farther out than
 one call layer seems like test-creep tho.

 connection_half_edge_add() should succeed until the opened list is empty
 and the half-closed list is full of 65536 IDs. By pigeon-hole principle
 with streamid_t (uint16_t), this should then fail due to a duplicate
 stream ID in the list. I already test failure on duplicates.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25573#comment:16>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online