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

[tor/master] Merge branch 'maint-0.3.3' into maint-0.3.4
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit 82b128277203b86f35739a424787bbb268a6cdeb Merge: 3ddfd5ff2 7c26f88fd Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Sep 14 09:25:34 2018 -0400 Merge branch 'maint-0.3.3' into maint-0.3.4 changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.3] rust/protover: validate unknown protocol names use only allowed characters
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit 7c26f88fd7bde6844f36f4810675688542c313bf Author: cypherpunks <cypherpunks(a)torproject.org> Date: Thu Sep 13 16:33:58 2018 +0000 rust/protover: validate unknown protocol names use only allowed characters --- changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/changes/bug27687 b/changes/bug27687 new file mode 100644 index 000000000..8b7903b63 --- /dev/null +++ b/changes/bug27687 @@ -0,0 +1,4 @@ + o Minor bugfixes (rust): + - protover parsed and accepted unknown protocol names containing invalid + characters outside the range [A-Za-z0-9-]. Fixes bug 27687; bugfix on + 0.3.3.1-alpha. diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs index 56473d12e..d9dc73381 100644 --- a/src/rust/protover/errors.rs +++ b/src/rust/protover/errors.rs @@ -18,6 +18,7 @@ pub enum ProtoverError { ExceedsExpansionLimit, UnknownProtocol, ExceedsNameLimit, + InvalidProtocol, } /// Descriptive error messages for `ProtoverError` variants. @@ -38,6 +39,8 @@ impl Display for ProtoverError { => write!(f, "A protocol in the protover string we attempted to parse is unknown."), ProtoverError::ExceedsNameLimit => write!(f, "An unrecognised protocol name was too long."), + ProtoverError::InvalidProtocol + => write!(f, "A protocol name includes invalid characters."), } } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 12ede53e5..b3563b063 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -113,11 +113,17 @@ impl fmt::Display for UnknownProtocol { } } +fn is_valid_proto(s: &str) -> bool { + s.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') +} + impl FromStr for UnknownProtocol { type Err = ProtoverError; fn from_str(s: &str) -> Result<Self, Self::Err> { - if s.len() <= MAX_PROTOCOL_NAME_LENGTH { + if !is_valid_proto(s) { + Err(ProtoverError::InvalidProtocol) + } else if s.len() <= MAX_PROTOCOL_NAME_LENGTH { Ok(UnknownProtocol(s.to_string())) } else { Err(ProtoverError::ExceedsNameLimit) @@ -129,6 +135,9 @@ impl UnknownProtocol { /// Create an `UnknownProtocol`, ignoring whether or not it /// exceeds MAX_PROTOCOL_NAME_LENGTH. fn from_str_any_len(s: &str) -> Result<Self, ProtoverError> { + if !is_valid_proto(s) { + return Err(ProtoverError::InvalidProtocol); + } Ok(UnknownProtocol(s.to_string())) } } @@ -777,6 +786,29 @@ mod test { use super::*; + macro_rules! parse_proto { + ($e:expr) => {{ + let proto: Result<UnknownProtocol, _> = $e.parse(); + let proto2 = UnknownProtocol::from_str_any_len($e); + assert_eq!(proto, proto2); + proto + }}; + } + + #[test] + fn test_protocol_from_str() { + assert!(parse_proto!("Cons").is_ok()); + assert!(parse_proto!("123").is_ok()); + assert!(parse_proto!("1-2-3").is_ok()); + + let err = Err(ProtoverError::InvalidProtocol); + assert_eq!(err, parse_proto!("a_b_c")); + assert_eq!(err, parse_proto!("a b")); + assert_eq!(err, parse_proto!("a,")); + assert_eq!(err, parse_proto!("b.")); + assert_eq!(err, parse_proto!("é")); + } + macro_rules! assert_protoentry_is_parseable { ($e:expr) => ( let protoentry: Result<ProtoEntry, ProtoverError> = $e.parse();

1 0

