Opened 6 years ago

Closed 2 weeks ago

#6947 closed enhancement (fixed)

Allow filtering relays by version ranges

Reported by: rransom Owned by: karsten
Priority: Medium Milestone: Onionoo 1.18.0
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: nusenu@…, metrics-team Actual Points:
Parent ID: Points:
Reviewer: irl Sponsor:

Description

It would be much easier to find out how many relays are still vulnerable to bugs like #6811 if there were an easy way to search for relays running versions of Tor in a specified range.

Child Tickets

Change History (19)

comment:1 Changed 6 years ago by cypherpunkx

That use case (How much overall tor bandwidth is provided by vulnerable relays) was also my motivation for #6855 - great to see someone else seeing that use case too.

You are probably interested in karsten's reply in #6855 - which somehow covers also your feature request in this ticket.

comment:2 Changed 4 years ago by tyseom

Cc: nusenu@… added

comment:3 Changed 12 months ago by karsten

Severity: Normal
Summary: Compass should allow filtering relays by version rangesAllow filtering relays by version ranges

Keep summary as short and precise as possible.

comment:4 Changed 12 months ago by karsten

Owner: set to metrics-team
Status: newassigned

comment:5 Changed 10 months ago by irl

Component: Metrics/CompassMetrics/Atlas

In #23517 it is planned to merge Compass functionality with Relay Search (formerly known as Atlas). These tickets may be relevant to that work and so these are being reassigned to the Metrics/Atlas component.

comment:6 Changed 10 months ago by irl

Component: Metrics/AtlasMetrics/Onionoo

Relay Search will magically support this for both single relay lookup and aggregations if it's supported by Onionoo.

comment:7 Changed 2 months ago by karsten

Cc: metrics-team added

I started to think about this today and came up with a few design questions that we need to answer first:

  • Dashes in tor versions: As of now, a query for /details?version=0.3.4.5-rc returns relays with version "0.3.4.5-rc". But the dash is also a quite intuitive notation for a range, just like we're using a dash in the first_seen_days and last_seen_days parameters. If we're going to support version ranges, we might want to use the dash for queries like /details?version=0.3.4.5-0.3.4.6. If we do, the first query won't work anymore, and would have to be changed to /details?version=0.3.4.5. The result set will remain the same, it's just that people will get it wrong in the beginning. Needs a major protocol update.
  • Prefix matching: The query /details?version=0.3.2.1 returns relays with version 0.3.2.1 as well as 0.3.2.10, 0.3.2.11, and so on. Following that logic, which versions would we expect that the query /details?version=0.3.2.2-0.3.2.1 returns? Everything from 0.3.2.1 to 0.3.2.29, plus 0.3.2.100 to 0.3.2.199, etc. Maybe we should consider changing the matching algorithm to attempt to parse the version (prefix) and do the matching numerically. In this case the example above would simply be rejected, and a query /details?version=0.3.2.1-0.3.2.2 would just return those two versions and nothing else. Needs a major protocol update.
  • Comma-separated versions or version ranges: I could imagine use cases where somebody wants all relays with non-consecutive version numbers. While we're changing this parameter for version ranges we could as well start supporting comma-separated versions or version ranges. As in: /details?version=0.3.2.1,0.3.2.5. Needs a minor protocol update unless combined with the changes above.

Thoughts?

comment:8 Changed 2 months ago by irl

We have version ordering logic now thanks to the version_status field. We can have a from_version and to_version parameter?

We can then separately also have comma separated versions for prefix matches.

comment:9 in reply to:  8 ; Changed 2 months ago by karsten

Replying to irl:

We have version ordering logic now thanks to the version_status field.

Yes! It might that we need to write some more code to also support version prefixes, but most of the code should already be there.

We can have a from_version and to_version parameter?

Hmm, let's ignore for a moment that we already have a version parameter and let's discuss how we would like to build version-related parameters. Wouldn't it be more intuitive if we had a single version parameter that supports single versions, dash-separated version ranges, and possibly even comma-separated versions or version ranges? Keep in mind that some users may want to use these parameters as qualified search terms.

Maybe my list of design questions looked too alarming, but that was not my intention. I think it's perfectly fine to make a major protocol update, if it's a useful change. We just need to give folks the chance to update their clients, bookmarks, etc. But if the result is a more intuitive interface, that's worth the effort.

However, I'm not sure what the most intuitive interface is. Maybe it's something else, like a different character for ranges, such as /details?version=0.3.2.1:0.3.2.5. (If so, we should consider changing first_seen_days and last_seen_days, too, for consistency.)

We can then separately also have comma separated versions for prefix matches.

comment:10 in reply to:  9 ; Changed 2 months ago by irl

Replying to karsten:

However, I'm not sure what the most intuitive interface is. Maybe it's something else, like a different character for ranges, such as /details?version=0.3.2.1:0.3.2.5. (If so, we should consider changing first_seen_days and last_seen_days, too, for consistency.)

I think using : for ranges is better, as long as we can be certain that we'll never see a : in a released Tor version. We should probably require this in torspec somewhere. If we do a major version change, which I think we should for this, then we would also change {first,last}_seen_days.

comment:11 in reply to:  10 ; Changed 2 months ago by karsten

Replying to irl:

Replying to karsten:

However, I'm not sure what the most intuitive interface is. Maybe it's something else, like a different character for ranges, such as /details?version=0.3.2.1:0.3.2.5. (If so, we should consider changing first_seen_days and last_seen_days, too, for consistency.)

I think using : for ranges is better, as long as we can be certain that we'll never see a : in a released Tor version. We should probably require this in torspec somewhere. If we do a major version change, which I think we should for this, then we would also change {first,last}_seen_days.

Hmm, on second thought, : wouldn't work so well with qualified search terms. Imagine a search for version:0.3.2.1:0.3.2.5 (for versions 0.3.2.1, 0.3.2.2, 0.3.2.3, 0.3.2.4, and 0.3.2.5) or even version::0.3.2.5 (for everything until 0.3.2.5) or version:0.3.2.1: for 0.3.2.1 or newer).

New idea (sorry for the brainstorming, I'm just trying to find the best solution): .., as in version:0.3.2.1..0.3.2.5 or version:..0.3.2.5 or version:0.3.2.1.. for the examples given above. Yet unimplemented, so no guarantee that it will work well in all cases. We'd only have to require that versions don't contain two consecutive ., but that should be a safe assumption. What do you think? Are there better alternatives?

comment:12 in reply to:  11 Changed 2 months ago by irl

Replying to karsten:

Hmm, on second thought, : wouldn't work so well with qualified search terms. Imagine a search for version:0.3.2.1:0.3.2.5 (for versions 0.3.2.1, 0.3.2.2, 0.3.2.3, 0.3.2.4, and 0.3.2.5) or even version::0.3.2.5 (for everything until 0.3.2.5) or version:0.3.2.1: for 0.3.2.1 or newer).

Good point.

New idea (sorry for the brainstorming, I'm just trying to find the best solution): .., as in version:0.3.2.1..0.3.2.5 or version:..0.3.2.5 or version:0.3.2.1.. for the examples given above. Yet unimplemented, so no guarantee that it will work well in all cases. We'd only have to require that versions don't contain two consecutive ., but that should be a safe assumption. What do you think? Are there better alternatives?

Yes, I think a double . is safer than a :. I know that : is perfectly valid in Debian package version numbers and perhaps there would be other clients that use it in the future.

comment:13 Changed 2 months ago by karsten

Owner: changed from metrics-team to karsten
Status: assignedaccepted

Okay, great, I think that's enough to start writing some code. Grabbing the ticket.

comment:14 Changed 2 months ago by karsten

Status: acceptedneeds_review

Please review commit 9740ce7 in my task-6947 branch. Once this patch is merge-ready, let's schedule the next major version for a month later and put the merge on hold for that time. Thanks!

comment:15 Changed 2 months ago by irl

Reviewer: irl

Will look at this tomorrow UTC morning unless anything more urgent comes up.

comment:16 Changed 8 weeks ago by irl

Status: needs_reviewmerge_ready

This looks good to me.

comment:17 Changed 8 weeks ago by karsten

Milestone: Onionoo 1.17.0

Schedule for protocol version 7.0, likely to be released in 1.17.0.

comment:18 Changed 3 weeks ago by karsten

Milestone: Onionoo 1.17.0Onionoo 1.18.0

Versions 1.17.0 and 1.17.1 are already released, the next released version will be 1.18.0.

comment:19 Changed 2 weeks ago by karsten

Resolution: fixed
Status: merge_readyclosed

This is now merged and deployed as part of 7.0-1.18.1. Closing. Thanks!

Note: See TracTickets for help on using tickets.