Tor considers routers in the same IPv6 /16 to be "in the same subnet"

When EnforceDistinctSubnets is enabled, tor uses:

/** Return true iff router1 and router2 have similar enough network addresses                                                                                                                                       
 * that we should treat them as being in the same family */
static INLINE int
addrs_in_same_network_family(const tor_addr_t *a1,
                             const tor_addr_t *a2)
{
  return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
}

to determine if an address is in the same family. For an example IPv6 address, 2001:1234::0:1, its /16 representation is 2001::/16, meaning that 2001:ffff:: would be in the same family. A \16 for IPv6 is huge, particularly considering that only one-eighth of all IPv6 space is currently allocated for use on the internet (2000::/3). For the path selection code, using /16 essentially means that no two IPv6 routers in the same country (or possibly even continent) will be in the same path, and might possibly provide extremely increased chance of selection to routers in weird/rare IPv6 subnets.

For a related ticket, see #15517 (moved) governing how BridgeDB's version of EnforceDistinctSubnets will work for IPv6. (In that ticket, I proposed using IPv6 /32s, since that is the minimum ARIN IPv6 subnet allocation for a LIR.

changed milestone to %Tor: 0.3.5.x-final

added component::core tor/tor easy ipv6 milestone::Tor: 0.3.5.x-final owner::neel parent::24393 path path-bias points::1 priority::high resolution::fixed reviewer::teor severity::normal status::closed tor-client type::defect version::tor 0.2.3.1-alpha labels

Trac:
Description: When EnforceDistinctSubnets is enabled, tor uses:

/** Return true iff router1 and router2 have similar enough network addresses                                                                                                                                       
 * that we should treat them as being in the same family */
static INLINE int
addrs_in_same_network_family(const tor_addr_t *a1,
                             const tor_addr_t *a2)
{
  return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
}

to determine if an address is in the same family. For an example IPv6 address, 2001:1234::0:1, its /16 representation is 2001::/16, meaning that 2001:ffff:: would be in the same family. A \16 for IPv6 is huge, particularly considering that only one-eighth of all IPv6 space is currently allocated for use on the internet (2000::/3).

For a related ticket, see #15517 (moved) governing how BridgeDB's version of EnforceDistinctSubnets will work for IPv6. (In that ticket, I proposed using IPv6 /32s, since that is the minimum ARIN IPv6 subnet allocation for a LIR.

to

When EnforceDistinctSubnets is enabled, tor uses:

/** Return true iff router1 and router2 have similar enough network addresses                                                                                                                                       
 * that we should treat them as being in the same family */
static INLINE int
addrs_in_same_network_family(const tor_addr_t *a1,
                             const tor_addr_t *a2)
{
  return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
}

to determine if an address is in the same family. For an example IPv6 address, 2001:1234::0:1, its /16 representation is 2001::/16, meaning that 2001:ffff:: would be in the same family. A \16 for IPv6 is huge, particularly considering that only one-eighth of all IPv6 space is currently allocated for use on the internet (2000::/3). for the path selection code, using /16 essentially means that no two IPv6 routers in the same country (or possibly even continent) will be in the same path, and might possibly provide extremely increased chance of selection to routers in weird/rare IPv6 subnets.

For a related ticket, see #15517 (moved) governing how BridgeDB's version of EnforceDistinctSubnets will work for IPv6. (In that ticket, I proposed using IPv6 /32s, since that is the minimum ARIN IPv6 subnet allocation for a LIR.

Trac:
Description: When EnforceDistinctSubnets is enabled, tor uses:

/** Return true iff router1 and router2 have similar enough network addresses                                                                                                                                       
 * that we should treat them as being in the same family */
static INLINE int
addrs_in_same_network_family(const tor_addr_t *a1,
                             const tor_addr_t *a2)
{
  return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
}

to determine if an address is in the same family. For an example IPv6 address, 2001:1234::0:1, its /16 representation is 2001::/16, meaning that 2001:ffff:: would be in the same family. A \16 for IPv6 is huge, particularly considering that only one-eighth of all IPv6 space is currently allocated for use on the internet (2000::/3). for the path selection code, using /16 essentially means that no two IPv6 routers in the same country (or possibly even continent) will be in the same path, and might possibly provide extremely increased chance of selection to routers in weird/rare IPv6 subnets.

For a related ticket, see #15517 (moved) governing how BridgeDB's version of EnforceDistinctSubnets will work for IPv6. (In that ticket, I proposed using IPv6 /32s, since that is the minimum ARIN IPv6 subnet allocation for a LIR.

