Opened 2 years ago

Closed 2 years ago

#22139 closed defect (implemented)

last_restarted and platform missing even though it is available in descriptor

Reported by: cypherpunks Owned by: metrics-team
Priority: Medium Milestone: metrics-lib 1.9.0
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

there are currently 3 such relays, one example:
256F183F252DBBF080F2E70E5CB0F523A6323D0F

Also note that recommended_version is set to true even though that depends on the relay's platform string.

https://onionoo.torproject.org/details?fingerprint=256F183F252DBBF080F2E70E5CB0F523A6323D0F

{"version":"4.0",
"relays_published":"2017-05-03 08:00:00",
"relays":[
{"nickname":"UbuntuCore169","fingerprint":"256F183F252DBBF080F2E70E5CB0F523A6323D0F","or_addresses":["176.158.53.63:44583"],"last_seen":"2017-05-02 18:00:00","last_changed_address_or_port":"2017-05-02 18:00:00","first_seen":"2017-05-02 18:00:00","running":false,"flags":["Running","V2Dir","Valid"],"country":"fr","country_name":"France","region_name":"\u00CEle-de-France","city_name":"Paris","latitude":48.8628,"longitude":2.3292,"as_number":"AS5410","as_name":"Bouygues Telecom SA","consensus_weight":0,"host_name":"static-176-158-53-63.ftth.abo.bbox.fr","exit_policy_summary":{"reject":["1-65535"]},"recommended_version":true,"measured":false}
],
"bridges_published":"2017-05-03 06:57:32",
"bridges":[
]}

https://collector.torproject.org/recent/relay-descriptors/server-descriptors/2017-05-02-18-05-00-server-descriptors

@type server-descriptor 1.0
router UbuntuCore169 176.158.53.63 44583 0 0
identity-ed25519
-----BEGIN ED25519 CERT-----
AQQABleiAZ2Ce5QY1oSL0F79WeaPhL/zWomAVJG1vwTioPBkpeG7AQAgBABF3iK6
clXuNv2ZbfNSbmrJkKRLKsC41BZAVs1BSWQndRMNDsZJ/s6GmOd5IiU6axR5z2Nn
XTUR0TMGOc5KNJHqKi9Ht+iSIH02OeV1Gm/PNfos7KBKSJJROme1YQQsvwQ=
-----END ED25519 CERT-----
master-key-ed25519 Rd4iunJV7jb9mW3zUm5qyZCkSyrAuNQWQFbNQUlkJ3U
platform Tor 0.3.0.6 on Linux
proto Cons=1-2 Desc=1-2 DirCache=1 HSDir=1-2 HSIntro=3-4 HSRend=1-2 Link=1-4 LinkAuth=1,3 Microdesc=1-2 Relay=1-2
published 2017-05-02 17:25:22
fingerprint 256F 183F 252D BBF0 80F2 E70E 5CB0 F523 A632 3D0F
uptime 12
bandwidth 4194304 6291456 0
extra-info-digest 357F399E5A0FE2EEDEB7B3AD3D9328440EC17582 OgEu6BAQLUeTFjGofg0WTT9CYQsUGH9tiDENt/tiAD0
onion-key
-----BEGIN RSA PUBLIC KEY-----
MIGJAoGBAMYpYIFcAGOcfZBWt+nUPDu1ovbG8uamDBN4A/XTla74p6A3Ozl8/06D
1E/CcX6N2UahjDs+iM9EmND0k1CFgnkkkU7qBhm4aeOwfzSjDGXA52ab9vS0yEpa
aFHORGn88LRqcSvm9zRtChde5Ez0QJpBOuhyh19qIsSwT4EVa6CXAgMBAAE=
-----END RSA PUBLIC KEY-----
signing-key
-----BEGIN RSA PUBLIC KEY-----
MIGJAoGBAL6touSlbyMx2frcjIrLXcUUhN9rydnQhZrREZEdpALondnaEZzu3LE8
AeQI+VUTpZBlYbWR3Wh+wMDrdPzB3B07ATjAV3N07x6CtKk8YHE5RgShLlEr1k9c
DhN1VZi3rEA63pVfGTC1n7jXpAkMgYMW4KSHk40kgueu+3JxNSe1AgMBAAE=
-----END RSA PUBLIC KEY-----
onion-key-crosscert
-----BEGIN CROSSCERT-----
ITr+XCRVFqFE5o/5utRst/j8cZjEj43Ucd6n4Xoo566rVS9VPvUszduvPAZJECVS
QHPmshTsvXFH5+LEzCk0nN3cR5+iZX5zT15+1EoplE97doHQqtSTcA1CJSSFvoRj
1iobnqDn1lHLFyTMBJ4VV38a1NeovFmy4YkodTrtztk=
-----END CROSSCERT-----
ntor-onion-key-crosscert 1
-----BEGIN ED25519 CERT-----
AQoABlV6AUXeIrpyVe42/Zlt81JuasmQpEsqwLjUFkBWzUFJZCd1ALsQt0Q8mBNP
FcAXX6E+2oX2nGto910Sb1CBMPenMopKXaqArOPeqEQQx4+4x/waBLw7niBtEVjb
+WZ5cSha6Aw=
-----END ED25519 CERT-----
hidden-service-dir
ntor-onion-key fhiVUl9Ff0OlXd6zyqnfEA8u86KmewZISILHeU33Diw=
reject *:*
tunnelled-dir-server
router-sig-ed25519 pyHeZ3dimbx4cBOAjlhLbnav2F9FLrmy+CqO+QIv01VI4qK5xihG6s75HLj3s6dpa52xGBE6HNRdx2rCk2r3Bg
router-signature
-----BEGIN SIGNATURE-----
gJGxrxrbBVnO5x34450bKkBBBGZGJrgfYBLL6tfN6BhEYtENy9cWqt556boXsEuW
cN8z+OdNYr+LGJqUJgGWTSb1am26lU9lyHHHzVIhp9I9K4CXYq93POHCSore0M0c
PgAHPTkUN6WJvxachkEXwftzYaOLvJOqP+GFj+QvsVg=
-----END SIGNATURE-----

