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

[tor/release-0.3.4] Bug 26214: Test updates.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit 46b06cd6999439608f4a8a34bb68f6fe6a6311e9 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Jun 19 19:53:58 2018 +0000 Bug 26214: Test updates. --- src/test/test_relaycell.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 841174982..1bd17b73b 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -240,11 +240,26 @@ test_circbw_relay(void *arg) circ->cpath); ASSERT_UNCOUNTED_BW(); - /* Sendme on stream: not counted */ + /* Sendme on valid stream: counted */ ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, circ->cpath); + ASSERT_COUNTED_BW(); + + /* Sendme on valid stream with full window: not counted */ + ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; + PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); + edgeconn->package_window = 500; + connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, + circ->cpath); + ASSERT_UNCOUNTED_BW(); + + /* Sendme on unknown stream: not counted */ + ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; + PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); + connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, + circ->cpath); ASSERT_UNCOUNTED_BW(); /* Sendme on circuit with full window: not counted */

1 0

[tor/master] Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit bfcfeaed0772cd9238c3fa4d1c4a6db4f23ad2a3 Merge: cf8c3abff 46b06cd69 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 2 13:22:07 2018 -0400 Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4 changes/bug26214 | 3 +++ src/or/or.h | 1 + src/or/relay.c | 24 ++++++++++++++++++++++-- src/test/test_relaycell.c | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-)

1 0

[tor/release-0.3.4] Bug 26214: Changes file.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit 11d993869447d08a3fe4ab3b36d34fc53aa21ba1 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Sun May 27 01:43:19 2018 +0000 Bug 26214: Changes file. --- changes/bug26214 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changes/bug26214 b/changes/bug26214 new file mode 100644 index 000000000..4277b9c6e --- /dev/null +++ b/changes/bug26214 @@ -0,0 +1,3 @@ + o Minor bugfixes (correctness, flow control): + - Upon receiving a stream-level SENDME cell, verify that our window has + not grown too large. Fixes bug 26214; bugfix on svn r54 (pre-0.0.1)

1 0

[tor/release-0.3.4] Bug 26214: Check stream SENDME against max.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit dc397f9a61e2e2caeea1acd46beab0e7205eaa9f Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Sun May 27 01:42:35 2018 +0000 Bug 26214: Check stream SENDME against max. --- src/or/or.h | 1 + src/or/relay.c | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index e106ec66f..db8f9544f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -929,6 +929,7 @@ typedef enum { /** Initial value on both sides of a stream transmission window when the * stream is initialized. Measured in cells. */ #define STREAMWINDOW_START 500 +#define STREAMWINDOW_START_MAX 500 /** Amount to increment a stream window when we get a stream SENDME. */ #define STREAMWINDOW_INCREMENT 50 diff --git a/src/or/relay.c b/src/or/relay.c index 50f59d6b9..3632678af 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1752,8 +1752,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, circuit_resume_edge_reading(circ, layer_hint); /* We count circuit-level sendme's as valid delivered data because - * they are rate limited. Note that we cannot count stream - * sendme's because the other end could send as many as they like. + * they are rate limited. */ if (CIRCUIT_IS_ORIGIN(circ)) { circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), @@ -1783,6 +1782,27 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, rh.stream_id); return 0; } + + /* Don't allow the other endpoint to request more than our maximim + * (ie initial) stream SENDME window worth of data. Well-behaved + * stock clients will not request more than this max (as per the check + * in the while loop of connection_edge_consider_sending_sendme()). + */ + if (conn->package_window + STREAMWINDOW_INCREMENT > + STREAMWINDOW_START_MAX) { + static struct ratelim_t stream_warn_ratelim = RATELIM_INIT(600); + log_fn_ratelim(&stream_warn_ratelim,LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unexpected stream sendme cell. Closing circ (window %d).", + conn->package_window); + return -END_CIRC_REASON_TORPROTOCOL; + } + + /* At this point, the stream sendme is valid */ + if (CIRCUIT_IS_ORIGIN(circ)) { + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), + rh.length); + } + conn->package_window += STREAMWINDOW_INCREMENT; log_debug(domain,"stream-level sendme, packagewindow now %d.", conn->package_window);

1 0

[tor/release-0.3.4] Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit bfcfeaed0772cd9238c3fa4d1c4a6db4f23ad2a3 Merge: cf8c3abff 46b06cd69 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 2 13:22:07 2018 -0400 Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4 changes/bug26214 | 3 +++ src/or/or.h | 1 + src/or/relay.c | 24 ++++++++++++++++++++++-- src/test/test_relaycell.c | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-)

