Check IPv6 exit policies on microdescs

In node_exit_policy_rejects_all(), we check IPv4 and IPv6 policies on ri, but on md we only check IPv4:

  else if (node->md)
    return node->md->exit_policy == NULL ||
      short_policy_is_reject_star(node->md->exit_policy);

One way to fix this issue is to refactor the existing code to check a new policy_is_reject_star, and then populate policy_is_reject_star when the md is parsed. (Like we already do with the ri.)

changed milestone to %Tor: 0.4.2.x-final

added 040-deferred-20190220 asn-merge component::core tor/tor ipv6 milestone::Tor: 0.4.2.x-final no-backport owner::neel parent::27248 priority::medium resolution::fixed reviewer::nickm severity::normal status::closed teor-unreached-2019-03-08 type::defect version::tor 0.2.3.1-alpha labels

Flatten the tree

Trac:
Parent: #27236 (moved) to #27080 (moved)

Defer to 0.3.6, because it doesn't have any clear benefit in 0.3.5 just before a freeze.

Trac:
Keywords: N/A deleted, ipv6 added
Milestone: Tor: 0.3.5.x-final to Tor: 0.3.6.x-final

Un-parenting, it's not required for 0.3.4.

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

Trac:
Keywords: 034-backport-maybe deleted, N/A added

Tor 0.3.6.x has been renamed to 0.4.0.x.

Trac:
Milestone: Tor: 0.3.6.x-final to Tor: 0.4.0.x-final

#27248 (moved) would also fix this issue, and many similar issues.

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

Deferring 51 tickets from 0.4.0.x-final. Tagging them with 040-deferred-20190220 for visibility. These are the tickets that did not get 040-must, 040-can, or tor-ci.

Trac:
Keywords: N/A deleted, 040-deferred-20190220 added
Milestone: Tor: 0.4.0.x-final to Tor: unspecified

I'd like to do these tickets, but not in the next few months.

Trac:
Keywords: N/A deleted, teor-unreached-2019-03-08 added
Owner: teor to N/A

if this is the case with md only, is the journey until fix to go with:

UseMicrodescriptors 0

?

Replying to cypherpunks:

if this is the case with md only, is the journey until fix to go with: {{{ UseMicrodescriptors 0 }}} ? This is a question not a fix? But the question is, it is the fix? Who knows?

Trac:
Owner: N/A to neel

Trac:
Cc: dgoulet, intrigeri to dgoulet, intrigeri, neel

https://github.com/torproject/tor/pull/1261

Setting as needs review. PR is in Comment 13.

Trac:
Status: assigned to needs_review

This code has bugs, and it has no unit tests.

Trac:
Status: needs_review to needs_revision
Milestone: Tor: unspecified to Tor: 0.4.2.x-final
Keywords: N/A deleted, 042-backport, 029-backport, 040-backport, consider-backport-after-042-stable, 041-backport, 035-backport added
Version: N/A to Tor: 0.2.3.1-alpha

The statement policy == NULL exists because a pure !policy doesn't work (I believe it's a pointer) and fails the existing tests. It may also be a bug.

I will add tests tomorrow and then set as needs review.

Nevermind, I have added tests as they were trivial. Setting as needs review.

Trac:
Status: needs_revision to needs_review

Replying to neel:

The statement policy == NULL exists because a pure !policy doesn't work (I believe it's a pointer) and fails the existing tests. It may also be a bug.

I will add tests tomorrow and then set as needs review.

policy == NULL and !policy should be equivalent?

Trac:
Reviewer: N/A to nickm

I've left a few comments on the PR; I believe Teor is correct about the C.

Trac:
Status: needs_review to needs_revision

I made the necessary changes and pushed it as fixup commits.

Trac:
Status: needs_revision to needs_review

This looks better to me. I'd like to turn the boolean back into a bitfield, but move it to be next to the other bitfields, in order to save memory.

Teor, do you think we will want to backport this? If so I want to rebase it before merging--but I am not sure how necessary a backport is, or how difficult it would be.

I made the boolean back into a bitfield and moved it to the other bitfields in a fixup commit.

That change LGTM. Before we merge, let's decide if it's a backport candidate.

Trac:
Status: needs_review to merge_ready

I think this is "no backport". Here's why:

Before this fix, exits that reject IPv4 but accept IPv6 are marked as "rejects all". After this fix, exits that reject IPv4 but accept IPv6 are marked as "accepts some".

These exits are rare, so the change is not worth backporting.

Thanks for the info! Your reasoning makes sense to me.

Trac:
Keywords: N/A deleted, asn-merge added

Trac:
Keywords: 042-backport, 029-backport, 040-backport, consider-backport-after-042-stable, 041-backport, 035-backport deleted, no-backport added

merged!

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

closed

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