April 2018 - tor-commits - lists.torproject.org

[torspec/master] prop291: dos2unix the file so we get a Unix format
by dgoulet＠torproject.org 16 Apr '18

16 Apr '18

commit 5e2e04eb9791bb9f92384bcd541de3c1f26dbca8 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 16 11:12:06 2018 -0400 prop291: dos2unix the file so we get a Unix format No change, just removes the annoying ^M at the end of each line. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- proposals/291-two-guard-nodes.txt | 502 +++++++++++++++++++------------------- 1 file changed, 251 insertions(+), 251 deletions(-) diff --git a/proposals/291-two-guard-nodes.txt b/proposals/291-two-guard-nodes.txt index a900ee9..dc0aa74 100644 --- a/proposals/291-two-guard-nodes.txt +++ b/proposals/291-two-guard-nodes.txt @@ -1,251 +1,251 @@ -Filename: 291-two-guard-nodes.txt -Title: The move to two guard nodes -Author: Mike Perry -Created: 2018-03-22 -Supersedes: Proposal 236 -Status: Open - -0. Background - - Back in 2014, Tor moved from three guard nodes to one guard node[1,2,3]. - - We made this change primarily to limit points of observability of entry - into the Tor network for clients and onion services, as well as to - reduce the ability of an adversary to track clients as they move from - one internet connection to another by their choice of guards. - - -1. Proposed changes - -1.1. Switch to two guards per client - - When this proposal becomes effective, clients will switch to using - two guard nodes. The guard node selection algorithms of Proposal 271 - will remain unchanged. Instead of having one primary guard "in use", - Tor clients will always use two. - - This will be accomplished by setting the guard-n-primary-guards-to-use - consensus parameter to 2, as well as guard-n-primary-guards to 2. - (Section 3.1 covers the reason for both parameters). This is equivalent - to using the torrc option NumEntryGuards=3D2, which can be used for - testing behavior prior to the consensus update. - -1.2. Enforce Tor's path restrictions across this guard layer - - In order to ensure that Tor can always build circuits using two guards - without resorting to a third, they must be chosen such that Tor's path - restrictions could still build a path with at least one of them, - regardless of the other nodes in the path. - - In other words, we must ensure that both guards are not chosen from the - same /16 or the same node family. In this way, Tor will always be able to - build a path using these guards, preventing the use of a third guard. - - -2. Discussion - -2.1. Why two guards? - - The main argument for switching to two guards is that because of Tor's - path restrictions, we're already using two guards, but we're using them - in a suboptimal and potentially dangerous way. - - Tor's path restrictions enforce the condition that the same node cannot - appear twice in the same circuit, nor can nodes from the same /16 subnet - or node family be used in the same circuit. - - Tor's paths are also built such that the exit node is chosen first and - held fixed during guard node choice, as are the IP, HSDIR, and RPs for - onion services. This means that whenever one of these nodes happens to - be the guard[4], or be in the same /16 or node family as the guard, Tor - will build that circuit using a second "primary" guard, as per proposal - 271[7]. - - Worse still, the choice of RP, IP, and exit can all be controlled by an - adversary (to varying degrees), enabling them to force the use of a - second guard at will. - - Because this happens somewhat infrequently in normal operation, a fresh - TLS connection will typically be created to the second "primary" guard, - and that TLS connection will be used only for the circuit for that - particular request. This property makes all sorts of traffic analysis - attacks easier, because this TLS connection will not benefit from any - multiplexing. - - This is more serious than traffic injection via an already in-use - guard because the lack of multiplexing means that the data retention - level required to gain information from this activity is very low, and - may exist for other reasons. To gain information from this behavior, an - adversary needs only connection 5-tuples + timestamps, as opposed to - detailed timeseries data that is polluted by other concurrent activity - and padding. - - In the most severe form of this attack, the adversary can take a suspect - list of Tor client IP addresses (or the list of all Guard node IP addresses) - and observe when secondary Tor connections are made to them at the time when - they cycle through all guards as RPs for connections to an onion - service. This adversary does not require collusion on the part of observers - beyond the ability to provide 5-tuple connection logs (which ISPs may retain - for reasons such as netflow accounting, IDS, or DoS protection systems). - - A fully passive adversary can also make use of this behavior. Clients - unlucky enough to pick guard nodes in heavily used /16s or in large node - families will tend to make use of a second guard more frequently even - without effort from the adversary. In these cases, the lack of - multiplexing also means that observers along the path to this secondary - guard gain more information per observation. - -2.2. Why not MOAR guards? - - We do not want to increase the number of observation points for client - activity into the Tor network[1]. We merely want better multiplexing for - the cases where this already happens. - -2.3. Can you put some numbers on that? - - The Changing of the Guards[13] paper studies this from a few different - angles, but one of the crucially missing graphs is how long a client - can expect to run with N guards before it chooses a malicious guard. - - However, we do have tables in section 3.2.1 of proposal 247 that cover - this[14]. There are three tables there: one for a 1% adversary, one for - a 5% adversary, and one for a 10% adversary. You can see the probability - of adversary success for one and two guards in terms of the number of - rotations needed before the adversary's node is chosen. Not surprisingly, - the two guard adversary gets to compromise clients roughly twice as - quickly, but the timescales are still rather large even for the 10% - adversary: they only have 50% chance of success after 4 rotations, which - will take about 14 months with Tor's 3.5 month guard rotation. - -2.4. What about guard fingerprinting? - - More guards also means more fingerprinting[8]. However, even one guard - may be enough to fingerprint a user who moves around in the same area, - if that guard is low bandwidth or there are not many Tor users in that - area. - - Furthermore, our use of separate directory guards (and three of them) - means that we're not really changing the situation much with the - addition of another regular guard. Right now, directory guard use alone - is enough to track all Tor users across the entire world. - - While the directory guard problem could be fixed[12] (and should be - fixed), it is still the case that another mechanism should be used for - the general problem of guard-vs-location management[9]. - - -3. Alternatives - - There are two other solutions that also avoid the use of secondary guard - in the path restriction case. - -3.1. Eliminate path restrictions entirely - - If Tor decided to stop enforcing /16, node family, and also allowed the - guard node to be chosen twice in the path, then under normal conditions, - it should retain the use of its primary guard. - - This approach is not as extreme as it seems on face. In fact, it is hard - to come up with arguments against removing these restrictions. Tor's - /16 restriction is of questionable utility against monitoring, and it can - be argued that since only good actors use node family, it gives influence - over path selection to bad actors in ways that are worse than the benefit - it provides to paths through good actors[10,11]. - - However, while removing path restrictions will solve the immediate - problem, it will not address other instances where Tor temporarily opts - use a second guard due to congestion, OOM, or failure of its primary - guard, and we're still running into bugs where this can be adversarially - controlled or just happen randomly[5]. - - While using two guards means twice the surface area for these types of - bugs, it also means that instances where they happen simultaneously on - both guards (thus forcing a third guard) are much less likely than with - just one guard. (In the passive adversary model, consider that one guard - fails at any point with probability P1. If we assume that such passive - failures are independent events, both guards would fail concurrently - with probability P1*P2. Even if the events are correlated, the maximum - chance of concurrent failure is still MIN(P1,P2)). - - Note that for this analysis to hold, we have to ensure that nodes that - are at RESOURCELIMIT or otherwise temporarily unresponsive do not cause - us to consider other primary guards beyond than the two we have chosen. - This is accomplished by setting guard-n-primary-guards to 2 (in addition - to setting guard-n-primary-guards-to-use to 2). With this parameter - set, the proposal 271 algorithm will avoid considering more than our two - guards, unless *both* are down at once. - -3.2. No Guard-flagged nodes as exit, RP, IP, or HSDIRs - - Similar to 3.1, we could instead forbid the use of Guard-flagged nodes - for the exit, IP, RP, and HSDIR positions. - - This solution has two problems: First, like 3.1, it also does not handle - the case where resource exhaustion could force the use of a second - guard. Second, it requires clients to upgrade to the new behavior and - stop using Guard flagged nodes before it can be deployed. - - -4. The future is confluxed - - An additional benefit of using a second guard is that it enables us to - eventually use conflux[6]. - - Conflux works by giving circuits a 256bit cookie that is sent to the - exit/RP, and circuits that are then built to the same exit/RP with the - same cookie can then be fused together. Throughput estimates are used to - balance traffic between these circuits, depending on their performance. - - We have unfortunately signaled to the research community that conflux is - not worth pursuing, because of our insistence on a single guard. While - not relevant to this proposal (indeed, conflux requires its own proposal - and also concurrent research), it is worth noting that whichever way we - go here, the door remains open to conflux because of its utility against - similar issues. - - If our conflux implementation includes packet acking, then circuits can - still survive the loss of one guard node due to DoS, OOM, or other - failures because the second half of the path will remain open and - usable (see the probability of concurrent failure arguments in Section - 3.1). - - If exits remember this cookie for a short period of time after the last - circuit is closed, the technique can be used to protect against - DoS/OOM/guard downtime conditions that take down both guard nodes or - destroy many circuits to confirm both guard node choices. In these - cases, circuits could be rebuilt along an alternate path and resumed - without end-to-end circuit connectivity loss. This same technique will - also make things like ephemeral bridges (ie Snowflake/Flashproxy) more - usable, because bridge uptime will no longer be so crucial to usability. - It will also improve mobile usability by allowing us to resume - connections after mobile Tor apps are briefly suspended, or if the user - switches between cell and wifi networks. - - Furthermore, it is likely that conflux will also be useful against traffic - analysis and congestion attacks. Since the load balancing is dynamic and - hard to predict by an external observer and also increases overall - traffic multiplexing, traffic correlation and website traffic - fingerprinting attacks will become harder, because the adversary can no - longer be sure what percentage of the traffic they have seen (depending - on their position and other potential concurrent activity). Similarly, - it should also help dampen congestion attacks, since traffic will - automatically shift away from a congested guard. - - - -References: - -1. https://blog.torproject.org/improving-tors-anonymity-changing-guard-paramet… -2. https://trac.torproject.org/projects/tor/ticket/12206 -3. https://gitweb.torproject.org/torspec.git/tree/proposals/236-single-guard-n… -4. https://trac.torproject.org/projects/tor/ticket/14917 -5. https://trac.torproject.org/projects/tor/ticket/25347#comment:14 -6. https://www.cypherpunks.ca/~iang/pubs/conflux-pets.pdf -7. https://gitweb.torproject.org/torspec.git/tree/proposals/271-another-guard-… -8. https://trac.torproject.org/projects/tor/ticket/9273#comment:3 -9. https://tails.boum.org/blueprint/persistent_Tor_state/ -10. https://trac.torproject.org/projects/tor/ticket/6676#comment:3 -11. https://bugs.torproject.org/15060 -12. https://trac.torproject.org/projects/tor/ticket/10969 -13. https://www.freehaven.net/anonbib/cache/wpes12-cogs.pdf -14. https://gitweb.torproject.org/torspec.git/tree/proposals/247-hs-guard-disco… +Filename: 291-two-guard-nodes.txt +Title: The move to two guard nodes +Author: Mike Perry +Created: 2018-03-22 +Supersedes: Proposal 236 +Status: Open + +0. Background + + Back in 2014, Tor moved from three guard nodes to one guard node[1,2,3]. + + We made this change primarily to limit points of observability of entry + into the Tor network for clients and onion services, as well as to + reduce the ability of an adversary to track clients as they move from + one internet connection to another by their choice of guards. + + +1. Proposed changes + +1.1. Switch to two guards per client + + When this proposal becomes effective, clients will switch to using + two guard nodes. The guard node selection algorithms of Proposal 271 + will remain unchanged. Instead of having one primary guard "in use", + Tor clients will always use two. + + This will be accomplished by setting the guard-n-primary-guards-to-use + consensus parameter to 2, as well as guard-n-primary-guards to 2. + (Section 3.1 covers the reason for both parameters). This is equivalent + to using the torrc option NumEntryGuards=3D2, which can be used for + testing behavior prior to the consensus update. + +1.2. Enforce Tor's path restrictions across this guard layer + + In order to ensure that Tor can always build circuits using two guards + without resorting to a third, they must be chosen such that Tor's path + restrictions could still build a path with at least one of them, + regardless of the other nodes in the path. + + In other words, we must ensure that both guards are not chosen from the + same /16 or the same node family. In this way, Tor will always be able to + build a path using these guards, preventing the use of a third guard. + + +2. Discussion + +2.1. Why two guards? + + The main argument for switching to two guards is that because of Tor's + path restrictions, we're already using two guards, but we're using them + in a suboptimal and potentially dangerous way. + + Tor's path restrictions enforce the condition that the same node cannot + appear twice in the same circuit, nor can nodes from the same /16 subnet + or node family be used in the same circuit. + + Tor's paths are also built such that the exit node is chosen first and + held fixed during guard node choice, as are the IP, HSDIR, and RPs for + onion services. This means that whenever one of these nodes happens to + be the guard[4], or be in the same /16 or node family as the guard, Tor + will build that circuit using a second "primary" guard, as per proposal + 271[7]. + + Worse still, the choice of RP, IP, and exit can all be controlled by an + adversary (to varying degrees), enabling them to force the use of a + second guard at will. + + Because this happens somewhat infrequently in normal operation, a fresh + TLS connection will typically be created to the second "primary" guard, + and that TLS connection will be used only for the circuit for that + particular request. This property makes all sorts of traffic analysis + attacks easier, because this TLS connection will not benefit from any + multiplexing. + + This is more serious than traffic injection via an already in-use + guard because the lack of multiplexing means that the data retention + level required to gain information from this activity is very low, and + may exist for other reasons. To gain information from this behavior, an + adversary needs only connection 5-tuples + timestamps, as opposed to + detailed timeseries data that is polluted by other concurrent activity + and padding. + + In the most severe form of this attack, the adversary can take a suspect + list of Tor client IP addresses (or the list of all Guard node IP addresses) + and observe when secondary Tor connections are made to them at the time when + they cycle through all guards as RPs for connections to an onion + service. This adversary does not require collusion on the part of observers + beyond the ability to provide 5-tuple connection logs (which ISPs may retain + for reasons such as netflow accounting, IDS, or DoS protection systems). + + A fully passive adversary can also make use of this behavior. Clients + unlucky enough to pick guard nodes in heavily used /16s or in large node + families will tend to make use of a second guard more frequently even + without effort from the adversary. In these cases, the lack of + multiplexing also means that observers along the path to this secondary + guard gain more information per observation. + +2.2. Why not MOAR guards? + + We do not want to increase the number of observation points for client + activity into the Tor network[1]. We merely want better multiplexing for + the cases where this already happens. + +2.3. Can you put some numbers on that? + + The Changing of the Guards[13] paper studies this from a few different + angles, but one of the crucially missing graphs is how long a client + can expect to run with N guards before it chooses a malicious guard. + + However, we do have tables in section 3.2.1 of proposal 247 that cover + this[14]. There are three tables there: one for a 1% adversary, one for + a 5% adversary, and one for a 10% adversary. You can see the probability + of adversary success for one and two guards in terms of the number of + rotations needed before the adversary's node is chosen. Not surprisingly, + the two guard adversary gets to compromise clients roughly twice as + quickly, but the timescales are still rather large even for the 10% + adversary: they only have 50% chance of success after 4 rotations, which + will take about 14 months with Tor's 3.5 month guard rotation. + +2.4. What about guard fingerprinting? + + More guards also means more fingerprinting[8]. However, even one guard + may be enough to fingerprint a user who moves around in the same area, + if that guard is low bandwidth or there are not many Tor users in that + area. + + Furthermore, our use of separate directory guards (and three of them) + means that we're not really changing the situation much with the + addition of another regular guard. Right now, directory guard use alone + is enough to track all Tor users across the entire world. + + While the directory guard problem could be fixed[12] (and should be + fixed), it is still the case that another mechanism should be used for + the general problem of guard-vs-location management[9]. + + +3. Alternatives + + There are two other solutions that also avoid the use of secondary guard + in the path restriction case. + +3.1. Eliminate path restrictions entirely + + If Tor decided to stop enforcing /16, node family, and also allowed the + guard node to be chosen twice in the path, then under normal conditions, + it should retain the use of its primary guard. + + This approach is not as extreme as it seems on face. In fact, it is hard + to come up with arguments against removing these restrictions. Tor's + /16 restriction is of questionable utility against monitoring, and it can + be argued that since only good actors use node family, it gives influence + over path selection to bad actors in ways that are worse than the benefit + it provides to paths through good actors[10,11]. + + However, while removing path restrictions will solve the immediate + problem, it will not address other instances where Tor temporarily opts + use a second guard due to congestion, OOM, or failure of its primary + guard, and we're still running into bugs where this can be adversarially + controlled or just happen randomly[5]. + + While using two guards means twice the surface area for these types of + bugs, it also means that instances where they happen simultaneously on + both guards (thus forcing a third guard) are much less likely than with + just one guard. (In the passive adversary model, consider that one guard + fails at any point with probability P1. If we assume that such passive + failures are independent events, both guards would fail concurrently + with probability P1*P2. Even if the events are correlated, the maximum + chance of concurrent failure is still MIN(P1,P2)). + + Note that for this analysis to hold, we have to ensure that nodes that + are at RESOURCELIMIT or otherwise temporarily unresponsive do not cause + us to consider other primary guards beyond than the two we have chosen. + This is accomplished by setting guard-n-primary-guards to 2 (in addition + to setting guard-n-primary-guards-to-use to 2). With this parameter + set, the proposal 271 algorithm will avoid considering more than our two + guards, unless *both* are down at once. + +3.2. No Guard-flagged nodes as exit, RP, IP, or HSDIRs + + Similar to 3.1, we could instead forbid the use of Guard-flagged nodes + for the exit, IP, RP, and HSDIR positions. + + This solution has two problems: First, like 3.1, it also does not handle + the case where resource exhaustion could force the use of a second + guard. Second, it requires clients to upgrade to the new behavior and + stop using Guard flagged nodes before it can be deployed. + + +4. The future is confluxed + + An additional benefit of using a second guard is that it enables us to + eventually use conflux[6]. + + Conflux works by giving circuits a 256bit cookie that is sent to the + exit/RP, and circuits that are then built to the same exit/RP with the + same cookie can then be fused together. Throughput estimates are used to + balance traffic between these circuits, depending on their performance. + + We have unfortunately signaled to the research community that conflux is + not worth pursuing, because of our insistence on a single guard. While + not relevant to this proposal (indeed, conflux requires its own proposal + and also concurrent research), it is worth noting that whichever way we + go here, the door remains open to conflux because of its utility against + similar issues. + + If our conflux implementation includes packet acking, then circuits can + still survive the loss of one guard node due to DoS, OOM, or other + failures because the second half of the path will remain open and + usable (see the probability of concurrent failure arguments in Section + 3.1). + + If exits remember this cookie for a short period of time after the last + circuit is closed, the technique can be used to protect against + DoS/OOM/guard downtime conditions that take down both guard nodes or + destroy many circuits to confirm both guard node choices. In these + cases, circuits could be rebuilt along an alternate path and resumed + without end-to-end circuit connectivity loss. This same technique will + also make things like ephemeral bridges (ie Snowflake/Flashproxy) more + usable, because bridge uptime will no longer be so crucial to usability. + It will also improve mobile usability by allowing us to resume + connections after mobile Tor apps are briefly suspended, or if the user + switches between cell and wifi networks. + + Furthermore, it is likely that conflux will also be useful against traffic + analysis and congestion attacks. Since the load balancing is dynamic and + hard to predict by an external observer and also increases overall + traffic multiplexing, traffic correlation and website traffic + fingerprinting attacks will become harder, because the adversary can no + longer be sure what percentage of the traffic they have seen (depending + on their position and other potential concurrent activity). Similarly, + it should also help dampen congestion attacks, since traffic will + automatically shift away from a congested guard. + + + +References: + +1. https://blog.torproject.org/improving-tors-anonymity-changing-guard-paramet… +2. https://trac.torproject.org/projects/tor/ticket/12206 +3. https://gitweb.torproject.org/torspec.git/tree/proposals/236-single-guard-n… +4. https://trac.torproject.org/projects/tor/ticket/14917 +5. https://trac.torproject.org/projects/tor/ticket/25347#comment:14 +6. https://www.cypherpunks.ca/~iang/pubs/conflux-pets.pdf +7. https://gitweb.torproject.org/torspec.git/tree/proposals/271-another-guard-… +8. https://trac.torproject.org/projects/tor/ticket/9273#comment:3 +9. https://tails.boum.org/blueprint/persistent_Tor_state/ +10. https://trac.torproject.org/projects/tor/ticket/6676#comment:3 +11. https://bugs.torproject.org/15060 +12. https://trac.torproject.org/projects/tor/ticket/10969 +13. https://www.freehaven.net/anonbib/cache/wpes12-cogs.pdf +14. https://gitweb.torproject.org/torspec.git/tree/proposals/247-hs-guard-disco…

