Include both declared and reachable IPv6 OR addresses
When a relay declares an IPv6 OR address, it puts it in its descriptor, and it gets placed in the microdescriptor automatically.
But the IPv6 addresses in the full consensus are different: authorities on IPv6 only vote for an IPv6 address if they believe it is reachable.
(There are no IPv6 addresses in the microdesc consensus, see #20916 (moved).)
This makes a difference on Atlas tickets like #10401 (moved).
(It doesn't make a difference to client behaviour yet, because microdescs are the default. We'll fix that in #20916 (moved).)
Activity
changed milestone to %Onionoo-1.7.0
Replying to karsten:
I believe we're taking addresses from consensuses, not from server descriptors, so we're only including reachable IPv6 OR addresses. And you want to learn about declared and unreachable IPv6 OR addresses, right?
Ok, so that's good: we're only displaying IPv6 addresses that work.
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Replying to teor:
Replying to karsten:
I believe we're taking addresses from consensuses, not from server descriptors, so we're only including reachable IPv6 OR addresses. And you want to learn about declared and unreachable IPv6 OR addresses, right?
Ok, so that's good: we're only displaying IPv6 addresses that work.
Somebody (possibly I) should confirm by belief here by looking at the code.
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Yes, makes sense.
-
Replying to karsten:
Replying to teor:
Replying to karsten:
I believe we're taking addresses from consensuses, not from server descriptors, so we're only including reachable IPv6 OR addresses. And you want to learn about declared and unreachable IPv6 OR addresses, right?
Ok, so that's good: we're only displaying IPv6 addresses that work.
Somebody (possibly I) should confirm by belief here by looking at the code.
So, I did look at the code and some sample descriptors and now feel more confident to confirm: we're only displaying IPv6 addresses that work.
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Yes, makes sense.
How about we add a new field "unreachable_or_addresses" that is an optional array of strings and that contains an "Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty." If you can think of specifying this more clearly, please feel free to suggest a better text. And if you can think of a better way to include this data, just state it here.
Regarding the suggested flag, I believe that that shouldn't be a relay flag like the existing ones, but it could be a custom notification like Atlas adds for relays running an non-recommended version. In other words, that's for Atlas land, not Onionoo land. Hope that matches your idea. If not, please elaborate.
Replying to karsten:
Replying to karsten:
Replying to teor:
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Yes, makes sense.
How about we add a new field "unreachable_or_addresses" that is an optional array of strings and that contains an "Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty." If you can think of specifying this more clearly, please feel free to suggest a better text. And if you can think of a better way to include this data, just state it here.
Do we need to explain:
- claims to accept onion-routing connections in its descriptor
- the directory authorities failed to confirm as reachable. (If a relay has an unreachable IPv4 address, the relay is removed from the consensus. If a relay has an unreachable IPv6 address, the relay is included in the consensus without the IPv6 address.)
This is how things will work when authorities upgrade to 0.3.3, assuming we merge the IPv6 fixes in #20916 (moved).
Regarding the suggested flag, I believe that that shouldn't be a relay flag like the existing ones, but it could be a custom notification like Atlas adds for relays running an non-recommended version. In other words, that's for Atlas land, not Onionoo land. Hope that matches your idea. If not, please elaborate.
Sounds good to me. This will help us prompt operators when their IPv6 is broken.
-
Replying to teor:
Replying to karsten:
Replying to karsten:
Replying to teor:
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Yes, makes sense.
How about we add a new field "unreachable_or_addresses" that is an optional array of strings and that contains an "Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty." If you can think of specifying this more clearly, please feel free to suggest a better text. And if you can think of a better way to include this data, just state it here.
Do we need to explain:
- claims to accept onion-routing connections in its descriptor
Sure, we can add the "in its descriptor" part.
- the directory authorities failed to confirm as reachable. (If a relay has an unreachable IPv4 address, the relay is removed from the consensus. If a relay has an unreachable IPv6 address, the relay is included in the consensus without the IPv6 address.)
Yes, let's try to add that. Maybe we can make the distinction between primary and additional addresses rather than IPv4 and IPv6, just in case it will be possible to use an IPv6 address as primary address or an IPv4 address as additional address in the future.
This is how things will work when authorities upgrade to 0.3.3, assuming we merge the IPv6 fixes in #20916 (moved).
Hmm, can you elaborate how either the upgrade or the bugfix will affect us here?
In any case, here's the edited specification with additional parts being written in italics:
"Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims in its descriptor to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Contains only additional addresses of a relay that are found unreachable, whereas relays with an unreachable primary address are excluded entirely. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty."
Please continue editing or suggesting as necessary!
Regarding the suggested flag, I believe that that shouldn't be a relay flag like the existing ones, but it could be a custom notification like Atlas adds for relays running an non-recommended version. In other words, that's for Atlas land, not Onionoo land. Hope that matches your idea. If not, please elaborate.
Sounds good to me. This will help us prompt operators when their IPv6 is broken.
Sounds great!
-
My task-21637 branch contains a small patch that adds this new field. Requires a minor protocol version bump when merging.
Trac:
Status: new to needs_review Replying to karsten:
Replying to teor:
Replying to karsten:
Replying to karsten:
Replying to teor:
It would be nice to have the IPv6 address from the descriptor, and then we can synthesise a flag if it's missing from the consensus (or different).
Yes, makes sense.
How about we add a new field "unreachable_or_addresses" that is an optional array of strings and that contains an "Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty." If you can think of specifying this more clearly, please feel free to suggest a better text. And if you can think of a better way to include this data, just state it here.
Do we need to explain:
- claims to accept onion-routing connections in its descriptor
Sure, we can add the "in its descriptor" part.
- the directory authorities failed to confirm as reachable. (If a relay has an unreachable IPv4 address, the relay is removed from the consensus. If a relay has an unreachable IPv6 address, the relay is included in the consensus without the IPv6 address.)
Yes, let's try to add that. Maybe we can make the distinction between primary and additional addresses rather than IPv4 and IPv6, just in case it will be possible to use an IPv6 address as primary address or an IPv4 address as additional address in the future.
Yes. Or some combination of primary and alternate addresses regardless of version.
This is how things will work when authorities upgrade to 0.3.3, assuming we merge the IPv6 fixes in #20916 (moved).
Hmm, can you elaborate how either the upgrade or the bugfix will affect us here?
Authorities will vote Running for relays with unreachable IPv6 addresses, but drop the unreachable address from the vote. At the moment, they vote not Running, and drop the entire relay from the consensus.
In any case, here's the edited specification with additional parts being written in italics:
"Array of IPv4 or IPv6 addresses and TCP ports or port lists where the relay claims in its descriptor to accept onion-routing connections but that the directory authorities failed to confirm as reachable. Contains only additional addresses of a relay that are found unreachable, whereas relays with an unreachable primary address are excluded entirely. Addresses are in arbitrary order. IPv6 hex characters are all lower-case. Omitted if empty."
Please continue editing or suggesting as necessary!
Looks great!
Regarding the suggested flag, I believe that that shouldn't be a relay flag like the existing ones, but it could be a custom notification like Atlas adds for relays running an non-recommended version. In other words, that's for Atlas land, not Onionoo land. Hope that matches your idea. If not, please elaborate.
Sounds good to me. This will help us prompt operators when their IPv6 is broken.
Sounds great!
-
Replying to teor:
Replying to karsten:
Replying to teor:
This is how things will work when authorities upgrade to 0.3.3, assuming we merge the IPv6 fixes in #20916 (moved).
Hmm, can you elaborate how either the upgrade or the bugfix will affect us here?
Authorities will vote Running for relays with unreachable IPv6 addresses, but drop the unreachable address from the vote. At the moment, they vote not Running, and drop the entire relay from the consensus.
It took me a while to figure out why I'm seeing relays with unreachable IPv6 address and the Running flag in the consensus. Turns out that only tor26 and gabelmoo check whether an IPv6 address is reachable and hence remove the Running flag from relays in their vote. But given that all the other authorities don't perform this check and assign the Running flag to relays, they end up in the consensus as Running. And #20916 (moved) will fix the part about votes not containing the Running flag for these relays? If so, okay. This shouldn't block deploying this Onionoo feature, though.
I also found a minor bug in my branch and pushed a fixup commit.
I'd say that my updated task-21637 branch is ready for code review now!
-
Replying to Sebastian:
Can someone please summarize this "only some dirauths vote running" stuff and send it to the dirauth list? That seems super broken and we really should get all dirauths on the same page wrt ipv6
Sure, I'll send my findings to that list, and teor can follow up as needed. (I thought that all of this is already known information. Maybe it is, we'll find out.)
Replying to Sebastian:
Can someone please summarize this "only some dirauths vote running" stuff and send it to the dirauth list? That seems super broken and we really should get all dirauths on the same page wrt ipv6
Sure!
I have a draft proposal that summarises the changes here:
https://github.com/teor2345/torspec/blob/bug20916/proposals/xxx-ipv6-in-micro-consensus.txt
It describes the current state of IPv6 voting. But I want to improve the explanation before sending it out.
Replying to karsten:
... I also found a minor bug in my branch and pushed a fixup commit.
I'd say that my updated task-21637 branch is ready for code review now!
Looks ok. What about tests for the new field? Other than that existing checks and tests pass.
-
Please review the last commit in my updated task-21637 branch which contains a new test class with unit tests for this new field.
Trac:
Status: needs_revision to needs_review mentioned in issue #23761 (moved)
mentioned in issue #24287 (moved)
mentioned in issue #24344 (moved)
mentioned in issue tpo/network-health/depictor#6