Opened 11 years ago

Last modified 7 years ago

#768 closed defect (Implemented)

one-hop circuit restriction should not apply to special-purpose routers

Reported by: goodell Owned by: nickm
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.1.2-alpha
Severity: Keywords:
Cc: goodell, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

At some point (r9735?), code was added to src/or/control.c that prevents
controllers from attaching streams to one-hop circuits. The idea seems to be
that we can use the cost of forking and maintaining a patch as a lever to
prevent people from writing controllers that jeopardize the operational
security of routers and the anonymity properties of the Tor network by creating
and using one-hop circuits rather than the standard three-hop circuits. It may
be, for example, that some users do not actually seek true anonymity but simply
reachability through network perspectives afforded by the Tor network, and
since anonymity is stronger in numbers, forcing users to contribute to
anonymity and decrease the risk to server operators by using full-length paths
may be reasonable.

Whether or not we agree that the particular approach of using hardcoded,
immutable policy in the Tor client to limit self-determinism on the part of
clients is the right way to address the risks posed by one-hop circuit
utilization (for example, I think that routers ought to take responsibility for
ensuring that they are not allowing exit from one-hop circuits), it remains
true that as presently implemented, the sweeping restriction of one-hop
circuits for all routers limits the usefulness of Tor as a general-purpose
technology for building circuits. In particular, we should allow for
controllers, such as Blossom, that create and use single-hop circuits involving
routers that are not part of the Tor network.

There are several ways to address the problem while maintaining the approach of
using hardcoded restrictions in the client code. The simplest solution is to
just have a new configuration parameter that allows the attachment of streams
to one-hop circuits. An objection to that approach might be that the abusive
controllers will simply set the configuration paramter and carry on. Another
solution is to require that we can only exit from one-hop circuits if the
(single) router in the circuit has a descriptor whose associated purpose is not
general. Again, controllers can set purposes on descriptors, or feed the Tor
descriptors via POSTDESCRIPTOR, so controllers could still be abusive, but the
overhead would be somewhat greater.

A third solution is to require descriptors to have a special option that
indicates to the client that they can be used in one-hop circuits, and have
clients object, as they do now for all routers, to attaching streams to one-hop
circuits for routers that do not have this option set. That solution seems to
eliminate the worry about operational router security, since server operators
will not set this bit unless they are willing to take on such risk. If we
worry about the impact on anonymity of the network resulting from including
such "risky" routers in regular Tor path selection, we can just systematically
exclude those.

Tactically, we should do something to allow Blossom to say "yes, I really do
mean to use this one-hop circuit." What is the right way to achieve this in
the short-term?

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (9)

comment:1 Changed 11 years ago by nickm

Option 3 is the only acceptable one; servers should never allow one-hop circuits inadvertently.

This needs a proposal.

comment:2 Changed 11 years ago by goodell

(patch from TheJash on irc.oftc.net)

Index: or/circuituse.c
===================================================================
--- or/circuituse.c (revision 16033)
+++ or/circuituse.c (working copy)
@@ -349,6 +349,7 @@

(!circ->timestamp_dirty

circ->timestamp_dirty + get_options()->MaxCircuitDirtiness > now)) {

cpath_build_state_t *build_state = TO_ORIGIN_CIRCUIT(circ)->build_state;

+ 152: INVESTIGATE: what do we want to do here?

if (build_state->is_internal
build_state->onehop_tunnel)

continue;


@@ -982,6 +983,7 @@

!connection_edge_is_rendezvous_stream(conn);

want_onehop = conn->want_onehop;


+ 152: INVESTIGATE: what do we want to do here?

need_uptime = !conn->want_onehop && !conn->use_begindir &&

smartlist_string_num_isin(options->LongLivedPorts,

conn->socks_request->port);

Index: or/config.c
===================================================================
--- or/config.c (revision 16033)
+++ or/config.c (working copy)
@@ -191,6 +191,8 @@

V(DNSPort, UINT, "0"),
V(DNSListenAddress, LINELIST, NULL),
V(DownloadExtraInfo, BOOL, "0"),

+ 152: config var defaults
+ V(SingleHopExits, BOOL, "0"),

V(EnforceDistinctSubnets, BOOL, "1"),
V(EntryNodes, STRING, NULL),
V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "10 minutes"),

Index: or/connection_edge.c
===================================================================
--- or/connection_edge.c (revision 16033)
+++ or/connection_edge.c (working copy)
@@ -2004,9 +2004,10 @@

begin_type = ap_conn->use_begindir ?

