Opened 7 years ago

Closed 7 years ago

#10841 closed enhancement (fixed)

Remove hidden service version 0 directory code

Reported by: karsten Owned by:
Priority: Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

From the ChangeLog:

Changes in version 0.2.2.1-alpha - 2009-08-26
    - Hidden services no longer publish version 0 descriptors, and clients
      do not request or use version 0 descriptors. However, the old hidden
      service authorities still accept and serve version 0 descriptors
      when contacted by older hidden services/clients.

Nick, would you accept a patch that removes version 0 hidden service code? Or would you prefer writing that patch yourself and me reviewing it?

Child Tickets

TicketTypeStatusOwnerSummary
#10881defectclosedDrop AlternativeHSAuthority and friends

Change History (8)

comment:1 Changed 7 years ago by nickm

I'd accept a patch like this if it turns out to be simple. If it looks like it's going to be tricky or complicated, though, let me know.

comment:2 Changed 7 years ago by nickm

Ticket #10881 is related, but just to removing the options here.

comment:3 Changed 7 years ago by karsten

Status: newneeds_revision

Please find my branch bug10841 that removes all remaining hidden service version 0 directory code I could find. Tested with a hidden service and a hidden service client.

comment:4 Changed 7 years ago by nickm

Did you mean needs_revision or needs_review?

Notes while reviewing:

  • I'm surprised to see DIR_PURPOSE_FETCH_RENDDESC and DIR_PURPOSE_UPLOAD_RENDDESC disappear. I guess they were v0 only. (Next time we add a v1 or a v2, we should make sure that we rename the old functions to "*_v0", so that we don't get confused this way again.)
  • I wondered about removing the "case -1" case from connection_dir_client_reached_eof. I guess that return value shouldn't happen anymore.... but having these semi-enumerated error codes makes me twitchy. Maybe we should be using an enum for these. I guess that's another commit. I can do that one if you like.
  • Do we still need rend_parse_service_descriptor? I think you may have removed the only thing that called it.

comment:5 in reply to:  4 ; Changed 7 years ago by karsten

Status: needs_revisionneeds_review

Replying to nickm:

Did you mean needs_revision or needs_review?

Yes, needs_review. Oops.

Notes while reviewing:

  • I'm surprised to see DIR_PURPOSE_FETCH_RENDDESC and DIR_PURPOSE_UPLOAD_RENDDESC disappear. I guess they were v0 only. (Next time we add a v1 or a v2, we should make sure that we rename the old functions to "*_v0", so that we don't get confused this way again.)

Correct, those two were only used for v0.

  • I wondered about removing the "case -1" case from connection_dir_client_reached_eof. I guess that return value shouldn't happen anymore.... but having these semi-enumerated error codes makes me twitchy. Maybe we should be using an enum for these. I guess that's another commit. I can do that one if you like.

Okay. Do you mind making that change to an enum?

  • Do we still need rend_parse_service_descriptor? I think you may have removed the only thing that called it.

True! Please find my updated branch bug10841.

I wonder, is there a simple way to find unused functions in tor?

comment:6 in reply to:  5 ; Changed 7 years ago by nickm

Replying to karsten:

Okay. Do you mind making that change to an enum?

Done in branch "bug10841" in my public repo. Looks okay to you?

  • Do we still need rend_parse_service_descriptor? I think you may have removed the only thing that called it.

True! Please find my updated branch bug10841.

Thanks!

I wonder, is there a simple way to find unused functions in tor?

Opening a new ticket.

comment:7 in reply to:  6 Changed 7 years ago by karsten

Replying to nickm:

Replying to karsten:

Okay. Do you mind making that change to an enum?

Done in branch "bug10841" in my public repo. Looks okay to you?

Looks good! Thanks!

comment:8 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Great; merging it. Thanks!

Note: See TracTickets for help on using tickets.