[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #24031 [Core Tor/Tor]: Protover.rs could use a better algorithm
#24031: Protover.rs could use a better algorithm
---------------------------------------------+-----------------------------
Reporter: nickm | Owner: isis
Type: defect | Status:
| needs_review
Priority: Very High | Milestone: Tor:
| 0.3.3.x-final
Component: Core Tor/Tor | Version: Tor:
| 0.3.3.1-alpha
Severity: Normal | Resolution:
Keywords: rust 033-must protover security | Actual Points: 5
Parent ID: | Points: 1
Reviewer: | Sponsor: SponsorM-
| can
---------------------------------------------+-----------------------------
Changes (by isis):
* status: needs_revision => needs_review
* actualpoints: => 5
* priority: High => Very High
* version: => Tor: 0.3.3.1-alpha
* sponsor: => SponsorM-can
* milestone: Tor: 0.3.4.x-final => Tor: 0.3.3.x-final
* keywords: rust 033-must protover => rust 033-must protover security
Comment:
I've cleaned up (mostly! sorry sorry!) my code into more understandable
commits in my `bug24031_r4` branch.
The changes to the unittests might be a little hard to follow, that commit
is not quite pleasant (although the integration test changes in that same
commit should be simple to follow to see where/where behaviour has
changed, so I'd argue that the internal unittests changing precariously
isn't as much of an issue?). If test changes (or any changes) are
difficult for the reviewer to understand, please feel free to ask
questions, or make me split that commit up better.
The other thing is that one test I've added intentionally fails:
`protover_all_supported_should_include_version_we_actually_do_support`.
The behavioural difference is this: if we take a protover string
`"Link=3-999"` and ask if it is supported, when `"Link=1-5"` ''is''
supported:
* the C version returns `"Link=3-999"` (which is, IMHO, wrong, since we
''do'' support 3, 4, and 5.
* the Rust version returns `"Link=6-999"`, which I believe to be the
correct implementation of the spec?
Upping the priority because potential DoS.
Upping the actual points to 5, although only ~3 were spent on this ticket,
and another 2 finding/fixing related bugs elsewhere.
Marking as SponsorM-can, although that may be wrong; feel free to correct.
Changing the version to reflect inclusion in 0.3.3, along with the
appropriate keywords.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24031#comment:10>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs