Opened 18 months ago

Closed 10 months ago

Last modified 8 months ago

#22488 closed enhancement (fixed)

Include relay version listed in consensus in addition to platform line from server descriptor

Reported by: cypherpunks Owned by: metrics-team
Priority: High Milestone: Onionoo 2.0.0
Component: Metrics/Onionoo Version:
Severity: Normal Keywords: metrics-2018
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Example:
https://onionoo.torproject.org/details?search=MYCROFTsOtherChild&fields=platform,recommended_version

{"version":"4.0",
"relays_published":"2017-06-04 18:00:00",
"relays":[
{"platform":"Tor 0.3.0.7 on FreeBSD","recommended_version":false}
],
"bridges_published":"2017-06-04 16:57:33",
"bridges":[
]}

but this version is recommended:

server-versions 0.2.4.27, 0.2.4.28, 0.2.5.12, 0.2.5.13, 0.2.6.11, 0.2.7.6, 0.2.7.7, 0.2.8.9, 0.2.8.10, 0.2.8.11, 0.2.8.12, 0.2.8.13, 0.2.9.9, 0.2.9.10, 0.3.0.7, 0.3.1.2-alpha }}}

How often is the version checked against the recommended_version list?

This should happen with every consensus, we should not have false positives here since this will confuse tor relay operators.

'''Note:''' We prominently tell (red flag on atlas) everyone that they run an outdated version if recommended_version is false.

Child Tickets

Attachments (1)

task-22488-relay-versions.csv.gz (14.3 KB) - added by karsten 10 months ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 18 months ago by karsten

Resolution: not a bug
Status: newclosed

This relay was last included in a consensus on 2017-05-31 at 22:00:00 UTC. It was listed there with version 0.3.0.6 which was not a recommended server version at the time. That's why recommended_version is false.

The relay may have uploaded a server descriptor with "Tor 0.3.0.7 on FreeBSD" as platform string after that (I haven't checked when that was), but that descriptor never made it into a consensus.

We only look at the consensus for determining recommended versions, which is also what Tor clients do.

This is not a bug in Onionoo, just a rare edge case of which there are probably a dozen more. Closing as not-a-bug. Thanks for the report anyway!

comment:2 Changed 18 months ago by karsten

Resolution: not a bug
Status: closedreopened
Summary: recommended_version is false but it should be trueInclude relay version listed in consensus in addition to platform line from server descriptor
Type: defectenhancement

From the metrics-team@ mailing list:

thanks for your prompt reply, since the ticket has been closed I'm answering here.

We display whatever version was last uploaded by the relay but we only care about what made it into the consensus.

What do you think about _also_ just displaying whatever made it into the consensus so there is no mismatch between what atlas displays and what is actually in the consensus?

Sounds plausible to me. I'm currently focusing on metrics-lib, so I'm reopening this ticket for now.

comment:3 Changed 14 months ago by karsten

Keywords: metrics-2018 added

comment:4 Changed 14 months ago by karsten

Keywords: metrics-2017 added; metrics-2018 removed

comment:5 Changed 12 months ago by karsten

Status: reopenedneeds_review

(I worked on all currently open (recommended) version related Onionoo tickets including this one: #22488, #23962, #21827, and #23544.)

I suggest we add a new "version" field in addition to the existing "platform" field. For relays we include the version listed in the consensus in that field. Then we have both versions: "platform" with the version from the server descriptor and "version" with the version from the consensus.

And for bridges we include the version substring from the "platform" line. That's somewhat redundant. But in some cases it might be easier for clients to process that version string without the rest of the rest of the "platform" line around it. No harm in adding that new field to both relay and bridge details documents.

Please review the first commit bd5b45a in my tasks-22488-23962-21827-23544 branch together with specification changes in commit 75c7626 in my corresponding metrics-web branch.

comment:6 Changed 12 months ago by iwakeh

Status: needs_reviewmerge_ready

Patch looks fine and it is a good idea to provide the 'version' field.
Tests and checks pass, spec changes describe the changes. => ready for merge.

comment:7 Changed 12 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

Great, thanks for checking! Merged. Closing.

comment:8 Changed 12 months ago by iwakeh

Milestone: Onionoo-1.8.0

This will be part of the next release.

comment:9 Changed 11 months ago by cypherpunks

Resolution: fixed
Status: closedreopened

Is this fix already deployed?
A few relays that have recommended_version set to False even though they are running a recommended version:

+----------------+-------------+------------------------------------------+---------------------+---------+
| nickname       | tor_version | fingerprint                              | last_restarted      | running |
+----------------+-------------+------------------------------------------+---------------------+---------+
| TOris          | 0.3.1.9     | A2888CB01CEAD3BBD84F63707F45318A7A71D141 | 2018-01-08 07:39:07 |       0 |
| NoSpyOrg       | 0.2.9.14    | 1AFD68DF6403140E32E73F1DB72415CE4229832F | 2018-01-09 10:17:15 |       0 |
| DeltaIV        | 0.3.1.9     | 75CCC14FB38F2E473AC2C31F255355110FCE8D38 | 2018-01-06 21:01:47 |       0 |
| cosmicsurfin   | 0.3.1.9     | 4A0242BED06138AD6BB1FE103A1D5D3AA9E97C05 | 2018-01-08 21:32:15 |       0 |
| wintermute     | 0.3.1.9     | 276EAD6119C327930C0B8D3E370BF163EFB0244B | 2018-01-09 15:40:38 |       1 |
| heteigenwijsje | 0.3.1.9     | 1283EBDEEC2B9D745F1E7FBE83407655B984FD66 | 2018-01-09 16:38:02 |       1 |
| monoculus      | 0.3.1.9     | A1A01D1B3353F3CFB065E9CBA113AEEB948DF8E1 | 2018-01-09 20:47:16 |       1 |
| Unnamed        | 0.2.9.14    | 7C8F045FC6710FBA57B2C84D008F91BCEABD6A36 | 2018-01-03 15:32:15 |       0 |
| propub01       | 0.3.2.8-rc  | 4290EEF4276FEACAD3A4694770B3B905FED51302 | 2018-01-03 22:27:11 |       0 |
+----------------+-------------+------------------------------------------+---------------------+---------+

comment:10 Changed 11 months ago by cypherpunks

This causing confusion for tor relay operators
example:
http://lists.nycbug.org/pipermail/tor-bsd/2018-January/000620.html

Changed 10 months ago by karsten

comment:11 Changed 10 months ago by karsten

Status: reopenedneeds_information

Hmm, you might have found something here, but I'm not certain yet. (And I have too many other things going on to take a closer look.)

For the record, recommended server versions as of your comment were: 0.2.5.16, 0.2.5.17, 0.2.9.14, 0.2.9.15, 0.3.0.13, 0.3.0.14, 0.3.1.9, 0.3.1.10, 0.3.2.8-rc, 0.3.2.9.

Please find the attached compressed CSV file which contains:

  • fingerprint: the relay fingerprint, limited to the relays you listed
  • timestamp: the timestamp when a descriptor was published or after which a status was declared valid
  • descriptor: "server" for server descriptors published by the relay, "vote" for network status votes, "consensus" for network status consensuses
  • authority: authority nickname in case of a vote and blank otherwise
  • version: tor software version

Again, I'm not certain yet, but it looks like there's a delay of a few hours between relays publishing a server descriptor with a new tor software version number and authorities including that version in their votes. This delay is different between authorities, with some authorities picking up the change earlier than others.

If this is true, and somebody would have to look through the attached file in detail, that's a bug in the authorities. Totally wild guess: authorities might consider platform changes "cosmetic" and continue using an older descriptor of the relay until that gets too old.

However, from Onionoo's perspective I think we don't have a bug here. Onionoo uses whatever version it sees in the consensus to say whether a relay runs a recommended version or not. If the authorities don't update that version number properly, that's not something that Onionoo can fix.

Want to analyze this more and possibly open a ticket in Core Tor/Tor if you can confirm that this is a bug in tor? (But please don't move this ticket to that component, because 90% of what's discussed above is irrelevant for the issue you may have found.)

comment:12 Changed 10 months ago by cypherpunks

Hi Karsten,
thanks for that CSV file. I can confirm what you suggested:
https://lists.torproject.org/pipermail/tor-dev/2018-January/012786.html

comment:13 Changed 10 months ago by teor

I did some analysis of the examples in the Core Tor ticket, Tor appears to be working fine:
https://trac.torproject.org/projects/tor/ticket/24864#comment:6

Perhaps this is just a bug in Onionoo?

I am waiting for some more examples in the Core Tor ticket.

comment:14 in reply to:  13 ; Changed 10 months ago by karsten

Replying to teor:

I did some analysis of the examples in the Core Tor ticket, Tor appears to be working fine:
https://trac.torproject.org/projects/tor/ticket/24864#comment:6

Perhaps this is just a bug in Onionoo?

Hmm, I don't see how Onionoo would be the issue here. The preliminary analysis that I did in comment 11 above is not based on Onionoo, but on a new analysis made using metrics-lib.

I am waiting for some more examples in the Core Tor ticket.

Did you check the CSV file that is attached to this ticket? That just contains the relays above. I'm going to upload the full file with all relay fingerprints now and will post the link here in 10-20 minutes.

comment:15 in reply to:  14 Changed 10 months ago by karsten

Replying to karsten:

[...] I'm going to upload the full file with all relay fingerprints now and will post the link here in 10-20 minutes.

https://people.torproject.org/~karsten/volatile/task-22488-relay-versions-full.csv.xz

comment:16 Changed 10 months ago by iwakeh

Keywords: metrics-2018 added; metrics-2017 removed
Milestone: Onionoo-1.8.0Onionoo-2.0.0

Adapting to new year and next version.

comment:17 Changed 10 months ago by karsten

So, from the discussion in #24864 it sounds like this is either a bug in the tor daemon or somewhat non-intuitive behavior of the tor daemon. But I don't see where Onionoo would be doing something wrong here. If there's no bug, we should close this ticket again.

comment:18 Changed 10 months ago by teor

Resolution: not a bug
Status: needs_informationclosed

Not a bug in Onionoo

comment:19 Changed 10 months ago by cypherpunks

I believe there is still something that is related to onionoo regardless of the other related tickets.

If onionoo says tor_version:some good tor version
and it displays recommended_version:false
than there seems to be a mismatch.

Onionoo should display the tor version it uses to compare against the list of recommended_versions
and not any other version. If it does, this mismatch will disappear, right?

Last edited 10 months ago by cypherpunks (previous) (diff)

comment:20 Changed 10 months ago by cypherpunks

Resolution: not a bug
Status: closedreopened

comment:21 Changed 10 months ago by teor

Onionoo displays the platform from the descriptor.
But it uses the relay version and recommended versions list from the consensus to calculate the relay recommended version field.
This is a correct behaviour.

There are three ways to resolve this issue:

  • clarify that the recommended version uses the consensus version at https://metrics.torproject.org/onionoo.html#details_relay_recommended_version
  • make Onionoo parse the platform field in the descriptor and compare it to the consensus version
  • fix the underlying bug in Tor and wait 6 months for the directory authorities to deploy the fix (see #24864)
    • since this isn't causing any network issues, it's low priority right now. I'm not sure when we will get time to fix it.

comment:22 in reply to:  21 ; Changed 10 months ago by cypherpunks

Replying to teor:

Yes, it would be great if onionoo includes both and atlas could then display a clear info when they are not the same (and display only one if they are the same, so to not confuse operators).

Last edited 10 months ago by cypherpunks (previous) (diff)

comment:23 in reply to:  22 ; Changed 10 months ago by teor

Status: reopenedneeds_review

Replying to cypherpunks:

Replying to teor:

Replying to teor:

...

Please see my branch bug22488 at https://github.com/teor2345/metrics-web.git

It updates the Onionoo documentation for recommended_version. The versions used in recommended_version come from the consensus or bridge networkstatus. I checked the code for relays and bridges.

  • make Onionoo parse the platform field in the descriptor and compare it to the consensus version

Yes, it would be great if onionoo includes both and atlas could then display a clear info when they are not the same (and display only one if they are the same, so to not confuse operators).

  • make Onionoo parse the platform field in the descriptor and compare it to the consensus version

I am not sure if this fix is a good idea. We don't trust descriptors when they are not in a consensus. (Their details can be wrong.) So why are any of our tools using them? And what should Onionoo do if a relay has multiple descriptors?

And I don't have time to write this fix. It looks like new feature that involves at least one new field. Or a rethink of how we parse descriptors and which ones we should parse.

For an alternative fix where Onionoo stops parsing untrusted descriptors, see #24932.

Version 0, edited 10 months ago by teor (next)

comment:24 in reply to:  23 Changed 10 months ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Replying to teor:

Replying to cypherpunks:

Replying to teor:

Please see my branch bug22488 at https://github.com/teor2345/metrics-web.git

It updates the Onionoo documentation for recommended_version. The versions used in recommended_version come from the consensus or bridge networkstatus. I checked the code for relays and bridges.

Thanks for providing that patch! Merged with trivial tweaks, and deployed.

  • make Onionoo parse the platform field in the descriptor and compare it to the consensus version

Yes, it would be great if onionoo includes both and atlas could then display a clear info when they are not the same (and display only one if they are the same, so to not confuse operators).

I am not sure if this fix is a good idea. We don't trust descriptors when they are not in a consensus. (Their details can be wrong.) So why are any of our tools using them? And what should Onionoo do if a relay has multiple descriptors?

And I don't have time to write this fix. It looks like new feature that involves at least one new field. Or a rethink of how we parse descriptors and which ones we should parse.

In theory, this should be as simple as checking whether the "platform" string starts with "Tor " + version_field_content + " ". This could already be done in Relay Search and doesn't require a new Onionoo field. At least it could be a start, if we really care.

For an alternative fix where Onionoo stops parsing untrusted descriptors, see #24932.

Indeed, this is worth thinking about. I'll comment on the ticket why this might be tricky to implement, though. But it seems like something we should do.

Okay, I think with the documentation patch merged and #24864 and #24932 still open, it's safe to close this ticket now. Doing so. Thanks, everyone!

comment:25 Changed 10 months ago by cypherpunks

I just realized that onionoo provides already both data points (platform and version as seen in consensus):

https://onionoo.torproject.org/details?fields=version,platform,recommended_version,fingerprint&search=fastor&type=relay

{"version":"5.0",
"build_revision":"0bce98a",
"relays_published":"2018-01-23 08:00:00",
"relays":[
{"fingerprint":"0EE3D3F6988B868F9018FFEC7567289E791D4785","platform":"Tor 0.3.2.9 on Linux","version":"0.2.9.11","recommended_version":false}
],
"bridges_published":"2018-01-23 08:04:05",
"bridges":[
]}

Filed a low prio ticket:
https://trac.torproject.org/projects/tor/ticket/24974

comment:26 Changed 8 months ago by karsten

Milestone: Onionoo-2.0.0Onionoo 2.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.