Missing descriptor ID in some HS_DESC control event

The control-spec does have it specified (see section 4.1.25) but Tor doesn't provide it for both RECEIVED and FAILED.

changed milestone to %Tor: 0.2.7.x-final

added component::core tor/tor controller milestone::Tor: 0.2.7.x-final points::medium priority::medium resolution::fixed sponsor::R status::closed type::defect labels

Trac:
Description: The control-spec does have it specified (see section 4.1.25) but Tor doesn't provide it.

to

The control-spec does have it specified (see section 4.1.25) but Tor doesn't provide it for both RECEIVED and FAILED.
Summary: Missing descriptor ID in the failed HS_DESC control event to Missing descriptor ID in some HS_DESC control event

Right now it's pretty difficult to print the descriptor ID every time without some "significant" change to Tor on how HS descriptor query are handled. Ticket #15816 (moved) is part of the solution which would help us fix the full issue.

For now, we can print it when a descriptor ID is used for the fetch (HSFETCH command).

See branch bug15881_027_01.

Trac:
Status: new to needs_review

#15816 (moved) is being merged so hold up this one so I can provide a second version with the full complete fix.

Trac:
Status: needs_review to needs_revision

Full fix: bug15881_027_02

Trac:
Status: needs_revision to needs_review

• Can this have tests?
• In get_desc_id_from_query(), is it possible that the set of directories we knew about when we launched the query will not match the set that we know about when we call get_desc_id_from_query()? This could happen if we get a new networkstatus in between.
• Does get_desc_id_from_query() return a (binary) digest or a hex string or a base32 string or what? It should be documented.

Replying to nickm:

• Can this have tests?

I think so yes, we have already HS control test, I'll see what I can do.

• In get_desc_id_from_query(), is it possible that the set of directories we knew about when we launched the query will not match the set that we know about when we call get_desc_id_from_query()? This could happen if we get a new networkstatus in between.

Hrm... indeed, new consensus, churn occurs and thus we have an erroneous HSDir list. Fix to that seems to keep our responsible HSDir in rend_data but that's a bit more complicated because that means one different rend_data per HSDir connection. So would keeping the full list of HSDir fingerprint a reasonable idea?

• Does get_desc_id_from_query() return a (binary) digest or a hex string or a base32 string or what? It should be documented.

Ack!

Trac:
Status: needs_review to needs_revision

This came up for the UPLOADED event as well. It would be really nice if the descriptor ID was provided, so that the event can be used to determine if a new HS should be reachable or not.

All the fixes now in branch bug15881_027_03.

Extra commit added on top to fix the testing.

Trac:
Status: needs_revision to needs_review
Points: small to medium

Looks good; merging. Thanks!

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Initialization that I missed in rend_data_service_create which causes the rend_data free and dup to fail on a NULL pointer.

Maybe it would be better to check if hsdirs_fp is NULL in those functions, I don't have a preference here.

Trac:
Status: closed to reopened
Resolution: fixed to N/A

Trac:
0001-Fix-init-HSDirs-list-in-rend_data_service_create.patch

Merged that. Thanks!

Trac:
Status: reopened to closed
Resolution: N/A to fixed

Yawning mentioned the UPLOADED action. I just checked and that still reports as 'UNKNOWN'. Should that be a separate ticket?

Replying to atagar:

Yawning mentioned the UPLOADED action. I just checked and that still reports as 'UNKNOWN'. Should that be a separate ticket?

Yeah let's make another ticket about it because this one is already merged and I've looked to adding it to the UPLOADED event and it's a bit more work.

Sounds good. Filed #16023 (moved) for that.

Trac:
Sponsor: N/A to SponsorR
Keywords: SponsorR deleted, N/A added

closed

mentioned in issue #16021 (moved)

mentioned in issue #16023 (moved)

moved to tpo/core/tor#15881 (closed)

mentioned in issue tpo/core/tor#16021 (closed)

mentioned in issue tpo/core/tor#16023 (closed)