extend_info_describe should list IPv6 address (if present)

changed milestone to %Tor: 0.4.2.x-final

added actualpoints::0.3 asn-merge component::core tor/tor easy fast-fix intro ipv6 logging milestone::Tor: 0.4.2.x-final nickm-merge no-backport parent::31088 points::0.5 priority::medium resolution::fixed reviewer::dgoulet severity::normal sponsor::27-can status::closed type::enhancement labels

Trac:
Description: When running an IPv6-only client using: src/or/tor ClientUseIPv4 0 DataDirectory mktemp -d UseMicrodescriptors 0

I get log messages like [info] add_an_entry_guard(): Chose $906933A309F15D7CF379127078A6791DF9B0C763~Unnamed at 91.121.82.25 as new entry guard

But I'm really contacting them on their IPv6 address. So we should list it along with the IPv4 address.

to

When running an IPv6-only client using:

src/or/tor ClientUseIPv4 0 DataDirectory `mktemp -d` UseMicrodescriptors 0

I get log messages like [info] add_an_entry_guard(): Chose $906933A309F15D7CF379127078A6791DF9B0C763~Unnamed at 91.121.82.25 as new entry guard

But I'm really contacting them on their IPv6 address. So we should list it along with the IPv4 address.

(This only makes sense for entry nodes: (directory) guards, bridges, fallbacks, authorities - all other relays are referred to by their IPv4 addresses.)

This is a problem with the address chosen by the calling code: an extend_info only has one tor_addr_t in it.

Trac:
Keywords: easy intro ipv6 deleted, easy intro ipv6 logging added

Is this ticket still valid?

According to teor comments, only entry nodes should be concerned by it. I've run an IPv6 client (ClientUseIPv4 0 UseMicrodescriptors 0) and I can't find any log of entry points using an IPv4 address. However, other nodes are referred to by their IPv4 addresses.

An example:

[debug] onion_extend_cpath(): Chose router $A6EDA578BA3CA94AB207841DBD468B0FF991643B~FalkensteinTor01 at 2a01:4f8:1c17:4216::1 for hop #1 (exit is inwaiting)
[debug] onion_extend_cpath(): Chose router $AC153580D1DE33051AFC975C7108BCD31CBBC9A1~spaghetti2 at 176.31.181.71 for hop #2 (exit is inwaiting)
[debug] onion_extend_cpath(): Chose router $18BFF1DEAA073003108CFDA2C0DE970D8F604478~inwaiting at 178.175.138.98 for hop #3 (exit is inwaiting)

Trac:
Username: valentecaio

Yes, this ticket is about log messages that look like:

[info] add_an_entry_guard(): Chose %s as new entry guard

where %s is the output of extend_info_describe().

But it is good to see that messages like:

[debug] onion_extend_cpath(): Chose router %s for hop #%d (%s)

work correctly.

I can't reproduce this output:

[info] add_an_entry_guard(): Chose %s as new entry guard

I think the function: add_an_entry_guard() has been deleted somewhere between the versions 0.2.3 and 0.2.4 . Is there any other function with the same bug?

Trac:
Username: meryemz

I would add that we have four functions for describing routers and nodes:

const char *router_describe(const routerinfo_t *ri);
const char *node_describe(const node_t *node);
const char *routerstatus_describe(const routerstatus_t *ri);
const char *extend_info_describe(const extend_info_t *ei);

But three of them use IPv4 addresses when describing the node. The only one that can use IPv6 is extend_info_describe().

Trac:
Username: valentecaio

Ok, then they should all be modified so they can use IPv6. In particular, each of the first 3 functions takes a struct with an IPv4 and IPv6 address, and so it should report both, if they are present.

Do you think it would be better to be able to show both addresses?

By now, the buffer used has space for this output format:

"$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at 
[ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]"

But I think the functions only return strings in one of the following formats (please confirm this if you can):

"$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at ffff:ffff:ffff:ffff:ffff:ffff"
or
"$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at 255.255.255.255"

We could add a separator and show both addresses, something like (increasing the buffer size by 2):

