Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+
Changes (by catalyst):

 * cc: catalyst (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by teor):

 Replying to [comment:26 cjb]:
 > Thank you!  I think the line is just named `transport` in the extra-info
 doc, and there can be 0-to-many of them.  So it sounds like a new
 `transports` field in the `extrainfo_t` struct should be a smartlist of
 `transport` line `char *`s, do you agree?  And then as the bridgeauth we
 reject the entire descriptor during parsing if a single `transport` line
 contains an internal address?

 Yes, I think that's our standard design for lists of strings.

 > (If this takes me a while, perhaps it might make sense to split out the
 bridge-side change from the bridgeauth-side change and merge them
 independently?)

 Splitting changes is usually helpful. Particularly when they need to be
 merged to separate repositories.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by cjb):

 Thank you!  I think the line is just named `transport` in the extra-info
 doc, and there can be 0-to-many of them.  So it sounds like a new
 `transports` field in the `extrainfo_t` struct should be a smartlist of
 `transport` line `char *`s, do you agree?  And then as the bridgeauth we
 reject the entire descriptor during parsing if a single `transport` line
 contains an internal address?

 (If this takes me a while, perhaps it might make sense to split out the
 bridge-side change from the bridgeauth-side change and merge them
 independently?)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by teor):

 Good question!

 It looks like extrainfo documents aren't really parsed by tor right now.
 Instead, they're parsed by BridgeDB and metrics.

 Here's how you can change that:
 1. Add the server transport line to the token table at:
 
https://github.com/torproject/tor/blob/4f02812242d1fd90d859eb98ac3fb1ed182f18cf/src/feature/dirparse/routerparse.c#L127
 2. Add a server transport line variable to the extrainfo struct at:
 
https://github.com/torproject/tor/blob/4f02812242d1fd90d859eb98ac3fb1ed182f18cf/src/feature/nodelist/extrainfo_st.h#L18
 3. Parse the server transport line when tor parses the extrainfo:
 
https://github.com/torproject/tor/blob/4f02812242d1fd90d859eb98ac3fb1ed182f18cf/src/feature/dirparse/routerparse.c#L960
 4. Free the new string when an extrainfo struct is freed.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by cjb):

 Replying to [comment:22 teor]:
 > I did a quick review, and saw that the bridge authority is checking the
 address in the routerinfo, not the address in the ServerTransport line in
 the extrainfo,

 Oops, thanks!  Please could you point me towards a preferred way to access
 extrainfo lines from an `ei`?  I couldn't find any similar examples in the
 codebase.  So far I've found things like
 `router_parse_list_from_string()`, `signed_descriptor_get_body()`,
 `munge_extrainfo_into_routerinfo()` (deprecated).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by teor):

 Thanks for this PR,

 I did a quick review, and saw that the bridge authority is checking the
 address in the routerinfo, not the address in the ServerTransport line in
 the extrainfo,

 There's also a few places that the logs could be improved.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-pt|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+
Changes (by teor):

 * keywords:   => tor-pt
 * status:  needs_review => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by cjb):

 Replying to [comment:20 ahf]:
 > cjb: Hey, thanks for the patch. Normally for more complicated stuff, we
 prefer if people create pull requests on Github rather than upload patches
 directly on Trac.

 Thanks, created https://github.com/torproject/tor/pull/1742.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by ahf):

 cjb: Hey, thanks for the patch. Normally for more complicated stuff, we
 prefer if people create pull requests on Github rather than upload patches
 directly on Trac.

 This allows our CI to run before we start diving too much into the code.
 Is this something you are up for doing or do you want me to try it out
 first? :-)

 Dropping a link to a PR on a ticket is perfectly fine.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+