[tor/release-0.3.3] Merge branch 'maint-0.3.3' into release-0.3.3
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit a2823f106bde9c4667bc1ca8946e74c1cd81b5c6 Merge: 7597340b3 7c26f88fd Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Sep 14 09:25:34 2018 -0400 Merge branch 'maint-0.3.3' into release-0.3.3 changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.4] rust/protover: validate unknown protocol names use only allowed characters
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit 7c26f88fd7bde6844f36f4810675688542c313bf Author: cypherpunks <cypherpunks(a)torproject.org> Date: Thu Sep 13 16:33:58 2018 +0000 rust/protover: validate unknown protocol names use only allowed characters --- changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/changes/bug27687 b/changes/bug27687 new file mode 100644 index 000000000..8b7903b63 --- /dev/null +++ b/changes/bug27687 @@ -0,0 +1,4 @@ + o Minor bugfixes (rust): + - protover parsed and accepted unknown protocol names containing invalid + characters outside the range [A-Za-z0-9-]. Fixes bug 27687; bugfix on + 0.3.3.1-alpha. diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs index 56473d12e..d9dc73381 100644 --- a/src/rust/protover/errors.rs +++ b/src/rust/protover/errors.rs @@ -18,6 +18,7 @@ pub enum ProtoverError { ExceedsExpansionLimit, UnknownProtocol, ExceedsNameLimit, + InvalidProtocol, } /// Descriptive error messages for `ProtoverError` variants. @@ -38,6 +39,8 @@ impl Display for ProtoverError { => write!(f, "A protocol in the protover string we attempted to parse is unknown."), ProtoverError::ExceedsNameLimit => write!(f, "An unrecognised protocol name was too long."), + ProtoverError::InvalidProtocol + => write!(f, "A protocol name includes invalid characters."), } } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 12ede53e5..b3563b063 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -113,11 +113,17 @@ impl fmt::Display for UnknownProtocol { } } +fn is_valid_proto(s: &str) -> bool { + s.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') +} + impl FromStr for UnknownProtocol { type Err = ProtoverError; fn from_str(s: &str) -> Result<Self, Self::Err> { - if s.len() <= MAX_PROTOCOL_NAME_LENGTH { + if !is_valid_proto(s) { + Err(ProtoverError::InvalidProtocol) + } else if s.len() <= MAX_PROTOCOL_NAME_LENGTH { Ok(UnknownProtocol(s.to_string())) } else { Err(ProtoverError::ExceedsNameLimit) @@ -129,6 +135,9 @@ impl UnknownProtocol { /// Create an `UnknownProtocol`, ignoring whether or not it /// exceeds MAX_PROTOCOL_NAME_LENGTH. fn from_str_any_len(s: &str) -> Result<Self, ProtoverError> { + if !is_valid_proto(s) { + return Err(ProtoverError::InvalidProtocol); + } Ok(UnknownProtocol(s.to_string())) } } @@ -777,6 +786,29 @@ mod test { use super::*; + macro_rules! parse_proto { + ($e:expr) => {{ + let proto: Result<UnknownProtocol, _> = $e.parse(); + let proto2 = UnknownProtocol::from_str_any_len($e); + assert_eq!(proto, proto2); + proto + }}; + } + + #[test] + fn test_protocol_from_str() { + assert!(parse_proto!("Cons").is_ok()); + assert!(parse_proto!("123").is_ok()); + assert!(parse_proto!("1-2-3").is_ok()); + + let err = Err(ProtoverError::InvalidProtocol); + assert_eq!(err, parse_proto!("a_b_c")); + assert_eq!(err, parse_proto!("a b")); + assert_eq!(err, parse_proto!("a,")); + assert_eq!(err, parse_proto!("b.")); + assert_eq!(err, parse_proto!("é")); + } + macro_rules! assert_protoentry_is_parseable { ($e:expr) => ( let protoentry: Result<ProtoEntry, ProtoverError> = $e.parse();

1 0

[tor/release-0.3.4] Merge branch 'maint-0.3.3' into maint-0.3.4
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit 82b128277203b86f35739a424787bbb268a6cdeb Merge: 3ddfd5ff2 7c26f88fd Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Sep 14 09:25:34 2018 -0400 Merge branch 'maint-0.3.3' into maint-0.3.4 changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.4] Merge branch 'maint-0.3.4' into release-0.3.4
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit bd4c50f36b366f61f18769c17e17d77d75891465 Merge: 5e94c9384 82b128277 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Sep 14 09:25:34 2018 -0400 Merge branch 'maint-0.3.4' into release-0.3.4 changes/bug27687 | 4 ++++ src/rust/protover/errors.rs | 3 +++ src/rust/protover/protover.rs | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-)

1 0