1 0

[torspec/master] prop291: Change MOAR to MORE
by dgoulet＠torproject.org 16 Apr '18

16 Apr '18

commit f7fa4b925b40779731c401ddb5b827fe6635e3bd Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 16 11:12:52 2018 -0400 prop291: Change MOAR to MORE Some of us understands that "MOAR" is a funny word that means MORE++ but unfortunately, not everyone can get that reference especially when you don't speak English as your first language. Closes #25758 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- proposals/291-two-guard-nodes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/291-two-guard-nodes.txt b/proposals/291-two-guard-nodes.txt index dc0aa74..2a884ef 100644 --- a/proposals/291-two-guard-nodes.txt +++ b/proposals/291-two-guard-nodes.txt @@ -95,7 +95,7 @@ Status: Open multiplexing also means that observers along the path to this secondary guard gain more information per observation. -2.2. Why not MOAR guards? +2.2. Why not MORE guards? We do not want to increase the number of observation points for client activity into the Tor network[1]. We merely want better multiplexing for

1 0

[torspec/master] dir-spec: Add consensus param circ_max_cell_queue_size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 7a25d149332a737a0116ddf0d562bdb24894ad91 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Apr 16 10:56:27 2018 -0400 dir-spec: Add consensus param circ_max_cell_queue_size Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- dir-spec.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dir-spec.txt b/dir-spec.txt index 7657e39..1adfe69 100644 --- a/dir-spec.txt +++ b/dir-spec.txt @@ -2042,6 +2042,11 @@ "DoSRefuseSingleHopClientRendezvous" -- Refuse establishment of rendezvous points for single hop clients. + "circ_max_cell_queue_size" -- This parameter determines the maximum + number of cells allowed per circuit queue. + Min 1000. Max 4294967295. Default 50000. + First-appeared: 0.3.3.6-rc. + "shared-rand-previous-value" SP NumReveals SP Value NL [At most once]

