Exits should tell clients when they are connecting to an IPv6-only hostname

changed milestone to %Tor: unspecified

added 034-removed-20180328 034-triage-20180328 actualpoints::0.1 component::core tor/tor ipv6 milestone::Tor: unspecified parent::17811 points::1 priority::medium severity::normal status::needs-revision type::defect version::tor 0.2.4.7-alpha labels

These are fixed as part of #21311 (moved), as they touch the same code.

Can we get this in 0.3.0?

Trac:
Milestone: Tor: unspecified to Tor: 0.3.1.x-final
Parent: N/A to #21311 (moved)
Version: N/A to Tor: 0.2.4.7-alpha
Status: new to needs_review
Description: This code is wrong for at least two reasons:

it should also unset BEGIN_FLAG_IPV6_OK, and
it's way too early in the function: we might end up resolving an IPv6-only hostname, learn that it doesn't match our exit policy, and send the address back in the REASON_EXITPOLICY RELAY_END cell (See https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1436 )

  if (! options->IPv6Exit) {
    /* I don't care if you prefer IPv6; I can't give you any. */
    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
    /* If you don't want IPv4, I can't help. */
    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
      tor_free(address);
      relay_send_end_cell_from_edge(rh.stream_id, circ,
                                    END_STREAM_REASON_EXITPOLICY, NULL);
      return 0;
    }
  }

to

This code is wrong for at least two reasons:

it should also unset BEGIN_FLAG_IPV6_OK, and
~~it's way too early in the function: we might end up resolving an IPv6-only hostname, learn that it doesn't match our exit policy, and send the address back in the REASON_EXITPOLICY RELAY_END cell~~ ~~(See https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1436 )~~ (Apparently this works anyway.)

  if (! options->IPv6Exit) {
    /* I don't care if you prefer IPv6; I can't give you any. */
    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
    /* If you don't want IPv4, I can't help. */
    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
      tor_free(address);
      relay_send_end_cell_from_edge(rh.stream_id, circ,
                                    END_STREAM_REASON_EXITPOLICY, NULL);
      return 0;
    }
  }

See my branch bug21310 on github. It turns out it doesn't touch the same code, but it does require #21311 (moved) to work.

Trac:
Actualpoints: N/A to 0.1

Trac:
Description: This code is wrong for at least two reasons:

it should also unset BEGIN_FLAG_IPV6_OK, and
~~it's way too early in the function: we might end up resolving an IPv6-only hostname, learn that it doesn't match our exit policy, and send the address back in the REASON_EXITPOLICY RELAY_END cell~~ ~~(See https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1436 )~~ (Apparently this works anyway.)

  if (! options->IPv6Exit) {
    /* I don't care if you prefer IPv6; I can't give you any. */
    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
    /* If you don't want IPv4, I can't help. */
    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
      tor_free(address);
      relay_send_end_cell_from_edge(rh.stream_id, circ,
                                    END_STREAM_REASON_EXITPOLICY, NULL);
      return 0;
    }
  }

to

Edit: Turns out that these IPv6Exit option checks prevent clients ever seeing that they're trying to connect to an IPv6-only hostname

This code is wrong for at least two reasons:

it should also unset BEGIN_FLAG_IPV6_OK, and
~~it's way too early in the function: we might end up resolving an IPv6-only hostname, learn that it doesn't match our exit policy, and send the address back in the REASON_EXITPOLICY RELAY_END cell~~ ~~(See https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1436 )~~ (Apparently this works anyway.)

  if (! options->IPv6Exit) {
    /* I don't care if you prefer IPv6; I can't give you any. */
    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
    /* If you don't want IPv4, I can't help. */
    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
      tor_free(address);
      relay_send_end_cell_from_edge(rh.stream_id, circ,
                                    END_STREAM_REASON_EXITPOLICY, NULL);
      return 0;
    }
  }

Summary: Fix IPv6Exit support in connection_exit_begin_conn() to Exits should tell clients when they are connecting to an IPv6-only hostname

-  if (! options->IPv6Exit) {
-    /* I don't care if you prefer IPv6; I can't give you any. */
-    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
-    /* If you don't want IPv4, I can't help. */
-    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
-      tor_free(address);
-      relay_send_end_cell_from_edge(rh.stream_id, circ,
-                                    END_STREAM_REASON_EXITPOLICY, NULL);
-      return 0;
-    }
-  }

Shouldn't we leave the "if you won't be ok with v4, and I won't exit to v6, then return an end cell immediately" clause in there?

I think if clients are going to say they're not ok with v4, then it's their responsibility to pick an exit that says it can do v6.

Replying to teor:

Can we get this in 0.3.0?

I'm a big fan of getting everything in as early as possible, but if 0.3.0 is meant to be frozen right about now, and this fix could destabilize things in ways we haven't tested much, getting it into 0.3.0 makes me a bit nervous.

