[tor-bugs] #16069 [Tor]: ipv4 + ipv6 exit : v6 policy is displayed twice, v4 isn't displayed

[Tor Bug Tracker & Wiki]

#16069: ipv4 + ipv6 exit : v6 policy is displayed twice, v4 isn't displayed
--------------------------+-----------------------------------------------
     Reporter:  toralf    |      Owner:
         Type:  defect    |     Status:  needs_review
     Priority:  critical  |  Milestone:  Tor: 0.2.7.x-final
    Component:  Tor       |    Version:  Tor: 0.2.7
   Resolution:            |   Keywords:  026-backport, ipv6, PostFreeze027
Actual Points:            |  Parent ID:
       Points:            |
--------------------------+-----------------------------------------------

Comment (by teor):

 Replying to [comment:29 nickm]:
 > okay, thoughts!
 >
 > {{{
 > +      log_notice(LD_GENERAL,
 > +                 "accept/reject * expands into rules which apply to all
 IPv4 "
 > +                 "and IPv6 addresses.");
 > }}}
 >
 > Maybe this should:
 >    * mention the actual policy that's getting extended?

 Yes, I'll fix both instances where a similar message is used (torrc
 parsing and descriptor parsing) so they have the actual policy.

 >    * tell the user what to do if they only wanted it to apply to IPv4?
 >    * not happen once per policy line per hup. :)
 >    * Give the user some way to avoid the message if they really did mean
 "all ipv4 and ipv6".

 Hmm, yes, this is a complex one to fix. I suggest a split solution:
  * downgrading the severity of the every-line-every-hup message to INFO or
 DEBUG
  * telling the user to use *4 for IPv4 or *6 for IPv6

 Then creating another message that is NOTICE that only occurs once per
 torrc parse when:
  * the policy has an `accept *:*` or `reject *:*` line
  * other lines occur after that line (and will be ignored)
 We can then create a message telling the user:
  * that lines after `accept/reject *:*` are being ignored
  * to use *4 for IPv4 or *6 for IPv6 or put `accept/reject *:*` at the end
 of the policy to silence this notice

 For toralf's torrc and similar torrcs:
 * this patch makes the IPv6 section would now be IPv6 only
 * the accept/reject section would only apply to IPv4 due to `ExitPolicy
 reject6 *:*` at the end of the accept6/reject6 section
 * no warnings would be issued, as `reject *:*` occurs last

 If someone goes against the advice to end with `accept/reject *:*`, and
 puts `accept6/reject6/accept/reject` after an `accept/reject *:*`, then
 they will get this NOTICE once on every torrc parse.

 > {{{
 > +    if (flags & TAPMP_STAR_IPV4_ONLY) {
 > +      family = AF_INET;
 > +      tor_addr_from_ipv4h(addr_out, 0);
 > }}}
 >
 > This doesn't match the documentation, which says that
 TAPMP_STAR_IPV4_ONLY has no effect unless TAPMP_EXTENDED_STAR is also set.

 Yes, the conditionals are nested incorrectly here. I'll fix it.

 There's no need for it to be active in other contexts, and in a late
 change, I prefer to limit the scope of new flags.

 > {{{
 > -                                              EXIT_POLICY_IPV6_ENABLED
 |
 > +                                              ~EXIT_POLICY_IPV6_ENABLED
 |
 >
 EXIT_POLICY_ADD_DEFAULT,0));
 > }}}
 >
 > Should this bitwise "or" be an "and"?

 Uh, no, the line with the bitwise "not" should have been deleted entirely.
 The bitwise "not" clears the flag, but at the cost of setting every other
 flag.
 I think I'll split this test and do it once with the flag, and once
 without.

 Thanks for picking these up!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16069#comment:30>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online