RELAY_COMMAND_BEGIN_DIR : RELAY_COMMAND_BEGIN;

  • if (begin_type == RELAY_COMMAND_BEGIN) {
  • tor_assert(circ->build_state->onehop_tunnel == 0);
  • }

+ 152: removed client assert:
+
if (begin_type == RELAY_COMMAND_BEGIN) {
+ tor_assert(circ->build_state->onehop_tunnel == 0);
+
}

if (connection_edge_send_command(ap_conn, begin_type,

begin_type == RELAY_COMMAND_BEGIN ? payload : NULL,

@@ -2440,7 +2441,8 @@

tor_free(address);
return 0;

}

  • if (or_circ && or_circ->is_first_hop) {

+ 152: check server flag here
+ if (or_circ && or_circ->is_first_hop && !get_options()->SingleHopExits) {

/* Don't let clients use us as a single-hop proxy; it attracts attackers

  • and users who'd be better off with, well, single-hop proxies. */

Index: or/control.c
===================================================================
--- or/control.c (revision 16033)
+++ or/control.c (working copy)
@@ -2311,11 +2311,26 @@

conn);

return 0;

}

+ 152: this is how we're supposed to be attaching supposedly. changed to:

if (circ && (circuit_get_cpath_len(circ)<2
hop==1)) {
  • connection_write_str_to_buf(
  • "551 Can't attach stream to one-hop circuit.\r\n", conn);
  • return 0;

+ routerinfo_t *r;
+ char* exit_digest;
+ assert(circ->build_state);
+ assert(circ->build_state->chosen_exit);
+ exit_digest = circ->build_state->chosen_exit->identity_digest;
+ assert(exit_digest);
+ also need to check that the router allows single hop exits:
+ r = router_get_by_digest(exit_digest);

+ if(!r
!r->single_hop_exits) {

+ connection_write_str_to_buf(
+ "551 Can't attach stream to this one-hop circuit.\r\n", conn);
+ return 0;
+ }
+ ap_conn->chosen_exit_name = tor_strdup(hex_str(exit_digest, DIGEST_LEN));
+ circ->build_state->onehop_tunnel = 1;
+ ap_conn->want_onehop = 1;

}

+

if (circ && hop>0) {

/* find this hop in the circuit, and set cpath */
cpath = circuit_get_cpath_hop(circ, hop);

Index: or/or.h
===================================================================
--- or/or.h (revision 16033)
+++ or/or.h (working copy)
@@ -1284,6 +1284,9 @@

  • dnsworker code. */

unsigned int caches_extra_info:1; /< Whether the router caches and serves

  • extrainfo documents. */

+ 152: declaration of descriptor variable:
+ unsigned int single_hop_exits:1; /< Whether the router allows single hop
+ * exits. */

/* local info */
unsigned int is_running:1; /< As far as we know, is this OR currently

@@ -2339,6 +2342,11 @@

  • if we are a cache). For authorities, this is always true. */

int DownloadExtraInfo;


+ 152: config var declarations
+ / If true, and we are acting as a relay, allow exit circuits even when
+ * we are the first hop of a circuit. */
+ int SingleHopExits;
+

/ If true, do not believe anybody who tells us that a domain resolves

  • to an internal address, or that an internal address has a PTR mapping.
  • Helps avoid some cross-site attacks. */

Index: or/router.c
===================================================================
--- or/router.c (revision 16033)
+++ or/router.c (working copy)
@@ -1675,7 +1675,7 @@

"opt extra-info-digest %s\n%s"
"onion-key\n%s"
"signing-key\n%s"

  • "%s%s%s",

+ "%s%s%s%s",

router->nickname,
router->address,
router->or_port,

@@ -1692,7 +1692,9 @@

onion_pkey, identity_pkey,
family_line,
we_are_hibernating() ? "opt hibernating 1\n" : "",

  • options->HidServDirectoryV2 ? "opt hidden-service-dir\n" : "");

+ options->HidServDirectoryV2 ? "opt hidden-service-dir\n" : "",
+ 152: add text to descriptor:
+ options->SingleHopExits ? "opt single-hop-exits\n" : "");

tor_free(family_line);
tor_free(onion_pkey);

Index: or/routerlist.c
===================================================================
--- or/routerlist.c (revision 16033)
+++ or/routerlist.c (working copy)
@@ -1731,6 +1731,8 @@

smartlist_t *sl, *excludednodes;
routerinfo_t *choice = NULL, *r;
bandwidth_weight_rule_t rule;

+ 152: hopefully this doesnt slow things down much...
+ routerlist_t *rl = router_get_routerlist();

tor_assert(!(weight_for_exit && need_guard));
rule = weight_for_exit ? WEIGHT_FOR_EXIT :

@@ -1739,6 +1741,12 @@

excludednodes = smartlist_create();
add_nickname_list_to_smartlist(excludednodes,excluded,0);


+ 152: dont let single hop servers be part of Tor circuits
+ SMARTLIST_FOREACH(rl->routers, routerinfo_t *, r,
+ if (r->single_hop_exits) {
+ smartlist_add(excludednodes, r);
+ });
+

if ((r = routerlist_find_my_routerinfo())) {

smartlist_add(excludednodes, r);
routerlist_add_family(excludednodes, r);

Index: or/routerparse.c
===================================================================
--- or/routerparse.c (revision 16033)
+++ or/routerparse.c (working copy)
@@ -59,6 +59,8 @@

K_EXTRA_INFO_DIGEST,
K_CACHES_EXTRA_INFO,
K_HIDDEN_SERVICE_DIR,

+ 152: enum declaration
+ K_SINGLE_HOP_EXITS,

K_DIR_KEY_CERTIFICATE_VERSION,
K_DIR_IDENTITY_KEY,

@@ -228,6 +230,8 @@

T01("write-history", K_WRITE_HISTORY, ARGS, NO_OBJ ),
T01("extra-info-digest", K_EXTRA_INFO_DIGEST, GE(1), NO_OBJ ),
T01("hidden-service-dir", K_HIDDEN_SERVICE_DIR, NO_ARGS, NO_OBJ ),

+ 152: define descriptor text:
+ T01("single-hop-exits", K_SINGLE_HOP_EXITS, NO_ARGS, NO_OBJ ),

T01("family", K_FAMILY, ARGS, NO_OBJ ),
T01("caches-extra-info", K_CACHES_EXTRA_INFO, NO_ARGS, NO_OBJ ),

@@ -1340,6 +1344,10 @@

if ((tok = find_first_by_keyword(tokens, K_CACHES_EXTRA_INFO)))

router->caches_extra_info = 1;


+ 152: check if the option is defined for descriptor:
+ if ((tok = find_first_by_keyword(tokens, K_SINGLE_HOP_EXITS)))
+ router->single_hop_exits = 1;
+

if ((tok = find_first_by_keyword(tokens, K_EXTRA_INFO_DIGEST))) {

tor_assert(tok->n_args >= 1);
if (strlen(tok->args[0]) == HEX_DIGEST_LEN) {

comment:3 Changed 11 years ago by nickm

Good start! Here are a few quick comments. Somebody will need to deal with all of these before this patch is
applied. That somebody could be me, but if it is, it probably will be a while before I get to it. ;)

AllowSingleHopExits would be a better name for the server-side option. "SingleHopExits" doesn't really make clear
what the option *does*. Similarly, allows_single_hop_exits would be a better name for the field in routerinfo_t.

The 152 comments are not something we want in the code long-term. Once proposal 152 is implemented and closed, it
will be merged into tor-spec.txt, and "proposal 152" will no longer be the canonical way to refer to the feature.

Check out the doc/HACKING file for our coding conventions; you can run "make check-spaces" on your code to detect
some kinds of deviations from these conventions. For example, the rest of Tor does "if (x)", not "if(x)".

The proposal says that clients MAY exclude such routers from their circuits; this patch makes all clients follow this
behavior. That doesn't match.

I'm not 100% sure that the onehop_tunnel or want_onehop flags do what you think: does this patch still work okay if
you leave them alone? {Geoff, how much have you tested this?}

Also, there doesn't seem to be any documentation in tor.1.in for the SingleHopExits option.

comment:4 Changed 11 years ago by TheJash

Sorry about the slow response. I've tried to address each of your comments with the new patch below.

  • The option has been renamed to AllowSingleHopExits.
  • The code formatting issues should hopefully all be gone (the previous patch was not meant for submission, there must have been a miscommunication between Geoff and me)
  • A new option was added, AllowSingleHopRelays, for client control over whether to include these "risky" relays in circuits built by Tor, so the behavior of the patch should be exactly as described in proposal 152 now.
  • I looked at the onehop_tunnel and want_onehop, and you were definitely right, they dont seem necessary, so they're no longer touched at all.
  • Documentation added to tor.1.in for both options.

Let me know if there is anything else I should change, I promise I'll be faster about it this time.

  • Josh

Index: doc/tor.1.in
===================================================================
--- doc/tor.1.in (revision 16926)
+++ doc/tor.1.in (working copy)
@@ -387,6 +387,14 @@

"middle,rendezvous", and other choices are not advised.
.LP
.TP

+\fBAllowSingleHopRelays \fR\fB0\fR|\fB1\fR\fP
+This option controls whether circuits built by Tor will include relays with
+the AllowSingleHopExits flag set to true. If AllowSingleHopRelays is set to
+1, these relays will be included. Note that these relays might be at higher
+risk of being seized or observed, so they are not normally included.
+(Default: 0)
+.LP
+.TP

\fBBridge \fR\fIIP:ORPort\fR [fingerprint]\fP
When set along with UseBridges, instructs Tor to use the relay at
"IP:ORPort" as a "bridge" relaying into the Tor network. If "fingerprint"

@@ -772,6 +780,12 @@

leave this unset, and Tor will guess your IP address.
.LP
.TP

+\fBAllowSingleHopExits \fR\fB0\fR|\fB1\fR\fP
+This option controls whether clients can use this server as a single hop
+proxy. If set to 1, clients can use this server as an exit even if it is
+the only hop in the circuit. (Default: 0)
+.LP
+.TP

\fBAssumeReachable \fR\fB0\fR|\fB1\fR\fP
This option is used when bootstrapping a new Tor network. If set to 1,
don't do self-reachability testing; just upload your server descriptor

Index: src/or/config.c
===================================================================
--- src/or/config.c (revision 16926)
+++ src/or/config.c (working copy)
@@ -193,6 +193,8 @@

V(DNSPort, UINT, "0"),
V(DNSListenAddress, LINELIST, NULL),
V(DownloadExtraInfo, BOOL, "0"),

+ V(AllowSingleHopExits, BOOL, "0"),
+ V(AllowSingleHopRelays, BOOL, "0"),

V(EnforceDistinctSubnets, BOOL, "1"),
V(EntryNodes, STRING, NULL),
V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "10 minutes"),

Index: src/or/connection_edge.c
===================================================================
--- src/or/connection_edge.c (revision 16926)
+++ src/or/connection_edge.c (working copy)
@@ -2481,8 +2481,10 @@

tor_free(address);
return 0;

}

  • if (or_circ && or_circ->is_first_hop) {
  • /* Don't let clients use us as a single-hop proxy; it attracts attackers

+ if (or_circ && or_circ->is_first_hop &&
+ !get_options()->AllowSingleHopExits) {
+ /* Don't let clients use us as a single-hop proxy, unless the user
+ * has explicitly allowed that in the config. It attracts attackers

  • and users who'd be better off with, well, single-hop proxies. */

log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,

Index: src/or/control.c
===================================================================
--- src/or/control.c (revision 16926)
+++ src/or/control.c (working copy)
@@ -2324,11 +2324,26 @@

conn);