Child Tickets

Attachments (1)

2017-05-02-18-05-00-server-descriptors.xz (531.1 KB) - added by karsten 2 years ago.

Download all attachments as: .zip

Change History (6)

Changed 2 years ago by karsten

comment:1 Changed 2 years ago by karsten

Not a solution, but a first analysis: The issue is that the very first descriptor contained in that file contains a corrupt line, causing Onionoo/metrics-lib to skip the entire file. I attached the (compressed) file for later analysis. The corrupt line is: extra-info-digest 8FD8EF05DE6B65A8AC86E3E1FD8694E3F7D000DC PDZfioAN8N4ghKnLyCAMj4gRGQliaOtPYzHs@uploaded-at 2017-05-02 17:45:59. Thanks for the report!

comment:2 Changed 2 years ago by karsten

Component: Metrics/OnionooMetrics/metrics-lib
Milestone: metrics-lib 1.7.0

This is probably something we should fix in metrics-lib, ideally in the next release.

comment:3 Changed 2 years ago by iwakeh

Milestone: metrics-lib 1.7.0metrics-lib 1.8.0

comment:4 Changed 2 years ago by karsten

Milestone: metrics-lib 1.8.0metrics-lib 1.9.0

I looked more at this issue and believe that it's not exactly a mad bug but a rather sad design decision. Let me explain.

The situation is that we have a descriptor file starting with a truncated descriptor D0 followed by several complete descriptors D1 to Dn. In this specific case D0 is truncated in the middle of a base64-encoded digest string that we're trying to validate, so we're throwing a DescriptorParseException. But we're not catching that exception and moving on to D1. Instead, we're catching the exception in DescriptorReader where we're passing that exception to DescriptorFileImpl#setException() and not touching DescriptorFileImpl#setDescriptors() at all. Note that we would have done the exact same thing when running into a DescriptorParseException in Dn! As a result, a DescriptorFile either contains parsed descriptors or an exception. All or nothing.

This is not a bug, because we never claimed that we're returning parsed descriptors even if we run into an exception. That would have been the better design: just skip the descriptor we can't parse, set the exception if it's the first exception we ran into (as documented), and then move on to the next descriptor. But we can't say that the current implementation is incorrect. Even worse, if we change this behavior now, that might be considered a backward-incompatible change.

History lesson: when metrics-lib was designed, descriptors were usually stored as one descriptor per file. We later switched to concatenating server descriptors and extra-info descriptors because handling many small files was less efficient. And even later, last year, we considered concatenating even more descriptors including votes. But that was all not the case when metrics-lib came to life. End of history lesson.

My suggestion would be to implement #22141 first, so that the smallest returned unit is a Descriptor, not a DescriptorFile. We can fail a single descriptor that we can't parse, but we don't have to fail all other descriptors that happen to be contained in the same descriptor file. I'm changing the milestone to 1.9.0 for now.

comment:5 Changed 2 years ago by karsten

Resolution: implemented
Status: newclosed

This issue will be fixed in 1.9.0 with #22141 being just merged to master. The change is also backward-compatible, because we added a new readDescriptors(File...) method that can handle single malformed descriptors (by returning a UnparseableDescriptor), so we didn't have to wait for 2.0.0 to make this change. The readDescriptors() method that doesn't support single malformed descriptors is now deprecated and will be removed in 2.0.0. That means we can now close this ticket, which I'll do.

Note: See TracTickets for help on using tickets.