"$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at 
[ffff:ffff:ffff:ffff:ffff:ffff | 255.255.255.255]"

And we could let the caller decide what address it want as output (only IPv4, only IPv6 or both), letting "only IPv4" as default.

What do you think about this?

Trac:
Username: valentecaio

Replying to valentecaio:

Do you think it would be better to be able to show both addresses?

By now, the buffer used has space for this output format: {{{ "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]" }}}

This legacy format prints the last 4 bytes of the IPv6 address like an IPv4 address, it is never used in Tor.

But I think the functions only return strings in one of the following formats (please confirm this if you can): {{{ "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~~xxxxxxxxxxxxxxxxxxx at ffff:ffff:ffff:ffff:ffff:ffff" or "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~~xxxxxxxxxxxxxxxxxxx at 255.255.255.255" }}}

An IPv6 address has 128 bits, yours is 32 bits short. We are trying to always print IPv6 addresses between brackets.

We could add a separator and show both addresses, something like (increasing the buffer size by 2): {{{ "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at [ffff:ffff:ffff:ffff:ffff:ffff | 255.255.255.255]" }}}

The IPv6 address should be last, and in brackets. The IPv4 address should not be in brackets. I suggest using "or", to match "at".

And we could let the caller decide what address it want as output (only IPv4, only IPv6 or both), letting "only IPv4" as default.

We should always print all the addresses that are present.

What do you think about this?

Let's make it happen

Edit: spelling

My commit c83e48e82 has a draft version of this change. I need to fix the format, and add tests.

Trac:
Owner: N/A to teor
Status: new to assigned

Trac:
Status: assigned to needs_revision

Trac:
Keywords: easy intro ipv6 logging deleted, easy intro ipv6 logging fast-fix added

These items are not on our roadmap, so I won't be revising them any time soon.

Trac:
Keywords: easy intro ipv6 logging fast-fix deleted, teor-backlog-revise, fast-fix, logging, intro, easy, ipv6 added
Status: needs_revision to assigned
Owner: teor to N/A

Trac:
Status: assigned to needs_revision

We needed this feature to test #31088 (moved).

I made a pull request on master for this feature: https://github.com/torproject/tor/pull/1234

I still need to do some more unit tests, and a changes file. But I would like CI and an initial review.

I am not sure if we should backport this feature, so older versions get better diagnostics. Probably not.

Trac:
Parent: N/A to #31088 (moved)
Status: needs_revision to needs_review
Type: defect to enhancement
Milestone: Tor: unspecified to Tor: 0.4.2.x-final
Actualpoints: N/A to 0.3

Trac:
Keywords: teor-backlog-revise deleted, no-backport added

Trac:
Reviewer: N/A to dgoulet

Replying to teor:

I made a pull request on master for this feature: https://github.com/torproject/tor/pull/1234

I still need to do some more unit tests, and a changes file. But I would like CI and an initial review.

I did the unit tests and changes file, and did a force-push to the PR branch.

Hi Gaba, this ticket is also useful for debugging #23818 (moved) and #23507 (moved). Can we make it Sponsor27-can?

Trac:
Cc: N/A to gaba

Looks good!

One of my worry was that we were removing one side of the LongName format from the control spec with this:

-   buf[1+HEX_DIGEST_LEN] = is_named ? '=' : '~';
+   buf[1+HEX_DIGEST_LEN] = '~';

where LongName from the control spec is:

LongName   = Fingerprint [ ( "=" / "~" ) Nickname ]

I went over the control subsystem and we appear to be OK there.

Trac:
Keywords: N/A deleted, asn-merge nickm-merge added
Sponsor: N/A to Sponsor27-can
Status: needs_review to merge_ready

The Named flag is obsolete, so I think it is ok to remove code that uses is_named.

merged!

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

changed time estimate to 4h

added 2h 24m of time spent

mentioned in issue #31088 (moved)

mentioned in issue #31545 (moved)

moved to tpo/core/tor#21003 (closed)

mentioned in issue tpo/core/tor#31545 (closed)

extend_info_describe should list IPv6 address (if present)

Child items 0

Activity