[tor/master] Ticket #27678: Emit CIRC_BW events immediately for dropped cells.
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit efa20756701b3434fce29b41f7210bc33423d764 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu Sep 13 01:23:17 2018 +0000 Ticket #27678: Emit CIRC_BW events immediately for dropped cells. We determine that a cell was dropped by inspecting CIRC_BW fields. If we did not update the delivered or overhead fields after processing the cell, the cell was dropped/not processed. Also emit CIRC_BW events for cases where we decide to close the circuit in this function, so vanguards can print messages about dropped cells in those cases, too. --- src/core/or/command.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/core/or/command.c b/src/core/or/command.c index 461e49b62..ebddc4a35 100644 --- a/src/core/or/command.c +++ b/src/core/or/command.c @@ -480,6 +480,8 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) const or_options_t *options = get_options(); circuit_t *circ; int reason, direction; + uint32_t orig_delivered_bw = 0; + uint32_t orig_overhead_bw = 0; circ = circuit_get_by_circid_channel(cell->circ_id, chan); @@ -512,6 +514,15 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) /* Count the payload bytes only. We don't care about cell headers */ ocirc->n_read_circ_bw = tor_add_u32_nowrap(ocirc->n_read_circ_bw, CELL_PAYLOAD_SIZE); + + /* Stash the original delivered and overhead values. These values are + * updated by circuit_read_valid_data() during cell processing by + * connection_edge_process_relay_cell(), called from + * circuit_receive_relay_cell() below. If they do not change, we inform + * the control port about dropped cells immediately after the call + * to circuit_receive_relay_cell() below. */ + orig_delivered_bw = ocirc->n_delivered_read_circ_bw; + orig_overhead_bw = ocirc->n_overhead_read_circ_bw; } if (!CIRCUIT_IS_ORIGIN(circ) && @@ -535,6 +546,8 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) (unsigned)cell->circ_id); if (CIRCUIT_IS_ORIGIN(circ)) { circuit_log_path(LOG_WARN, LD_OR, TO_ORIGIN_CIRCUIT(circ)); + /* Always emit a bandwidth event for closed circs */ + control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ)); } else if (circ->n_chan) { log_warn(LD_OR, " upstream=%s", channel_get_actual_remote_descr(circ->n_chan)); @@ -560,9 +573,33 @@ command_process_relay_cell(cell_t *cell, channel_t *chan) log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,"circuit_receive_relay_cell " "(%s) failed. Closing.", direction==CELL_DIRECTION_OUT?"forward":"backward"); + /* Always emit a bandwidth event for closed circs */ + if (CIRCUIT_IS_ORIGIN(circ)) { + control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ)); + } circuit_mark_for_close(circ, -reason); } + if (CIRCUIT_IS_ORIGIN(circ)) { + origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); + + /* If neither the delivered nor overhead values changed, this cell + * was dropped due to being invalid by one of the error codepaths in + * connection_edge_process_relay_cell(), called by + * circuit_receive_relay_cell(). + * + * Valid cells, on the other hand, call circuit_read_valid_data() + * to update these values upon processing them. + * + * So, if the values are the same as those stored above, + * emit a control port event for CIRC_BW, so the controller can + * react quickly to invalid cells. */ + if (orig_delivered_bw == ocirc->n_delivered_read_circ_bw && + orig_overhead_bw == ocirc->n_overhead_read_circ_bw) { + control_event_circ_bandwidth_used_for_circ(ocirc); + } + } + /* If this is a cell in an RP circuit, count it as part of the hidden service stats */ if (options->HiddenServiceStatistics &&

1 0

[tor/master] Ticket #27678: Changes file.
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit e86593c40df1956741fd3a53b68e36abfa205532 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu Sep 13 01:25:57 2018 +0000 Ticket #27678: Changes file. --- changes/ticket27678 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/ticket27678 b/changes/ticket27678 new file mode 100644 index 000000000..daeb262e1 --- /dev/null +++ b/changes/ticket27678 @@ -0,0 +1,5 @@ + o Minor features (controller): + - Emit CIRC_BW events as soon as we detect that we processed an invalid + or otherwise dropped cell on a circuit. This allows vanguards and + other controllers to react more quickly to dropped cells. Closes + ticket 27678.

1 0

[tor/master] Control port call to emit a CIRC_BW event for a single circuit.
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit 80ffedd3ca78e76e706ef2ac3f469a4fa4aa7a96 Author: Mike Perry <mikeperry-git(a)torproject.org> Date: Thu Sep 13 01:22:02 2018 +0000 Control port call to emit a CIRC_BW event for a single circuit. This commit only moves code. No functionality has been changed. --- src/feature/control/control.c | 74 +++++++++++++++++++++++++++++-------------- src/feature/control/control.h | 2 +- 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 2fac3bd6d..d56fbcf8e 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -6049,41 +6049,69 @@ control_event_stream_bandwidth_used(void) int control_event_circ_bandwidth_used(void) { - origin_circuit_t *ocirc; - struct timeval now; - char tbuf[ISO_TIME_USEC_LEN+1]; if (!EVENT_IS_INTERESTING(EVENT_CIRC_BANDWIDTH_USED)) return 0; SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) { if (!CIRCUIT_IS_ORIGIN(circ)) continue; - ocirc = TO_ORIGIN_CIRCUIT(circ); - if (!ocirc->n_read_circ_bw && !ocirc->n_written_circ_bw) - continue; - tor_gettimeofday(&now); - format_iso_time_nospace_usec(tbuf, &now); - send_control_event(EVENT_CIRC_BANDWIDTH_USED, - "650 CIRC_BW ID=%d READ=%lu WRITTEN=%lu TIME=%s " - "DELIVERED_READ=%lu OVERHEAD_READ=%lu " - "DELIVERED_WRITTEN=%lu OVERHEAD_WRITTEN=%lu\r\n", - ocirc->global_identifier, - (unsigned long)ocirc->n_read_circ_bw, - (unsigned long)ocirc->n_written_circ_bw, - tbuf, - (unsigned long)ocirc->n_delivered_read_circ_bw, - (unsigned long)ocirc->n_overhead_read_circ_bw, - (unsigned long)ocirc->n_delivered_written_circ_bw, - (unsigned long)ocirc->n_overhead_written_circ_bw); - ocirc->n_written_circ_bw = ocirc->n_read_circ_bw = 0; - ocirc->n_overhead_written_circ_bw = ocirc->n_overhead_read_circ_bw = 0; - ocirc->n_delivered_written_circ_bw = ocirc->n_delivered_read_circ_bw = 0; + + control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ)); } SMARTLIST_FOREACH_END(circ); return 0; } +/** + * Emit a CIRC_BW event line for a specific circuit. + * + * This function sets the values it emits to 0, and does not emit + * an event if there is no new data to report since the last call. + * + * Therefore, it may be called at any frequency. + */ +int +control_event_circ_bandwidth_used_for_circ(origin_circuit_t *ocirc) +{ + struct timeval now; + char tbuf[ISO_TIME_USEC_LEN+1]; + + tor_assert(ocirc); + + if (!EVENT_IS_INTERESTING(EVENT_CIRC_BANDWIDTH_USED)) + return 0; + + /* n_read_circ_bw and n_written_circ_bw are always updated + * when there is any new cell on a circuit, and set to 0 after + * the event, below. + * + * Therefore, checking them is sufficient to determine if there + * is new data to report. */ + if (!ocirc->n_read_circ_bw && !ocirc->n_written_circ_bw) + return 0; + + tor_gettimeofday(&now); + format_iso_time_nospace_usec(tbuf, &now); + send_control_event(EVENT_CIRC_BANDWIDTH_USED, + "650 CIRC_BW ID=%d READ=%lu WRITTEN=%lu TIME=%s " + "DELIVERED_READ=%lu OVERHEAD_READ=%lu " + "DELIVERED_WRITTEN=%lu OVERHEAD_WRITTEN=%lu\r\n", + ocirc->global_identifier, + (unsigned long)ocirc->n_read_circ_bw, + (unsigned long)ocirc->n_written_circ_bw, + tbuf, + (unsigned long)ocirc->n_delivered_read_circ_bw, + (unsigned long)ocirc->n_overhead_read_circ_bw, + (unsigned long)ocirc->n_delivered_written_circ_bw, + (unsigned long)ocirc->n_overhead_written_circ_bw); + ocirc->n_written_circ_bw = ocirc->n_read_circ_bw = 0; + ocirc->n_overhead_written_circ_bw = ocirc->n_overhead_read_circ_bw = 0; + ocirc->n_delivered_written_circ_bw = ocirc->n_delivered_read_circ_bw = 0; + + return 0; +} + /** Print out CONN_BW event for a single OR/DIR/EXIT <b>conn</b> and reset * bandwidth counters. */ int diff --git a/src/feature/control/control.h b/src/feature/control/control.h index 3445eb0a9..cd5402d45 100644 --- a/src/feature/control/control.h +++ b/src/feature/control/control.h @@ -146,6 +146,7 @@ int control_event_bandwidth_used(uint32_t n_read, uint32_t n_written); int control_event_stream_bandwidth(edge_connection_t *edge_conn); int control_event_stream_bandwidth_used(void); int control_event_circ_bandwidth_used(void); +int control_event_circ_bandwidth_used_for_circ(origin_circuit_t *ocirc); int control_event_conn_bandwidth(connection_t *conn); int control_event_conn_bandwidth_used(void); int control_event_circuit_cell_stats(void); @@ -246,7 +247,6 @@ void control_event_hs_descriptor_content(const char *onion_address, const char *desc_id, const char *hsdir_fp, const char *content); - void control_free_all(void); #ifdef CONTROL_PRIVATE

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/329'
by nickm＠torproject.org 14 Sep '18

14 Sep '18

commit bb012d7941d3e4fd4ad31e3d64e9a820f25c34a2 Merge: b67f3b751 e86593c40 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Sep 14 09:22:23 2018 -0400 Merge remote-tracking branch 'tor-github/pr/329' changes/ticket27678 | 5 +++ src/core/or/command.c | 37 ++++++++++++++++++++++ src/feature/control/control.c | 74 +++++++++++++++++++++++++++++-------------- src/feature/control/control.h | 2 +- 4 files changed, 94 insertions(+), 24 deletions(-)

1 0