Flash proxy facilitator should not give IPv6 addresses to IPv4 proxies

The facilitator understands IPv4 and IPv6 client addresses. It should avoid giving an IPv6 client address to an IPv4-only proxy. Perhaps a reasonable heuristic is to only give IPv6 client addresses to proxies that talk to the facilitator over IPv6. It may be okay to assume that IPv6 proxies also support IPv4 (but also may not buy us much as long as IPv4 far outnumbers IPv6).

added component::archived/flashproxy owner::jct priority::medium resolution::fixed status::closed type::defect labels

For the moment I'm taking the facilitator's IPv6 address out of DNS to make it less likely that clients will try to register IPv6 addresses.

Trac:
Username: jct

patch_ticket_6124.patch

A patch with the candidate code in order to solve the ticket.

Trac:
Username: jct

The attached patch is doing the following:

• Guessing the IP version of the Proxy
• Guessing the IP version of the Client:
  • The Client's address in the list of registered clients ('''RegSet''') has a different encoding that the Proxy's address
• Giving to the Proxy a Client from RegSet with the same IP version:
  • It is easy to change it in order to give IPv4 clients to IPv6 Proxies

The patch contains an automatic test: ticket-6124-test.py

Trac:
Username: jct

Replying to jct:

The attached patch is doing the following:

• Guessing the IP version of the Proxy
• Guessing the IP version of the Client:
  • The Client's address in the list of registered clients (RegSet) has a different encoding that the Proxy's address
• Giving to the Proxy a Client from RegSet with the same IP version:
  • It is easy to change it in order to give IPv4 clients to IPv6 Proxies

The patch contains an automatic test: ticket-6124-test.py

Use two RegSets for IPv4 and IPv6, instead of searching the set for a reg of the appropriate family. fetch should remain a constant-time operation.

Functions like ip_ver that don't depend on a class instance (that don't use the self variable) should be ordinary functions, not part of a class. Put them in fac.py.

It looks like you want to factor out the address family-detection code from fac.parse_addr_spec into a new function (and have fac.parse_addr_spec call that function). That, or create a new function that can be run on the first element of an addr tuple (i.e., a numeric address string, not including brackets for IPv6), and returns the address family.

In general, use socket.AF_INET and socket.AF_INET6, not the numbers 4 and 6.

In the test code, change comments like "# IPv6 client and IPv4 proxy" into docstrings so they appear in the unittest output. Thank you for making new tests!

Trac:
Status: new to needs_revision

Trac:
Owner: dcf to jct
Status: needs_revision to assigned

Working on that!

Trac:
Username: jct

Trac:
Username: jct

ticket_6124.patch

A candidate patch in order to resolve the ticket.

Trac:
Username: jct

The new attached patch is doing the suggested previously.

The included suite test (ticket-6124-test.py) is testing the new function ip_ver, the modified parse_addr_spec and the Facilitator behaviour with IPv4/IPv6 proxies and clients.

Trac:
Username: jct

Replying to jct:

The new attached patch is doing the suggested previously.

The included suite test (ticket-6124-test.py) is testing the new function ip_ver, the modified parse_addr_spec and the Facilitator behaviour with IPv4/IPv6 proxies and clients.

There is also another change: when the proxy has IPv6 support, the Facilitator tries first to give it a IPv6 client, but if there isn't one, then tries to give it a IPv4 client.

Trac:
Username: jct

Replying to jct:

Replying to jct:

The new attached patch is doing the suggested previously.

The included suite test (ticket-6124-test.py) is testing the new function ip_ver, the modified parse_addr_spec and the Facilitator behaviour with IPv4/IPv6 proxies and clients.

There is also another change: when the proxy has IPv6 support, the Facilitator tries first to give it a IPv6 client, but if there isn't one, then tries to give it a IPv4 client.

I don't want this. IPv4 proxies get only IPv4 clients. IPv6 proxies get only IPv6 clients.

Replying to jct:

The new attached patch is doing the suggested previously.

The included suite test (ticket-6124-test.py) is testing the new function ip_ver, the modified parse_addr_spec and the Facilitator behaviour with IPv4/IPv6 proxies and clients.

I am not quite happy with this patch. It rewrites parse_addr_spec in a less straightforward way, and changes its behavior in ways that are not obvious.

I added some of the test cases to master; these are the only new tests I want. I don't want an ip_ver that is complicated enough to need lots of tests.

I'll attach a patch that is more in line with how I want this to be implemented. Please base the remaining work off of it. (Apply it with git am.)

Trac:
Status: assigned to needs_revision

Trac:
6124-twopools.patch

Replying to dcf:

Replying to jct:

The new attached patch is doing the suggested previously.

The included suite test (ticket-6124-test.py) is testing the new function ip_ver, the modified parse_addr_spec and the Facilitator behaviour with IPv4/IPv6 proxies and clients.

I am not quite happy with this patch. It rewrites parse_addr_spec in a less straightforward way, and changes its behavior in ways that are not obvious.

I added some of the test cases to master; these are the only new tests I want. I don't want an ip_ver that is complicated enough to need lots of tests.

I'll attach a patch that is more in line with how I want this to be implemented. Please base the remaining work off of it. (Apply it with git am.)

Done at: https://github.com/uyjco0/flashproxy/tree/6124

Trac:
Username: jct

Replying to jct:

Done at: https://github.com/uyjco0/flashproxy/tree/6124

Thanks, now merged.

I've reenabled the AAAA record for tor-facilitator.bamsoftware.com.

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

closed