1 0

[tor/maint-0.3.4] Bug 26214: Changes file.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit 11d993869447d08a3fe4ab3b36d34fc53aa21ba1 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Sun May 27 01:43:19 2018 +0000 Bug 26214: Changes file. --- changes/bug26214 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changes/bug26214 b/changes/bug26214 new file mode 100644 index 000000000..4277b9c6e --- /dev/null +++ b/changes/bug26214 @@ -0,0 +1,3 @@ + o Minor bugfixes (correctness, flow control): + - Upon receiving a stream-level SENDME cell, verify that our window has + not grown too large. Fixes bug 26214; bugfix on svn r54 (pre-0.0.1)

1 0

[tor/maint-0.3.4] Bug 26214: Check stream SENDME against max.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit dc397f9a61e2e2caeea1acd46beab0e7205eaa9f Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Sun May 27 01:42:35 2018 +0000 Bug 26214: Check stream SENDME against max. --- src/or/or.h | 1 + src/or/relay.c | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index e106ec66f..db8f9544f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -929,6 +929,7 @@ typedef enum { /** Initial value on both sides of a stream transmission window when the * stream is initialized. Measured in cells. */ #define STREAMWINDOW_START 500 +#define STREAMWINDOW_START_MAX 500 /** Amount to increment a stream window when we get a stream SENDME. */ #define STREAMWINDOW_INCREMENT 50 diff --git a/src/or/relay.c b/src/or/relay.c index 50f59d6b9..3632678af 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1752,8 +1752,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, circuit_resume_edge_reading(circ, layer_hint); /* We count circuit-level sendme's as valid delivered data because - * they are rate limited. Note that we cannot count stream - * sendme's because the other end could send as many as they like. + * they are rate limited. */ if (CIRCUIT_IS_ORIGIN(circ)) { circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), @@ -1783,6 +1782,27 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, rh.stream_id); return 0; } + + /* Don't allow the other endpoint to request more than our maximim + * (ie initial) stream SENDME window worth of data. Well-behaved + * stock clients will not request more than this max (as per the check + * in the while loop of connection_edge_consider_sending_sendme()). + */ + if (conn->package_window + STREAMWINDOW_INCREMENT > + STREAMWINDOW_START_MAX) { + static struct ratelim_t stream_warn_ratelim = RATELIM_INIT(600); + log_fn_ratelim(&stream_warn_ratelim,LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Unexpected stream sendme cell. Closing circ (window %d).", + conn->package_window); + return -END_CIRC_REASON_TORPROTOCOL; + } + + /* At this point, the stream sendme is valid */ + if (CIRCUIT_IS_ORIGIN(circ)) { + circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ), + rh.length); + } + conn->package_window += STREAMWINDOW_INCREMENT; log_debug(domain,"stream-level sendme, packagewindow now %d.", conn->package_window);

1 0

[tor/maint-0.3.4] Bug 26214: Test updates.
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit 46b06cd6999439608f4a8a34bb68f6fe6a6311e9 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Tue Jun 19 19:53:58 2018 +0000 Bug 26214: Test updates. --- src/test/test_relaycell.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 841174982..1bd17b73b 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -240,11 +240,26 @@ test_circbw_relay(void *arg) circ->cpath); ASSERT_UNCOUNTED_BW(); - /* Sendme on stream: not counted */ + /* Sendme on valid stream: counted */ ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, circ->cpath); + ASSERT_COUNTED_BW(); + + /* Sendme on valid stream with full window: not counted */ + ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; + PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); + edgeconn->package_window = 500; + connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn, + circ->cpath); + ASSERT_UNCOUNTED_BW(); + + /* Sendme on unknown stream: not counted */ + ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0; + PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234"); + connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, + circ->cpath); ASSERT_UNCOUNTED_BW(); /* Sendme on circuit with full window: not counted */

