effective_family sometimes contains the relay's own fingerprint
This was originally reported as a Relay Search bug #25034 (moved).
Sometimes, but not always, the effective_family for a relay will include the relay itself. The specification does not explicitly say that a relay's own fingerprint will or won't be included, but from reading it I would assume that it is not included.
Activity
changed milestone to %Onionoo 1.11.0
Hmm, yes, I think it makes more sense to exclude the relay itself from its
"effective_family".It looks like the reason is that some relays include their own fingerprint in the family line of their server descriptor. For example,
03603092cf69a41857aed9131aba531cf4e71f27:@type server-descriptor 1.0 router sipuliDE01 94.130.181.81 6381 0 0 identity-ed25519 -----BEGIN ED25519 CERT----- AQQABnCOAQpGlUrHIKxdR7v7BHwzvtyu5vjWJb3q52EWHg6F8npMAQAgBAA1+uBN NBr4hTuW+Kbo0w4AdV1ZzBWqbP5pzKa1/WtPGBWiu91zkxM0APYsY17lf9/eu0dI SnmejAv3CB65kb20XbhG8fJzvbbAvKUApYaOVCO6FmJEwRlNBSVA6my6Ugo= -----END ED25519 CERT----- master-key-ed25519 NfrgTTQa+IU7lvim6NMOAHVdWcwVqmz+acymtf1rTxg or-address [2a01:4f8:1c0c:414b::1]:8573 platform Tor 0.2.9.14 on Linux proto Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2 published 2018-02-05 09:04:34 fingerprint 4357 0F86 2FE7 CAF7 B486 6B9B D95D B276 6221 8BBD uptime 3515 bandwidth 1073741824 2147483647 57535 extra-info-digest C3F79852F13BD8F15EB221F6D0A298D416242EE0 xbEm5P5s0wcubfLRIYV4ms6cFBz0764X24OpNpa0SGI onion-key -----BEGIN RSA PUBLIC KEY----- MIGJAoGBALkbUUVGfsDVS9Vi8BRc0CLeS0PbRKoAh3CICoychg00B6nQmxGTJX0o okKUGikA+AmTDcii7VQd3M1INVT7XPIjy7PMAiiiBZqogREMMW1r70+AZEK/whJu 2CjesaY70rP0yUZYsT8QNZgOUNR14jQyB5z2ofYSMZWAacQRdYjJAgMBAAE= -----END RSA PUBLIC KEY----- signing-key -----BEGIN RSA PUBLIC KEY----- MIGJAoGBAOGWq/4J9CsBA06Re32X8U522p97sQeRY1jXNgS+6cQ14rjfGyU7ydq6 jab34lHe2lLdyENhYroOeUSJWFRKX276IrZBJ1CS+UMPVHcIQvdDMSImg4zEESAX PNHk2tdnESTVlaUIAStVnQU9Nu3wH7VzQDhupEEYpOJY7siVAJObAgMBAAE= -----END RSA PUBLIC KEY----- onion-key-crosscert -----BEGIN CROSSCERT----- TKINsu+xKXjn+/4IP8pT7VD7BKKBGF7Pywoo3X+YLzcoESIUgZ9oyUdwt1tPQ4Qr Yar+hxh1dcJZ9ge7uVp+v/m6zXwFfUTka5MUAgaaUPMqryIp+wV+A7W6rv/7cSBB Six6IsOJPIVkUKuzYDlMspdRjR6/DcBvlhif40v62SQ= -----END CROSSCERT----- ntor-onion-key-crosscert 1 -----BEGIN ED25519 CERT----- AQoABm+aATX64E00GviFO5b4pujTDgB1XVnMFaps/mnMprX9a08YANimuVWhML71 rmBaLJlfawceE7k5P69VKqXZGis3cnSovIsvFK3p4+vELEwdZEsVu66W5Znumyc8 a3Rg5jQrRQk= -----END ED25519 CERT----- family $43570F862FE7CAF7B4866B9BD95DB27662218BBD $AEB76F909EE1730926195E92F0D6BBDDD0AC60ED $AFC637373810BE6EC4411F58D575CE5D137326CF $C552FF01540030F675DCF4793F1C32828ACD3FF1 hidden-service-dir contact O_O <tor at lin dot fi> ntor-onion-key IWbsv6SwQv06fSdlybpnF3RuXG4q1tA11FbXB+/ELV8= accept 8.8.8.8:53 accept 8.8.4.4:53 reject *:* router-sig-ed25519 TbKPy+MSIG9Cv2GyOGtCykcDJRD04bCESaFDHGqoxjLxQrcV12m3HDzKNBqgYWNXuX054kVDqmNQkbkaFfSfDw router-signature -----BEGIN SIGNATURE----- GuoAa7adXOktLBp7V0hvK4r9TT2tfGkLcxWQlfDf2dtv1NNwQ+DvLXAE+tnWaw8B 8foFO7UAMTPNdePk7uMVOkwRlodEoAw1cwuE0U9rnrcM4jUA/xAnMiqs3bVqTh42 L9JD2IfRQGmINm9pKeY/T1rPNF41Vwtdh0Csx0IPbqU= -----END SIGNATURE-----Onionoo returns the following details document for this relay:
{ "version": "5.0", "build_revision": "a66bfd1", "relays_published": "2018-02-14 10:00:00", "relays": [ { "nickname": "sipuliDE01", "fingerprint": "43570F862FE7CAF7B4866B9BD95DB27662218BBD", "or_addresses": [ "94.130.181.81:6381", "[2a01:4f8:1c0c:414b::1]:8573" ], "last_seen": "2018-02-12 05:00:00", "last_changed_address_or_port": "2018-01-24 22:00:00", "first_seen": "2018-01-23 14:00:00", "running": false, "flags": [ "Fast", "Running", "Stable", "Valid" ], "country": "de", "country_name": "Germany", "latitude": 51.2993, "longitude": 9.491, "as_number": "AS24940", "as_name": "Hetzner Online GmbH", "consensus_weight": 66900, "host_name": "static.81.181.130.94.clients.your-server.de", "last_restarted": "2018-02-11 18:31:40", "bandwidth_rate": 1073741824, "bandwidth_burst": 2147483647, "observed_bandwidth": 0, "advertised_bandwidth": 0, "exit_policy": [ "accept 8.8.8.8:53", "accept 8.8.4.4:53", "reject *:*" ], "exit_policy_summary": { "reject": [ "1-65535" ] }, "contact": "O_O <tor at lin dot fi>", "platform": "Tor 0.2.9.14 on Linux", "version": "0.2.9.14", "effective_family": [ "43570F862FE7CAF7B4866B9BD95DB27662218BBD", "AEB76F909EE1730926195E92F0D6BBDDD0AC60ED", "AFC637373810BE6EC4411F58D575CE5D137326CF", "C552FF01540030F675DCF4793F1C32828ACD3FF1" ], "recommended_version": true, "hibernating": true, "measured": true } ], "bridges_published": "2018-02-14 10:04:06", "bridges": [] }I wrote a quick fix that I'm currently testing. Grabbing this ticket and posting a branch as soon as it's tested.
Trac:
Status: new to accepted
Owner: metrics-team to karsten-
Just in case we need more examples, here are all details documents that I found that have their
"fingerprint"as one element in their"effective_family"field:8FF0B5BD7AE8A20DD2033E06053EE7B8BA609FAA 23AA8C61EC325C1F9B2443A64F5721C8D1074E0C 43570F862FE7CAF7B4866B9BD95DB27662218BBD 95C4EF3787069A3A1A3B5FD4057A34CA97C171C3 695DBEE606CA6CD99CD925CAA07DDF5CD8C9190B CF9831F5CE1068F083D0092F30CD77C4847CFC56 13557448AD1632BF080757C2FF7769BB23DF1DF3 0820262CA0468D56A20C3ABC62F58CC5F73CB53D 1B37A891E0BCB1911F1E442ADE9A3C5CC7542692 940850BDE514895F68B30D05C99637A0D27D07A9 
This is fixed for Relay Search (#25034 (moved)) so that there's no incorrect family counts showing up there (just deployed), but this should still be fixed in Onionoo too. Thanks for the example relays, handy for testing. (:
-
Glad to hear that the fingerprints helped and that Relay Search is already fixed.
Please somebody (iwakeh or irl) review commit 5da2d85 in my task-25241 branch.
Trac:
Status: accepted to needs_review 
Wondering, if his shouldn't go in a different direction. I would expect the family to contain the listing relay (the 'self'). Isn't it a use case to compare families of relays? Then I would need to somewhat artificially add the 'self's. And, the field's name also yields that the entire family is listed incl. 'self'. So, Onionoo should rather add 'self' always (, which is also a slightly simpler patch). I couldn't find any reasons for removing the 'self' in the other tickets referenced here. Thoughts?
Trac:
Status: needs_review to needs_informationIirc I'm the reason that effective_family has been added to onionoo and I'm post-processing onionoo data to always add self to all effective_family sets to have uniform effective_family sets for relays that are in the same effective_family. (That said I'm happy with either, remove it always or add it always)
-
Please find two commits on top of the above branch.
(Pending question still in comment:6)
Trac:
Status: needs_information to needs_review -
Hmm. Interesting idea. Some thoughts/questions:
- What would the
"effective_family"field contain if a relay is not in an effective family with any other relay? - The suggestion is to include the relay itself to
"effective_family"only, and not to"alleged_family"or"indirect_family", right?
I'm open to this idea, especially if it makes processing easier for Onionoo clients. It's a bit redundant to include the same fingerprint twice in a document, but we made similar decisions in the past where some redundancy is acceptable if that makes it easier to process results.
I'll look closer at the branch as soon as we have made a decision here.
- What would the
-
Replying to karsten:
Hmm. Interesting idea. Some thoughts/questions:
- What would the
"effective_family"field contain if a relay is not in an effective family with any other relay?
A family consists of that relay then, or we decide that it needs more than one to be a family, but that seems counter intuitive.
- The suggestion is to include the relay itself to
"effective_family"only, and not to"alleged_family"or"indirect_family", right?
The protocol states clearly that the effective family relays are not contained therein. So, actually, the 'self' needs to be in the effective family to make these correct.
I'm open to this idea, especially if it makes processing easier for Onionoo clients. It's a bit redundant to include the same fingerprint twice in a document, but we made similar decisions in the past where some redundancy is acceptable if that makes it easier to process results.
It follows the protocol more closely, I think.
I'll look closer at the branch as soon as we have made a decision here.
Even the other way wouldn't need big changes to the patch, only replacing 'remove' with 'add' and test adaption.
- What would the
-
Replying to irl:
If we do include a relay in its own effective family, Relay Search will need to be updated to not perform that step as it currently does.
Not sure I understand. The spec seems vague right now, so how about Relay Search just either adds or removes a relay's own fingerprint to ensure it's contained or not contained, whichever state it is that Relay Search expects? That is, what prevents us from updating Relay Search now and making the Onionoo change at a later time?
-
Currently Relay Search removes the relay. This means it has to iterate over 2000 (worst case) lists to remove fingerprints for the search view as effective family count is shown in the results table. When I added the code to filter the fingerprints, I added a comment to say this was temporary and should be removed after this ticket was resolved.
https://gitweb.torproject.org/atlas.git/commit/?id=409e410b5c10666c202cd56b2ff42db0c0a8f766
The important thing to achieve in this ticket is consistency. My comment was not very clear, it's a different change that should be made in Relay Search depending on which way we choose to go making it consistent.
If we always add the relay to its family:
- Stop filtering the relay's own fingerprint
- Stop adding 1 to the effective family count
If we always remove the relay from its family:
- Stop filtering the relay's own fingerprint
- Continue to add 1 to the effective family count
-
Open steps:
- decide for adding 'self' always to the effective family, because clients (like RS) interpret it like that (and had to work around the inconsistent situation so far) or against it because it would add too much to current documents
- explicitly state whether the relay belongs to its effective family or not in the protocol (if not it should also explicitly be added that the 'self' doesn't show up in the other two family types either)
- make & merge the appropriate changes (everywhere)
-
Replying to iwakeh:
Open steps:
- decide for adding 'self' always to the effective family, because clients (like RS) interpret it like that (and had to work around the inconsistent situation so far) or against it because it would add too much to current documents
I'm fine with this. From comments above it sounds like there are no concerns. Let's do it.
- explicitly state whether the relay belongs to its effective family or not in the protocol (if not it should also explicitly be added that the 'self' doesn't show up in the other two family types either)
- make & merge the appropriate changes (everywhere)
Do you want to do this? I just reviewed your task-25241 branch until commit df11f4d. Looks good! Do you want to change it to always add a relays own fingerprint to its declared family? And I guess we'll squash those commits in the end, to avoid making a change and undoing it right afterwards.
If you're already juggling too many tickets, I can also take this one. Let me know.
-
Replying to karsten:
Replying to iwakeh:
Open steps:
- decide for adding 'self' always to the effective family, because clients (like RS) interpret it like that (and had to work around the inconsistent situation so far) or against it because it would add too much to current documents
I'm fine with this. From comments above it sounds like there are no concerns. Let's do it.
- explicitly state whether the relay belongs to its effective family or not in the protocol (if not it should also explicitly be added that the 'self' doesn't show up in the other two family types either)
- make & merge the appropriate changes (everywhere)
Do you want to do this? I just reviewed your task-25241 branch until commit df11f4d. Looks good! Do you want to change it to always add a relays own fingerprint to its declared family? And I guess we'll squash those commits in the end, to avoid making a change and undoing it right afterwards.
If you're already juggling too many tickets, I can also take this one. Let me know.
You might be more efficient on this task because the tests need to be adapted; and you reviewed my suggested branch already, in which only small simple changes will be necessary.
-
Please review another commit that implements the changes discussed above.
Trac:
Status: accepted to needs_review -
Looks good. Here's a minor tweak in commit 2817936 of my task-25241 branch. If that looks good, I'll squash and merge.
-
Moving the specification part to #25476 (moved) and closing this ticket.
Trac:
Resolution: N/A to fixed
Status: merge_ready to closed mentioned in issue #25476 (moved)