Opened 9 months ago

Closed 8 days ago

#21615 closed enhancement (fixed)

Use hashed fingerprint in all lookups

Reported by: cypherpunks Owned by: irl
Priority: Medium Milestone:
Component: Metrics/Atlas Version:
Severity: Normal Keywords: metrics-2017
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Currently the fingerprints in detailed lookups performed by the search page are not hashed.

Child Tickets

Attachments (1)

0001-Hash-the-fingerprints-from-summary-documents.patch (2.6 KB) - added by cypherpunks 6 months ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 9 months ago by irl

Status: newneeds_information
Type: defectenhancement

Atlas doesn't claim to hash fingerprints and we instead provide instructions on how to look up bridges using the hashed fingerprint. I'm not convinced this is a defect, as clearly lookups using fingerprints work.

Is Onionoo generally happy to respond to hashed fingerprints in place of fingerprints for both relays and bridges then? What is the gain for this over the loss that a bridge fingerprint could be entered into the browser and perhaps leaked?

I'm not saying it's a bad idea, I'm saying I'm not sure I understand the motivation yet.

comment:2 in reply to:  1 Changed 9 months ago by cypherpunks

Replying to irl:

Atlas doesn't claim to hash fingerprints and we instead provide instructions on how to look up bridges using the hashed fingerprint. I'm not convinced this is a defect, as clearly lookups using fingerprints work.

To give some back story, I noticed the inconsistency while working on #15415. Looking at the requests, the hash would change between requests because the search page doesn't hash the fingerprint and the details page does. For example, looking at the requests to Onionoo when browsing to https://atlas.torproject.org/#search/BC630CBBB518BE7E9F4E09712AB0269E9DC7D626, the first details request (from the search page) uses the actual fingerprint while subsequent calls (from the details page) use the hashed fingerprint.

Is Onionoo generally happy to respond to hashed fingerprints in place of fingerprints for both relays and bridges then?

Onionoo does seem to be happy. For example, browsing to https://atlas.torproject.org/#details/C7A9862525665B3E5E48D27FCBC0D8B6E8A9F3C7 which is a bridge shows a different hash in the request to Onionoo because it is hashed again.

What is the gain for this over the loss that a bridge fingerprint could be entered into the browser and perhaps leaked?

I'm not sure. The hashing of fingerprints can be traced back to commit ba56727fad15316814a7caf4acee4e49c173b86c which doesn't give a motivation other than that Onionoo supports it.

I'm not saying it's a bad idea, I'm saying I'm not sure I understand the motivation yet.

I just like it to be consistent. Either everything should hash before requesting Onionoo or nothing should. Because its obviously hard to remember and check that every request should hash before sending it, i lean towards not hashing. It would remove the dependency on the JavaScript SHA library, remove some code, and speed Atlas up slightly.

comment:3 Changed 9 months ago by cypherpunks

Removing hashing would also make fixing #21612 obsolete.

comment:4 Changed 9 months ago by karsten

Wait, removing hashing is not a good idea. See this part of the spec: "Complete hex-encoded fingerprints should always be hashed using SHA-1, regardless of searching for a relay or a bridge, in order to not accidentally leak non-hashed bridge fingerprints in the URL." Happy to explain this more if needed.

comment:5 Changed 9 months ago by irl

Ok, I see the inconsistency now. The search page, when fetching the details documents, isn't hashing but the details page is regardless of whether or not the fingerprint has already been hashed.

So, to add to the examples of what is happening, I have two bridges that with the following search will do lookups for details documents of the hashed fingerprint but when going to the details pages, the hashed fingerprint will be hashed once more.

https://atlas.torproject.org/#search/irlDn42

The hashed fingerprints are already leaked in the search step, so either this is a defect in that we're leaking hashed fingerprints that need to be hashed again or there is an enhancement to be had in removing the re-hashing when fetching the details documents for the details pages.

Currently, by the time we get to the details pages we've already leaked the hashed fingerprint for the bridge.

karsten: I guess the question is, how many times do we need to hash the fingerprint?

comment:6 Changed 9 months ago by karsten

Leaking a hashed fingerprint is not problematic. It's the original, unhashed bridge fingerprint that we should not leak.

Let's assume B is an original, unhashed bridge fingerprint that we don't want to leak. If the user looks up B, Atlas shouldn't send B to the Onionoo server, but it should send H(B) instead. In fact, Onionoo wouldn't find anything under B, because it doesn't even know original, unhashed bridge fingerprints. So far so good, but what if the user did the right thing and put in H(B) to look up their bridge? In that case Atlas would send H(H(B)) to Onionoo, in which case Onionoo would still provide the same bridge.

