[tor-commits] [tor/master] Fix some hidden service edge cases.
nickm at torproject.org
nickm at torproject.org
Wed Dec 26 04:34:55 UTC 2012
commit 5f733ccd7382e8bb8289e4f8adf07f8ac001c28a
Author: Mike Perry <mikeperry-git at fscked.org>
Date: Sat Dec 8 12:07:58 2012 -0800
Fix some hidden service edge cases.
---
src/or/circuitbuild.c | 60 ++++++++++++++++++++++++++++++++++--------------
src/or/circuituse.c | 10 ++++++++
src/or/rendclient.c | 7 +++++
3 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index af36cb2..7eae0e7 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -1155,10 +1155,14 @@ pathbias_should_count(origin_circuit_t *circ)
char *rate_msg = NULL;
/* We can't do path bias accounting without entry guards.
- * Testing and controller circuits also have no guards. */
+ * Testing and controller circuits also have no guards.
+ * We also don't count server-side rends, because their
+ * endpoint could be chosen maliciously. */
if (get_options()->UseEntryGuards == 0 ||
circ->base_.purpose == CIRCUIT_PURPOSE_TESTING ||
- circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER) {
+ circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER ||
+ circ->base_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND ||
+ circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED) {
return 0;
}
@@ -1384,22 +1388,37 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
{
circuit_t *circ = ô->base_;
+ if (!pathbias_should_count(circ)) {
+ return;
+ }
+
if (ocirc->path_state == PATH_STATE_BUILD_SUCCEEDED) {
if (circ->timestamp_dirty) {
+ /* Any circuit where there were attempted streams but no successful
+ * streams could be bias */
// XXX: May open up attacks if the adversary can force connections
// on unresponsive hosts to use new circs. Vidalia displayes a "Retrying"
// state.. Can we use that? Does optimistic data change this?
- // XXX: For the hidserv side, we could only care about INTRODUCING purposes
- // for server+client, and REND purposes for the server... Can we
- // somehow only count those?
- /* Any circuit where there were attempted streams but no successful
- * streams could be bias */
- log_info(LD_CIRC,
+ // XXX: Sub-attack: in collusion with an intro point, you can induce bias
+ // through the web. Need a Torbutton patch to prevent this.
+
+ /* FIXME: This is not ideal, but it prevents the case where a
+ * CPU overloaded intro point is chosen.
+ * XXX: Is this reason code authenticated? */
+ if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING &&
+ reason ==
+ END_CIRC_REASON_FLAG_REMOTE|END_CIRC_REASON_RESOURCELIMIT) {
+ log_info(LD_CIRC,
+ "Ignoring CPU overload intro circuit without successful use. "
+ "Circuit purpose %d currently %s.",
+ reason, circ->purpose, circuit_state_to_string(circ->state));
+ } else {
+ log_info(LD_CIRC,
"Circuit closed without successful use for reason %d. "
- "Circuit is a %s currently %s.",
- reason, circuit_purpose_to_string(circ->purpose),
- circuit_state_to_string(circ->state));
- pathbias_count_unusable(ocirc);
+ "Circuit purpose %d currently %s.",
+ reason, circ->purpose, circuit_state_to_string(circ->state));
+ pathbias_count_unusable(ocirc);
+ }
} else {
if (reason & END_CIRC_REASON_FLAG_REMOTE) {
/* Unused remote circ close reasons all could be bias */
@@ -1409,9 +1428,8 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
// == reasons: 2,3,8. Client-side timeouts?
log_info(LD_CIRC,
"Circuit remote-closed without successful use for reason %d. "
- "Circuit is a %s currently %s.",
- reason, circuit_purpose_to_string(circ->purpose),
- circuit_state_to_string(circ->state));
+ "Circuit purpose %d currently %s.",
+ reason, circ->purpose, circuit_state_to_string(circ->state));
pathbias_count_collapse(ocirc);
} else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE)
== END_CIRC_REASON_CHANNEL_CLOSED &&
@@ -1423,10 +1441,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
* What about clock jumps/suspends? */
log_info(LD_CIRC,
"Circuit's channel closed without successful use for reason %d, "
- "channel reason %d. Circuit is a %s currently %s.",
+ "channel reason %d. Circuit purpose %d currently %s.",
reason, circ->n_chan->reason_for_closing,
- circuit_purpose_to_string(circ->purpose),
- circuit_state_to_string(circ->state));
+ circ->purpose, circuit_state_to_string(circ->state));
pathbias_count_collapse(ocirc);
} else {
pathbias_count_successful_close(ocirc);
@@ -1548,6 +1565,13 @@ pathbias_count_timeout(origin_circuit_t *circ)
return;
}
+ /* For hidden service circs, they can actually be used
+ * successfully and then time out later (because
+ * the other side declines to use them). */
+ if (circ->path_state == PATH_STATE_USE_SUCCEEDED) {
+ return;
+ }
+
if (circ->cpath && circ->cpath->extend_info) {
guard = entry_guard_get_by_id_digest(
circ->cpath->extend_info->identity_digest);
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 77822a3..9362e24 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1402,6 +1402,16 @@ circuit_launch_by_extend_info(uint8_t purpose,
build_state_get_exit_nickname(circ->build_state), purpose,
circuit_purpose_to_string(purpose));
+ if (purpose == CIRCUIT_PURPOSE_S_CONNECT_REND &&
+ circ->path_state == PATH_STATE_BUILD_SUCCEEDED) {
+ /* Path bias: Cannibalized rends pre-emptively count as a
+ * successfully used circ. We don't wait until the extend,
+ * because the rend point could be malicious. */
+ circ->path_state = PATH_STATE_USE_SUCCEEDED;
+ /* This must be called before the purpose change */
+ pathbias_check_close(circ);
+ }
+
circuit_change_purpose(TO_CIRCUIT(circ), purpose);
/* Reset the start date of this circ, else expire_building
* will see it and think it's been trying to build since it
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 3393e0f..88241a4 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -862,6 +862,13 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request,
/* Set timestamp_dirty, because circuit_expire_building expects it
* to specify when a circuit entered the _C_REND_READY state. */
circ->base_.timestamp_dirty = time(NULL);
+
+ /* From a path bias point of view, this circuit is now successfully used.
+ * Waiting any longer opens us up to attacks from Bob. He could induce
+ * Alice to attempt to connect to his hidden service and never reply
+ * to her rend requests */
+ circ->path_state = PATH_STATE_USE_SUCCEEDED;
+
/* XXXX This is a pretty brute-force approach. It'd be better to
* attach only the connections that are waiting on this circuit, rather
* than trying to attach them all. See comments bug 743. */
More information about the tor-commits
mailing list