to

When EnforceDistinctSubnets is enabled, tor uses:

/** Return true iff router1 and router2 have similar enough network addresses                                                                                                                                       
 * that we should treat them as being in the same family */
static INLINE int
addrs_in_same_network_family(const tor_addr_t *a1,
                             const tor_addr_t *a2)
{
  return 0 == tor_addr_compare_masked(a1, a2, 16, CMP_SEMANTIC);
}

to determine if an address is in the same family. For an example IPv6 address, 2001:1234::0:1, its /16 representation is 2001::/16, meaning that 2001:ffff:: would be in the same family. A \16 for IPv6 is huge, particularly considering that only one-eighth of all IPv6 space is currently allocated for use on the internet (2000::/3). For the path selection code, using /16 essentially means that no two IPv6 routers in the same country (or possibly even continent) will be in the same path, and might possibly provide extremely increased chance of selection to routers in weird/rare IPv6 subnets.

For a related ticket, see #15517 (moved) governing how BridgeDB's version of EnforceDistinctSubnets will work for IPv6. (In that ticket, I proposed using IPv6 /32s, since that is the minimum ARIN IPv6 subnet allocation for a LIR.

Trac:
Keywords: path-bias deleted, path-bias 026-backport added
Milestone: N/A to Tor: 0.2.7.x-final
Priority: normal to major

Fortunately, that function is only called on the output of node_get_addr(), which calls node_get_prim_orport(), which can only return an IPv4 address. But we should come back to this when we do more with IPv6 addresses, and also when we fix #7193 (moved).

Trac:
Milestone: Tor: 0.2.7.x-final to Tor: 0.2.???
Priority: major to normal

This could be in 0.2.9, and there's no reason for an 0.2.6 backport from there.

Trac:
Sponsor: N/A to N/A
Severity: N/A to Normal
Milestone: Tor: 0.2.??? to Tor: 0.2.9.x-final
Keywords: path-bias 026-backport deleted, path-bias added

We should probably do this in 0.2.9 if the IPv6 client bootstrap in #17840 (moved) gets merged in 0.2.8.

Trac:
Points: N/A to small
Reviewer: N/A to N/A

Trac:
Points: small to 1

Trac:
Milestone: Tor: 0.2.9.x-final to Tor: 0.2.???
Keywords: N/A deleted, isaremoved added

Milestone renamed

Trac:
Milestone: Tor: 0.2.??? to Tor: 0.3.???

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

Trac:
Keywords: N/A deleted, tor-03-unspecified-201612 added
Milestone: Tor: 0.3.??? to Tor: unspecified

Remove an old triaging keyword.

Trac:
Keywords: tor-03-unspecified-201612 deleted, N/A added

Trac:
Keywords: isaremoved deleted, N/A added

Trac:
Keywords: N/A deleted, tor-client easy added
Priority: Medium to High

Trac:
Parent: N/A to #7193 (moved)

Trac:
Cc: isis, nicoo, Sebastian to isis, nicoo, Sebastian, Samdney

Trac:
Parent: #7193 (moved) to #24393 (moved)

Hey!! I am a beginner! Can I take this up!

Trac:
Username: aruna1234

You can! Try to stick with the same coding style we use, And submit a patch as a diff attachment or a git branch.

argh I forgot to assign it to me. argh. I have already spent some time with this.

But it was my mistake, hence you can take it aruna1234. ;)

Edit: Please give me a short feedback, if you want to do it. If possible I would prefer to work on it.

I am okay in working with you, as I am a beginner, I'll get to learn in the process. Can you give me a small feedback on how much you have worked. We can communicate via irc or e-mail.

Trac:
Username: aruna1234

Here's how to fix this bug:

• tor_addr_t has a family member, it can be AF_INET or AF_INET6.
• if the addresses have different families, they are not "in the same subnet".
• if the addresses are both IPv4, use the existing code to compare /16s.
• if the addresses are both IPv6, add new code that compares /32s.

Thanks for the guideline. I'm already beyond on this :P.

I have two queries, namely-

1. What is the use of AF_INET and AF_INET6?
2. Which file should I search for the function tor_addr_t?

Trac:
Username: aruna1234

1. AF_INET for IPv4 and AF_INET6 for IPv6.
2. src/common/address.h

I found tor_addr_compare and tor_addr_compare_masked in address.h which compares the address:

int tor_addr_compare(const tor_addr_t *addr1, const tor_addr_t *addr2, tor_addr_comparison_t how); int tor_addr_compare_masked(const tor_addr_t *addr1, const tor_addr_t *addr2, maskbits_t mask, tor_addr_comparison_t how); I also came across this snippet in address.c which is below. Is the the block of code which compares for /16 i.e IPv4? case AF_INET: { uint32_t a1 = tor_addr_to_ipv4h(addr1); uint32_t a2 = tor_addr_to_ipv4h(addr2); if (mbits <= 0) return 0; if (mbits > 32) mbits = 32; a1 >>= (32-mbits); a2 >>= (32-mbits); r = TRISTATE(a1, a2); return r; }

Trac:
Username: aruna1234

The current code in addrs_in_same_network_family() calls tor_addr_compare_masked() to compare all addresses by /16.

You need to add more calls to tor_addr_compare_masked() in addrs_in_same_network_family() so that:

• if the family of a1 is AF_INET, it uses /16 for IPv4
• if the family of a1 is AF_INET6 and tor_addr_is_v4(a1), it uses /112 for an IPv6-mapped IPv4 address (96 bits for the v4 map and 16 for IPv6)
• otherwise, it is native IPv6, and it uses /32.

I realise this is a change from what I said above, I just read tor_addr_compare_masked() and it's more complicated than I thought.

where is the function defination of addr_in_same_network_family()? I searched in both address.h and address.c but couldn't find it.

Trac:
Username: aruna1234

Hi, btw, I already spent time with this. If you can wait, I can help you tomorrow with my solution ;). I have only been very distracted by other stuff, the last days. It's a longer thing, and I'm sleepy. >.< See you.

Hi aruna1234,

Some development environments will index C code and let you search it. Or you can use an editor or search application that looks for a string in all the files in the tor source directory.

Or you can use command line tools to do the search like this:

git clone https://git.torproject.org/tor.git
grep -r addrs_in_same_network_family tor/src

When I use those commands, I can see that the function is defined in nodelist.c.

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

Trac:
Cc: isis, nicoo, Sebastian, Samdney to isis, nicoo, Sebastian, Samdney, neel@neelc.org
Owner: Samdney to neel

Trac:
Owner: neel to Samdney

Samdney,

Are you still interested in this bug report? If not, could I take it?

EDIT: I'm asking because I don't want to steal this bug report. I accidentally reassigned it to myself but then realized you had it so I gave it back to you.

Thanks,

Neel

There's been no progress on this ticket for a month, throwing it back in the pool.

Trac:
Owner: Samdney to N/A

Trac:
Owner: N/A to neel

PR is here: https://github.com/torproject/tor/pull/276

Thanks!

Please fix the missing newline in the changes file, and add some unit tests for each of the new combinations:

• IPv4 and IPv6
• IPv6 and IPv4
• IPv4 and IPv4-mapped-IPv6
• IPv4-mapped-IPv6 and IPv4
• IPv6 and IPv6

If you'd like, you can remove the IPv4-mapped-IPv6 code, I don't think we actually use it.

Trac:
Status: assigned to needs_revision
Reviewer: N/A to teor

I have added a unit test and have fixed the newline in the changes file.

I have also decided to remove the IPv4 mapped IPv6 code.

The CI failed, please run "make check-spaces" on your code, and fix the errors: https://travis-ci.org/torproject/tor/jobs/417076922#L2919

Fixed the spaces and pushed.

Your pull request has some extra commits a358d29, 438fc91 and a merge commit 81a2829. Next time you're updating a pull request, please add commits that just contain the new changes. (Re-doing all the changes, and doing merges, makes it harder to review your code.)

Here's how you can add changes to an existing branch in a terminal:

git checkout b15518
git pull
(use your editor to make changes)
git add -p
git add new-file-name
git commit
git push origin b15518 

I cherry-picked the commit 1f4f47e on master, and pushed it to https://github.com/teor2345/tor.git as b15518-min.

If the CI passes, we can merge it.

Thanks!

Trac:
Status: needs_revision to merge_ready

The CI succeeded at https://travis-ci.org/teor2345/tor/branches

lgtm; merging

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

This was merged to 0.3.5.

Trac:
Milestone: Tor: unspecified to Tor: 0.3.5.x-final

This bug was introduced in f9ea242aca in 0.2.3.1-alpha.

Trac:
Version: N/A to Tor: 0.2.3.1-alpha

closed

changed time estimate to 8h