1 0

[tor/master] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3d5bf12ac24bfbabad5f14c6cd45373b227d2816 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 14:58:12 2018 -0400 relay: Remove max middle cells dead code Next commit is addressing the circuit queue cell limit so cleanup before doing anything else. Part of #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/or.h | 6 ------ src/or/relay.c | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2397f6651..8fcba42ad 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3524,12 +3524,6 @@ typedef struct or_circuit_t { * exit-ward queues of this circuit; reset every time when writing * buffer stats to disk. */ uint64_t total_cell_waiting_time; - - /** Maximum cell queue size for a middle relay; this is stored per circuit - * so append_cell_to_circuit_queue() can adjust it if it changes. If set - * to zero, it is initialized to the default value. - */ - uint32_t max_middle_cells; } or_circuit_t; #if REND_COOKIE_LEN != DIGEST_LEN diff --git a/src/or/relay.c b/src/or/relay.c index 506b7eccb..c8e0a2e61 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -99,9 +99,6 @@ static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ, entry_connection_t *conn, node_t *node, const tor_addr_t *addr); -#if 0 -static int get_max_middle_cells(void); -#endif /** Stop reading on edge connections when we have this many cells * waiting on the appropriate queue. */

1 0

[tor/maint-0.3.3] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3d5bf12ac24bfbabad5f14c6cd45373b227d2816 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 14:58:12 2018 -0400 relay: Remove max middle cells dead code Next commit is addressing the circuit queue cell limit so cleanup before doing anything else. Part of #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/or.h | 6 ------ src/or/relay.c | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2397f6651..8fcba42ad 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3524,12 +3524,6 @@ typedef struct or_circuit_t { * exit-ward queues of this circuit; reset every time when writing * buffer stats to disk. */ uint64_t total_cell_waiting_time; - - /** Maximum cell queue size for a middle relay; this is stored per circuit - * so append_cell_to_circuit_queue() can adjust it if it changes. If set - * to zero, it is initialized to the default value. - */ - uint32_t max_middle_cells; } or_circuit_t; #if REND_COOKIE_LEN != DIGEST_LEN diff --git a/src/or/relay.c b/src/or/relay.c index 506b7eccb..c8e0a2e61 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -99,9 +99,6 @@ static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ, entry_connection_t *conn, node_t *node, const tor_addr_t *addr); -#if 0 -static int get_max_middle_cells(void); -#endif /** Stop reading on edge connections when we have this many cells * waiting on the appropriate queue. */

