Opened 2 years ago

Closed 2 years ago

#26549 closed defect (fixed)

Revision counter for v3 ephemeral hidden service is lost

Reported by: akwizgran Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor:
Severity: Normal Keywords: tor-hs, prop224, 035-roadmap-subtask, 035-triaged-in-20180711
Cc: Actual Points:
Parent ID: #25552 Points:
Reviewer: Sponsor:


When a controller is using a client to provide two or more v3 ephemeral hidden services, with the private keys managed by the controller, and there's a client session where the controller activates one of the hidden services but not the others, the revision counters for the other hidden services are lost. This prevents the other services from being activated in future sessions because their descriptors are rejected by the HSDirs.

This happens because increment_descriptor_revision_counter() in hs_service.c calls update_revision_counters_in_state(), which loops over all the services currently being provided by the client, saves their counters, and removes any other counters from the state file. Thus if any hidden service is activated during a session, the revision counters of any services not activated during that session are lost.

Steps to reproduce:

  • Use ADD_ONION NEW:ED25519-V3 ... to create two hidden services
  • Save the private keys
  • Shut down and restart tor
  • Use ADD_ONION ED25519-V3:<private_key_1> ... to activate the first service
  • Shut down and restart tor
  • Use SETEVENTS HS_DESC to register for HS descriptor events
  • Use ADD_ONION ED25519-V3:<private_key_1> ... to activate the first service
  • The descriptor should be published successfully
  • Use ADD_ONION ED25519-V3:<private_key_2> ... to activate the second service
  • The controller receives HS_DESC_FAILED events with REASON=UPLOAD_REJECTED

It looks like this bug is related to #25552. I don't know whether the solution to that ticket will fix it.

Child Tickets

Change History (7)

comment:1 Changed 2 years ago by akwizgran

Perhaps the problem could be solved by using the hash of the identity public key, rather than the blinded public key, to identify the service in the state file. This would make it possible to remove any stale entries for the same service when storing the incremented counter, without affecting any entries for other services.

However, this would allow entries for ephemeral services that are no longer used to accumulate in the state file. This seems difficult to avoid: on one hand we have to keep state for any service the controller may one day want to reactivate, while on the other hand we'd prefer not to accumulate state for services the controller no longer intends to use.

comment:2 Changed 2 years ago by asn

Thanks for the analysis of this bug. Great find!

Fortunately, the patch from #25552 will fix this issue, since it completely removes the need of state file caching of rev counters. The rev counters are made on the fly.

A variant of this bug was also reported by the onionshare team here:

Please test out the #25552 branch and let us know if this doesn't fix it :)


comment:3 Changed 2 years ago by teor

Parent ID: #25552

This can close when #25552 merges,

comment:4 Changed 2 years ago by akwizgran

Confirming that the bug25552_ope branch fixes this bug.

comment:5 Changed 2 years ago by nickm

Keywords: 035-roadmap-subtask added
Milestone: Tor: 0.3.5.x-final

comment:6 Changed 2 years ago by nickm

Keywords: 035-triaged-in-20180711 added

comment:7 Changed 2 years ago by asn

Resolution: fixed
Status: newclosed

Closing this because of the fix in #25552 that got merged upstream! :)

Note: See TracTickets for help on using tickets.