Opened 10 months ago

Closed 9 months ago

#26980 closed defect (fixed)

HSv3 descriptors rejected because of bad SRV start time computation

Reported by: asn Owned by:
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 035-must regression tor-hs hsv3 reachability
Cc: dmr Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description (last modified by asn)

When we introduced #25552, we started OPE encrypting the time diff since the start of the SRV run. We also have some logic on which SRV period we should use to calculate the time diff:

  if (is_current) {
    srv_start = sr_state_get_start_time_of_previous_protocol_run();
  } else {
    srv_start = sr_state_get_start_time_of_current_protocol_run();
  }

There is a bug here, because when we cross from the 23:00 consensus to the 01:00 consensus, the start of the SRV protocol changes and screws up the revision counter monotonicity.

This causes one descriptor batch upload to fail.

Child Tickets

Attachments (1)

bug26980.log (5.1 KB) - added by asn 9 months ago.
demonstration of bug and fix

Download all attachments as: .zip

Change History (9)

comment:1 Changed 9 months ago by asn

Priority: MediumHigh

Marking as High because this is a reachability issue for onion services.

comment:2 Changed 9 months ago by asn

Keywords: reachability added

comment:3 Changed 9 months ago by asn

Description: modified (diff)

comment:4 Changed 9 months ago by dmr

Cc: dmr added

comment:5 Changed 9 months ago by asn

Status: newneeds_review

FWIW, I have made a branch here (with unittest): https://github.com/torproject/tor/pull/289

I've been testing this on my hsv3 for the past week, but unfortunately I still have not encountered the edge-case yet.

I'm marking this as needs_review to move this forward for now, but let's not move it to merge_ready before we've had a chance to see this edge-case actually happen.

Changed 9 months ago by asn

Attachment: bug26980.log added

demonstration of bug and fix

comment:6 Changed 9 months ago by asn

OK, seems like the patch does fix the edge-case that I just reproduced in my HSv3.

I attached some logs above with a buggy instance and a patched instance with some comments (search for "ASN" for comments).

This is ready to merge_ready when it gets a first round of review.

comment:7 Changed 9 months ago by dgoulet

Cc: dgoulet removed
Reviewer: dgoulet
Status: needs_reviewmerge_ready

Big find! lgtm;

comment:8 Changed 9 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged! And thanks for all the comments!

Note: See TracTickets for help on using tickets.