1 0

[tor/maint-0.3.3] Merge remote-tracking branch 'dgoulet/bug25226_033_02' into maint-0.3.3
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 22845df2a7503ed73ed325c3a98916f289918caa Merge: 3ee4c9b1f d064122e7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 10:04:36 2018 -0400 Merge remote-tracking branch 'dgoulet/bug25226_033_02' into maint-0.3.3 changes/bug25226 | 4 +++ src/or/networkstatus.c | 1 + src/or/or.h | 6 ----- src/or/relay.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++--- src/or/relay.h | 1 + 5 files changed, 70 insertions(+), 9 deletions(-)

1 0

[tor/maint-0.3.3] relay: Implement a circuit cell queue maximum size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit d064122e706575c99e8d2162de93f7f08cc8d41e Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 15:27:58 2018 -0400 relay: Implement a circuit cell queue maximum size This commit introduces the consensus parameter "circ_max_cell_queue_size" which controls the maximum number of cells a circuit queue should have. The default value is currently 50000 cells which is above what should be expected but keeps us a margin of error for padding cells. Related to this is #9072. Back in 0.2.4.14-alpha, we've removed that limit due to a Guard discovery attack. Ticket #25226 details why we are putting back the limit due to the memory pressure issue on relays. Fixes #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug25226 | 4 ++++ src/or/networkstatus.c | 1 + src/or/relay.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/relay.h | 1 + 4 files changed, 70 insertions(+) diff --git a/changes/bug25226 b/changes/bug25226 new file mode 100644 index 000000000..b594a7a42 --- /dev/null +++ b/changes/bug25226 @@ -0,0 +1,4 @@ + o Major bugfixes (relay, denial of service): + - Impose a limit on circuit cell queue size. The limit can be controlled by + a consensus parameter. Fixes bug 25226; bugfix on 0.2.4.14-alpha. + diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 7777473f1..1f21e8adb 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1596,6 +1596,7 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c, { notify_control_networkstatus_changed(old_c, new_c); dos_consensus_has_changed(new_c); + relay_consensus_has_changed(new_c); } /* Called after a new consensus has been put in the global state. It is safe diff --git a/src/or/relay.c b/src/or/relay.c index c8e0a2e61..5a3c43e05 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2951,6 +2951,60 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) return n_flushed; } +/* Minimum value is the maximum circuit window size. + * + * SENDME cells makes it that we can control how many cells can be inflight on + * a circuit from end to end. This logic makes it that on any circuit cell + * queue, we have a maximum of cells possible. + * + * Because the Tor protocol allows for a client to exit at any hop in a + * circuit and a circuit can be of a maximum of 8 hops, so in theory the + * normal worst case will be the circuit window start value times the maximum + * number of hops (8). Having more cells then that means something is wrong. + * + * However, because padding cells aren't counted in the package window, we set + * the maximum size to a reasonably large size for which we expect that we'll + * never reach in theory. And if we ever do because of future changes, we'll + * be able to control it with a consensus parameter. + * + * XXX: Unfortunately, END cells aren't accounted for in the circuit window + * which means that for instance if a client opens 8001 streams, the 8001 + * following END cells will queue up in the circuit which will get closed if + * the max limit is 8000. Which is sad because it is allowed by the Tor + * protocol. But, we need an upper bound on circuit queue in order to avoid + * DoS memory pressure so the default size is a middle ground between not + * having any limit and having a very restricted one. This is why we can also + * control it through a consensus parameter. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MIN CIRCWINDOW_START_MAX +/* We can't have a consensus parameter above this value. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MAX INT32_MAX +/* Default value is set to a large value so we can handle padding cells + * properly which aren't accounted for in the SENDME window. Default is 50000 + * allowed cells in the queue resulting in ~25MB. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT \ + (50 * RELAY_CIRC_CELL_QUEUE_SIZE_MIN) + +/* The maximum number of cell a circuit queue can contain. This is updated at + * every new consensus and controlled by a parameter. */ +static int32_t max_circuit_cell_queue_size = + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT; + +/* Called when the consensus has changed. At this stage, the global consensus + * object has NOT been updated. It is called from + * notify_before_networkstatus_changes(). */ +void +relay_consensus_has_changed(const networkstatus_t *ns) +{ + tor_assert(ns); + + /* Update the circuit max cell queue size from the consensus. */ + max_circuit_cell_queue_size = + networkstatus_get_param(ns, "circ_max_cell_queue_size", + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT, + RELAY_CIRC_CELL_QUEUE_SIZE_MIN, + RELAY_CIRC_CELL_QUEUE_SIZE_MAX); +} + /** Add cell to the queue of circ writing to chan * transmitting in direction. * @@ -2980,6 +3034,16 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, streams_blocked = circ->streams_blocked_on_p_chan; } + if (PREDICT_UNLIKELY(queue->n >= max_circuit_cell_queue_size)) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "%s circuit has %d cells in its queue, maximum allowed is %d. " + "Closing circuit for safety reasons.", + (exitward) ? "Outbound" : "Inbound", queue->n, + max_circuit_cell_queue_size); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } + /* Very important that we copy to the circuit queue because all calls to * this function use the stack for the cell memory. */ cell_queue_append_packed_copy(circ, queue, exitward, cell, diff --git a/src/or/relay.h b/src/or/relay.h index f0fa7e987..c9281c5ea 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -15,6 +15,7 @@ extern uint64_t stats_n_relay_cells_relayed; extern uint64_t stats_n_relay_cells_delivered; +void relay_consensus_has_changed(const networkstatus_t *ns); int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, cell_direction_t cell_direction); size_t cell_queues_get_total_allocation(void);