return 0;

}

+ /* Is this a single hop circuit? */

if (circ && (circuit_get_cpath_len(circ)<2
hop==1)) {
  • connection_write_str_to_buf(
  • "551 Can't attach stream to one-hop circuit.\r\n", conn);
  • return 0;

+ routerinfo_t *r = NULL;
+ char* exit_digest;
+ if (circ->build_state &&
+ circ->build_state->chosen_exit &&
+ circ->build_state->chosen_exit->identity_digest) {
+ exit_digest = circ->build_state->chosen_exit->identity_digest;
+ r = router_get_by_digest(exit_digest);
+ }
+ /* Do both the client and relay allow one-hop exit circuits? */

+ if (!r
!r->allow_single_hop_exits

+ !get_options()->AllowSingleHopRelays) {
+ connection_write_str_to_buf(
+ "551 Can't attach stream to this one-hop circuit.\r\n", conn);
+ return 0;
+ }
+ ap_conn->chosen_exit_name = tor_strdup(hex_str(exit_digest, DIGEST_LEN));

}

+

if (circ && hop>0) {

/* find this hop in the circuit, and set cpath */
cpath = circuit_get_cpath_hop(circ, hop);

Index: src/or/or.h
===================================================================
--- src/or/or.h (revision 16926)
+++ src/or/or.h (working copy)
@@ -1308,6 +1308,8 @@

  • dnsworker code. */

unsigned int caches_extra_info:1; /< Whether the router caches and serves

  • extrainfo documents. */

+ unsigned int allow_single_hop_exits:1; /< Whether the router allows
+ * single hop exits. */

/* local info */
unsigned int is_running:1; /< As far as we know, is this OR currently

@@ -2384,6 +2386,13 @@

  • if we are a cache). For authorities, this is always true. */

