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.
This commit is contained in:
Roger Dingledine 2016-03-24 18:53:33 -04:00
parent d5f50cb052
commit f590a303db

View File

@ -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;
}