1 0

[tor/release-0.3.3] relay: Remove max middle cells dead code
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3d5bf12ac24bfbabad5f14c6cd45373b227d2816 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 14:58:12 2018 -0400 relay: Remove max middle cells dead code Next commit is addressing the circuit queue cell limit so cleanup before doing anything else. Part of #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/or.h | 6 ------ src/or/relay.c | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2397f6651..8fcba42ad 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3524,12 +3524,6 @@ typedef struct or_circuit_t { * exit-ward queues of this circuit; reset every time when writing * buffer stats to disk. */ uint64_t total_cell_waiting_time; - - /** Maximum cell queue size for a middle relay; this is stored per circuit - * so append_cell_to_circuit_queue() can adjust it if it changes. If set - * to zero, it is initialized to the default value. - */ - uint32_t max_middle_cells; } or_circuit_t; #if REND_COOKIE_LEN != DIGEST_LEN diff --git a/src/or/relay.c b/src/or/relay.c index 506b7eccb..c8e0a2e61 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -99,9 +99,6 @@ static void adjust_exit_policy_from_exitpolicy_failure(origin_circuit_t *circ, entry_connection_t *conn, node_t *node, const tor_addr_t *addr); -#if 0 -static int get_max_middle_cells(void); -#endif /** Stop reading on edge connections when we have this many cells * waiting on the appropriate queue. */

