[tor-bugs] #31011 [Core Tor/Tor]: Make the bridge authority reject private PT addresses when DirAllowPrivateAddresses is 0

[Tor Bug Tracker & Wiki]

#31011: Make the bridge authority reject private PT addresses when
DirAllowPrivateAddresses is 0
--------------------------+----------------------------------
 Reporter:  teor          |          Owner:  cjb
     Type:  defect        |         Status:  assigned
 Priority:  Medium        |      Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:                |  Actual Points:
Parent ID:  #31009        |         Points:  1
 Reviewer:                |        Sponsor:  Sponsor28-can
--------------------------+----------------------------------

Comment (by cjb):

 Replying to [comment:10 teor]:
 > We can add bridge authority code that rejects extra-info descriptors
 with a private address in any `transport` line.
 >
 > We should probably also add a config error on the bridge side, if
 ServerTransportListenAddress is an internal address,
 compute_publishserverdescriptor() is bridge, and the bridge is using the
 default bridge authority.

 Thanks for the hints!  Here's a patch that attempts to do these things:

 {{{
 From 0725e4a2848c8a6e41640d4af784e36417b37aa5 Mon Sep 17 00:00:00 2001
 From: Chris Ball <chris at printf.net>
 Date: Tue, 28 Jan 2020 17:07:06 -0800
 Subject: [PATCH] Refuse to publish a bridge with internal addr to
 bridgeauth

 If a PT:
   * has ServerTransportListenAddr set to an internal address
   * and it's a bridge
   * and it's publishing its descriptor to the default bridge authorities
 then fail the config as invalid, since we don't want internal IP info
 being sent to bridgeauths for distribution.

 And for clients that aren't running with this fix: if we're a bridgeauth,
 reject extra-info descs containing those internal addresses.
 ---
  src/feature/dirauth/process_descs.c  |  9 +++++++++
  src/feature/relay/transport_config.c | 12 +++++++++++-
  src/test/conf_examples/pt_10/error   |  1 +
  src/test/conf_examples/pt_10/torrc   |  9 +++++++++
  4 files changed, 30 insertions(+), 1 deletion(-)
  create mode 100644 src/test/conf_examples/pt_10/error
  create mode 100644 src/test/conf_examples/pt_10/torrc

 diff --git a/src/feature/dirauth/process_descs.c
 b/src/feature/dirauth/process_descs.c
 index baf8f8c21..1c2c48ea3 100644
 --- a/src/feature/dirauth/process_descs.c
 +++ b/src/feature/dirauth/process_descs.c
 @@ -895,6 +895,15 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char
 **msg)
      goto fail;
    }

 +  /* If the extrainfo descriptor contains an internal address, reject it.
 */
 +  if (dirserv_router_has_valid_address(ri) < 0) {
 +    log_notice(LD_DIR, "Rejecting an extrainfo descriptor '%s' "
 +               "with an internal address.", ri->nickname);
 +    *msg = "Router extrainfo descriptor contained an internal address.";
 +    rv = ROUTER_AUTHDIR_REJECTS;
 +    goto fail;
 +  }
 +
    if ((r = routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei,
                                                    &ri->cache_info, msg)))
 {
      if (r<0) {
 diff --git a/src/feature/relay/transport_config.c
 b/src/feature/relay/transport_config.c
 index 7dcce70e3..0bd844bd9 100644
 --- a/src/feature/relay/transport_config.c
 +++ b/src/feature/relay/transport_config.c
 @@ -16,7 +16,7 @@

  #include "lib/encoding/confline.h"
  #include "lib/encoding/keyval.h"
 -
 +#include "lib/net/address.h"
  #include "lib/container/smartlist.h"

  /* Required for dirinfo_type_t in or_options_t */
 @@ -230,6 +230,16 @@ options_validate_server_transport(const or_options_t
 *old_options,
      char *bindaddr = get_bindaddr_from_transport_listen_line(cl->value,
 NULL);
      if (!bindaddr)
        REJECT("ServerTransportListenAddr did not parse. See logs for
 details.");
 +
 +    tor_addr_t tor_addr;
 +    uint16_t tor_port;
 +    if (tor_addr_port_parse(LOG_WARN, bindaddr, &tor_addr, &tor_port, 0)
 > -1 &&
 +        tor_addr_is_internal(&tor_addr, 0) &&
 +        options->PublishServerDescriptor_ == BRIDGE_DIRINFO &&
 +        !options->AlternateBridgeAuthority)
 +      REJECT("ServerTransportListenAddr is an internal address, "
 +             "refusing to publish this bridge to the default bridge
 authorities.");
 +
      tor_free(bindaddr);
    }

 diff --git a/src/test/conf_examples/pt_10/error
 b/src/test/conf_examples/pt_10/error
 new file mode 100644
 index 000000000..17a200942
 --- /dev/null
 +++ b/src/test/conf_examples/pt_10/error
 @@ -0,0 +1 @@
 +ServerTransportListenAddr is an internal address, refusing to publish
 this bridge to the default bridge authorities.
 diff --git a/src/test/conf_examples/pt_10/torrc
 b/src/test/conf_examples/pt_10/torrc
 new file mode 100644
 index 000000000..c08f34091
 --- /dev/null
 +++ b/src/test/conf_examples/pt_10/torrc
 @@ -0,0 +1,9 @@
 +# Relay PT tests
 +# Options from relay/transport_config.c
 +# Try a valid minimal config, with a bad ServerTransportListenAddr
 +ORPort 2
 +ExtORPort 1
 +BridgeRelay 1
 +ServerTransportPlugin bad3 exec /
 +ServerTransportListenAddr bad3 10.1.2.3:4567
 +PublishServerDescriptor 1
 --
 2.20.1
 }}}

 How does it look?  I'm not sure how to get setup to test the bridgeauth
 side of the change.  It looks like this is my first Tor patch since 2010
 so please give extra scrutiny.  :)  I'm still catching up with codebase
 and process.

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