1 0

[tor/maint-0.3.4] Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4
by nickm＠torproject.org 02 Jul '18

02 Jul '18

commit bfcfeaed0772cd9238c3fa4d1c4a6db4f23ad2a3 Merge: cf8c3abff 46b06cd69 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jul 2 13:22:07 2018 -0400 Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4 changes/bug26214 | 3 +++ src/or/or.h | 1 + src/or/relay.c | 24 ++++++++++++++++++++++-- src/test/test_relaycell.c | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-)

1 0

[stem/master] Update 'bandwidth-file-header' parsing
by atagar＠torproject.org 02 Jul '18

02 Jul '18

commit 8a1f48ff48dd611feee1d89e1ad27d4d16474de9 Author: Damian Johnson <atagar(a)torproject.org> Date: Mon Jul 2 09:44:32 2018 -0700 Update 'bandwidth-file-header' parsing Thanks to teor and juga our bandwidth-file-header specification has been improved... https://trac.torproject.org/projects/tor/ticket/26541 In particular... * Field renamed to 'bandwidth-file-headers'. Adjusted our attribute name to match. * Values are now required ('timetamp=' is no longer valid). * 'bandwidth-file-headers' is now documented as only residing in votes. I implemented it that way so no change needed, but nice that we now match the spec. --- stem/descriptor/networkstatus.py | 29 +++++++++++++---------- test/unit/descriptor/networkstatus/document_v3.py | 28 ++++++++++++++-------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index 0e1af80b..eac556a6 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -130,7 +130,7 @@ HEADER_STATUS_DOCUMENT_FIELDS = ( ('shared-rand-commit', True, False, False), ('shared-rand-previous-value', True, True, False), ('shared-rand-current-value', True, True, False), - ('bandwidth-file', True, False, False), + ('bandwidth-file-headers', True, False, False), ('recommended-client-protocols', True, True, False), ('recommended-relay-protocols', True, True, False), ('required-client-protocols', True, True, False), @@ -784,25 +784,29 @@ def _parse_shared_rand_current_value(descriptor, entries): raise ValueError("A network status document's 'shared-rand-current-value' line must be a pair of values, the first an integer but was '%s'" % value) -def _parse_bandwidth_file(descriptor, entries): - # "bandwidth-file" KeyValues - # Value ::= any printing ASCII character except NL and SP. - # KeyValue ::= Keyword '=' Value +def _parse_bandwidth_file_headers(descriptor, entries): + # "bandwidth-file-headers" KeyValues # KeyValues ::= KeyValue | KeyValues SP KeyValue + # KeyValue ::= Keyword '=' Value + # Value ::= ArgumentChar+ - value = _value('bandwidth-file', entries) + value = _value('bandwidth-file-headers', entries) results = {} for entry in value.split(' '): if not entry: continue elif '=' not in entry: - raise ValueError("'bandwidth-file' lines must be a series of 'key=value' pairs: %s" % value) + raise ValueError("'bandwidth-file-headers' lines must be a series of 'key=value' pairs: %s" % value) k, v = entry.split('=', 1) + + if not v: + raise ValueError("'bandwidth-file-headers' mappings should all have values: %s" % value) + results[k] = v - descriptor.bandwidth_file = results + descriptor.bandwidth_file_headers = results _parse_header_valid_after_line = _parse_timestamp_line('valid-after', 'valid_after') @@ -874,7 +878,8 @@ class NetworkStatusDocumentV3(NetworkStatusDocument): :var dict recommended_relay_protocols: recommended protocols for relays :var dict required_client_protocols: required protocols for clients :var dict required_relay_protocols: required protocols for relays - :var dict bandwidth_file: bandwidth authority headers that generated this vote + :var dict bandwidth_file_headers: headers from the bandwidth authority that + generated this vote **\*** attribute is either required when we're parsed with validation or has a default value, others are left as None if undefined @@ -905,7 +910,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument): counterpart. .. versionchanged:: 1.7.0 - Added the bandwidth_file attributbute. + Added the bandwidth_file_headers attributbute. """ ATTRIBUTES = { @@ -936,7 +941,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument): 'shared_randomness_previous_value': (None, _parse_shared_rand_previous_value), 'shared_randomness_current_reveal_count': (None, _parse_shared_rand_current_value), 'shared_randomness_current_value': (None, _parse_shared_rand_current_value), - 'bandwidth_file': ({}, _parse_bandwidth_file), + 'bandwidth_file_headers': ({}, _parse_bandwidth_file_headers), 'signatures': ([], _parse_footer_directory_signature_line), 'bandwidth_weights': ({}, _parse_footer_bandwidth_weights_line), @@ -964,7 +969,7 @@ class NetworkStatusDocumentV3(NetworkStatusDocument): 'params': _parse_header_parameters_line, 'shared-rand-previous-value': _parse_shared_rand_previous_value, 'shared-rand-current-value': _parse_shared_rand_current_value, - 'bandwidth-file': _parse_bandwidth_file, + 'bandwidth-file-headers': _parse_bandwidth_file_headers, } FOOTER_PARSER_FOR_LINE = { diff --git a/test/unit/descriptor/networkstatus/document_v3.py b/test/unit/descriptor/networkstatus/document_v3.py index 6b11fe6c..fca7a0af 100644 --- a/test/unit/descriptor/networkstatus/document_v3.py +++ b/test/unit/descriptor/networkstatus/document_v3.py @@ -1255,14 +1255,13 @@ DnN5aFtYKiTc19qIC7Nmo+afPdDEf0MlJvEOP5EWl3w= self.assertEqual(None, authority.shared_randomness_current_reveal_count) self.assertEqual(None, authority.shared_randomness_current_value) - def test_bandwidth_file(self): + def test_bandwidth_file_headers(self): """ - Parses a 'bandwidth-file' line of votes. + Parses a 'bandwidth-file-headers' line of votes. """ test_values = { '': {}, - 'timestamp=': {'timestamp': ''}, 'timestamp=12=34': {'timestamp': '12=34'}, 'timestamp=123': {'timestamp': '123'}, 'timestamp=123 version=1.0': {'timestamp': '123', 'version': '1.0'}, @@ -1270,16 +1269,25 @@ DnN5aFtYKiTc19qIC7Nmo+afPdDEf0MlJvEOP5EWl3w= } for test_value, expected_value in test_values.items(): - document = NetworkStatusDocumentV3.create({'vote-status': 'vote', 'bandwidth-file': test_value}) - self.assertEqual(expected_value, document.bandwidth_file) + document = NetworkStatusDocumentV3.create({'vote-status': 'vote', 'bandwidth-file-headers': test_value}) + self.assertEqual(expected_value, document.bandwidth_file_headers) - # there really isn't much that *isn't* valid :P + def test_bandwidth_file_headers_malformed(self): + """ + Parses 'bandwidth-file-headers' with invalid content. + """ - content = NetworkStatusDocumentV3.content({'vote-status': 'vote', 'bandwidth-file': 'key_without_value'}) - self.assertRaises(ValueError, NetworkStatusDocumentV3, content, True) + test_values = ( + 'timestamp=', + 'key_without_value', + ) - document = NetworkStatusDocumentV3(content, False) - self.assertEqual(DEFAULT_PARAMS, document.params) + for attr in test_values: + content = NetworkStatusDocumentV3.content({'vote-status': 'vote', 'bandwidth-file-headers': attr}) + self.assertRaises(ValueError, NetworkStatusDocumentV3, content, True) + + document = NetworkStatusDocumentV3(content, False) + self.assertEqual({}, document.bandwidth_file_headers) def test_with_legacy_directory_authorities(self): """

1 0