int DownloadExtraInfo;


+ / If true, and we are acting as a relay, allow exit circuits even when
+ * we are the first hop of a circuit. */
+ int AllowSingleHopExits;
+ /
If true, allow relays with AllowSingleHopExits=1 to be used in circuits
+ * that we build. */
+ int AllowSingleHopRelays;
+

/ If true, do not believe anybody who tells us that a domain resolves

  • to an internal address, or that an internal address has a PTR mapping.
  • Helps avoid some cross-site attacks. */

Index: src/or/router.c
===================================================================
--- src/or/router.c (revision 16926)
+++ src/or/router.c (working copy)
@@ -1685,7 +1685,7 @@

"opt extra-info-digest %s\n%s"
"onion-key\n%s"
"signing-key\n%s"

  • "%s%s%s",

+ "%s%s%s%s",

router->nickname,
router->address,
router->or_port,

@@ -1703,7 +1703,8 @@

family_line,
we_are_hibernating() ? "opt hibernating 1\n" : "",
(options->DirPort && options->HidServDirectoryV2) ?

  • "opt hidden-service-dir\n" : "");

+ "opt hidden-service-dir\n" : "",
+ options->AllowSingleHopExits ? "opt single-hop-exits\n" : "");

tor_free(family_line);
tor_free(onion_pkey);

Index: src/or/routerlist.c
===================================================================
--- src/or/routerlist.c (revision 16926)
+++ src/or/routerlist.c (working copy)
@@ -1767,6 +1767,16 @@

excludednodes = smartlist_create();


+ /* Exclude relays that allow single hop exit circuits, if the user
+ * wants to (such relays might be risky) */
+ if (!get_options()->AllowSingleHopRelays) {
+ routerlist_t *rl = router_get_routerlist();
+ SMARTLIST_FOREACH(rl->routers, routerinfo_t *, r,
+ if (r->allow_single_hop_exits) {
+ smartlist_add(excludednodes, r);
+ });
+ }
+

if ((r = routerlist_find_my_routerinfo())) {

smartlist_add(excludednodes, r);
routerlist_add_family(excludednodes, r);

Index: src/or/routerparse.c
===================================================================
--- src/or/routerparse.c (revision 16926)
+++ src/or/routerparse.c (working copy)
@@ -63,6 +63,7 @@

K_EXTRA_INFO_DIGEST,
K_CACHES_EXTRA_INFO,
K_HIDDEN_SERVICE_DIR,

+ K_SINGLE_HOP_EXITS,

K_DIR_KEY_CERTIFICATE_VERSION,
K_DIR_IDENTITY_KEY,

@@ -239,6 +240,7 @@

T01("write-history", K_WRITE_HISTORY, ARGS, NO_OBJ ),
T01("extra-info-digest", K_EXTRA_INFO_DIGEST, GE(1), NO_OBJ ),
T01("hidden-service-dir", K_HIDDEN_SERVICE_DIR, NO_ARGS, NO_OBJ ),

+ T01("single-hop-exits", K_SINGLE_HOP_EXITS, NO_ARGS, NO_OBJ ),

T01("family", K_FAMILY, ARGS, NO_OBJ ),
T01("caches-extra-info", K_CACHES_EXTRA_INFO, NO_ARGS, NO_OBJ ),

@@ -1362,6 +1364,9 @@

if ((tok = find_first_by_keyword(tokens, K_CACHES_EXTRA_INFO)))

router->caches_extra_info = 1;


+ if ((tok = find_first_by_keyword(tokens, K_SINGLE_HOP_EXITS)))
+ router->allow_single_hop_exits = 1;
+

if ((tok = find_first_by_keyword(tokens, K_EXTRA_INFO_DIGEST))) {

tor_assert(tok->n_args >= 1);
if (strlen(tok->args[0]) == HEX_DIGEST_LEN) {

comment:5 Changed 11 years ago by nickm

Applied this most recent version, with a little tweaking and renaming.

I've split AllowSingleHopRelays into two options: one of which controls which nodes are in general allowed, and one
of which controls which circuits are allowable in controllers.

Note that the code still needs to get adjusted so that the other points that also pick nodes without using
router_choose_random_node also respect ExcludeSingleHopRelays.

comment:6 Changed 11 years ago by nickm

(commit was r16983; see XXXX021 items for remaining issues)

comment:7 Changed 11 years ago by nickm

Fixed one in r16984; decided the other wasn't actually a bug.

comment:8 Changed 11 years ago by nickm

flyspray2trac: bug closed.

comment:9 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.