1 0

[tor/master] Merge branch 'maint-0.3.3'
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit 3463b4e0652bacca51fecd2c256e3e9d61ce920e Merge: 197d1992d 22845df2a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Apr 16 10:06:21 2018 -0400 Merge branch 'maint-0.3.3' changes/bug25226 | 4 +++ src/or/networkstatus.c | 1 + src/or/or.h | 6 ----- src/or/relay.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++--- src/or/relay.h | 1 + 5 files changed, 70 insertions(+), 9 deletions(-)

1 0

[tor/master] relay: Implement a circuit cell queue maximum size
by nickm＠torproject.org 16 Apr '18

16 Apr '18

commit d064122e706575c99e8d2162de93f7f08cc8d41e Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Mar 20 15:27:58 2018 -0400 relay: Implement a circuit cell queue maximum size This commit introduces the consensus parameter "circ_max_cell_queue_size" which controls the maximum number of cells a circuit queue should have. The default value is currently 50000 cells which is above what should be expected but keeps us a margin of error for padding cells. Related to this is #9072. Back in 0.2.4.14-alpha, we've removed that limit due to a Guard discovery attack. Ticket #25226 details why we are putting back the limit due to the memory pressure issue on relays. Fixes #25226 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug25226 | 4 ++++ src/or/networkstatus.c | 1 + src/or/relay.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/relay.h | 1 + 4 files changed, 70 insertions(+) diff --git a/changes/bug25226 b/changes/bug25226 new file mode 100644 index 000000000..b594a7a42 --- /dev/null +++ b/changes/bug25226 @@ -0,0 +1,4 @@ + o Major bugfixes (relay, denial of service): + - Impose a limit on circuit cell queue size. The limit can be controlled by + a consensus parameter. Fixes bug 25226; bugfix on 0.2.4.14-alpha. + diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 7777473f1..1f21e8adb 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1596,6 +1596,7 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c, { notify_control_networkstatus_changed(old_c, new_c); dos_consensus_has_changed(new_c); + relay_consensus_has_changed(new_c); } /* Called after a new consensus has been put in the global state. It is safe diff --git a/src/or/relay.c b/src/or/relay.c index c8e0a2e61..5a3c43e05 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2951,6 +2951,60 @@ channel_flush_from_first_active_circuit, (channel_t *chan, int max)) return n_flushed; } +/* Minimum value is the maximum circuit window size. + * + * SENDME cells makes it that we can control how many cells can be inflight on + * a circuit from end to end. This logic makes it that on any circuit cell + * queue, we have a maximum of cells possible. + * + * Because the Tor protocol allows for a client to exit at any hop in a + * circuit and a circuit can be of a maximum of 8 hops, so in theory the + * normal worst case will be the circuit window start value times the maximum + * number of hops (8). Having more cells then that means something is wrong. + * + * However, because padding cells aren't counted in the package window, we set + * the maximum size to a reasonably large size for which we expect that we'll + * never reach in theory. And if we ever do because of future changes, we'll + * be able to control it with a consensus parameter. + * + * XXX: Unfortunately, END cells aren't accounted for in the circuit window + * which means that for instance if a client opens 8001 streams, the 8001 + * following END cells will queue up in the circuit which will get closed if + * the max limit is 8000. Which is sad because it is allowed by the Tor + * protocol. But, we need an upper bound on circuit queue in order to avoid + * DoS memory pressure so the default size is a middle ground between not + * having any limit and having a very restricted one. This is why we can also + * control it through a consensus parameter. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MIN CIRCWINDOW_START_MAX +/* We can't have a consensus parameter above this value. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_MAX INT32_MAX +/* Default value is set to a large value so we can handle padding cells + * properly which aren't accounted for in the SENDME window. Default is 50000 + * allowed cells in the queue resulting in ~25MB. */ +#define RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT \ + (50 * RELAY_CIRC_CELL_QUEUE_SIZE_MIN) + +/* The maximum number of cell a circuit queue can contain. This is updated at + * every new consensus and controlled by a parameter. */ +static int32_t max_circuit_cell_queue_size = + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT; + +/* Called when the consensus has changed. At this stage, the global consensus + * object has NOT been updated. It is called from + * notify_before_networkstatus_changes(). */ +void +relay_consensus_has_changed(const networkstatus_t *ns) +{ + tor_assert(ns); + + /* Update the circuit max cell queue size from the consensus. */ + max_circuit_cell_queue_size = + networkstatus_get_param(ns, "circ_max_cell_queue_size", + RELAY_CIRC_CELL_QUEUE_SIZE_DEFAULT, + RELAY_CIRC_CELL_QUEUE_SIZE_MIN, + RELAY_CIRC_CELL_QUEUE_SIZE_MAX); +} + /** Add cell to the queue of circ writing to chan * transmitting in direction. * @@ -2980,6 +3034,16 @@ append_cell_to_circuit_queue(circuit_t *circ, channel_t *chan, streams_blocked = circ->streams_blocked_on_p_chan; } + if (PREDICT_UNLIKELY(queue->n >= max_circuit_cell_queue_size)) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "%s circuit has %d cells in its queue, maximum allowed is %d. " + "Closing circuit for safety reasons.", + (exitward) ? "Outbound" : "Inbound", queue->n, + max_circuit_cell_queue_size); + circuit_mark_for_close(circ, END_CIRC_REASON_RESOURCELIMIT); + return; + } + /* Very important that we copy to the circuit queue because all calls to * this function use the stack for the cell memory. */ cell_queue_append_packed_copy(circ, queue, exitward, cell, diff --git a/src/or/relay.h b/src/or/relay.h index f0fa7e987..c9281c5ea 100644 --- a/src/or/relay.h +++ b/src/or/relay.h @@ -15,6 +15,7 @@ extern uint64_t stats_n_relay_cells_relayed; extern uint64_t stats_n_relay_cells_delivered; +void relay_consensus_has_changed(const networkstatus_t *ns); int circuit_receive_relay_cell(cell_t *cell, circuit_t *circ, cell_direction_t cell_direction); size_t cell_queues_get_total_allocation(void);

1 0