Kill the pre-version 3 intro protocol code with fire.

We still have code for dealing with version 0, 1, and 2 of the HS intro protocol.

From rend-spec.txt:

As of Tor 0.2.0.7-alpha and 0.1.2.18, clients switched to using the v2 intro format.

From the release notes:

Bugfix on 0.2.1.6-alpha, when the v3 intro-point protocol (the first one which sent a timestamp field in the INTRODUCE2 cell) was introduced;

Anything that generates INTRODUCE cells with these versions are long dead, so the code for handling this protocol version should be removed.

changed milestone to %Tor: unspecified

added component::core tor/tor deprecation milestone::Tor: unspecified parent::6418 points::1 priority::medium prop224 reviewer::asn severity::normal sponsor::R-must status::new technical-debt tor-hs type::enhancement labels

The only reason we haven't removed them so far is the possibility of identifying the version of Tor running a hidden service based on which versions it won't accept. Not sure if that's a compelling reason.

Trac:
Keywords: N/A deleted, TorCoreTeam201507 added

I would be fine removing this code. David, I know you're interested in in hacking on this code; is this an area you'd like to hack on?

Trac:
Cc: N/A to dgoulet

Trac:
Owner: N/A to dgoulet
Status: new to assigned
Keywords: TorCoreTeam201507 deleted, TorCoreTeam201508 added

Trac:
Keywords: TorCoreTeam201508 deleted, TorCoreTeam201509 added

Trac:
Milestone: Tor: 0.2.7.x-final to Tor: 0.2.8.x-final

Move a few tickets out of 0.2.8. I would take a good patch for most of these if somebody writes one. (If you do, please make the ticket needs_review and move it back into maint-0.2.8 milestone. :) )

Trac:
Milestone: Tor: 0.2.8.x-final to Tor: 0.2.???

Is this a subset of #6418 (moved)? And there, we decided to phase out the old unused parts of the HS protocol while we roll out the new one, to minimize partitioning issues?

Does that mean we should close this one as a duplicate of #6418 (moved)?

Trac:
Severity: N/A to Normal
Sponsor: N/A to N/A

Trac:
Parent: N/A to #6418 (moved)

I've made a branch for this one that kills v0/v1/v2 protocol version. Not only that but makes client and HSDir cache only store v3 descriptors for now on. I might of miss some part since version check are spread around the code...

This works in chutney perfectly.

Branch: ticket15621_028_01

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to N/A
Type: enhancement to defect
Version: Tor: unspecified to N/A
Sponsor: N/A to None
Keywords: TorCoreTeam201509 deleted, N/A added
Milestone: Tor: 0.2.??? to Tor: 0.2.9.x-final

Trac:
Type: defect to enhancement
Cc: dgoulet to N/A
Points: N/A to small
Sponsor: None to SponsorR-can

Every postponed needs_review ticket should get a review in April

Trac:
Keywords: N/A deleted, TorCoreTeam201604 added

Trac:
Reviewer: N/A to asn

Patch looks good to me.

Glad to see all that code getting deleted.

Trac:
Status: needs_review to merge_ready

• NM1: I don't like the looks of the hardcoded (1<<3) refereneces; can these become a macro or move into a hs_protocols_supported() function or something? Similarly the "if version != 3" one.
• NM2: Should the union in rend_intro_cell_s go away? Unions can be scary. This could be another ticket.
• NM3: err_msg_out isn't used in rend_service_parse_intro_unsupported. Should it be? There's probably some message to give there, right?
• NM4: I am concerned about fingerprinting attacks here. Do we have a good story about fingerprinting hidden services and their users as they upgrade, and why that will/won't matter?

Trac:
Status: merge_ready to needs_revision

Replying to nickm:

• NM1: I don't like the looks of the hardcoded (1<<3) refereneces; can these become a macro or move into a hs_protocols_supported() function or something? Similarly the "if version != 3" one.

For supported protocols, see commit: 0c0c76095c370817051d2b46596b29cf59cefba6

As for the version != 3 where the number is hardcoded. Every code site where it's hardcoded, we are in a code path dedicated for v3. The functions name are sometimes misleading like we use the _v2 parse intro function for the v3... Unless we refactor cleanly, I think it's "ok" to leave them like that considering the potential refactor that 224 will bring for those functions (if any). Plausible?

• NM2: Should the union in rend_intro_cell_s go away? Unions can be scary. This could be another ticket.

Yup it could! Although, as a minimal change for killing version protocol < 3, I would make this one another ticket. Chances are though that 224 will not touch this so this union will only see v3 in the future.

• NM3: err_msg_out isn't used in rend_service_parse_intro_unsupported. Should it be? There's probably some message to give there, right?

That function is actually never reached because of the version check in rend_service_parse_intro_plaintext prior to the use of intro_version_handlers. And on error, it also returns an undocumented status code so tbh I'm not even sure that this is needed anymore. Should we rip it off?

• NM4: I am concerned about fingerprinting attacks here. Do we have a good story about fingerprinting hidden services and their users as they upgrade, and why that will/won't matter?

Hrm... so the fingerprinting attack here is basically knowing that this HS did upgrade to 029 or is there something else? It's doable but what did we do when we moved to v0->v1 and v1->v2 ?

Note that here it will only indicate the the HS is at >= 0.2.9.0, no the exact version. Not sure what it can give to the attacker other than try to find an attack on that version? If this is a real issue, we'll have a hard time with users on 224 :).

Trac:
Status: needs_revision to needs_review

Replying to dgoulet:

• NM4: I am concerned about fingerprinting attacks here. Do we have a good story about fingerprinting hidden services and their users as they upgrade, and why that will/won't matter? Note that here it will only indicate the the HS is at >= 0.2.9.0, no the exact version. Not sure what it can give to the attacker other than try to find an attack on that version? If this is a real issue, we'll have a hard time with users on 224 :).

That was why we originally (comment:8) had in mind to do one switch, rather than several switches. That is, to do this change when we roll over to the next-gen design so there's only one partition.

That said, I am increasingly a fan of just getting the coding done and not worrying too much about intermediate states for users (if that worrying means slowing everything else down). But I can definitely see both sides.