Can't search by fingerprint with spaces

added component::metrics/onionoo owner::phw priority::medium reporter::strugee resolution::fixed severity::normal status::closed type::defect labels

I spent 30 minutes trying to fix this, without success. Leaving it open and tagging it as "easy," for somebody who actually knows JavaScript.

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

Oh, this is probably not something you can fix in Atlas, but something that we'd have to fix in Onionoo. The issue is that Onionoo interprets spaces as separators between search terms, and if you put in ten space-separated hex blocks, it looks for fingerprints/IP addresses/nicknames starting with all ten of them. I'm not yet clear how to fix this in Onionoo. I'm moving it to the Onionoo component. Please move it back to Atlas if I understood the issue completely wrong though.

Trac:
Component: Atlas to Onionoo

Replying to karsten:

Oh, this is probably not something you can fix in Atlas, but something that we'd have to fix in Onionoo. Couldn't Atlas just sanitise fingerprints before it sends them to Onionoo? After all, it should probably be conservative in what it sends anyway.

Agreed, Atlas should sanitize fingerprints before it sends them to Onionoo. In fact, Onionoo already suggests that when it says "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."

This case is a bit different, because it's not always possible to distinguish a 4-character hex block from other valid input like (part of) a nickname. For example, should "DEFA CB7E 7D73" be considered the beginning of fingerprint DEFACB7E7D73, or is it supposed to be a search for the (existing) relay with nickname default and fingerprint CB7E7D734E28312337DE322C1A0E4DE53578D2AE?

But I'm inclined to improve usability by allowing fingerprints with spaces even at the risk of returning false negatives for mixed searches with 4-character hex nicknames or nickname parts. After all, users shouldn't rely on nicknames anymore.

Suggestion (not implemented yet, might be implemented differently):

Onionoo clients, including Atlas, are advised to remove spaces between any two search terms consisting of exactly 4 hex characters. (If the result is a search term consisting of 40 hex characters, clients are advised to hash that using SHA-1.)
The Onionoo server performs the same operation, which would cover any clients that don't follow this advice. This is a major protocol change that needs to be announced at least 1 month before becoming effective.

What do you think? What did I overlook?

Wait, maybe there's a simpler way to implement this: we can extend the search parameter to support exact matching of any 4-character hex block in a space-separated fingerprint. That is, in addition to checking if a search term is the beginning of a relay's fingerprint we can check if it matches one of the ten 4 hex characters in the relay's space-separated fingerprint. This wouldn't require any input processing on client or server side.

In the example above, this would work as follows:

"DEFA" matches part of the relay's nickname "default",
"CB7E" is the beginning of the relay's fingerprint "CB7E7D734E28312337DE322C1A0E4DE53578D2AE" and exactly matches the first hex block, and
"7D73" exactly matches the second hex block of the relay's fingerprint.

I implemented this idea in branch task-17115 in my public repository. I have some concerns that this will put more load on the Onionoo server, but I think I'm willing to risk that. It's still a major protocol change where we should give client developers a 1-month warning. But I'd want to start running it on https://onionoo.thecthulhu.com/ even before that to gather some feedback. I think I'll deploy it there in a few days, unless there are objections. Deployment on https://onionoo.torproject.org/ would happen around November 15.

Feedback welcome!

Trac:
Status: new to needs_review
Cc: N/A to iwakeh
Keywords: easy deleted, N/A added

I cannot say if the above use-case is satisfied.

Concerning the implementation: In case of space issues it might be useful to only use an array String[] of the ten hex-strings. The hashset will have a size multiple the size of the string array. For the look-up use Arrays.binarySearch, which should have a similar performance in the size of the search set we consider here. The sorting before storing the array should be about similar or less of the book-keeping done by the hashset.

iwakeh, good suggestion. I think I implemented that. Mind taking a look at the updated branch?

Yeah, that looks fine.

Maybe, also add a test in order to verify that fingerprintFourHexBlocks is indeed only available as sorted array.

Maybe, even adjust its name to fingerprintSortedHexBlocks or similar as reminder.

Thanks for the suggestions, I added them as another squash commit.

I'll wait another day or two and then deploy this branch on the Onionoo mirror and also announce the upcoming major protocol change on tor-dev@. And I'll set "next_major_version_scheduled" to "2015-11-15" (writing this here mostly as reminder to myself).

Replying to karsten:

Thanks for the suggestions, I added them as another squash commit.

I'll wait another day or two and then deploy this branch on the Onionoo mirror and also announce the upcoming major protocol change on tor-dev@. And I'll set "next_major_version_scheduled" to "2015-11-15" (writing this here mostly as reminder to myself).

Did that. I'll leave this ticket open as a reminder to squash and merge in a month from now.

Trac:
Status: needs_review to new
Severity: N/A to Normal

Merged and deployed. Closing.

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

closed

mentioned in issue #17231 (moved)

Can't search by fingerprint with spaces

Child items ...

Activity