Similarly for relays, let's assume that R is an original, unhashed relay fingerprint, however, that we don't mind leaking. If Atlas sees that it sends H(R) to Onionoo, which is fine. But Onionoo would also respond to R as search input. What Onionoo would not understand is H(H(R)).

comment:7 Changed 9 months ago by cypherpunks

Thanks for the detailed explanation. This means that any fingerprint we get back from Onionoo (being either R or H(B)) we can safely rehash. And because we expect users to give us either R or H(B) too we can also safely rehash those. This is the current situation.

The problem is with the detailed lookups performed by the search page which uses whatever Onionoo returns and thus sends requests using R and H(B) directly. FWICT this is the only edge case where there's no rehashing.

The solution would be to let the relay model on the Atlas side always hash whatever fingerprint is assigned to it when it's created and store it in a separate field. Then we would have one field which contains either R or H(B) for showing on the details page and one field which contains H(R) or H(H(B)) for making subsequent requests (such as bandwidth, weights, etc).

comment:8 Changed 9 months ago by karsten

I'm still not sure I understand the problem you're trying to solve. Whatever Onionoo tells you in a response you can safely include in a subsequent request without hashing. It's the part about hashing user input that should be hashed before sending it to Onionoo, because you cannot be sure that the user did not tell you B. But of course, if you want, you can hash everything you get from Onionoo. It doesn't help but it also doesn't hurt. But I may be overlooking the real issue here. Sorry that this is so confusing, but if it's any relief, it's confusing me, too. ;)

comment:9 Changed 9 months ago by cypherpunks

I understand that hashing fingerprints from Onionoo isn't necessary, but i like consistency. IMO it's confusing to see requests with two different hashes when information is requested about a single relay. Also, performing the hashing in one central place makes it harder to make mistakes in the future.

comment:10 Changed 9 months ago by karsten

Hmm, this doesn't sound like a real issue to me. To me it seems unnecessary to hash fingerprints that Atlas receives from Onionoo, but if this makes things easier, okay. I guess most users wouldn't even notice. Anyway, as long as Atlas keeps hashing full fingerprints it receives as input from the user, I'm happy. :)

comment:11 Changed 7 months ago by irl

Resolution: not a bug
Status: needs_informationclosed

From reading the log here, I don't think there is a bug here.

comment:12 Changed 6 months ago by cypherpunks

Parent ID: #22307
Resolution: not a bug
Status: closedreopened

Turns out this is a bug after all. See ticket:22307#comment:9.

comment:13 Changed 6 months ago by cypherpunks

Status: reopenedneeds_review

I've attached a patch which hashes the fingerprints returned by searches before requesting their details documents. As explained in comment:7 this is the only edge case where fingerprints were not rehashed.

When the patch is merged into master we can continue with #22307 and ask if it fixed their issue (my testing indicates that it does).

comment:14 Changed 4 months ago by irl

Resolution: fixed
Status: needs_reviewclosed

Merged. Thanks. (:

comment:15 Changed 4 months ago by irl

Parent ID: #22307
Resolution: fixed
Status: closedreopened

See #23156.

comment:16 Changed 4 months ago by cypherpunks

The cause of #23156 is a combination of the patch hashing the fingerprints from summary documents to fix #22307 and the details page also hashing the fingerprint it has been given (to prevent bridge fingerprints from being leaked).

This is also related to #15415 where functionality was added to redirect searches to the details page when there is only one result. The redirect uses the internal fingerprint and this ticket caused that fingerprint to be hashed. The details page would then doubly hash relay fingerprints and triply hash bridge fingerprints which Onionoo does not support.

comment:17 Changed 8 weeks ago by karsten

Keywords: metrics-2018 added

comment:18 Changed 8 weeks ago by karsten

Keywords: metrics-2017 added; metrics-2018 removed

comment:19 Changed 8 weeks ago by karsten

Owner: changed from irl to metrics-team
Status: reopenedassigned

comment:20 Changed 9 days ago by irl

Owner: changed from metrics-team to irl

Moving this into my queue.

comment:21 Changed 8 days ago by irl

Status: assignedaccepted

comment:22 Changed 8 days ago by irl

Resolution: fixed
Status: acceptedclosed

I am happy that we're not using any fingerprints that we shouldn't be using.

Note: See TracTickets for help on using tickets.