Nick Mathewson schreef op 02/01/15 om 15:27:
On Mon, Dec 29, 2014 at 3:33 PM, Tom van der Woerdt info@tvdw.eu wrote:
Sounds good!
I spent some time writing a patch that removes v1 of the link protocol from both the server and client, and so far it seems to work nicely: the code compiles nicely, all test cases pass, and the resulting binary has relayed a few gigabytes of data without any problems.
As I didn't really have a place to put the branch, I uploaded it to Github: https://github.com/TvdW/tor/commits/master
It's a rather large patch, though not as large as the patch that will remove v2 of the protocol. However, before I write that one, can someone please check whether my patch is sane and I'm not violating any standards or policies?
Hi, Tom!
Sorry about the delay; this has been a busy time, between CCC drama and the new year.
A few procedural suggestions:
- Maybe call branches like this something other than "master"? It's
a good idea to have a separate topic branch for each patch series you write, so that the upstream project can merge them independently.
- It's a good idea to link to branches like this from the
appropriate tickets on the bugtracker, so that we can't lose track of them.
Thanks, I'll create an account.
- Every Tuesday on the #tor-dev IRC channel on OFTC, at 1800 UTC,
it's patch workshop time, where a bunch of people get together to review one another's patches. Maybe you'd like to stop by? :)
Sounds fun!
And, some fast notes on the patch itself:
- Actually, it's not that long. "git log -p -b" only says it's 607
lines, which is comparatively easy to review.
- Some of the stuff can't actually get removed yet by the terms of
what we've said we're doing. The V2_CIPHER_LIST, for example, is used to detect Tor versions up through 0.2.3.17-beta. We've only mentioned dropping support for 0.2.2 and earlier, right?. Similarly with the tor_tls_session_secret_cb logic.
Now, maybe we _should_ drop support for versions before 0.2.3.17-beta as well. If so, we can rip out even more code. (And that might be a good idea.) What do people on the list think?
I'd definitely like ripping out more legacy support code :-) According to the consensus, 0.2.3.24-rc is the lowest recommended version right now, and I (mis)interpreted that as the lowest version that must still be supported after I rip out code.
On a related note, I've uploaded a branch of the torspec [1] to reword (most of) the handshake part to match what I hope the code will do as soon as I'm done cleaning. Spec changes may need to go through proposals though, should I add it as one?
Also, in case anyone is wondering why a complete stranger is suddenly so interested in ripping parts out of Tor: I attempted to implement a Tor client and was constantly faced with legacy stuff in the spec. Might as well get rid of it if you don't need it - also makes my client implementation easier.
Tom
[1] https://github.com/TvdW/torspec/commits/remove-legacy-protocol-support