That said, if we put it into 0.3.1 and there's a problem with it, it will be a long time until we notice the problem, and we'll have forgotten about this ticket in the mean time. So that isn't obviously a winning move either. :)

Replying to arma:

Replying to teor:

Can we get this in 0.3.0?

I'm a big fan of getting everything in as early as possible, but if 0.3.0 is meant to be frozen right about now, and this fix could destabilize things in ways we haven't tested much, getting it into 0.3.0 makes me a bit nervous.

You're right, I forgot we were doing the freeze this week. (I'm used to them being about a month later.)

That said, if we put it into 0.3.1 and there's a problem with it, it will be a long time until we notice the problem, and we'll have forgotten about this ticket in the mean time. So that isn't obviously a winning move either. :)

Just make me the IPv6 person :-) Although even I forget things I've written by the time they're in stable.

Replying to arma:

{{{
if (! options->IPv6Exit) {

/* I don't care if you prefer IPv6; I can't give you any. */

bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;

/* If you don't want IPv4, I can't help. */

if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
 tor_free(address);
 relay_send_end_cell_from_edge(rh.stream_id, circ,
                               END_STREAM_REASON_EXITPOLICY, NULL);
 return 0;
}

} }}}
Shouldn't we leave the "if you won't be ok with v4, and I won't exit to v6, then return an end cell immediately" clause in there?

I think if clients are going to say they're not ok with v4, then it's their responsibility to pick an exit that says it can do v6.

I think we could keep this in, but we can't adjust the flags on the cell.

Trac:
Status: needs_review to needs_revision

Replying to teor:

Replying to arma:

... I think if clients are going to say they're not ok with v4, then it's their responsibility to pick an exit that says it can do v6.

I think we could keep this in, but we can't adjust the flags on the cell.

What do current clients do?

It seems like tor logs a warning in connection_ap_get_begincell_flags, when it should actually fail and choose another exit. (Or, even better, only choose IPv6-supporting exits on connections with NoIPv4Traffic set, and, although redundant in the current network, only choose IPv4-supporting exits on exits with NoIPv6Traffic set. We can't do this with PreferIPv6, because that would put all Tor Browser users on 12% of the exits.)

So I guess we need to open another child ticket and make that change.

Opened #21346 (moved) for the client change

0.3.1 feature freeze today: these don't seem like they will be all sorted out in time. Let's try to make progress in 0.3.2.

Trac:
Milestone: Tor: 0.3.1.x-final to Tor: 0.3.2.x-final

Trac:
Status: needs_revision to accepted
Owner: N/A to teor

Trac:
Status: accepted to needs_revision

I'm not going to get time to do these in 0.3.2. Moving them to 0.3.3.

Trac:
Milestone: Tor: 0.3.2.x-final to Tor: 0.3.3.x-final

Moving most of my assigned tickets to 0.3.4

Trac:
Milestone: Tor: 0.3.3.x-final to Tor: 0.3.4.x-final

Trac:
Keywords: N/A deleted, 034-triage-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

Trac:
Keywords: N/A deleted, 034-removed-20180328 added

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

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

Trac:
Parent: #21311 (moved) to #17811 (moved)

This ticket requires #21311 (moved)

Trac:
Description: Edit: Turns out that these IPv6Exit option checks prevent clients ever seeing that they're trying to connect to an IPv6-only hostname

This code is wrong for at least two reasons:

it should also unset BEGIN_FLAG_IPV6_OK, and
~~it's way too early in the function: we might end up resolving an IPv6-only hostname, learn that it doesn't match our exit policy, and send the address back in the REASON_EXITPOLICY RELAY_END cell~~ ~~(See https://gitweb.torproject.org/torspec.git/tree/tor-spec.txt#n1436 )~~ (Apparently this works anyway.)

  if (! options->IPv6Exit) {
    /* I don't care if you prefer IPv6; I can't give you any. */
    bcell.flags &= ~BEGIN_FLAG_IPV6_PREFERRED;
    /* If you don't want IPv4, I can't help. */
    if (bcell.flags & BEGIN_FLAG_IPV4_NOT_OK) {
      tor_free(address);
      relay_send_end_cell_from_edge(rh.stream_id, circ,
                                    END_STREAM_REASON_EXITPOLICY, NULL);
      return 0;
    }
  }

to

When #21311 (moved) is finished, we need to make exits tell clients that the hostname they asked for is IPv6-only

Not on any roadmap yet.

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

Trac:
Status: assigned to needs_revision

changed time estimate to 8h

added 48m of time spent

mentioned in issue #21311 (moved)

mentioned in issue #21394 (moved)

mentioned in issue #26664 (moved)

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

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

Exits should tell clients when they are connecting to an IPv6-only hostname

Child items ...

Activity