[tor-commits] [tor/master] revert the or_connection and dir_connection flags
nickm at torproject.org
nickm at torproject.org
Fri Mar 25 21:20:09 UTC 2016
commit f590a303db344a18f4e77be803a615e259892d2d
Author: Roger Dingledine <arma at torproject.org>
Date: Thu Mar 24 18:53:33 2016 -0400
revert the or_connection and dir_connection flags
They incorrectly summarized what the function was planning to do,
leading to wrong behavior like making an http request to an orport,
or making a begindir request to a dirport.
This change backs out some of the changes made in commit e72cbf7a, and
most of the changes made in commit ba6509e9.
This patch resolves bug 18625. There more changes I want to make
after this one, for code clarity.
---
src/or/directory.c | 59 ++++++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/src/or/directory.c b/src/or/directory.c
index d057dac..c14c11d 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1062,28 +1062,31 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
dir_connection_t *conn;
const or_options_t *options = get_options();
int socket_error = 0;
+ /* Should the connection be to a relay's OR port (and inside that we will
+ * send our directory request)? */
const int use_begindir = directory_command_should_use_begindir(options,
&or_addr_port->addr, or_addr_port->port,
router_purpose, indirection);
- /* Is it an anonymous connection? Be careful, it could be either an OR or
- * directory connection. */
+ /* Will the connection go via a three-hop Tor circuit? Note that this
+ * is separate from whether it will use_begindir. */
const int anonymized_connection = dirind_is_anon(indirection);
- /* Is it a connection to our DirPort? */
- const int dir_connection = (indirection == DIRIND_ANON_DIRPORT ||
- indirection == DIRIND_DIRECT_CONN);
- /* It's an OR connection if we should use BEGIN_DIR or if it's an
- * anonymized connection but obviously not a directory connection. */
- const int or_connection = (use_begindir ||
- (anonymized_connection && !dir_connection));
+ /* What is the address we want to make the directory request to? If
+ * we're making a begindir request this is the ORPort of the relay
+ * we're contacting; if not a begindir request, this is its DirPort.
+ * Note that if anonymized_connection is true, we won't be initiating
+ * a connection directly to this address. */
tor_addr_t addr;
- tor_addr_copy(&addr, &(or_connection ? or_addr_port : dir_addr_port)->addr);
- uint16_t port = (or_connection ? or_addr_port : dir_addr_port)->port;
+ tor_addr_copy(&addr, &(use_begindir ? or_addr_port : dir_addr_port)->addr);
+ uint16_t port = (use_begindir ? or_addr_port : dir_addr_port)->port;
+ /* XXX dir_port is redundant and we can get rid of it. Just use port. */
uint16_t dir_port = dir_addr_port->port;
log_debug(LD_DIR, "anonymized %d, use_begindir %d.",
anonymized_connection, use_begindir);
+ /* XXX This clause is redundant with the "addr, port is viable" check below.
+ * We should remove it. */
if (!dir_port && !use_begindir) {
char ipaddr[TOR_ADDR_BUF_LEN];
tor_addr_to_str(ipaddr, &addr, TOR_ADDR_BUF_LEN, 0);
@@ -1104,28 +1107,20 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
/* ensure that we don't make direct connections when a SOCKS server is
* configured. */
- if (dir_connection && !options->HTTPProxy &&
+ if (!anonymized_connection && !use_begindir && !options->HTTPProxy &&
(options->Socks4Proxy || options->Socks5Proxy)) {
log_warn(LD_DIR, "Cannot connect to a directory server through a "
"SOCKS proxy!");
return;
}
- if (or_connection && (!or_addr_port->port
- || tor_addr_is_null(&or_addr_port->addr))) {
+ /* Make sure that the destination addr and port we picked is viable. */
+ if (!port || tor_addr_is_null(&addr)) {
static int logged_backtrace = 0;
- log_warn(LD_DIR, "Cannot make an outgoing OR connection without an OR "
- "port.");
- if (!logged_backtrace) {
- log_backtrace(LOG_INFO, LD_BUG, "Address came from");
- logged_backtrace = 1;
- }
- return;
- } else if (dir_connection && (!dir_addr_port->port
- || tor_addr_is_null(&dir_addr_port->addr))) {
- static int logged_backtrace = 0;
- log_warn(LD_DIR, "Cannot make an outgoing Dir connection without a Dir "
- "port.");
+ log_warn(LD_DIR,
+ "Cannot make an outgoing %sconnection without %sPort.",
+ use_begindir ? "begindir " : "",
+ use_begindir ? "an OR" : "a Dir");
if (!logged_backtrace) {
log_backtrace(LOG_INFO, LD_BUG, "Address came from");
logged_backtrace = 1;
@@ -1161,7 +1156,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
if (rend_query)
conn->rend_data = rend_data_dup(rend_query);
- if (dir_connection && !anonymized_connection) {
+ if (!anonymized_connection && !use_begindir) {
/* then we want to connect to dirport directly */
if (options->HTTPProxy) {
@@ -1190,8 +1185,12 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
/* writable indicates finish, readable indicates broken link,
error indicates broken link in windowsland. */
}
- } else { /* we want to connect via a tor connection */
+ } else {
+ /* We will use a Tor circuit (maybe 1-hop, maybe 3-hop, maybe with
+ * begindir, maybe not with begindir) */
+
entry_connection_t *linked_conn;
+
/* Anonymized tunneled connections can never share a circuit.
* One-hop directory connections can share circuits with each other
* but nothing else. */
@@ -1213,6 +1212,8 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
conn->base_.address, conn->base_.port,
digest,
SESSION_GROUP_DIRCONN, iso_flags,
+ /* XXX dirconn_direct is misleading below. we should use
+ * !anonymized_connection, since that's what we mean. */
use_begindir, conn->dirconn_direct);
if (!linked_conn) {
log_warn(LD_NET,"Making tunnel to dirserver failed.");
@@ -1225,6 +1226,8 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
connection_mark_for_close(TO_CONN(conn));
return;
}
+ /* XXX the below line is suspicious and uncommented. does it close all
+ * consensus fetches if we've already bootstrapped? investigate. */
if (connection_dir_close_consensus_conn_if_extra(conn)) {
return;
}
More information about the tor-commits
mailing list