Comment (by cjb):

 Replying to [comment:17 teor]:
 > In this case, I think we should log a warning or notice-level message.
 Because it is still likely to be a misconfiguration. (Im pretty sure tor
 understands all current PT addresses.)

 Thanks, here's a v2 of the patch that adds this notice.

 {{{
 From be21a19923793836918386fa8672932befb9e645 Mon Sep 17 00:00:00 2001
 From: Chris Ball 
 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 | 17 -
  src/test/conf_examples/pt_10/error   |  1 +
  src/test/conf_examples/pt_10/torrc   |  9 +
  4 files changed, 35 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,
>cache_info, msg)))
 {
  if (r<0) {
 diff --git a/src/feature/relay/transport_config.c
 b/src/feature/relay/transport_config.c
 index 7dcce70e3..7cf023bec 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,21 @@ 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;
 +int parse_rv = tor_addr_port_parse(LOG_WARN, bindaddr, _addr,
 _port, 0);
 +if (parse_rv < 0) {
 +log_notice(LD_CONFIG, "Your ServerTransportListenAddr failed to
 parse "
 +  "as a Tor address; check your
 configuration.");
 +}
 +if (parse_rv == 0 &&
 +tor_addr_is_internal(_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 0..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 0..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 /
 

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:  ahf   |Sponsor:  Sponsor28-can
--+
Changes (by dgoulet):

 * reviewer:   => ahf


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:|Sponsor:  Sponsor28-can
--+

Comment (by teor):

 Replying to [comment:16 cjb]:
 > Oh, something to mention:
 >
 > > `if (tor_addr_port_parse(LOG_WARN, bindaddr, _addr, _port, 0)
 > -1 &&`
 >
 > This patch will only reject an internal address if it successfully
 passes `tor_addr_port_parse`.  I was worried that there might be a valid
 `ServerTransportListenAddr` that fails `tor_addr_port_parse` and didn't
 think we should reject the config in that case.  Does that sound
 reasonable?

 We shouldn't restrict future pluggable transport addresses too much.
 (`ServerTransportListenAddr` is parsed by the PT, so it's possible that
 tor won't understand it.)

 In this case, I think we should log a warning or notice-level message.
 Because it is still likely to be a misconfiguration. (Im pretty sure tor
 understands all current PT addresses.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:|Sponsor:  Sponsor28-can
--+

Comment (by cjb):

 Oh, something to mention:

 > `if (tor_addr_port_parse(LOG_WARN, bindaddr, _addr, _port, 0) >
 -1 &&`

 This patch will only reject an internal address if it successfully passes
 `tor_addr_port_parse`.  I was worried that there might be a valid
 `ServerTransportListenAddr` that fails `tor_addr_port_parse` and didn't
 think we should reject the config in that case.  Does that sound
 reasonable?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:|Sponsor:  Sponsor28-can
--+
Changes (by cjb):

 * status:  assigned => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  1
 Reviewer:|Sponsor:  Sponsor28-can
--+
Changes (by nickm):

 * milestone:  Tor: unspecified => Tor: 0.4.4.x-final


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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 
 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,
>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, _addr, _port, 0)
 > -1 &&
 +tor_addr_is_internal(_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 0..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 0..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 

Re: [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
--+--
Changes (by phw):

 * keywords:  anti-censorship-roadmap-september =>


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by cjb):

 * owner:  (none) => cjb
 * status:  new => assigned


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---

Comment (by teor):

 Replying to [comment:9 phw]:
 > I prefer having the bridge authority reject descriptors with private
 addresses. In my opinion, a private address has no business being in the
 descriptor and we should reject such descriptors rather than guessing what
 the bridge operators meant to do.

 Thanks, that seems like a sensible decision.

 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.

 Here's how the `transport` line is created on the bridge side:
 
https://github.com/torproject/tor/blob/f6c9ca3a1d1c29a293915612e26cdbfeb050c192/src/feature/relay/router.c#L3190
 
https://github.com/torproject/tor/blob/60d5ff303d65bb7caf5c064675c661faac4cecf1/src/feature/client/transports.c#L1615

 Here's where we reject extra-info descriptors in dirserv_add_extrainfo():
 
https://github.com/torproject/tor/blob/53bdd21179b3507b8d8aa2788e4955df8619f6db/src/feature/dirauth/process_descs.c#L789

 See dirserv_router_has_valid_address() for some example code. This code
 rejects relay descriptors with private IPv4 or IPv6 addresses, when
 DirAllowPrivateAddresses is 0:
 
https://github.com/torproject/tor/blob/53bdd21179b3507b8d8aa2788e4955df8619f6db/src/feature/dirauth/process_descs.c#L456

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by phw):

 * cc: cohosh (added)


Comment:

 Replying to [comment:8 teor]:
 > I think we need to know how many bridges are affected by this issue,
 before we can make this decision.
 [[br]]
 As of Dec 17, 2019, 1,382 bridges support a pluggable transport. Among
 these, 10 bridges use an address in 10.0.0.0/8, 10 bridges use
 192.168.0.0/16, and 2 bridges use 172.16.0.0/12, so a total of 24 bridges
 use private addresses.
 [[br]]
 > Replying to [comment:7 cjb]:
 > > Replying to [comment:1 arma]:
 > > > Another option here is to leave the bridge authority alone, and
 teach bridgedb that if there's an internal address in the extrainfo
 descriptor, it should swap it out in favor of the public address in the
 descriptor.
 > > >
 > > > Then once the #31009 fix is sufficiently deployed, it shouldn't
 matter anymore.
 > > >
 > > > (That way we could make use of the current obfs4 bridges even if
 they haven't upgraded yet.)
 > >
 > > I think I could volunteer to work on this ticket, but it looks like we
 still need to decide what to do.  Options:
 >
 > There's a tradeoff here, so maybe we should ask the anti-censorship team
 what they'd like.
 [[br]]
 I prefer having the bridge authority reject descriptors with private
 addresses. In my opinion, a private address has no business being in the
 descriptor and we should reject such descriptors rather than guessing what
 the bridge operators meant to do.

 In parallel, we could teach BridgeDB to rewrite private to public IP
 addresses but given that only 24 bridges are affected by this issue, I
 don't consider this a priority.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by teor):

 * cc: phw (added)


Comment:

 I think we need to know how many bridges are affected by this issue,
 before we can make this decision.

 Replying to [comment:7 cjb]:
 > Replying to [comment:1 arma]:
 > > Another option here is to leave the bridge authority alone, and teach
 bridgedb that if there's an internal address in the extrainfo descriptor,
 it should swap it out in favor of the public address in the descriptor.
 > >
 > > Then once the #31009 fix is sufficiently deployed, it shouldn't matter
 anymore.
 > >
 > > (That way we could make use of the current obfs4 bridges even if they
 haven't upgraded yet.)
 >
 > I think I could volunteer to work on this ticket, but it looks like we
 still need to decide what to do.  Options:

 There's a tradeoff here, so maybe we should ask the anti-censorship team
 what they'd like.

 > 1) as in the summary, bridgeauth just refuses descriptors with internal
 addresses

 Rejecting descriptors means we have fewer bridges, until those bridges
 upgrade tor versions.
 But those bridges are more likely to have correct addresses.
 (Changing to an address the operator didn't provide means that port
 forwarding might not be set up.)

 > 2) arma's suggestion, bridgedb transforms internal addresses to external

 This option has the opposite tradeoff: more bridges, fast to deploy,
 potentially wrong data.

 > 3) Could we also consider having bridgeauth itself, rather than bridgedb
 downstream, perform that transformation?  Or perhaps there's a reason why
 that's not a good idea?

 I'm not sure if we can do this, because extra-info descriptors are signed
 by the bridge.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---

Comment (by cjb):

 Replying to [comment:1 arma]:
 > Another option here is to leave the bridge authority alone, and teach
 bridgedb that if there's an internal address in the extrainfo descriptor,
 it should swap it out in favor of the public address in the descriptor.
 >
 > Then once the #31009 fix is sufficiently deployed, it shouldn't matter
 anymore.
 >
 > (That way we could make use of the current obfs4 bridges even if they
 haven't upgraded yet.)

 I think I could volunteer to work on this ticket, but it looks like we
 still need to decide what to do.  Options:

 1) as in the summary, bridgeauth just refuses descriptors with internal
 addresses

 2) arma's suggestion, bridgedb transforms internal addresses to external

 3) Could we also consider having bridgeauth itself, rather than bridgedb
 downstream, perform that transformation?  Or perhaps there's a reason why
 that's not a good idea?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by neel):

 * status:  assigned => new


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  assigned
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by neel):

 * cc: neel (removed)
 * owner:  neel => (none)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  neel
 Type:  defect | Status:  assigned
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by neel):

 * cc: neel (added)
 * status:  new => assigned
 * owner:  (none) => neel


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:  Tor:
   |  unspecified
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  anti-censorship-roadmap-september  |  Actual Points:
Parent ID:  #31009 | Points:  1
 Reviewer: |Sponsor:
   |  Sponsor28-can
---+---
Changes (by gaba):

 * keywords:   => anti-censorship-roadmap-september
 * points:  0.5 => 1


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  0.5
 Reviewer:|Sponsor:  Sponsor28-can
--+--
Changes (by teor):

 * sponsor:   => Sponsor28-can


Comment:

 Gaba set #31009 to Sponsor 28 can, making this related ticket match.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [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:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:  #31009| Points:  0.5
 Reviewer:|Sponsor:
--+--

Comment (by arma):

 Another option here is to leave the bridge authority alone, and teach
 bridgedb that if there's an internal address in the extrainfo descriptor,
 it should swap it out in favor of the public address in the descriptor.

 Then once the #31009 fix is sufficiently deployed, it shouldn't matter
 anymore.

 (That way we could make use of the current obfs4 bridges even if they
 haven't upgraded yet.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[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:  (none)
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: unspecified
Component:  Core Tor/Tor  |Version:
 Severity:  Normal|   Keywords:
Actual Points:|  Parent ID:  #31009
   Points:  0.5   |   Reviewer:
  Sponsor:|
--+--
 When DirAllowPrivateAddresses is 0, the bridge authority should reject
 extra info descriptors with private addresses in their PT lines.

 We should add some text asking operators to upgrade, and deploy this
 change after #31009 is backported,

 Gaba, this ticket should go in the PT sponsor with #31009.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs