[tor-commits] [translation/tails-perl5lib] Update translations for tails-perl5lib
commit 5cf06067531cc1d442622515bdab5ea0aaf741f8 Author: Translation commit bot Date: Wed Apr 4 06:17:36 2018 + Update translations for tails-perl5lib --- he.po | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/he.po b/he.po index 823c5f42d..3c7919e22 100644 --- a/he.po +++ b/he.po @@ -3,14 +3,14 @@ # This file is distributed under the same license as the PACKAGE package. # # Translators: -# ION, 2017 +# ION, 2017-2018 msgid "" msgstr "" "Project-Id-Version: The Tor Project\n" "Report-Msgid-Bugs-To: Tails developers \n" "POT-Creation-Date: 2018-03-15 12:15+\n" -"PO-Revision-Date: 2018-03-31 13:21+\n" -"Last-Translator: carolyn \n" +"PO-Revision-Date: 2018-04-04 06:17+\n" +"Last-Translator: ION\n" "Language-Team: Hebrew (http://www.transifex.com/otf/torproject/language/he/)\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" @@ -26,10 +26,10 @@ msgstr "ש××××" msgid "" "The device Tails is running from cannot be found. Maybe you used the 'toram'" " option?" -msgstr "" +msgstr "×××ª×§× ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת 'toram'?" #: ../lib/Tails/RunningSystem.pm:192 msgid "" "The drive Tails is running from cannot be found. Maybe you used the 'toram' " "option?" -msgstr "" +msgstr "×××× × ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת 'toram'?" ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/tails-perl5lib_completed] Update translations for tails-perl5lib_completed
commit d648e68cf63e2ad8b0b5eb917626aa733274608c Author: Translation commit bot Date: Wed Apr 4 06:17:41 2018 + Update translations for tails-perl5lib_completed --- he.po | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/he.po b/he.po index 0409123a4..3c7919e22 100644 --- a/he.po +++ b/he.po @@ -3,13 +3,13 @@ # This file is distributed under the same license as the PACKAGE package. # # Translators: -# ION, 2017 +# ION, 2017-2018 msgid "" msgstr "" "Project-Id-Version: The Tor Project\n" "Report-Msgid-Bugs-To: Tails developers \n" -"POT-Creation-Date: 2017-05-20 10:59+0200\n" -"PO-Revision-Date: 2017-09-30 16:40+\n" +"POT-Creation-Date: 2018-03-15 12:15+\n" +"PO-Revision-Date: 2018-04-04 06:17+\n" "Last-Translator: ION\n" "Language-Team: Hebrew (http://www.transifex.com/otf/torproject/language/he/)\n" "MIME-Version: 1.0\n" @@ -24,12 +24,12 @@ msgstr "ש××××" #: ../lib/Tails/RunningSystem.pm:161 msgid "" -"The device Tails is running from cannot be found. Maybe you used the `toram'" +"The device Tails is running from cannot be found. Maybe you used the 'toram'" " option?" -msgstr "×××ª×§× ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת `toram'?" +msgstr "×××ª×§× ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת 'toram'?" #: ../lib/Tails/RunningSystem.pm:192 msgid "" -"The drive Tails is running from cannot be found. Maybe you used the `toram' " +"The drive Tails is running from cannot be found. Maybe you used the 'toram' " "option?" -msgstr "×××× × ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת `toram'?" +msgstr "×××× × ××× × Tails רץ ××× × ×××× ××××צ×. ×××× ×שת×שת ××פשר×ת 'toram'?" ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/tails-misc] Update translations for tails-misc
commit 13a321e5887ad1e555865f39e4de8ae5f50131e6 Author: Translation commit bot Date: Wed Apr 4 06:16:44 2018 + Update translations for tails-misc --- he.po | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/he.po b/he.po index 2004946cc..3ad852dff 100644 --- a/he.po +++ b/he.po @@ -14,7 +14,7 @@ msgstr "" "Project-Id-Version: The Tor Project\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: 2018-03-12 19:03+0100\n" -"PO-Revision-Date: 2018-03-19 18:04+\n" +"PO-Revision-Date: 2018-04-04 06:13+\n" "Last-Translator: ION\n" "Language-Team: Hebrew (http://www.transifex.com/otf/torproject/language/he/)\n" "MIME-Version: 1.0\n" @@ -79,7 +79,7 @@ msgstr "×פע×× ×××ש" #: config/chroot_local-includes/usr/share/gnome-shell/extensions/status-menu-hel...@tails.boum.org/extension.js:78 msgid "Lock screen" -msgstr "" +msgstr "× ×¢× ×ס×" #: config/chroot_local-includes/usr/share/gnome-shell/extensions/status-menu-hel...@tails.boum.org/extension.js:81 msgid "Power Off" @@ -202,7 +202,7 @@ msgstr "××××£ MAC × ××©× ×¢××ר ×ר××ס ×רשת ${nic_name} (${nic}). #: config/chroot_local-includes/usr/local/bin/tails-screen-locker:109 msgid "Lock Screen" -msgstr "" +msgstr "× ×¢× ×ס×" #: config/chroot_local-includes/usr/local/bin/tails-screen-locker:118 #: config/chroot_local-includes/usr/local/bin/tor-browser:46 ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/https_everywhere] Update translations for https_everywhere
commit 2a1bc7660968f554ad827353e063e4cecca5eb05 Author: Translation commit bot Date: Wed Apr 4 06:15:31 2018 + Update translations for https_everywhere --- he/https-everywhere.dtd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/he/https-everywhere.dtd b/he/https-everywhere.dtd index 10fed2a06..ade3f12c9 100644 --- a/he/https-everywhere.dtd +++ b/he/https-everywhere.dtd @@ -2,7 +2,7 @@ - + @@ -15,7 +15,7 @@ - + ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/https_everywhere_completed] Update translations for https_everywhere_completed
commit 747f09101bf284b82b727b3f0015d9b5e99cc024 Author: Translation commit bot Date: Wed Apr 4 06:15:39 2018 + Update translations for https_everywhere_completed --- he/https-everywhere.dtd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/he/https-everywhere.dtd b/he/https-everywhere.dtd index c9ee862fa..ade3f12c9 100644 --- a/he/https-everywhere.dtd +++ b/he/https-everywhere.dtd @@ -2,6 +2,7 @@ + @@ -14,6 +15,7 @@ + ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/tails-perl5lib_completed] Update translations for tails-perl5lib_completed
commit d9acf97794f66558063704912730366f611dc207 Author: Translation commit bot Date: Wed Apr 4 05:17:54 2018 + Update translations for tails-perl5lib_completed --- id.po | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/id.po b/id.po index 986dfd2e9..3d54e8929 100644 --- a/id.po +++ b/id.po @@ -5,6 +5,7 @@ # Translators: # Mohamad Hasan Al Banna , 2016 # moi, 2017 +# Robert Dafis , 2018 # Slamet Badwi , 2014 # zk, 2016 # zk, 2015 @@ -12,8 +13,8 @@ msgid "" msgstr "" "Project-Id-Version: The Tor Project\n" "Report-Msgid-Bugs-To: Tails developers \n" -"POT-Creation-Date: 2017-05-20 10:59+0200\n" -"PO-Revision-Date: 2018-02-20 19:04+\n" +"POT-Creation-Date: 2018-03-15 12:15+\n" +"PO-Revision-Date: 2018-04-04 04:57+\n" "Last-Translator: Robert Dafis \n" "Language-Team: Indonesian (http://www.transifex.com/otf/torproject/language/id/)\n" "MIME-Version: 1.0\n" @@ -28,12 +29,12 @@ msgstr "Kesalahan" #: ../lib/Tails/RunningSystem.pm:161 msgid "" -"The device Tails is running from cannot be found. Maybe you used the `toram'" +"The device Tails is running from cannot be found. Maybe you used the 'toram'" " option?" -msgstr "Perangkat asal Tails berjalan tidak dapat ditemukan. Mungkin Anda menggunakan opsi `toram'?" +msgstr "Alat yang sedang menjalankan Tails tidak dapat ditemukan. Mungkin Anda menggunakan opsi 'toram'?" #: ../lib/Tails/RunningSystem.pm:192 msgid "" -"The drive Tails is running from cannot be found. Maybe you used the `toram' " +"The drive Tails is running from cannot be found. Maybe you used the 'toram' " "option?" -msgstr "Perangkat Tails yang sedang berjalan tidak dapat ditemukan. Mungkin kamu menggunakan opsi 'toram'?" +msgstr "Hard drive tempat Tails dijalankan, tidak dapat ditemukan. Mungkin Anda menggunakan opsi 'toram'?" ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/tails-perl5lib] Update translations for tails-perl5lib
commit dedd4e18351503d9e5cbb10af423d20ff9d6d5dc Author: Translation commit bot Date: Wed Apr 4 05:17:49 2018 + Update translations for tails-perl5lib --- id.po | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/id.po b/id.po index 995fcee7f..3d54e8929 100644 --- a/id.po +++ b/id.po @@ -5,6 +5,7 @@ # Translators: # Mohamad Hasan Al Banna , 2016 # moi, 2017 +# Robert Dafis , 2018 # Slamet Badwi , 2014 # zk, 2016 # zk, 2015 @@ -13,8 +14,8 @@ msgstr "" "Project-Id-Version: The Tor Project\n" "Report-Msgid-Bugs-To: Tails developers \n" "POT-Creation-Date: 2018-03-15 12:15+\n" -"PO-Revision-Date: 2018-03-31 13:21+\n" -"Last-Translator: carolyn \n" +"PO-Revision-Date: 2018-04-04 04:57+\n" +"Last-Translator: Robert Dafis \n" "Language-Team: Indonesian (http://www.transifex.com/otf/torproject/language/id/)\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" @@ -30,10 +31,10 @@ msgstr "Kesalahan" msgid "" "The device Tails is running from cannot be found. Maybe you used the 'toram'" " option?" -msgstr "" +msgstr "Alat yang sedang menjalankan Tails tidak dapat ditemukan. Mungkin Anda menggunakan opsi 'toram'?" #: ../lib/Tails/RunningSystem.pm:192 msgid "" "The drive Tails is running from cannot be found. Maybe you used the 'toram' " "option?" -msgstr "" +msgstr "Hard drive tempat Tails dijalankan, tidak dapat ditemukan. Mungkin Anda menggunakan opsi 'toram'?" ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/https_everywhere] Update translations for https_everywhere
commit d0b9652f2e33ad38c7da24e3d2bd8ce51e7235d5 Author: Translation commit bot Date: Wed Apr 4 05:15:35 2018 + Update translations for https_everywhere --- id/https-everywhere.dtd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/id/https-everywhere.dtd b/id/https-everywhere.dtd index c5ebe1f1f..ceb93ed9a 100644 --- a/id/https-everywhere.dtd +++ b/id/https-everywhere.dtd @@ -2,7 +2,7 @@ - + @@ -15,7 +15,7 @@ - + ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/https_everywhere_completed] Update translations for https_everywhere_completed
commit d860f12989244a64a05e4460b86e9faf22a14029 Author: Translation commit bot Date: Wed Apr 4 05:15:46 2018 + Update translations for https_everywhere_completed --- id/https-everywhere.dtd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/id/https-everywhere.dtd b/id/https-everywhere.dtd index 80b49657c..ceb93ed9a 100644 --- a/id/https-everywhere.dtd +++ b/id/https-everywhere.dtd @@ -2,6 +2,7 @@ + @@ -14,6 +15,7 @@ + ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [translation/https_everywhere_completed] Update translations for https_everywhere_completed
commit a21bdd102b807c685243ed378d25d6b9d9aa1a2a Author: Translation commit bot Date: Wed Apr 4 02:16:54 2018 + Update translations for https_everywhere_completed --- templates/https-everywhere.dtd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/templates/https-everywhere.dtd b/templates/https-everywhere.dtd index e8752886b..c16099cb1 100644 --- a/templates/https-everywhere.dtd +++ b/templates/https-everywhere.dtd @@ -2,6 +2,7 @@ + @@ -14,6 +15,7 @@ + ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [torspec/master] specify that Guard needs the V2Dir flag too
commit a442ab92e84a044ffe90cad37cd517e0f98e1bea Author: Roger Dingledine Date: Mon Apr 2 00:32:56 2018 -0400 specify that Guard needs the V2Dir flag too part of the change made by ticket 22310, where dir auths no longer vote in favor of the Guard flag for relays that don't advertise directory support. --- dir-spec.txt | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/dir-spec.txt b/dir-spec.txt index eb2c599..7657e39 100644 --- a/dir-spec.txt +++ b/dir-spec.txt @@ -2492,13 +2492,16 @@ the top 7/8ths for known active routers or at least 100KB/s. "Guard" -- A router is a possible Guard if all of the following apply: - - It is Fast. - - It is Stable. + - It is Fast, + - It is Stable, - Its Weighted Fractional Uptime is at least the median for "familiar" active routers, - It is "familiar", - Its bandwidth is at least AuthDirGuardBWGuarantee (if set, 2 MB by - default), OR its bandwidth is among the 25% fastest relays. + default), OR its bandwidth is among the 25% fastest relays, + - It qualifies for the V2Dir flag as described below (this + constraint was added in 0.3.3.x, because in 0.3.0.x clients + started avoiding guards that didn't also have the V2Dir flag). To calculate weighted fractional uptime, compute the fraction of time that the router is up in any given day, weighting so that @@ -2513,8 +2516,11 @@ "V2Dir" -- A router supports the v2 directory protocol or higher if it has an open directory port OR a tunnelled-dir-server line in its router descriptor, and it is running a version of the directory - protocol that supports the functionality clients need. (Currently, this - is every supported version of Tor.) + protocol that supports the functionality clients need. (Currently, every + supported version of Tor supports the functionality that clients need, + but some relays might set "DirCache 0" or set really low rate limiting, + making them unqualified to be a directory mirror, i.e. they will omit + the tunnelled-dir-server line from their descriptor.) "HSDir" -- A router is a v2 hidden service directory if it stores and serves v2 hidden service descriptors, has the Stable and Fast flag, and the ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [torspec/master] Merge remote-tracking branch 'teor/bug25284'
commit 486455bbed02276c0dd5b228f06b15b90b521d47 Merge: 815a710 43c2f78 Author: Nick Mathewson Date: Tue Apr 3 19:17:06 2018 -0400 Merge remote-tracking branch 'teor/bug25284' dir-spec.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [torspec/master] Tor has never supported versions in the hidden-service-dir descriptor line
commit 43c2f78f0238e2cccd9f58d630f6f441f4504feb Author: teor Date: Fri Mar 30 07:16:59 2018 +1100 Tor has never supported versions in the hidden-service-dir descriptor line Instead, we use the HSDir "proto" versions, or default to version 2. Closes 25284. --- dir-spec.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dir-spec.txt b/dir-spec.txt index bc17913..e27d89a 100644 --- a/dir-spec.txt +++ b/dir-spec.txt @@ -711,14 +711,14 @@ [Versions before 0.2.7.2-alpha did not include a SHA256 digest.] [Versions before 0.2.0.1-alpha don't recognize this field at all.] - "hidden-service-dir" *(SP VersionNum) NL + "hidden-service-dir" NL [At most once.] Present only if this router stores and serves hidden service - descriptors. If any VersionNum(s) are specified, this router - supports those descriptor versions. If none are specified, it - defaults to version 2 descriptors. + descriptors. This router supports the descriptor versions declared + in the HSDir "proto" entry. If there is no "proto" entry, this + router supports version 2 descriptors. "protocols" SP "Link" SP LINK-VERSION-LIST SP "Circuit" SP CIRCUIT-VERSION-LIST NL ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] Merge branch 'maint-0.3.3' into release-0.3.3
commit 753840a8a7c946dbf2188cb4f8b03b8ff7cfbe61 Merge: 5bdc18b2d 7ccb1c5a8 Author: Nick Mathewson Date: Tue Apr 3 19:03:38 2018 -0400 Merge branch 'maint-0.3.3' into release-0.3.3 changes/bug24031| 13 + src/or/protover.c | 77 ++- src/rust/protover/errors.rs | 43 ++ src/rust/protover/ffi.rs| 94 ++- src/rust/protover/lib.rs|4 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 1265 --- src/rust/protover/tests/protover.rs | 421 +++- src/test/test_protover.c| 44 +- 9 files changed, 1690 insertions(+), 905 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] add a missing word
commit 7ccb1c5a859873656ab074c88935865bcf4b4c38 Author: Nick Mathewson Date: Tue Apr 3 15:31:30 2018 -0400 add a missing word --- changes/bug24031 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug24031 b/changes/bug24031 index adffa46d8..2bb0e8309 100644 --- a/changes/bug24031 +++ b/changes/bug24031 @@ -10,4 +10,4 @@ in outside crates and avoid mistakenly passing an internal error string to C over the FFI boundary. Many tests were added, and some previous differences between the C and Rust implementations have been - remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. + remedied. Fixes bug 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.
commit c65088cb1943748412e1a390de655e20bdb28692 Author: Isis Lovecruft Date: Tue Mar 27 22:46:14 2018 + rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C. Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied in Rust to the maximum number of version (total, for all subprotocols). Whereas in C, it was being applied to the number of subprotocols that were allowed. This changes the Rust to match C's behaviour. --- src/rust/protover/protoset.rs | 14 +++--- src/rust/protover/protover.rs | 18 +++--- src/rust/protover/tests/protover.rs | 14 -- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs index f94e6299c..4afc50edf 100644 --- a/src/rust/protover/protoset.rs +++ b/src/rust/protover/protoset.rs @@ -8,7 +8,6 @@ use std::slice; use std::str::FromStr; use std::u32; -use protover::MAX_PROTOCOLS_TO_EXPAND; use errors::ProtoverError; /// A single version number. @@ -183,10 +182,6 @@ impl ProtoSet { last_high = high; } -if self.len() > MAX_PROTOCOLS_TO_EXPAND { -return Err(ProtoverError::ExceedsMax); -} - Ok(self) } @@ -317,9 +312,15 @@ impl FromStr for ProtoSet { /// assert!(protoset.contains(&5)); /// assert!(!protoset.contains(&10)); /// -/// // We can also equivalently call `ProtoSet::from_str` by doing: +/// // We can also equivalently call `ProtoSet::from_str` by doing (all +/// // implementations of `FromStr` can be called this way, this one isn't +/// // special): /// let protoset: ProtoSet = "4-6,12".parse()?; /// +/// // Calling it (either way) can take really large ranges (up to `u32::MAX`): +/// let protoset: ProtoSet = "1-7".parse()?; +/// let protoset: ProtoSet = "1-4294967296".parse()?; +/// /// // There are lots of ways to get an `Err` from this function. Here are /// // a few: /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("=")); @@ -327,7 +328,6 @@ impl FromStr for ProtoSet { /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4")); -/// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-7")); /// /// // Things which would get parsed into an _empty_ `ProtoSet` are, /// // however, legal, and result in an empty `ProtoSet`: diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index fc89f70d4..5e5a31cd3 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// before concluding that someone is trying to DoS us /// /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` -pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); +const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Currently supported protocols and their versions, as a byte-slice. /// @@ -166,6 +166,10 @@ impl ProtoEntry { supported.parse() } +pub fn len(&self) -> usize { +self.0.len() +} + pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { self.0.get(protocol) } @@ -220,8 +224,11 @@ impl FromStr for ProtoEntry { let proto_name: Protocol = proto.parse()?; proto_entry.insert(proto_name, versions); -} +if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND { +return Err(ProtoverError::ExceedsMax); +} +} Ok(proto_entry) } } @@ -738,8 +745,13 @@ mod test { } #[test] +fn test_protoentry_from_str_allowed_number_of_versions() { +assert_protoentry_is_parseable!("Desc=1-4294967294"); +} + +#[test] fn test_protoentry_from_str_too_many_versions() { -assert_protoentry_is_unparseable!("Desc=1-65537"); +assert_protoentry_is_unparseable!("Desc=1-4294967295"); } #[test] diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 11015f35b..2db01a163 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { } #[test] -#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] -// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an -// UnvalidatedProtoEntry is defined as a Hashmap. -// Because it contains the ProtoSet part, there's still *some* validation -// happening, so in this case the DoS protections in the Rust code are kicking -// in because the range here is exceeds the m
[tor-commits] [tor/release-0.3.3] rust: Port all C protover_all_supported tests to Rust.
commit 4b4e36a413bb5d0e2ea499dd6bc34b3d24bd3375 Author: Isis Lovecruft Date: Tue Mar 27 21:38:29 2018 + rust: Port all C protover_all_supported tests to Rust. The behaviours still do not match, unsurprisingly, but now we know where a primary difference is: the Rust is validating version ranges more than the C, so in the C it's possible to call protover_all_supported on a ridiculous version range like "Sleen=0-4294967294" because the C uses MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust uses it to count the total number of *versions* of all subprotocols. --- src/rust/protover/tests/protover.rs | 86 +++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 9f8b5a443..11015f35b 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -6,6 +6,7 @@ extern crate protover; use protover::ProtoEntry; use protover::ProtoverVote; use protover::UnvalidatedProtoEntry; +use protover::errors::ProtoverError; #[test] fn parse_protocol_with_single_proto_and_single_version() { @@ -320,9 +321,88 @@ fn protover_compute_vote_may_exceed_limit() { } #[test] -fn protover_all_supported_should_include_version_we_actually_do_support() { +fn protover_all_supported_should_exclude_versions_we_actually_do_support() { let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap(); -let _result: String = proto.all_supported().unwrap().to_string(); +let result: String = proto.all_supported().unwrap().to_string(); -assert_eq!(_result, "Link=3-999".to_string()); +assert_eq!(result, "Link=6-999".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() { +let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=345-666".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex2() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer() { +let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-2147483648".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer_max_versions() { +let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-4294967294".to_string()); +} + +#[test] +// C_RUST_DIFFERS: The C will return true (e.g. saying "yes, that's supported") +// but set the msg to NULL (??? seems maybe potentially bad). The Rust will +// simply return a None. +fn protover_all_supported_should_return_empty_string_for_weird_thing() { +let proto: UnvalidatedProtoEntry = "Fribble=".parse().unwrap(); +let result: Option = proto.all_supported(); + +assert!(result.is_none()); +} + +#[test] +fn protover_unvalidatedprotoentry_should_err_entirely_unparseable_things() { +let proto: Result = "Fribble".parse(); + +assert_eq!(Err(ProtoverError::Unparseable), proto); +} + +#[test] +fn protover_all_supported_over_maximum_limit() {
[tor-commits] [tor/release-0.3.3] rust: Refactor Rust implementation of protover_is_supported_here().
commit fd127bfbfa13d407e5fb5d22a567f51a30af4c2e Author: Isis Lovecruft Date: Wed Mar 21 03:08:35 2018 + rust: Refactor Rust implementation of protover_is_supported_here(). It was changed to take borrows instead of taking ownership. * REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method signature on `protover::is_supported_here()`. --- src/rust/protover/ffi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 13d64821f..3632f5de8 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -218,7 +218,7 @@ pub extern "C" fn protover_is_supported_here( Err(_) => return 0, }; -let is_supported = is_supported_here(protocol, version); +let is_supported = is_supported_here(&protocol, &version); return if is_supported { 1 } else { 0 }; } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Refactor Rust impl of protover_list_supports_protocol().
commit 63eeda89ea11bf719ec6fddc7619994cc7f654ca Author: Isis Lovecruft Date: Wed Mar 21 02:52:04 2018 + rust: Refactor Rust impl of protover_list_supports_protocol(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types and methods. --- src/rust/protover/ffi.rs | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index c17696803..d9365bdd7 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -101,16 +101,18 @@ pub extern "C" fn protocol_list_supports_protocol( Ok(n) => n, Err(_) => return 1, }; - -let protocol = match translate_to_rust(c_protocol) { -Ok(n) => n, +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, Err(_) => return 0, }; - -let is_supported = -protover_string_supports_protocol(protocol_list, protocol, version); - -return if is_supported { 1 } else { 0 }; +let protocol: UnknownProtocol = match translate_to_rust(c_protocol) { +Ok(n) => n.into(), +Err(_) => return 0, +}; +match proto_entry.supports_protocol(&protocol, &version) { +false => return 0, +true => return 1, +} } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Implement error types for Rust protover implementation.
commit 88204f91df3e01b69e79b89ba029b42a4025d11f Author: Isis Lovecruft Date: Wed Mar 21 00:24:46 2018 + rust: Implement error types for Rust protover implementation. This will allow us to do actual error handling intra-crate in a more rusty manner, e.g. propogating errors in match statements, conversion between error types, logging messages, etc. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/errors.rs | 43 +++ src/rust/protover/lib.rs | 3 +++ src/rust/protover/protover.rs | 6 -- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs new file mode 100644 index 0..56473d12e --- /dev/null +++ b/src/rust/protover/errors.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Various errors which may occur during protocol version parsing. + +use std::fmt; +use std::fmt::Display; + +/// All errors which may occur during protover parsing routines. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] // See Display impl for error descriptions +pub enum ProtoverError { +Overlap, +LowGreaterThanHigh, +Unparseable, +ExceedsMax, +ExceedsExpansionLimit, +UnknownProtocol, +ExceedsNameLimit, +} + +/// Descriptive error messages for `ProtoverError` variants. +impl Display for ProtoverError { +fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +match *self { +ProtoverError::Overlap +=> write!(f, "Two or more (low, high) protover ranges would overlap once expanded."), +ProtoverError::LowGreaterThanHigh +=> write!(f, "The low in a (low, high) protover range was greater than high."), +ProtoverError::Unparseable +=> write!(f, "The protover string was unparseable."), +ProtoverError::ExceedsMax +=> write!(f, "The high in a (low, high) protover range exceeds u32::MAX."), +ProtoverError::ExceedsExpansionLimit +=> write!(f, "The protover string would exceed the maximum expansion limit."), +ProtoverError::UnknownProtocol +=> 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."), +} +} +} diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index fe8c0f9bb..371d1ae58 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -22,12 +22,15 @@ //! protocols to develop independently, without having to claim compatibility //! with specific versions of Tor. +#[deny(missing_docs)] + extern crate libc; extern crate smartlist; extern crate external; extern crate tor_allocate; extern crate tor_util; +pub mod errors; mod protover; pub mod ffi; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index e5dc69b9a..8ce182cf1 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -13,6 +13,8 @@ use std::u32; use tor_util::strings::NUL_BYTE; +use errors::ProtoverError; + /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// @@ -78,7 +80,7 @@ impl fmt::Display for Proto { /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` impl FromStr for Proto { -type Err = &'static str; +type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { @@ -92,7 +94,7 @@ impl FromStr for Proto { "LinkAuth" => Ok(Proto::LinkAuth), "Microdesc" => Ok(Proto::Microdesc), "Relay" => Ok(Proto::Relay), -_ => Err("Not a valid protocol type"), +_ => Err(ProtoverError::UnknownProtocol), } } } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] tests: Run all existing protover tests in both languages.
commit 6739a69c590a12af0f1cb2af62972f4305803670 Author: Isis Lovecruft Date: Tue Mar 27 21:36:10 2018 + tests: Run all existing protover tests in both languages. There's now no difference in these tests w.r.t. the C or Rust: both fail miserably (well, Rust fails with nice descriptive errors, and C gives you a traceback, because, well, C). --- src/test/test_protover.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index e7e17efe3..7bf1471eb 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -283,23 +283,22 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Sleen=0-4294967294"); tor_free(msg); - /* If we get an unparseable list, we say "yes, that's supported." */ -#ifndef HAVE_RUST - // let's make this section unconditional: rust should behave the - // same as C here! + /* If we get a (barely) valid (but unsupported list, we say "yes, that's + * supported." */ + tt_assert(protover_all_supported("Fribble=", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Fribble", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); tor_end_capture_bugs_(); - /* This case is forbidden. Since it came from a protover_all_supported, - * it can trigger a bug message. */ + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); - tor_free(msg); tor_end_capture_bugs_(); -#endif done: tor_end_capture_bugs_(); ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Refactor protover tests with new methods; note altered behaviours.
commit 493e565226fb6e5c985f787333bb0c89d661 Author: Isis Lovecruft Date: Wed Mar 21 02:13:40 2018 + rust: Refactor protover tests with new methods; note altered behaviours. Previously, the rust implementation of protover considered an empty string to be a valid ProtoEntry, while the C version did not (it must have a "=" character). Other differences include that unknown protocols must now be parsed as `protover::UnknownProtocol`s, and hence their entries as `protover::UnvalidatedProtoEntry`s, whereas before (nearly) all protoentries could be parsed regardless of how erroneous they might be considered by the C version. My apologies for this somewhat messy and difficult to read commit, if any part is frustrating to the reviewer, please feel free to ask me to split this into smaller changes (possibly hard to do, since so much changed), or ask me to comment on a specific line/change and clarify how/when the behaviours differ. The tests here should more closely match the behaviours exhibited by the C implementation, but I do not yet personally guarantee they match precisely. * REFACTOR unittests in protover::protover. * ADD new integration tests for previously untested behaviour. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/protover.rs | 214 +- src/rust/protover/tests/protover.rs | 355 2 files changed, 285 insertions(+), 284 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 247166c23..96e9dd582 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -659,154 +659,118 @@ mod test { use super::*; +macro_rules! assert_protoentry_is_parseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_ok(), format!("{:?}", protoentry.err())); +) +} + +macro_rules! assert_protoentry_is_unparseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_err()); +) +} + #[test] -fn test_versions_from_version_string() { -use std::collections::HashSet; +fn test_protoentry_from_str_multiple_protocols_multiple_versions() { +assert_protoentry_is_parseable!("Cons=3-4 Link=1,3-5"); +} -use super::Versions; +#[test] +fn test_protoentry_from_str_empty() { +assert_protoentry_is_unparseable!(""); +} -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("a,b")); -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("1,!")); +#[test] +fn test_protoentry_from_str_single_protocol_single_version() { +assert_protoentry_is_parseable!("HSDir=1"); +} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -assert_eq!(versions, Versions::from_version_string("1").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -assert_eq!(versions, Versions::from_version_string("1,2").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(3); -assert_eq!(versions, Versions::from_version_string("1-3").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1-2,5").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(3); -versions.insert(4); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1,3-5").unwrap().0); -} +#[test] +fn test_protoentry_from_str_unknown_protocol() { +assert_protoentry_is_unparseable!("Ducks=5-7,8"); } #[test] -fn test_contains_only_supported_protocols() { -use super::contains_only_supported_protocols; - -assert_eq!(false, contains_only_supported_protocols("")); -assert_eq!(true, contains_only_supported_protocols("Cons=")); -assert_eq!(true, contains_only_supported_protocols("Cons=1")); -assert_eq!(false, contains_only_supported_protocols("Cons=0")); -assert_eq!(false, contains_only_supported_protocols("Cons=0-1")); -assert_eq!(false, contains_only_supported_protocols("Cons=5")); -assert_eq!(false, contains_only_supported_protocols("Cons=1-5")); -assert_eq!(false, contains_only_supporte
[tor-commits] [tor/release-0.3.3] protover: Change protover_all_supported() to return only unsupported.
commit ad369313f87cba286a4f3347553e7322608dbd9c Author: Isis Lovecruft Date: Tue Mar 27 16:59:49 2018 + protover: Change protover_all_supported() to return only unsupported. Previously, if "Link=1-5" was supported, and you asked protover_all_supported() (or protover::all_supported() in Rust) if it supported "Link=3-999", the C version would return "Link=3-999" and the Rust would return "Link=6-999". These both behave the same now, i.e. both return "Link=6-999". --- src/or/protover.c| 77 +++- src/test/test_protover.c | 17 ++- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/or/protover.c b/src/or/protover.c index cb168085c..6532f09c2 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -671,7 +671,9 @@ int protover_all_supported(const char *s, char **missing_out) { int all_supported = 1; - smartlist_t *missing; + smartlist_t *missing_some; + smartlist_t *missing_completely; + smartlist_t *missing_all; if (!s) { return 1; @@ -684,7 +686,8 @@ protover_all_supported(const char *s, char **missing_out) return 1; } - missing = smartlist_new(); + missing_some = smartlist_new(); + missing_completely = smartlist_new(); SMARTLIST_FOREACH_BEGIN(entries, const proto_entry_t *, ent) { protocol_type_t tp; @@ -696,26 +699,86 @@ protover_all_supported(const char *s, char **missing_out) } SMARTLIST_FOREACH_BEGIN(ent->ranges, const proto_range_t *, range) { + proto_entry_t *unsupported = tor_malloc_zero(sizeof(proto_entry_t)); + proto_range_t *versions = tor_malloc_zero(sizeof(proto_range_t)); uint32_t i; + + unsupported->name = tor_strdup(ent->name); + unsupported->ranges = smartlist_new(); + for (i = range->low; i <= range->high; ++i) { if (!protover_is_supported_here(tp, i)) { - goto unsupported; + if (versions->low == 0 && versions->high == 0) { +versions->low = i; +/* Pre-emptively add the high now, just in case we're in a single + * version range (e.g. "Link=999"). */ +versions->high = i; + } + /* If the last one to be unsupported is one less than the current + * one, we're in a continous range, so set the high field. */ + if ((versions->high && versions->high == i - 1) || + /* Similarly, if the last high wasn't set and we're currently + * one higher than the low, add current index as the highest + * known high. */ + (!versions->high && versions->low == i - 1)) { +versions->high = i; +continue; + } +} else { + /* If we hit a supported version, and we previously had a range, + * we've hit a non-continuity. Copy the previous range and add it to + * the unsupported->ranges list and zero-out the previous range for + * the next iteration. */ + if (versions->low != 0 && versions->high != 0) { +proto_range_t *versions_to_add = tor_malloc(sizeof(proto_range_t)); + +versions_to_add->low = versions->low; +versions_to_add->high = versions->high; +smartlist_add(unsupported->ranges, versions_to_add); + +versions->low = 0; +versions->high = 0; + } } } + /* Once we've run out of versions to check, see if we had any unsupported + * ones and, if so, add them to unsupported->ranges. */ + if (versions->low != 0 && versions->high != 0) { +smartlist_add(unsupported->ranges, versions); + } + /* Finally, if we had something unsupported, add it to the list of + * missing_some things and mark that there was something missing. */ + if (smartlist_len(unsupported->ranges) != 0) { +smartlist_add(missing_some, (void*) unsupported); +all_supported = 0; + } else { +proto_entry_free(unsupported); +tor_free(versions); + } } SMARTLIST_FOREACH_END(range); continue; unsupported: all_supported = 0; -smartlist_add(missing, (void*) ent); +smartlist_add(missing_completely, (void*) ent); } SMARTLIST_FOREACH_END(ent); + /* We keep the two smartlists separate so that we can free the proto_entry_t + * we created and put in missing_some, so here we add them together to build + * the string. */ + missing_all = smartlist_new(); + smartlist_add_all(missing_all, missing_some); + smartlist_add_all(missing_all, missing_completely); + if (missing_out && !all_supported) { -tor_assert(0 != smartlist_len(missing)); -*missing_out = encode_protocol_list(missing); +tor_assert(smartlist_len(missing_all) != 0); +*missing_out = encode_protocol_list(missing_all); } - smartlist_free(missing); + SMARTLIST_FOREACH(missing_some, proto_entry_t *, ent, proto_entry_free(ent)); + smar
[tor-commits] [tor/release-0.3.3] Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3
commit 8d6b1da2e6729c0cf8331e663bdfeee5d5660237 Merge: 961d2ad59 b503df277 Author: Nick Mathewson Date: Tue Apr 3 15:29:29 2018 -0400 Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3 changes/bug24031| 13 + src/or/protover.c | 77 ++- src/rust/protover/errors.rs | 43 ++ src/rust/protover/ffi.rs| 94 ++- src/rust/protover/lib.rs|4 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 1265 --- src/rust/protover/tests/protover.rs | 421 +++- src/test/test_protover.c| 44 +- 9 files changed, 1690 insertions(+), 905 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Add new ProtoverVote type and refactor functions to methods.
commit 2eb1b7f2fd05eb0302e41b55f5cb61959cc9528e Author: Isis Lovecruft Date: Wed Mar 21 01:52:41 2018 + rust: Add new ProtoverVote type and refactor functions to methods. This adds a new type for votes upon `protover::ProtoEntry`s (technically, on `protover::UnvalidatedProtoEntry`s, because the C code does not validate based upon currently known protocols when voting, in order to maintain future-compatibility), and converts several functions which would have operated on this datatype into methods for ease-of-use and readability. This also fixes a behavioural differentce to the C version of protover_compute_vote(). The C version of protover_compute_vote() calls expand_protocol_list() which checks if there would be too many subprotocols *or* expanded individual version numbers, i.e. more than MAX_PROTOCOLS_TO_EXPAND, and does this *per vote* (but only in compute_vote(), everywhere else in the C seems to only care about the number of subprotocols, not the number of individual versions). We need to match its behaviour in Rust and ensure we're not allowing more than it would to get the votes to match. * ADD new `protover::ProtoverVote` datatype. * REMOVE the `protover::compute_vote()` function and refactor it into an equivalent-in-behaviour albeit more memory-efficient voting algorithm based on the new underlying `protover::protoset::ProtoSet` datatype, as `ProtoverVote::compute()`. * REMOVE the `protover::write_vote_to_string()` function, since this functionality is now generated by the impl_to_string_for_proto_entry!() macro for both `ProtoEntry` and `UnvalidatedProtoEntry` (the latter of which is the correct type to return from a voting protocol instance, since the entity voting may not know of all protocols being voted upon or known about by other voting parties). * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix a difference in compute_vote() behaviour to C version. --- src/rust/protover/protover.rs | 195 -- 1 file changed, 93 insertions(+), 102 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 2a1a5df9e..6f1ad768e 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -281,6 +281,15 @@ impl UnvalidatedProtoEntry { self.0.is_empty() } +pub fn len(&self) -> usize { +let mut total: usize = 0; + +for (_, versions) in self.iter() { +total += versions.len(); +} +total +} + /// Determine if we support every protocol a client supports, and if not, /// determine which protocols we do not have support for. /// @@ -478,120 +487,102 @@ impl From for UnvalidatedProtoEntry { } } -/// Protocol voting implementation. +/// A mapping of protocols to a count of how many times each of their `Version`s +/// were voted for or supported. /// -/// Given a list of strings describing protocol versions, return a new -/// string encoding all of the protocols that are listed by at -/// least threshold of the inputs. -/// -/// The string is sorted according to the following conventions: -/// - Protocols names are alphabetized -/// - Protocols are in order low to high -/// - Individual and ranges are listed together. For example, -/// "3, 5-10,13" -/// - All entries are unique -/// -/// # Examples -/// ``` -/// use protover::compute_vote; +/// # Warning /// -/// let protos = vec![String::from("Link=3-4"), String::from("Link=3")]; -/// let vote = compute_vote(protos, 2); -/// assert_eq!("Link=3", vote) -/// ``` -pub fn compute_vote( -list_of_proto_strings: Vec, -threshold: i32, -) -> String { -let empty = String::from(""); - -if list_of_proto_strings.is_empty() { -return empty; -} - -// all_count is a structure to represent the count of the number of -// supported versions for a specific protocol. For example, in JSON format: -// { -// "FirstSupportedProtocol": { -// "1": "3", -// "2": "1" -// } -// } -// means that FirstSupportedProtocol has three votes which support version -// 1, and one vote that supports version 2 -let mut all_count: HashMap> = -HashMap::new(); - -// parse and collect all of the protos and their versions and collect them -for vote in list_of_proto_strings { -let this_vote: HashMap = -match parse_protocols_from_string_with_no_validation(&vote) { -Ok(result) => result, -Err(_) => continue, -}; -for (protocol, versions) in this_vote { -let supported_vers: &mut HashMap = -all_count.entry(protocol).or_insert(HashMap::new()); - -for version in versions.0 { -let counter: &mut usize = -
[tor-commits] [tor/release-0.3.3] rust: Refactor protover::compute_for_old_tor().
commit cd28b4c7f5cefd319d6ded635d25911b4323b50b Author: Isis Lovecruft Date: Tue Mar 27 02:41:25 2018 + rust: Refactor protover::compute_for_old_tor(). During code review and discussion with Chelsea Komlo, she pointed out that protover::compute_for_old_tor() was a public function whose return type was `&'static CStr`. We both agree that C-like parts of APIs should: 1. not be exposed publicly (to other Rust crates), 2. only be called in the appropriate FFI code, 3. not expose types which are meant for FFI code (e.g. `*mut char`, `CString`, `*const c_int`, etc.) to the pure-Rust code of other crates. 4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called from pure-Rust, not even from other modules in its own crate (i.e. do not call `protover::ffi::*` from anywhere in `protover::protoset::*`, etc). With that in mind, this commit makes the following changes: * CHANGE `protover::compute_for_old_tor()` to be visible only at the `pub(crate)` level. * RENAME `protover::compute_for_old_tor()` to `protover::compute_for_old_tor_cstr()` to reflect the last change. * ADD a new `protover::compute_for_old_tor()` function wrapper which is public and intended for other Rust code to use, which returns a `&str`. --- src/rust/protover/ffi.rs | 2 +- src/rust/protover/protover.rs | 58 +-- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 3632f5de8..a40353eb1 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -246,7 +246,7 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const Err(_) => return empty.as_ptr(), }; -elder_protocols = compute_for_old_tor(&version); +elder_protocols = compute_for_old_tor_cstr(&version); // If we're going to pass it to C, there cannot be any intermediate NUL // bytes. An assert is okay here, since changing the const byte slice diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 96e9dd582..fc89f70d4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -611,8 +611,8 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { supported_versions.contains(vers) } -/// Older versions of Tor cannot infer their own subprotocols -/// Used to determine which subprotocols are supported by older Tor versions. +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. /// /// # Inputs /// @@ -626,24 +626,28 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { /// "HSDir=1-1 LinkAuth=1" /// /// This function returns the protocols that are supported by the version input, -/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS. +/// only for tor versions older than `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS` +/// (but not older than 0.2.4.19). For newer tors (or older than 0.2.4.19), it +/// returns an empty string. /// -/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` -pub fn compute_for_old_tor(version: &str) -> &'static [u8] { +/// # Note +/// +/// This function is meant to be called for/within FFI code. If you'd +/// like to use this code in Rust, please see `compute_for_old_tor()`. +// +// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` +pub(crate) fn compute_for_old_tor_cstr(version: &str) -> &'static [u8] { if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) { return NUL_BYTE; } - if c_tor_version_as_new_as(version, "0.2.9.1-alpha") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.7.5") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.4.19") { return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0"; @@ -652,6 +656,44 @@ pub fn compute_for_old_tor(version: &str) -> &'static [u8] { NUL_BYTE } +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. +/// +/// # Inputs +/// +/// * `version`, a string comprised of "[0-9a-z.-]" +/// +/// # Returns +/// +/// A `Result` whose `Ok` value is an `&'static str` encoding a list of protocol +/// names and supported versions. The string takes the following format: +/// +/// "HSDir=1-1 LinkAuth=1" +/// +/// This function returns the protocols that are supported by
[tor-commits] [tor/release-0.3.3] tests: Run all existing protover tests in both languages.
commit 6739a69c590a12af0f1cb2af62972f4305803670 Author: Isis Lovecruft Date: Tue Mar 27 21:36:10 2018 + tests: Run all existing protover tests in both languages. There's now no difference in these tests w.r.t. the C or Rust: both fail miserably (well, Rust fails with nice descriptive errors, and C gives you a traceback, because, well, C). --- src/test/test_protover.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index e7e17efe3..7bf1471eb 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -283,23 +283,22 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Sleen=0-4294967294"); tor_free(msg); - /* If we get an unparseable list, we say "yes, that's supported." */ -#ifndef HAVE_RUST - // let's make this section unconditional: rust should behave the - // same as C here! + /* If we get a (barely) valid (but unsupported list, we say "yes, that's + * supported." */ + tt_assert(protover_all_supported("Fribble=", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Fribble", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); tor_end_capture_bugs_(); - /* This case is forbidden. Since it came from a protover_all_supported, - * it can trigger a bug message. */ + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); - tor_free(msg); tor_end_capture_bugs_(); -#endif done: tor_end_capture_bugs_(); ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Refactor Rust impl of protover_all_supported().
commit c7bcca0233d1d4c9805f78da5e7186be2c1bcdca Author: Isis Lovecruft Date: Wed Mar 21 02:45:41 2018 + rust: Refactor Rust impl of protover_all_supported(). This includes differences in behaviour to before, which should now more closely match the C version: - If parsing a protover `char*` from C, and the string is not parseable, this function will return 1 early, which matches the C behaviour when protocols are unparseable. Previously, we would parse it and its version numbers simultaneously, i.e. there was no fail early option, causing us to spend more time unnecessarily parsing versions. * REFACTOR `protover::ffi::protover_all_supported()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index ce2837832..c17696803 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -59,19 +59,26 @@ pub extern "C" fn protover_all_supported( Err(_) => return 1, }; -let (is_supported, unsupported) = all_supported(relay_version); +let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() { +Ok(n) => n, +Err(_) => return 1, +}; +let maybe_unsupported: Option = relay_proto_entry.all_supported(); -if unsupported.len() > 0 { -let c_unsupported = match CString::new(unsupported) { +if maybe_unsupported.is_some() { +let unsupported: UnvalidatedProtoEntry = maybe_unsupported.unwrap(); +let c_unsupported: CString = match CString::new(unsupported.to_string()) { Ok(n) => n, Err(_) => return 1, }; let ptr = c_unsupported.into_raw(); unsafe { *missing_out = ptr }; + +return 0; } -return if is_supported { 1 } else { 0 }; +1 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] tests: Make inline comments in test_protover.c more accurate.
commit f769edd148bfbb381a48217e9016902f036b9ed8 Author: Isis Lovecruft Date: Tue Mar 27 21:34:00 2018 + tests: Make inline comments in test_protover.c more accurate. The DoS potential is slightly higher in C now due to some differences to the Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs. Also, the comment about "failing at the splitting stage" in Rust wasn't true, since when we split, we ignore empty chunks (e.g. "1--1" parses into "(1,None),(None,1)" and "None" can't be parsed into an integer). Finally, the comment about "Rust seems to experience an internal error" is only true in debug mode, where u32s are bounds-checked at runtime. In release mode, code expressing the equivalent of this test will error with `Err(ProtoverError::Unparseable)` because 4294967295 is too large. --- src/test/test_protover.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 95cc5f083..e7e17efe3 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -273,7 +273,7 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001"); tor_free(msg); - /* CPU/RAM DoS loop: Rust only */ + /* We shouldn't be able to DoS ourselves parsing a large range. */ tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg)); tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); tor_free(msg); @@ -546,8 +546,6 @@ test_protover_vote_roundtrip(void *args) { "Link=1,9-8,3", NULL }, { "Faux=-0", NULL }, { "Faux=0--0", NULL }, -// "These fail at the splitting stage in Rust, but the number parsing -// stage in C." { "Faux=-1", NULL }, { "Faux=-1-3", NULL }, { "Faux=1--1", NULL }, @@ -556,9 +554,9 @@ test_protover_vote_roundtrip(void *args) /* Large range */ { "Sleen=1-501", "Sleen=1-501" }, { "Sleen=1-65537", NULL }, -/* CPU/RAM DoS Loop: Rust only. */ +/* Both C/Rust implementations should be able to handle this mild DoS. */ { "Sleen=0-2147483648", NULL }, -/* Rust seems to experience an internal error here. */ +/* Rust tests are built in debug mode, so ints are bounds-checked. */ { "Sleen=0-4294967295", NULL }, }; unsigned u; ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Add new protover::ProtoEntry type which uses new datatypes.
commit 54c964332b27104e56442128f8ce85110af89c96 Author: Isis Lovecruft Date: Wed Mar 21 01:18:02 2018 + rust: Add new protover::ProtoEntry type which uses new datatypes. This replaces the `protover::SupportedProtocols` (why would you have a type just for things which are supported?) with a new, more generic type, `protover::ProtoEntry`, which utilises the new, more memory-efficient datatype in protover::protoset. * REMOVE `get_supported_protocols()` and `SupportedProtocols::tor_supported()` (since they were never used separately) and collapse their functionality into a single `ProtoEntry::supported()` method. * REMOVE `SupportedProtocols::from_proto_entries()` and reimplement its functionality as the more rusty `impl FromStr for ProtoEntry`. * REMOVE `get_proto_and_vers()` function and refactor it into the more rusty `impl FromStr for ProtoEntry`. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 144 ++ 1 file changed, 75 insertions(+), 69 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 1ccad4af4..1f62e70f1 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -1,20 +1,19 @@ // Copyright (c) 2016-2017, The Tor Project, Inc. */ // See LICENSE for licensing information */ -use external::c_tor_version_as_new_as; - +use std::collections::HashMap; +use std::collections::hash_map; +use std::fmt; use std::str; use std::str::FromStr; -use std::fmt; -use std::collections::{HashMap, HashSet}; -use std::ops::Range; use std::string::String; -use std::u32; use tor_util::strings::NUL_BYTE; +use external::c_tor_version_as_new_as; use errors::ProtoverError; use protoset::Version; +use protoset::ProtoSet; /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. @@ -142,82 +141,89 @@ pub fn get_supported_protocols() -> &'static str { .unwrap_or("") } -pub struct SupportedProtocols(HashMap); - -impl SupportedProtocols { -pub fn from_proto_entries(protocol_strs: I) -> Result -where -I: Iterator, -S: AsRef, -{ -let mut parsed = HashMap::new(); -for subproto in protocol_strs { -let (name, version) = get_proto_and_vers(subproto.as_ref())?; -parsed.insert(name, version); -} -Ok(SupportedProtocols(parsed)) +/// A map of protocol names to the versions of them which are supported. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProtoEntry(HashMap); + +impl Default for ProtoEntry { +fn default() -> ProtoEntry { +ProtoEntry( HashMap::new() ) } +} -/// Translates a string representation of a protocol list to a -/// SupportedProtocols instance. -/// -/// # Examples -/// -/// ``` -/// use protover::SupportedProtocols; -/// -/// let supported_protocols = SupportedProtocols::from_proto_entries_string( -/// "HSDir=1-2 HSIntro=3-4" -/// ); -/// ``` -pub fn from_proto_entries_string( -proto_entries: &str, -) -> Result { -Self::from_proto_entries(proto_entries.split(" ")) +impl ProtoEntry { +/// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. +pub fn iter(&self) -> hash_map::Iter { +self.0.iter() } /// Translate the supported tor versions from a string into a -/// HashMap, which is useful when looking up a specific +/// ProtoEntry, which is useful when looking up a specific /// subprotocol. -/// -fn tor_supported() -> Result { -Self::from_proto_entries_string(get_supported_protocols()) +pub fn supported() -> Result { +let supported: &'static str = get_supported_protocols(); + +supported.parse() +} + +pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { +self.0.get(protocol) +} + +pub fn insert(&mut self, key: Protocol, value: ProtoSet) { +self.0.insert(key, value); +} + +pub fn remove(&mut self, key: &Protocol) -> Option { +self.0.remove(key) +} + +pub fn is_empty(&self) -> bool { +self.0.is_empty() } } -/// Parse the subprotocol type and its version numbers. -/// -/// # Inputs -/// -/// * A `protocol_entry` string, comprised of a keyword, an "=" sign, and one -/// or more version numbers. -/// -/// # Returns -/// -/// A `Result` whose `Ok` value is a tuple of `(Proto, HashSet)`, where the -/// first element is the subprotocol type (see `protover::Proto`) and the last -/// element is a(n unordered) set of unique version numbers which are supported. -/// Otherwise, the `Err` value of this `Result` is a description of the error -/// -fn get_proto_and_vers<'a>( -protocol_entry: &'a str, -) -> Result<(Pr
[tor-commits] [tor/master] Fix bug24031 changes file
commit 21c81348a39dd235c40656c34abb76daf88e81f3 Author: Nick Mathewson Date: Tue Apr 3 19:03:33 2018 -0400 Fix bug24031 changes file --- changes/bug24031 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug24031 b/changes/bug24031 index adffa46d8..2bb0e8309 100644 --- a/changes/bug24031 +++ b/changes/bug24031 @@ -10,4 +10,4 @@ in outside crates and avoid mistakenly passing an internal error string to C over the FFI boundary. Many tests were added, and some previous differences between the C and Rust implementations have been - remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. + remedied. Fixes bug 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Refactor Rust impl of protover_list_supports_protocol_or_later().
commit 269053a3801ebe925707db5a8e519836ad2242c9 Author: Isis Lovecruft Date: Wed Mar 21 02:59:25 2018 + rust: Refactor Rust impl of protover_list_supports_protocol_or_later(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol_or_later()` to use new types and methods. --- src/rust/protover/ffi.rs | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index d9365bdd7..e7c821116 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -141,11 +141,15 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( Err(_) => return 0, }; -let is_supported = -protover_string_supports_protocol_or_later( -protocol_list, protocol, version); +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, +Err(_) => return 1, +}; -return if is_supported { 1 } else { 0 }; +if proto_entry.supports_protocol_or_later(&protocol.into(), &version) { +return 1; +} +0 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Add new protover::UnvalidatedProtoEntry type.
commit 3c47d31e1f29a016e2f0f21ca8445430ed7aad0a Author: Isis Lovecruft Date: Wed Mar 21 01:29:36 2018 + rust: Add new protover::UnvalidatedProtoEntry type. This adds a new protover::UnvalidatedProtoEntry type, which is the UnknownProtocol variant of a ProtoEntry, and refactors several functions which should operate on this type into methods. This also fixes what was previously another difference to the C implementation: if you asked the C version of protovet_compute_vote() to compute a single vote containing "Fribble=", it would return NULL. However, the Rust version would return "Fribble=" since it didn't check if the versions were empty before constructing the string of differences. ("Fribble=" is technically a valid protover string.) This is now fixed, and the Rust version in that case will, analogous to (although safer than) C returning a NULL, return None. * REMOVE internal `contains_only_supported_protocols()` function. * REMOVE `all_supported()` function and refactor it into `UnvalidatedProtoEntry::all_supported()`. * REMOVE `parse_protocols_from_string_with_no_validation()` and refactor it into the more rusty implementation of `impl FromStr for UnvalidatedProtoEntry`. * REMOVE `protover_string_supports_protocol()` and refactor it into `UnvalidatedProtoEntry::supports_protocol()`. * REMOVE `protover_string_supports_protocol_or_later()` and refactor it into `UnvalidatedProtoEntry::supports_protocol_or_later()`. * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix another C/Rust different in compute_vote(). This fixes the unittest from the prior commit by checking if the versions are empty before adding a protocol to a vote. --- src/rust/protover/protover.rs | 388 ++ 1 file changed, 208 insertions(+), 180 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 1f62e70f1..847406ca2 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -226,207 +226,235 @@ impl FromStr for ProtoEntry { } } -/// Parses a single subprotocol entry string into subprotocol and version -/// parts, and then checks whether any of those versions are unsupported. -/// Helper for protover::all_supported -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1" -/// -/// # Returns -/// -/// Returns `true` if the protocol entry is well-formatted and only contains -/// versions that are also supported in tor. Otherwise, returns false -/// -fn contains_only_supported_protocols(proto_entry: &str) -> bool { -let (name, mut vers) = match get_proto_and_vers(proto_entry) { -Ok(n) => n, -Err(_) => return false, -}; - -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(n) => n.0, -Err(_) => return false, -}; - -let supported_versions = match currently_supported.get(&name) { -Some(n) => n, -None => return false, -}; +/// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just +/// the supported ones enumerated in `Protocols`. The protocol versions are +/// validated, however. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnvalidatedProtoEntry(HashMap); -vers.0.retain(|x| !supported_versions.0.contains(x)); -vers.0.is_empty() +impl Default for UnvalidatedProtoEntry { +fn default() -> UnvalidatedProtoEntry { +UnvalidatedProtoEntry( HashMap::new() ) +} } -/// Determine if we support every protocol a client supports, and if not, -/// determine which protocols we do not have support for. -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1 LinkAuth=1-2" -/// -/// # Returns -/// -/// Return `true` if every protocol version is one that we support. -/// Otherwise, return `false`. -/// Optionally, return parameters which the client supports but which we do not -/// -/// # Examples -/// ``` -/// use protover::all_supported; -/// -/// let (is_supported, unsupported) = all_supported("Link=1"); -/// assert_eq!(true, is_supported); -/// -/// let (is_supported, unsupported) = all_supported("Link=5-6"); -/// assert_eq!(false, is_supported); -/// assert_eq!("Link=5-6", unsupported); -/// -pub fn all_supported(protocols: &str) -> (bool, String) { -let unsupported = protocols -.split_whitespace() -.filter(|v| !contains_only_supported_protocols(v)) -.collect::>(); +impl UnvalidatedProtoEntry { +/// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. +pub fn iter(&self) -> hash_map::Iter { +self.0.iter() +} -(unsupported.is_empty(), unsupported.join(" ")) -} +pub fn get(&self, protocol: &UnknownProtocol) -> Option<&ProtoSet> { +self.0.get(
[tor-commits] [tor/master] changes: Add changes file for #24031.
commit b503df2775d22cff0b74740357121ba5195e4a12 Author: Isis Lovecruft Date: Tue Apr 3 19:19:40 2018 + changes: Add changes file for #24031. (cherry picked from commit 5a8cdec3f8617920f19e3ab7707233ad3f02424f) --- changes/bug24031 | 13 + 1 file changed, 13 insertions(+) diff --git a/changes/bug24031 b/changes/bug24031 new file mode 100644 index 0..adffa46d8 --- /dev/null +++ b/changes/bug24031 @@ -0,0 +1,13 @@ + o Major bugfixes (protover, voting): +- Revise Rust implementation of protover to use a more memory-efficient + voting algorithm and corresponding data structures, thus avoiding a + potential (but small impact) DoS attack where specially crafted protocol + strings would expand to several potential megabytes in memory. In the + process, several portions of code were revised to be methods on new, + custom types, rather than functions taking interchangeable types, thus + increasing type safety of the module. Custom error types and handling + were added as well, in order to facilitate better error dismissal/handling + in outside crates and avoid mistakenly passing an internal error string to + C over the FFI boundary. Many tests were added, and some previous + differences between the C and Rust implementations have been + remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] changes: Add changes file for #24031.
commit b503df2775d22cff0b74740357121ba5195e4a12 Author: Isis Lovecruft Date: Tue Apr 3 19:19:40 2018 + changes: Add changes file for #24031. (cherry picked from commit 5a8cdec3f8617920f19e3ab7707233ad3f02424f) --- changes/bug24031 | 13 + 1 file changed, 13 insertions(+) diff --git a/changes/bug24031 b/changes/bug24031 new file mode 100644 index 0..adffa46d8 --- /dev/null +++ b/changes/bug24031 @@ -0,0 +1,13 @@ + o Major bugfixes (protover, voting): +- Revise Rust implementation of protover to use a more memory-efficient + voting algorithm and corresponding data structures, thus avoiding a + potential (but small impact) DoS attack where specially crafted protocol + strings would expand to several potential megabytes in memory. In the + process, several portions of code were revised to be methods on new, + custom types, rather than functions taking interchangeable types, thus + increasing type safety of the module. Custom error types and handling + were added as well, in order to facilitate better error dismissal/handling + in outside crates and avoid mistakenly passing an internal error string to + C over the FFI boundary. Many tests were added, and some previous + differences between the C and Rust implementations have been + remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust implementation of protover_is_supported_here().
commit fd127bfbfa13d407e5fb5d22a567f51a30af4c2e Author: Isis Lovecruft Date: Wed Mar 21 03:08:35 2018 + rust: Refactor Rust implementation of protover_is_supported_here(). It was changed to take borrows instead of taking ownership. * REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method signature on `protover::is_supported_here()`. --- src/rust/protover/ffi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 13d64821f..3632f5de8 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -218,7 +218,7 @@ pub extern "C" fn protover_is_supported_here( Err(_) => return 0, }; -let is_supported = is_supported_here(protocol, version); +let is_supported = is_supported_here(&protocol, &version); return if is_supported { 1 } else { 0 }; } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] add a missing word
commit 7ccb1c5a859873656ab074c88935865bcf4b4c38 Author: Nick Mathewson Date: Tue Apr 3 15:31:30 2018 -0400 add a missing word --- changes/bug24031 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug24031 b/changes/bug24031 index adffa46d8..2bb0e8309 100644 --- a/changes/bug24031 +++ b/changes/bug24031 @@ -10,4 +10,4 @@ in outside crates and avoid mistakenly passing an internal error string to C over the FFI boundary. Many tests were added, and some previous differences between the C and Rust implementations have been - remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. + remedied. Fixes bug 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor protover::compute_for_old_tor().
commit cd28b4c7f5cefd319d6ded635d25911b4323b50b Author: Isis Lovecruft Date: Tue Mar 27 02:41:25 2018 + rust: Refactor protover::compute_for_old_tor(). During code review and discussion with Chelsea Komlo, she pointed out that protover::compute_for_old_tor() was a public function whose return type was `&'static CStr`. We both agree that C-like parts of APIs should: 1. not be exposed publicly (to other Rust crates), 2. only be called in the appropriate FFI code, 3. not expose types which are meant for FFI code (e.g. `*mut char`, `CString`, `*const c_int`, etc.) to the pure-Rust code of other crates. 4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called from pure-Rust, not even from other modules in its own crate (i.e. do not call `protover::ffi::*` from anywhere in `protover::protoset::*`, etc). With that in mind, this commit makes the following changes: * CHANGE `protover::compute_for_old_tor()` to be visible only at the `pub(crate)` level. * RENAME `protover::compute_for_old_tor()` to `protover::compute_for_old_tor_cstr()` to reflect the last change. * ADD a new `protover::compute_for_old_tor()` function wrapper which is public and intended for other Rust code to use, which returns a `&str`. --- src/rust/protover/ffi.rs | 2 +- src/rust/protover/protover.rs | 58 +-- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 3632f5de8..a40353eb1 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -246,7 +246,7 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const Err(_) => return empty.as_ptr(), }; -elder_protocols = compute_for_old_tor(&version); +elder_protocols = compute_for_old_tor_cstr(&version); // If we're going to pass it to C, there cannot be any intermediate NUL // bytes. An assert is okay here, since changing the const byte slice diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 96e9dd582..fc89f70d4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -611,8 +611,8 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { supported_versions.contains(vers) } -/// Older versions of Tor cannot infer their own subprotocols -/// Used to determine which subprotocols are supported by older Tor versions. +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. /// /// # Inputs /// @@ -626,24 +626,28 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { /// "HSDir=1-1 LinkAuth=1" /// /// This function returns the protocols that are supported by the version input, -/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS. +/// only for tor versions older than `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS` +/// (but not older than 0.2.4.19). For newer tors (or older than 0.2.4.19), it +/// returns an empty string. /// -/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` -pub fn compute_for_old_tor(version: &str) -> &'static [u8] { +/// # Note +/// +/// This function is meant to be called for/within FFI code. If you'd +/// like to use this code in Rust, please see `compute_for_old_tor()`. +// +// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` +pub(crate) fn compute_for_old_tor_cstr(version: &str) -> &'static [u8] { if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) { return NUL_BYTE; } - if c_tor_version_as_new_as(version, "0.2.9.1-alpha") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.7.5") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.4.19") { return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0"; @@ -652,6 +656,44 @@ pub fn compute_for_old_tor(version: &str) -> &'static [u8] { NUL_BYTE } +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. +/// +/// # Inputs +/// +/// * `version`, a string comprised of "[0-9a-z.-]" +/// +/// # Returns +/// +/// A `Result` whose `Ok` value is an `&'static str` encoding a list of protocol +/// names and supported versions. The string takes the following format: +/// +/// "HSDir=1-1 LinkAuth=1" +/// +/// This function returns the protocols that are supported by
[tor-commits] [tor/release-0.3.3] rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`.
commit fa15ea104d5b9f05192afa3a023d0e2405c3842a Author: Isis Lovecruft Date: Wed Mar 21 01:44:59 2018 + rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`. This implements conversions from either a ProtoEntry or an UnvalidatedProtoEntry into a String, for use in replacing such functions as `protover::write_vote_to_string()`. * ADD macro for implementing ToString trait for ProtoEntry and UnvalidatedProtoEntry. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 21 + 1 file changed, 21 insertions(+) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 847406ca2..2a1a5df9e 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -226,6 +226,27 @@ impl FromStr for ProtoEntry { } } +/// Generate an implementation of `ToString` for either a `ProtoEntry` or an +/// `UnvalidatedProtoEntry`. +macro_rules! impl_to_string_for_proto_entry { +($t:ty) => ( +impl ToString for $t { +fn to_string(&self) -> String { +let mut parts: Vec = Vec::new(); + +for (protocol, versions) in self.iter() { +parts.push(format!("{}={}", protocol.to_string(), versions.to_string())); +} +parts.sort_unstable(); +parts.join(" ") +} +} +) +} + +impl_to_string_for_proto_entry!(ProtoEntry); +impl_to_string_for_proto_entry!(UnvalidatedProtoEntry); + /// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just /// the supported ones enumerated in `Protocols`. The protocol versions are /// validated, however. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Refactor protover::is_supported_here().
commit 35b86a12e60a8696b512261e96dc6f1c75ecebfc Author: Isis Lovecruft Date: Wed Mar 21 02:09:04 2018 + rust: Refactor protover::is_supported_here(). This changes `protover::is_supported_here()` to be aware of new datatypes (e.g. don't call `.0` on things which are no longer tuple structs) and also changes the method signature to take borrows, making it faster, threadable, and easier to read (i.e. the caller can know from reading the function signature that the function won't mutate values passed into it). * CHANGE the `protover::is_supported_here()` function to take borrows. * REFACTOR the `protover::is_supported_here()` function to be aware of new datatypes. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 6f1ad768e..247166c23 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -590,26 +590,25 @@ impl ProtoverVote { /// /// # Examples /// ``` -/// use protover::*; +/// use protover::is_supported_here; +/// use protover::Protocol; /// -/// let is_supported = is_supported_here(Proto::Link, 10); +/// let is_supported = is_supported_here(&Protocol::Link, &10); /// assert_eq!(false, is_supported); /// -/// let is_supported = is_supported_here(Proto::Link, 1); +/// let is_supported = is_supported_here(&Protocol::Link, &1); /// assert_eq!(true, is_supported); /// ``` -pub fn is_supported_here(proto: Proto, vers: Version) -> bool { -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(result) => result.0, +pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { +let currently_supported: ProtoEntry = match ProtoEntry::supported() { +Ok(result) => result, Err(_) => return false, }; - -let supported_versions = match currently_supported.get(&proto) { +let supported_versions = match currently_supported.get(proto) { Some(n) => n, None => return false, }; - -supported_versions.0.contains(&vers) +supported_versions.contains(vers) } /// Older versions of Tor cannot infer their own subprotocols ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Implement more memory-efficient protover datatype.
commit e6625113c98c281b0a649598d7daa347c28915e9 Author: Isis Lovecruft Date: Wed Mar 21 00:43:55 2018 + rust: Implement more memory-efficient protover datatype. * ADD new protover::protoset module. * ADD new protover::protoset::ProtoSet class for holding protover versions. * REMOVE protover::Versions type implementation and its method `from_version_string()`, and instead implement this behaviour in a more rust-like manner as `impl FromStr for ProtoSet`. * MOVE the `find_range()` utility function from protover::protover to protover::protoset since it's only used internally in the implementation of ProtoSet. * REMOVE the `contract_protocol_list()` function from protover::protover and instead refactor it (reusing nearly the entire thing, with minor superficial, i.e. non-behavioural, changes) into a more rusty `impl ToString for ProtoSet`. * REMOVE the `expand_version_range()` function from protover::protover and instead refactor it into a more rusty implementation of `impl Into> for ProtoSet` using the new error types in protover::errors. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/lib.rs | 1 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 215 +- 3 files changed, 641 insertions(+), 209 deletions(-) diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 371d1ae58..483260bca 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -31,6 +31,7 @@ extern crate tor_allocate; extern crate tor_util; pub mod errors; +pub mod protoset; mod protover; pub mod ffi; diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs new file mode 100644 index 0..f94e6299c --- /dev/null +++ b/src/rust/protover/protoset.rs @@ -0,0 +1,634 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Sets for lazily storing ordered, non-overlapping ranges of integers. + +use std::slice; +use std::str::FromStr; +use std::u32; + +use protover::MAX_PROTOCOLS_TO_EXPAND; +use errors::ProtoverError; + +/// A single version number. +pub type Version = u32; + +/// A `ProtoSet` stores an ordered `Vec` of `(low, high)` pairs of ranges of +/// non-overlapping protocol versions. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// +/// use protover::errors::ProtoverError; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// +/// # fn do_test() -> Result { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,8")?; +/// +/// // We could also equivalently call: +/// let protoset: ProtoSet = "3-5,8".parse()?; +/// +/// assert!(protoset.contains(&4)); +/// assert!(!protoset.contains(&7)); +/// +/// let expanded: Vec = protoset.clone().into(); +/// +/// assert_eq!(&expanded[..], &[3, 4, 5, 8]); +/// +/// let contracted: String = protoset.clone().to_string(); +/// +/// assert_eq!(contracted, "3-5,8".to_string()); +/// # Ok(protoset) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct ProtoSet { +pub(crate) pairs: Vec<(Version, Version)>, +} + +impl Default for ProtoSet { +fn default() -> Self { +let pairs: Vec<(Version, Version)> = Vec::new(); + +ProtoSet{ pairs } +} +} + +impl<'a> ProtoSet { +/// Create a new `ProtoSet` from a slice of `(low, high)` pairs. +/// +/// # Inputs +/// +/// We do not assume the input pairs are deduplicated or ordered. +pub fn from_slice(low_high_pairs: &'a [(Version, Version)]) -> Result { +let mut pairs: Vec<(Version, Version)> = Vec::with_capacity(low_high_pairs.len()); + +for &(low, high) in low_high_pairs { +pairs.push((low, high)); +} +// Sort the pairs without reallocation and remove all duplicate pairs. +pairs.sort_unstable(); +pairs.dedup(); + +ProtoSet{ pairs }.is_ok() +} +} + +/// Expand this `ProtoSet` to a `Vec` of all its `Version`s. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// # use protover::errors::ProtoverError; +/// +/// # fn do_test() -> Result, ProtoverError> { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,21")?; +/// let versions: Vec = protoset.into(); +/// +/// assert_eq!(&versions[..], &[3, 4, 5, 21]); +/// # +/// # Ok(versions) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +/// ``` +impl Into> for ProtoSet { +fn into(self) -> Vec { +let mut versions: Vec = Vec::new(); + +for &(low, high) in self.iter() { +versions.extend(low..high + 1); +} +versions +} +}
[tor-commits] [tor/master] rust: Port all C protover_all_supported tests to Rust.
commit 4b4e36a413bb5d0e2ea499dd6bc34b3d24bd3375 Author: Isis Lovecruft Date: Tue Mar 27 21:38:29 2018 + rust: Port all C protover_all_supported tests to Rust. The behaviours still do not match, unsurprisingly, but now we know where a primary difference is: the Rust is validating version ranges more than the C, so in the C it's possible to call protover_all_supported on a ridiculous version range like "Sleen=0-4294967294" because the C uses MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust uses it to count the total number of *versions* of all subprotocols. --- src/rust/protover/tests/protover.rs | 86 +++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 9f8b5a443..11015f35b 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -6,6 +6,7 @@ extern crate protover; use protover::ProtoEntry; use protover::ProtoverVote; use protover::UnvalidatedProtoEntry; +use protover::errors::ProtoverError; #[test] fn parse_protocol_with_single_proto_and_single_version() { @@ -320,9 +321,88 @@ fn protover_compute_vote_may_exceed_limit() { } #[test] -fn protover_all_supported_should_include_version_we_actually_do_support() { +fn protover_all_supported_should_exclude_versions_we_actually_do_support() { let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap(); -let _result: String = proto.all_supported().unwrap().to_string(); +let result: String = proto.all_supported().unwrap().to_string(); -assert_eq!(_result, "Link=3-999".to_string()); +assert_eq!(result, "Link=6-999".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() { +let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=345-666".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex2() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer() { +let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-2147483648".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer_max_versions() { +let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-4294967294".to_string()); +} + +#[test] +// C_RUST_DIFFERS: The C will return true (e.g. saying "yes, that's supported") +// but set the msg to NULL (??? seems maybe potentially bad). The Rust will +// simply return a None. +fn protover_all_supported_should_return_empty_string_for_weird_thing() { +let proto: UnvalidatedProtoEntry = "Fribble=".parse().unwrap(); +let result: Option = proto.all_supported(); + +assert!(result.is_none()); +} + +#[test] +fn protover_unvalidatedprotoentry_should_err_entirely_unparseable_things() { +let proto: Result = "Fribble".parse(); + +assert_eq!(Err(ProtoverError::Unparseable), proto); +} + +#[test] +fn protover_all_supported_over_maximum_limit() {
[tor-commits] [tor/release-0.3.3] rust: Refactor Rust impl of protover_compute_vote().
commit 32638ed4a6c47a03b899cd09582888674ae3bf08 Author: Isis Lovecruft Date: Wed Mar 21 03:05:56 2018 + rust: Refactor Rust impl of protover_compute_vote(). This includes a subtle difference in behaviour to the previous Rust implementation, where, for each vote that we're computing over, if a single one fails to parse, we skip it. This now matches the current behaviour in the C implementation. * REFACTOR `protover::ffi::protover_compute_vote()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index e7c821116..13d64821f 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -175,6 +175,8 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char { /// Provide an interface for C to translate arguments and return types for /// protover::compute_vote +// +// Why is the threshold a signed integer? âisis #[no_mangle] pub extern "C" fn protover_compute_vote( list: *const Stringlist, @@ -189,10 +191,19 @@ pub extern "C" fn protover_compute_vote( // Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. let data: Vec = unsafe { (*list).get_list() }; +let hold: usize = threshold as usize; +let mut proto_entries: Vec = Vec::new(); -let vote = compute_vote(data, threshold); +for datum in data { +let entry: UnvalidatedProtoEntry = match datum.parse() { +Ok(x) => x, +Err(_) => continue, +}; +proto_entries.push(entry); +} +let vote: UnvalidatedProtoEntry = ProtoverVote::compute(&proto_entries, &hold); -allocate_and_copy_string(&vote) +allocate_and_copy_string(&vote.to_string()) } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.
commit c65088cb1943748412e1a390de655e20bdb28692 Author: Isis Lovecruft Date: Tue Mar 27 22:46:14 2018 + rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C. Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied in Rust to the maximum number of version (total, for all subprotocols). Whereas in C, it was being applied to the number of subprotocols that were allowed. This changes the Rust to match C's behaviour. --- src/rust/protover/protoset.rs | 14 +++--- src/rust/protover/protover.rs | 18 +++--- src/rust/protover/tests/protover.rs | 14 -- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs index f94e6299c..4afc50edf 100644 --- a/src/rust/protover/protoset.rs +++ b/src/rust/protover/protoset.rs @@ -8,7 +8,6 @@ use std::slice; use std::str::FromStr; use std::u32; -use protover::MAX_PROTOCOLS_TO_EXPAND; use errors::ProtoverError; /// A single version number. @@ -183,10 +182,6 @@ impl ProtoSet { last_high = high; } -if self.len() > MAX_PROTOCOLS_TO_EXPAND { -return Err(ProtoverError::ExceedsMax); -} - Ok(self) } @@ -317,9 +312,15 @@ impl FromStr for ProtoSet { /// assert!(protoset.contains(&5)); /// assert!(!protoset.contains(&10)); /// -/// // We can also equivalently call `ProtoSet::from_str` by doing: +/// // We can also equivalently call `ProtoSet::from_str` by doing (all +/// // implementations of `FromStr` can be called this way, this one isn't +/// // special): /// let protoset: ProtoSet = "4-6,12".parse()?; /// +/// // Calling it (either way) can take really large ranges (up to `u32::MAX`): +/// let protoset: ProtoSet = "1-7".parse()?; +/// let protoset: ProtoSet = "1-4294967296".parse()?; +/// /// // There are lots of ways to get an `Err` from this function. Here are /// // a few: /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("=")); @@ -327,7 +328,6 @@ impl FromStr for ProtoSet { /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4")); -/// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-7")); /// /// // Things which would get parsed into an _empty_ `ProtoSet` are, /// // however, legal, and result in an empty `ProtoSet`: diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index fc89f70d4..5e5a31cd3 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// before concluding that someone is trying to DoS us /// /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` -pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); +const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Currently supported protocols and their versions, as a byte-slice. /// @@ -166,6 +166,10 @@ impl ProtoEntry { supported.parse() } +pub fn len(&self) -> usize { +self.0.len() +} + pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { self.0.get(protocol) } @@ -220,8 +224,11 @@ impl FromStr for ProtoEntry { let proto_name: Protocol = proto.parse()?; proto_entry.insert(proto_name, versions); -} +if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND { +return Err(ProtoverError::ExceedsMax); +} +} Ok(proto_entry) } } @@ -738,8 +745,13 @@ mod test { } #[test] +fn test_protoentry_from_str_allowed_number_of_versions() { +assert_protoentry_is_parseable!("Desc=1-4294967294"); +} + +#[test] fn test_protoentry_from_str_too_many_versions() { -assert_protoentry_is_unparseable!("Desc=1-65537"); +assert_protoentry_is_unparseable!("Desc=1-4294967295"); } #[test] diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 11015f35b..2db01a163 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { } #[test] -#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] -// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an -// UnvalidatedProtoEntry is defined as a Hashmap. -// Because it contains the ProtoSet part, there's still *some* validation -// happening, so in this case the DoS protections in the Rust code are kicking -// in because the range here is exceeds the m
[tor-commits] [tor/master] changes: Add changes file for #24031.
commit 5a8cdec3f8617920f19e3ab7707233ad3f02424f Author: Isis Lovecruft Date: Tue Apr 3 19:19:40 2018 + changes: Add changes file for #24031. --- changes/bug24031 | 13 + 1 file changed, 13 insertions(+) diff --git a/changes/bug24031 b/changes/bug24031 new file mode 100644 index 0..adffa46d8 --- /dev/null +++ b/changes/bug24031 @@ -0,0 +1,13 @@ + o Major bugfixes (protover, voting): +- Revise Rust implementation of protover to use a more memory-efficient + voting algorithm and corresponding data structures, thus avoiding a + potential (but small impact) DoS attack where specially crafted protocol + strings would expand to several potential megabytes in memory. In the + process, several portions of code were revised to be methods on new, + custom types, rather than functions taking interchangeable types, thus + increasing type safety of the module. Custom error types and handling + were added as well, in order to facilitate better error dismissal/handling + in outside crates and avoid mistakenly passing an internal error string to + C over the FFI boundary. Many tests were added, and some previous + differences between the C and Rust implementations have been + remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] Merge branch 'maint-0.3.3'
commit c2c616eb1958e32995577bdaa5f2c9507fd09c22 Merge: 3df954549 7ccb1c5a8 Author: Nick Mathewson Date: Tue Apr 3 15:33:29 2018 -0400 Merge branch 'maint-0.3.3' ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/release-0.3.3] rust: Add new protover::UnknownProtocol type.
commit 60daaa68b153cdca6d48b09f1951d6ba580609e5 Author: Isis Lovecruft Date: Wed Mar 21 01:07:18 2018 + rust: Add new protover::UnknownProtocol type. * ADD new type, protover::UnknownProtocol, so that we have greater type safety and our protover functionality which works with unsanitised protocol names is more clearly demarcated. * REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new protover::UnknownProtocol type name. * ADD a utility conversion of `impl From for UnknownProtocol` so that we can easily with known protocols and unknown protocols simultaneously (e.g. doing comparisons, checking their version numbers), while not allowing UnknownProtocols to be accidentally used in functions which should only take Protocols. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/ffi.rs | 30 src/rust/protover/protover.rs | 53 +++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 2ee0286ec..ce2837832 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -9,30 +9,32 @@ use libc::{c_char, c_int, uint32_t}; use std::ffi::CStr; use std::ffi::CString; -use protover::*; use smartlist::*; use tor_allocate::allocate_and_copy_string; use tor_util::strings::byte_slice_is_c_like; use tor_util::strings::empty_static_cstr; +use errors::ProtoverError; +use protover::*; + /// Translate C enums to Rust Proto enums, using the integer value of the C -/// enum to map to its associated Rust enum +/// enum to map to its associated Rust enum. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -fn translate_to_rust(c_proto: uint32_t) -> Result { +fn translate_to_rust(c_proto: uint32_t) -> Result { match c_proto { -0 => Ok(Proto::Link), -1 => Ok(Proto::LinkAuth), -2 => Ok(Proto::Relay), -3 => Ok(Proto::DirCache), -4 => Ok(Proto::HSDir), -5 => Ok(Proto::HSIntro), -6 => Ok(Proto::HSRend), -7 => Ok(Proto::Desc), -8 => Ok(Proto::Microdesc), -9 => Ok(Proto::Cons), -_ => Err("Invalid protocol type"), +0 => Ok(Protocol::Link), +1 => Ok(Protocol::LinkAuth), +2 => Ok(Protocol::Relay), +3 => Ok(Protocol::DirCache), +4 => Ok(Protocol::HSDir), +5 => Ok(Protocol::HSIntro), +6 => Ok(Protocol::HSRend), +7 => Ok(Protocol::Desc), +8 => Ok(Protocol::Microdesc), +9 => Ok(Protocol::Cons), +_ => Err(ProtoverError::UnknownProtocol), } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index d74ca00c1..1ccad4af4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -56,8 +56,8 @@ pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] = /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -#[derive(Hash, Eq, PartialEq, Debug)] -pub enum Proto { +#[derive(Clone, Hash, Eq, PartialEq, Debug)] +pub enum Protocol { Cons, Desc, DirCache, @@ -70,7 +70,7 @@ pub enum Proto { Relay, } -impl fmt::Display for Proto { +impl fmt::Display for Protocol { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self) } @@ -80,26 +80,51 @@ impl fmt::Display for Proto { /// Error if the string is an unrecognized protocol name. /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` -impl FromStr for Proto { +impl FromStr for Protocol { type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { -"Cons" => Ok(Proto::Cons), -"Desc" => Ok(Proto::Desc), -"DirCache" => Ok(Proto::DirCache), -"HSDir" => Ok(Proto::HSDir), -"HSIntro" => Ok(Proto::HSIntro), -"HSRend" => Ok(Proto::HSRend), -"Link" => Ok(Proto::Link), -"LinkAuth" => Ok(Proto::LinkAuth), -"Microdesc" => Ok(Proto::Microdesc), -"Relay" => Ok(Proto::Relay), +"Cons" => Ok(Protocol::Cons), +"Desc" => Ok(Protocol::Desc), +"DirCache" => Ok(Protocol::DirCache), +"HSDir" => Ok(Protocol::HSDir), +"HSIntro" => Ok(Protocol::HSIntro), +"HSRend" => Ok(Protocol::HSRend), +"Link" => Ok(Protocol::Link), +"LinkAuth" => Ok(Protocol::LinkAuth), +"Microdesc" => Ok(Protocol::Microdesc), +"Relay" => Ok(Protocol::Relay), _ => Err(ProtoverError::UnknownProtocol), } } } +/// A protocol string which is not one of the `Protocols` we currently know +/// about. +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct UnknownProtocol(String); + +impl fmt:
[tor-commits] [tor/master] tests: Make inline comments in test_protover.c more accurate.
commit f769edd148bfbb381a48217e9016902f036b9ed8 Author: Isis Lovecruft Date: Tue Mar 27 21:34:00 2018 + tests: Make inline comments in test_protover.c more accurate. The DoS potential is slightly higher in C now due to some differences to the Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs. Also, the comment about "failing at the splitting stage" in Rust wasn't true, since when we split, we ignore empty chunks (e.g. "1--1" parses into "(1,None),(None,1)" and "None" can't be parsed into an integer). Finally, the comment about "Rust seems to experience an internal error" is only true in debug mode, where u32s are bounds-checked at runtime. In release mode, code expressing the equivalent of this test will error with `Err(ProtoverError::Unparseable)` because 4294967295 is too large. --- src/test/test_protover.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 95cc5f083..e7e17efe3 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -273,7 +273,7 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001"); tor_free(msg); - /* CPU/RAM DoS loop: Rust only */ + /* We shouldn't be able to DoS ourselves parsing a large range. */ tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg)); tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); tor_free(msg); @@ -546,8 +546,6 @@ test_protover_vote_roundtrip(void *args) { "Link=1,9-8,3", NULL }, { "Faux=-0", NULL }, { "Faux=0--0", NULL }, -// "These fail at the splitting stage in Rust, but the number parsing -// stage in C." { "Faux=-1", NULL }, { "Faux=-1-3", NULL }, { "Faux=1--1", NULL }, @@ -556,9 +554,9 @@ test_protover_vote_roundtrip(void *args) /* Large range */ { "Sleen=1-501", "Sleen=1-501" }, { "Sleen=1-65537", NULL }, -/* CPU/RAM DoS Loop: Rust only. */ +/* Both C/Rust implementations should be able to handle this mild DoS. */ { "Sleen=0-2147483648", NULL }, -/* Rust seems to experience an internal error here. */ +/* Rust tests are built in debug mode, so ints are bounds-checked. */ { "Sleen=0-4294967295", NULL }, }; unsigned u; ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] Merge remote-tracking branch 'isis-github/bug24031_r5_squashed'
commit 29b3b485b9722ceead4b931af521b999d3c2aeb3 Merge: c2c616eb1 5a8cdec3f Author: Nick Mathewson Date: Tue Apr 3 15:33:50 2018 -0400 Merge remote-tracking branch 'isis-github/bug24031_r5_squashed' changes/bug24031| 13 + src/or/protover.c | 77 ++- src/rust/protover/errors.rs | 43 ++ src/rust/protover/ffi.rs| 96 ++- src/rust/protover/lib.rs|7 +- src/rust/protover/protoset.rs | 634 + src/rust/protover/protover.rs | 1289 --- src/rust/protover/tests/protover.rs | 421 +++- src/test/test_protover.c| 44 +- 9 files changed, 1684 insertions(+), 940 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Add new ProtoverVote type and refactor functions to methods.
commit 2eb1b7f2fd05eb0302e41b55f5cb61959cc9528e Author: Isis Lovecruft Date: Wed Mar 21 01:52:41 2018 + rust: Add new ProtoverVote type and refactor functions to methods. This adds a new type for votes upon `protover::ProtoEntry`s (technically, on `protover::UnvalidatedProtoEntry`s, because the C code does not validate based upon currently known protocols when voting, in order to maintain future-compatibility), and converts several functions which would have operated on this datatype into methods for ease-of-use and readability. This also fixes a behavioural differentce to the C version of protover_compute_vote(). The C version of protover_compute_vote() calls expand_protocol_list() which checks if there would be too many subprotocols *or* expanded individual version numbers, i.e. more than MAX_PROTOCOLS_TO_EXPAND, and does this *per vote* (but only in compute_vote(), everywhere else in the C seems to only care about the number of subprotocols, not the number of individual versions). We need to match its behaviour in Rust and ensure we're not allowing more than it would to get the votes to match. * ADD new `protover::ProtoverVote` datatype. * REMOVE the `protover::compute_vote()` function and refactor it into an equivalent-in-behaviour albeit more memory-efficient voting algorithm based on the new underlying `protover::protoset::ProtoSet` datatype, as `ProtoverVote::compute()`. * REMOVE the `protover::write_vote_to_string()` function, since this functionality is now generated by the impl_to_string_for_proto_entry!() macro for both `ProtoEntry` and `UnvalidatedProtoEntry` (the latter of which is the correct type to return from a voting protocol instance, since the entity voting may not know of all protocols being voted upon or known about by other voting parties). * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix a difference in compute_vote() behaviour to C version. --- src/rust/protover/protover.rs | 195 -- 1 file changed, 93 insertions(+), 102 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 2a1a5df9e..6f1ad768e 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -281,6 +281,15 @@ impl UnvalidatedProtoEntry { self.0.is_empty() } +pub fn len(&self) -> usize { +let mut total: usize = 0; + +for (_, versions) in self.iter() { +total += versions.len(); +} +total +} + /// Determine if we support every protocol a client supports, and if not, /// determine which protocols we do not have support for. /// @@ -478,120 +487,102 @@ impl From for UnvalidatedProtoEntry { } } -/// Protocol voting implementation. +/// A mapping of protocols to a count of how many times each of their `Version`s +/// were voted for or supported. /// -/// Given a list of strings describing protocol versions, return a new -/// string encoding all of the protocols that are listed by at -/// least threshold of the inputs. -/// -/// The string is sorted according to the following conventions: -/// - Protocols names are alphabetized -/// - Protocols are in order low to high -/// - Individual and ranges are listed together. For example, -/// "3, 5-10,13" -/// - All entries are unique -/// -/// # Examples -/// ``` -/// use protover::compute_vote; +/// # Warning /// -/// let protos = vec![String::from("Link=3-4"), String::from("Link=3")]; -/// let vote = compute_vote(protos, 2); -/// assert_eq!("Link=3", vote) -/// ``` -pub fn compute_vote( -list_of_proto_strings: Vec, -threshold: i32, -) -> String { -let empty = String::from(""); - -if list_of_proto_strings.is_empty() { -return empty; -} - -// all_count is a structure to represent the count of the number of -// supported versions for a specific protocol. For example, in JSON format: -// { -// "FirstSupportedProtocol": { -// "1": "3", -// "2": "1" -// } -// } -// means that FirstSupportedProtocol has three votes which support version -// 1, and one vote that supports version 2 -let mut all_count: HashMap> = -HashMap::new(); - -// parse and collect all of the protos and their versions and collect them -for vote in list_of_proto_strings { -let this_vote: HashMap = -match parse_protocols_from_string_with_no_validation(&vote) { -Ok(result) => result, -Err(_) => continue, -}; -for (protocol, versions) in this_vote { -let supported_vers: &mut HashMap = -all_count.entry(protocol).or_insert(HashMap::new()); - -for version in versions.0 { -let counter: &mut usize = -
[tor-commits] [tor/master] protover: Change protover_all_supported() to return only unsupported.
commit ad369313f87cba286a4f3347553e7322608dbd9c Author: Isis Lovecruft Date: Tue Mar 27 16:59:49 2018 + protover: Change protover_all_supported() to return only unsupported. Previously, if "Link=1-5" was supported, and you asked protover_all_supported() (or protover::all_supported() in Rust) if it supported "Link=3-999", the C version would return "Link=3-999" and the Rust would return "Link=6-999". These both behave the same now, i.e. both return "Link=6-999". --- src/or/protover.c| 77 +++- src/test/test_protover.c | 17 ++- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/or/protover.c b/src/or/protover.c index cb168085c..6532f09c2 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -671,7 +671,9 @@ int protover_all_supported(const char *s, char **missing_out) { int all_supported = 1; - smartlist_t *missing; + smartlist_t *missing_some; + smartlist_t *missing_completely; + smartlist_t *missing_all; if (!s) { return 1; @@ -684,7 +686,8 @@ protover_all_supported(const char *s, char **missing_out) return 1; } - missing = smartlist_new(); + missing_some = smartlist_new(); + missing_completely = smartlist_new(); SMARTLIST_FOREACH_BEGIN(entries, const proto_entry_t *, ent) { protocol_type_t tp; @@ -696,26 +699,86 @@ protover_all_supported(const char *s, char **missing_out) } SMARTLIST_FOREACH_BEGIN(ent->ranges, const proto_range_t *, range) { + proto_entry_t *unsupported = tor_malloc_zero(sizeof(proto_entry_t)); + proto_range_t *versions = tor_malloc_zero(sizeof(proto_range_t)); uint32_t i; + + unsupported->name = tor_strdup(ent->name); + unsupported->ranges = smartlist_new(); + for (i = range->low; i <= range->high; ++i) { if (!protover_is_supported_here(tp, i)) { - goto unsupported; + if (versions->low == 0 && versions->high == 0) { +versions->low = i; +/* Pre-emptively add the high now, just in case we're in a single + * version range (e.g. "Link=999"). */ +versions->high = i; + } + /* If the last one to be unsupported is one less than the current + * one, we're in a continous range, so set the high field. */ + if ((versions->high && versions->high == i - 1) || + /* Similarly, if the last high wasn't set and we're currently + * one higher than the low, add current index as the highest + * known high. */ + (!versions->high && versions->low == i - 1)) { +versions->high = i; +continue; + } +} else { + /* If we hit a supported version, and we previously had a range, + * we've hit a non-continuity. Copy the previous range and add it to + * the unsupported->ranges list and zero-out the previous range for + * the next iteration. */ + if (versions->low != 0 && versions->high != 0) { +proto_range_t *versions_to_add = tor_malloc(sizeof(proto_range_t)); + +versions_to_add->low = versions->low; +versions_to_add->high = versions->high; +smartlist_add(unsupported->ranges, versions_to_add); + +versions->low = 0; +versions->high = 0; + } } } + /* Once we've run out of versions to check, see if we had any unsupported + * ones and, if so, add them to unsupported->ranges. */ + if (versions->low != 0 && versions->high != 0) { +smartlist_add(unsupported->ranges, versions); + } + /* Finally, if we had something unsupported, add it to the list of + * missing_some things and mark that there was something missing. */ + if (smartlist_len(unsupported->ranges) != 0) { +smartlist_add(missing_some, (void*) unsupported); +all_supported = 0; + } else { +proto_entry_free(unsupported); +tor_free(versions); + } } SMARTLIST_FOREACH_END(range); continue; unsupported: all_supported = 0; -smartlist_add(missing, (void*) ent); +smartlist_add(missing_completely, (void*) ent); } SMARTLIST_FOREACH_END(ent); + /* We keep the two smartlists separate so that we can free the proto_entry_t + * we created and put in missing_some, so here we add them together to build + * the string. */ + missing_all = smartlist_new(); + smartlist_add_all(missing_all, missing_some); + smartlist_add_all(missing_all, missing_completely); + if (missing_out && !all_supported) { -tor_assert(0 != smartlist_len(missing)); -*missing_out = encode_protocol_list(missing); +tor_assert(smartlist_len(missing_all) != 0); +*missing_out = encode_protocol_list(missing_all); } - smartlist_free(missing); + SMARTLIST_FOREACH(missing_some, proto_entry_t *, ent, proto_entry_free(ent)); + smar
[tor-commits] [tor/master] Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3
commit 8d6b1da2e6729c0cf8331e663bdfeee5d5660237 Merge: 961d2ad59 b503df277 Author: Nick Mathewson Date: Tue Apr 3 15:29:29 2018 -0400 Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3 changes/bug24031| 13 + src/or/protover.c | 77 ++- src/rust/protover/errors.rs | 43 ++ src/rust/protover/ffi.rs| 94 ++- src/rust/protover/lib.rs|4 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 1265 --- src/rust/protover/tests/protover.rs | 421 +++- src/test/test_protover.c| 44 +- 9 files changed, 1690 insertions(+), 905 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Add new protover::UnknownProtocol type.
commit 60daaa68b153cdca6d48b09f1951d6ba580609e5 Author: Isis Lovecruft Date: Wed Mar 21 01:07:18 2018 + rust: Add new protover::UnknownProtocol type. * ADD new type, protover::UnknownProtocol, so that we have greater type safety and our protover functionality which works with unsanitised protocol names is more clearly demarcated. * REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new protover::UnknownProtocol type name. * ADD a utility conversion of `impl From for UnknownProtocol` so that we can easily with known protocols and unknown protocols simultaneously (e.g. doing comparisons, checking their version numbers), while not allowing UnknownProtocols to be accidentally used in functions which should only take Protocols. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/ffi.rs | 30 src/rust/protover/protover.rs | 53 +++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 2ee0286ec..ce2837832 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -9,30 +9,32 @@ use libc::{c_char, c_int, uint32_t}; use std::ffi::CStr; use std::ffi::CString; -use protover::*; use smartlist::*; use tor_allocate::allocate_and_copy_string; use tor_util::strings::byte_slice_is_c_like; use tor_util::strings::empty_static_cstr; +use errors::ProtoverError; +use protover::*; + /// Translate C enums to Rust Proto enums, using the integer value of the C -/// enum to map to its associated Rust enum +/// enum to map to its associated Rust enum. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -fn translate_to_rust(c_proto: uint32_t) -> Result { +fn translate_to_rust(c_proto: uint32_t) -> Result { match c_proto { -0 => Ok(Proto::Link), -1 => Ok(Proto::LinkAuth), -2 => Ok(Proto::Relay), -3 => Ok(Proto::DirCache), -4 => Ok(Proto::HSDir), -5 => Ok(Proto::HSIntro), -6 => Ok(Proto::HSRend), -7 => Ok(Proto::Desc), -8 => Ok(Proto::Microdesc), -9 => Ok(Proto::Cons), -_ => Err("Invalid protocol type"), +0 => Ok(Protocol::Link), +1 => Ok(Protocol::LinkAuth), +2 => Ok(Protocol::Relay), +3 => Ok(Protocol::DirCache), +4 => Ok(Protocol::HSDir), +5 => Ok(Protocol::HSIntro), +6 => Ok(Protocol::HSRend), +7 => Ok(Protocol::Desc), +8 => Ok(Protocol::Microdesc), +9 => Ok(Protocol::Cons), +_ => Err(ProtoverError::UnknownProtocol), } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index d74ca00c1..1ccad4af4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -56,8 +56,8 @@ pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] = /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -#[derive(Hash, Eq, PartialEq, Debug)] -pub enum Proto { +#[derive(Clone, Hash, Eq, PartialEq, Debug)] +pub enum Protocol { Cons, Desc, DirCache, @@ -70,7 +70,7 @@ pub enum Proto { Relay, } -impl fmt::Display for Proto { +impl fmt::Display for Protocol { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self) } @@ -80,26 +80,51 @@ impl fmt::Display for Proto { /// Error if the string is an unrecognized protocol name. /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` -impl FromStr for Proto { +impl FromStr for Protocol { type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { -"Cons" => Ok(Proto::Cons), -"Desc" => Ok(Proto::Desc), -"DirCache" => Ok(Proto::DirCache), -"HSDir" => Ok(Proto::HSDir), -"HSIntro" => Ok(Proto::HSIntro), -"HSRend" => Ok(Proto::HSRend), -"Link" => Ok(Proto::Link), -"LinkAuth" => Ok(Proto::LinkAuth), -"Microdesc" => Ok(Proto::Microdesc), -"Relay" => Ok(Proto::Relay), +"Cons" => Ok(Protocol::Cons), +"Desc" => Ok(Protocol::Desc), +"DirCache" => Ok(Protocol::DirCache), +"HSDir" => Ok(Protocol::HSDir), +"HSIntro" => Ok(Protocol::HSIntro), +"HSRend" => Ok(Protocol::HSRend), +"Link" => Ok(Protocol::Link), +"LinkAuth" => Ok(Protocol::LinkAuth), +"Microdesc" => Ok(Protocol::Microdesc), +"Relay" => Ok(Protocol::Relay), _ => Err(ProtoverError::UnknownProtocol), } } } +/// A protocol string which is not one of the `Protocols` we currently know +/// about. +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct UnknownProtocol(String); + +impl fmt:
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_all_supported().
commit c7bcca0233d1d4c9805f78da5e7186be2c1bcdca Author: Isis Lovecruft Date: Wed Mar 21 02:45:41 2018 + rust: Refactor Rust impl of protover_all_supported(). This includes differences in behaviour to before, which should now more closely match the C version: - If parsing a protover `char*` from C, and the string is not parseable, this function will return 1 early, which matches the C behaviour when protocols are unparseable. Previously, we would parse it and its version numbers simultaneously, i.e. there was no fail early option, causing us to spend more time unnecessarily parsing versions. * REFACTOR `protover::ffi::protover_all_supported()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index ce2837832..c17696803 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -59,19 +59,26 @@ pub extern "C" fn protover_all_supported( Err(_) => return 1, }; -let (is_supported, unsupported) = all_supported(relay_version); +let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() { +Ok(n) => n, +Err(_) => return 1, +}; +let maybe_unsupported: Option = relay_proto_entry.all_supported(); -if unsupported.len() > 0 { -let c_unsupported = match CString::new(unsupported) { +if maybe_unsupported.is_some() { +let unsupported: UnvalidatedProtoEntry = maybe_unsupported.unwrap(); +let c_unsupported: CString = match CString::new(unsupported.to_string()) { Ok(n) => n, Err(_) => return 1, }; let ptr = c_unsupported.into_raw(); unsafe { *missing_out = ptr }; + +return 0; } -return if is_supported { 1 } else { 0 }; +1 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor protover::is_supported_here().
commit 35b86a12e60a8696b512261e96dc6f1c75ecebfc Author: Isis Lovecruft Date: Wed Mar 21 02:09:04 2018 + rust: Refactor protover::is_supported_here(). This changes `protover::is_supported_here()` to be aware of new datatypes (e.g. don't call `.0` on things which are no longer tuple structs) and also changes the method signature to take borrows, making it faster, threadable, and easier to read (i.e. the caller can know from reading the function signature that the function won't mutate values passed into it). * CHANGE the `protover::is_supported_here()` function to take borrows. * REFACTOR the `protover::is_supported_here()` function to be aware of new datatypes. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 6f1ad768e..247166c23 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -590,26 +590,25 @@ impl ProtoverVote { /// /// # Examples /// ``` -/// use protover::*; +/// use protover::is_supported_here; +/// use protover::Protocol; /// -/// let is_supported = is_supported_here(Proto::Link, 10); +/// let is_supported = is_supported_here(&Protocol::Link, &10); /// assert_eq!(false, is_supported); /// -/// let is_supported = is_supported_here(Proto::Link, 1); +/// let is_supported = is_supported_here(&Protocol::Link, &1); /// assert_eq!(true, is_supported); /// ``` -pub fn is_supported_here(proto: Proto, vers: Version) -> bool { -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(result) => result.0, +pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { +let currently_supported: ProtoEntry = match ProtoEntry::supported() { +Ok(result) => result, Err(_) => return false, }; - -let supported_versions = match currently_supported.get(&proto) { +let supported_versions = match currently_supported.get(proto) { Some(n) => n, None => return false, }; - -supported_versions.0.contains(&vers) +supported_versions.contains(vers) } /// Older versions of Tor cannot infer their own subprotocols ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_list_supports_protocol().
commit 63eeda89ea11bf719ec6fddc7619994cc7f654ca Author: Isis Lovecruft Date: Wed Mar 21 02:52:04 2018 + rust: Refactor Rust impl of protover_list_supports_protocol(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types and methods. --- src/rust/protover/ffi.rs | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index c17696803..d9365bdd7 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -101,16 +101,18 @@ pub extern "C" fn protocol_list_supports_protocol( Ok(n) => n, Err(_) => return 1, }; - -let protocol = match translate_to_rust(c_protocol) { -Ok(n) => n, +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, Err(_) => return 0, }; - -let is_supported = -protover_string_supports_protocol(protocol_list, protocol, version); - -return if is_supported { 1 } else { 0 }; +let protocol: UnknownProtocol = match translate_to_rust(c_protocol) { +Ok(n) => n.into(), +Err(_) => return 0, +}; +match proto_entry.supports_protocol(&protocol, &version) { +false => return 0, +true => return 1, +} } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_compute_vote().
commit 32638ed4a6c47a03b899cd09582888674ae3bf08 Author: Isis Lovecruft Date: Wed Mar 21 03:05:56 2018 + rust: Refactor Rust impl of protover_compute_vote(). This includes a subtle difference in behaviour to the previous Rust implementation, where, for each vote that we're computing over, if a single one fails to parse, we skip it. This now matches the current behaviour in the C implementation. * REFACTOR `protover::ffi::protover_compute_vote()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index e7c821116..13d64821f 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -175,6 +175,8 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char { /// Provide an interface for C to translate arguments and return types for /// protover::compute_vote +// +// Why is the threshold a signed integer? âisis #[no_mangle] pub extern "C" fn protover_compute_vote( list: *const Stringlist, @@ -189,10 +191,19 @@ pub extern "C" fn protover_compute_vote( // Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. let data: Vec = unsafe { (*list).get_list() }; +let hold: usize = threshold as usize; +let mut proto_entries: Vec = Vec::new(); -let vote = compute_vote(data, threshold); +for datum in data { +let entry: UnvalidatedProtoEntry = match datum.parse() { +Ok(x) => x, +Err(_) => continue, +}; +proto_entries.push(entry); +} +let vote: UnvalidatedProtoEntry = ProtoverVote::compute(&proto_entries, &hold); -allocate_and_copy_string(&vote) +allocate_and_copy_string(&vote.to_string()) } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_list_supports_protocol_or_later().
commit 269053a3801ebe925707db5a8e519836ad2242c9 Author: Isis Lovecruft Date: Wed Mar 21 02:59:25 2018 + rust: Refactor Rust impl of protover_list_supports_protocol_or_later(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol_or_later()` to use new types and methods. --- src/rust/protover/ffi.rs | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index d9365bdd7..e7c821116 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -141,11 +141,15 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( Err(_) => return 0, }; -let is_supported = -protover_string_supports_protocol_or_later( -protocol_list, protocol, version); +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, +Err(_) => return 1, +}; -return if is_supported { 1 } else { 0 }; +if proto_entry.supports_protocol_or_later(&protocol.into(), &version) { +return 1; +} +0 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Add new protover::ProtoEntry type which uses new datatypes.
commit 54c964332b27104e56442128f8ce85110af89c96 Author: Isis Lovecruft Date: Wed Mar 21 01:18:02 2018 + rust: Add new protover::ProtoEntry type which uses new datatypes. This replaces the `protover::SupportedProtocols` (why would you have a type just for things which are supported?) with a new, more generic type, `protover::ProtoEntry`, which utilises the new, more memory-efficient datatype in protover::protoset. * REMOVE `get_supported_protocols()` and `SupportedProtocols::tor_supported()` (since they were never used separately) and collapse their functionality into a single `ProtoEntry::supported()` method. * REMOVE `SupportedProtocols::from_proto_entries()` and reimplement its functionality as the more rusty `impl FromStr for ProtoEntry`. * REMOVE `get_proto_and_vers()` function and refactor it into the more rusty `impl FromStr for ProtoEntry`. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 144 ++ 1 file changed, 75 insertions(+), 69 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 1ccad4af4..1f62e70f1 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -1,20 +1,19 @@ // Copyright (c) 2016-2017, The Tor Project, Inc. */ // See LICENSE for licensing information */ -use external::c_tor_version_as_new_as; - +use std::collections::HashMap; +use std::collections::hash_map; +use std::fmt; use std::str; use std::str::FromStr; -use std::fmt; -use std::collections::{HashMap, HashSet}; -use std::ops::Range; use std::string::String; -use std::u32; use tor_util::strings::NUL_BYTE; +use external::c_tor_version_as_new_as; use errors::ProtoverError; use protoset::Version; +use protoset::ProtoSet; /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. @@ -142,82 +141,89 @@ pub fn get_supported_protocols() -> &'static str { .unwrap_or("") } -pub struct SupportedProtocols(HashMap); - -impl SupportedProtocols { -pub fn from_proto_entries(protocol_strs: I) -> Result -where -I: Iterator, -S: AsRef, -{ -let mut parsed = HashMap::new(); -for subproto in protocol_strs { -let (name, version) = get_proto_and_vers(subproto.as_ref())?; -parsed.insert(name, version); -} -Ok(SupportedProtocols(parsed)) +/// A map of protocol names to the versions of them which are supported. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProtoEntry(HashMap); + +impl Default for ProtoEntry { +fn default() -> ProtoEntry { +ProtoEntry( HashMap::new() ) } +} -/// Translates a string representation of a protocol list to a -/// SupportedProtocols instance. -/// -/// # Examples -/// -/// ``` -/// use protover::SupportedProtocols; -/// -/// let supported_protocols = SupportedProtocols::from_proto_entries_string( -/// "HSDir=1-2 HSIntro=3-4" -/// ); -/// ``` -pub fn from_proto_entries_string( -proto_entries: &str, -) -> Result { -Self::from_proto_entries(proto_entries.split(" ")) +impl ProtoEntry { +/// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. +pub fn iter(&self) -> hash_map::Iter { +self.0.iter() } /// Translate the supported tor versions from a string into a -/// HashMap, which is useful when looking up a specific +/// ProtoEntry, which is useful when looking up a specific /// subprotocol. -/// -fn tor_supported() -> Result { -Self::from_proto_entries_string(get_supported_protocols()) +pub fn supported() -> Result { +let supported: &'static str = get_supported_protocols(); + +supported.parse() +} + +pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { +self.0.get(protocol) +} + +pub fn insert(&mut self, key: Protocol, value: ProtoSet) { +self.0.insert(key, value); +} + +pub fn remove(&mut self, key: &Protocol) -> Option { +self.0.remove(key) +} + +pub fn is_empty(&self) -> bool { +self.0.is_empty() } } -/// Parse the subprotocol type and its version numbers. -/// -/// # Inputs -/// -/// * A `protocol_entry` string, comprised of a keyword, an "=" sign, and one -/// or more version numbers. -/// -/// # Returns -/// -/// A `Result` whose `Ok` value is a tuple of `(Proto, HashSet)`, where the -/// first element is the subprotocol type (see `protover::Proto`) and the last -/// element is a(n unordered) set of unique version numbers which are supported. -/// Otherwise, the `Err` value of this `Result` is a description of the error -/// -fn get_proto_and_vers<'a>( -protocol_entry: &'a str, -) -> Result<(Pr
[tor-commits] [tor/master] rust: Add new protover::UnvalidatedProtoEntry type.
commit 3c47d31e1f29a016e2f0f21ca8445430ed7aad0a Author: Isis Lovecruft Date: Wed Mar 21 01:29:36 2018 + rust: Add new protover::UnvalidatedProtoEntry type. This adds a new protover::UnvalidatedProtoEntry type, which is the UnknownProtocol variant of a ProtoEntry, and refactors several functions which should operate on this type into methods. This also fixes what was previously another difference to the C implementation: if you asked the C version of protovet_compute_vote() to compute a single vote containing "Fribble=", it would return NULL. However, the Rust version would return "Fribble=" since it didn't check if the versions were empty before constructing the string of differences. ("Fribble=" is technically a valid protover string.) This is now fixed, and the Rust version in that case will, analogous to (although safer than) C returning a NULL, return None. * REMOVE internal `contains_only_supported_protocols()` function. * REMOVE `all_supported()` function and refactor it into `UnvalidatedProtoEntry::all_supported()`. * REMOVE `parse_protocols_from_string_with_no_validation()` and refactor it into the more rusty implementation of `impl FromStr for UnvalidatedProtoEntry`. * REMOVE `protover_string_supports_protocol()` and refactor it into `UnvalidatedProtoEntry::supports_protocol()`. * REMOVE `protover_string_supports_protocol_or_later()` and refactor it into `UnvalidatedProtoEntry::supports_protocol_or_later()`. * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix another C/Rust different in compute_vote(). This fixes the unittest from the prior commit by checking if the versions are empty before adding a protocol to a vote. --- src/rust/protover/protover.rs | 388 ++ 1 file changed, 208 insertions(+), 180 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 1f62e70f1..847406ca2 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -226,207 +226,235 @@ impl FromStr for ProtoEntry { } } -/// Parses a single subprotocol entry string into subprotocol and version -/// parts, and then checks whether any of those versions are unsupported. -/// Helper for protover::all_supported -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1" -/// -/// # Returns -/// -/// Returns `true` if the protocol entry is well-formatted and only contains -/// versions that are also supported in tor. Otherwise, returns false -/// -fn contains_only_supported_protocols(proto_entry: &str) -> bool { -let (name, mut vers) = match get_proto_and_vers(proto_entry) { -Ok(n) => n, -Err(_) => return false, -}; - -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(n) => n.0, -Err(_) => return false, -}; - -let supported_versions = match currently_supported.get(&name) { -Some(n) => n, -None => return false, -}; +/// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just +/// the supported ones enumerated in `Protocols`. The protocol versions are +/// validated, however. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnvalidatedProtoEntry(HashMap); -vers.0.retain(|x| !supported_versions.0.contains(x)); -vers.0.is_empty() +impl Default for UnvalidatedProtoEntry { +fn default() -> UnvalidatedProtoEntry { +UnvalidatedProtoEntry( HashMap::new() ) +} } -/// Determine if we support every protocol a client supports, and if not, -/// determine which protocols we do not have support for. -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1 LinkAuth=1-2" -/// -/// # Returns -/// -/// Return `true` if every protocol version is one that we support. -/// Otherwise, return `false`. -/// Optionally, return parameters which the client supports but which we do not -/// -/// # Examples -/// ``` -/// use protover::all_supported; -/// -/// let (is_supported, unsupported) = all_supported("Link=1"); -/// assert_eq!(true, is_supported); -/// -/// let (is_supported, unsupported) = all_supported("Link=5-6"); -/// assert_eq!(false, is_supported); -/// assert_eq!("Link=5-6", unsupported); -/// -pub fn all_supported(protocols: &str) -> (bool, String) { -let unsupported = protocols -.split_whitespace() -.filter(|v| !contains_only_supported_protocols(v)) -.collect::>(); +impl UnvalidatedProtoEntry { +/// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. +pub fn iter(&self) -> hash_map::Iter { +self.0.iter() +} -(unsupported.is_empty(), unsupported.join(" ")) -} +pub fn get(&self, protocol: &UnknownProtocol) -> Option<&ProtoSet> { +self.0.get(
[tor-commits] [tor/master] rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`.
commit fa15ea104d5b9f05192afa3a023d0e2405c3842a Author: Isis Lovecruft Date: Wed Mar 21 01:44:59 2018 + rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`. This implements conversions from either a ProtoEntry or an UnvalidatedProtoEntry into a String, for use in replacing such functions as `protover::write_vote_to_string()`. * ADD macro for implementing ToString trait for ProtoEntry and UnvalidatedProtoEntry. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 21 + 1 file changed, 21 insertions(+) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 847406ca2..2a1a5df9e 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -226,6 +226,27 @@ impl FromStr for ProtoEntry { } } +/// Generate an implementation of `ToString` for either a `ProtoEntry` or an +/// `UnvalidatedProtoEntry`. +macro_rules! impl_to_string_for_proto_entry { +($t:ty) => ( +impl ToString for $t { +fn to_string(&self) -> String { +let mut parts: Vec = Vec::new(); + +for (protocol, versions) in self.iter() { +parts.push(format!("{}={}", protocol.to_string(), versions.to_string())); +} +parts.sort_unstable(); +parts.join(" ") +} +} +) +} + +impl_to_string_for_proto_entry!(ProtoEntry); +impl_to_string_for_proto_entry!(UnvalidatedProtoEntry); + /// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just /// the supported ones enumerated in `Protocols`. The protocol versions are /// validated, however. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor protover tests with new methods; note altered behaviours.
commit 493e565226fb6e5c985f787333bb0c89d661 Author: Isis Lovecruft Date: Wed Mar 21 02:13:40 2018 + rust: Refactor protover tests with new methods; note altered behaviours. Previously, the rust implementation of protover considered an empty string to be a valid ProtoEntry, while the C version did not (it must have a "=" character). Other differences include that unknown protocols must now be parsed as `protover::UnknownProtocol`s, and hence their entries as `protover::UnvalidatedProtoEntry`s, whereas before (nearly) all protoentries could be parsed regardless of how erroneous they might be considered by the C version. My apologies for this somewhat messy and difficult to read commit, if any part is frustrating to the reviewer, please feel free to ask me to split this into smaller changes (possibly hard to do, since so much changed), or ask me to comment on a specific line/change and clarify how/when the behaviours differ. The tests here should more closely match the behaviours exhibited by the C implementation, but I do not yet personally guarantee they match precisely. * REFACTOR unittests in protover::protover. * ADD new integration tests for previously untested behaviour. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/protover.rs | 214 +- src/rust/protover/tests/protover.rs | 355 2 files changed, 285 insertions(+), 284 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 247166c23..96e9dd582 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -659,154 +659,118 @@ mod test { use super::*; +macro_rules! assert_protoentry_is_parseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_ok(), format!("{:?}", protoentry.err())); +) +} + +macro_rules! assert_protoentry_is_unparseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_err()); +) +} + #[test] -fn test_versions_from_version_string() { -use std::collections::HashSet; +fn test_protoentry_from_str_multiple_protocols_multiple_versions() { +assert_protoentry_is_parseable!("Cons=3-4 Link=1,3-5"); +} -use super::Versions; +#[test] +fn test_protoentry_from_str_empty() { +assert_protoentry_is_unparseable!(""); +} -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("a,b")); -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("1,!")); +#[test] +fn test_protoentry_from_str_single_protocol_single_version() { +assert_protoentry_is_parseable!("HSDir=1"); +} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -assert_eq!(versions, Versions::from_version_string("1").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -assert_eq!(versions, Versions::from_version_string("1,2").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(3); -assert_eq!(versions, Versions::from_version_string("1-3").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1-2,5").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(3); -versions.insert(4); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1,3-5").unwrap().0); -} +#[test] +fn test_protoentry_from_str_unknown_protocol() { +assert_protoentry_is_unparseable!("Ducks=5-7,8"); } #[test] -fn test_contains_only_supported_protocols() { -use super::contains_only_supported_protocols; - -assert_eq!(false, contains_only_supported_protocols("")); -assert_eq!(true, contains_only_supported_protocols("Cons=")); -assert_eq!(true, contains_only_supported_protocols("Cons=1")); -assert_eq!(false, contains_only_supported_protocols("Cons=0")); -assert_eq!(false, contains_only_supported_protocols("Cons=0-1")); -assert_eq!(false, contains_only_supported_protocols("Cons=5")); -assert_eq!(false, contains_only_supported_protocols("Cons=1-5")); -assert_eq!(false, contains_only_supporte
[tor-commits] [tor/master] rust: Implement error types for Rust protover implementation.
commit 88204f91df3e01b69e79b89ba029b42a4025d11f Author: Isis Lovecruft Date: Wed Mar 21 00:24:46 2018 + rust: Implement error types for Rust protover implementation. This will allow us to do actual error handling intra-crate in a more rusty manner, e.g. propogating errors in match statements, conversion between error types, logging messages, etc. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/errors.rs | 43 +++ src/rust/protover/lib.rs | 3 +++ src/rust/protover/protover.rs | 6 -- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs new file mode 100644 index 0..56473d12e --- /dev/null +++ b/src/rust/protover/errors.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Various errors which may occur during protocol version parsing. + +use std::fmt; +use std::fmt::Display; + +/// All errors which may occur during protover parsing routines. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] // See Display impl for error descriptions +pub enum ProtoverError { +Overlap, +LowGreaterThanHigh, +Unparseable, +ExceedsMax, +ExceedsExpansionLimit, +UnknownProtocol, +ExceedsNameLimit, +} + +/// Descriptive error messages for `ProtoverError` variants. +impl Display for ProtoverError { +fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +match *self { +ProtoverError::Overlap +=> write!(f, "Two or more (low, high) protover ranges would overlap once expanded."), +ProtoverError::LowGreaterThanHigh +=> write!(f, "The low in a (low, high) protover range was greater than high."), +ProtoverError::Unparseable +=> write!(f, "The protover string was unparseable."), +ProtoverError::ExceedsMax +=> write!(f, "The high in a (low, high) protover range exceeds u32::MAX."), +ProtoverError::ExceedsExpansionLimit +=> write!(f, "The protover string would exceed the maximum expansion limit."), +ProtoverError::UnknownProtocol +=> 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."), +} +} +} diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index fe8c0f9bb..371d1ae58 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -22,12 +22,15 @@ //! protocols to develop independently, without having to claim compatibility //! with specific versions of Tor. +#[deny(missing_docs)] + extern crate libc; extern crate smartlist; extern crate external; extern crate tor_allocate; extern crate tor_util; +pub mod errors; mod protover; pub mod ffi; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index e5dc69b9a..8ce182cf1 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -13,6 +13,8 @@ use std::u32; use tor_util::strings::NUL_BYTE; +use errors::ProtoverError; + /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// @@ -78,7 +80,7 @@ impl fmt::Display for Proto { /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` impl FromStr for Proto { -type Err = &'static str; +type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { @@ -92,7 +94,7 @@ impl FromStr for Proto { "LinkAuth" => Ok(Proto::LinkAuth), "Microdesc" => Ok(Proto::Microdesc), "Relay" => Ok(Proto::Relay), -_ => Err("Not a valid protocol type"), +_ => Err(ProtoverError::UnknownProtocol), } } } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.
commit f2daf82794c59c37756abeaf3e41e5ebe1e7fcde Author: Isis Lovecruft Date: Tue Mar 27 22:46:14 2018 + rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C. Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied in Rust to the maximum number of version (total, for all subprotocols). Whereas in C, it was being applied to the number of subprotocols that were allowed. This changes the Rust to match C's behaviour. --- src/rust/protover/protoset.rs | 14 +++--- src/rust/protover/protover.rs | 18 +++--- src/rust/protover/tests/protover.rs | 14 -- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs index f94e6299c..4afc50edf 100644 --- a/src/rust/protover/protoset.rs +++ b/src/rust/protover/protoset.rs @@ -8,7 +8,6 @@ use std::slice; use std::str::FromStr; use std::u32; -use protover::MAX_PROTOCOLS_TO_EXPAND; use errors::ProtoverError; /// A single version number. @@ -183,10 +182,6 @@ impl ProtoSet { last_high = high; } -if self.len() > MAX_PROTOCOLS_TO_EXPAND { -return Err(ProtoverError::ExceedsMax); -} - Ok(self) } @@ -317,9 +312,15 @@ impl FromStr for ProtoSet { /// assert!(protoset.contains(&5)); /// assert!(!protoset.contains(&10)); /// -/// // We can also equivalently call `ProtoSet::from_str` by doing: +/// // We can also equivalently call `ProtoSet::from_str` by doing (all +/// // implementations of `FromStr` can be called this way, this one isn't +/// // special): /// let protoset: ProtoSet = "4-6,12".parse()?; /// +/// // Calling it (either way) can take really large ranges (up to `u32::MAX`): +/// let protoset: ProtoSet = "1-7".parse()?; +/// let protoset: ProtoSet = "1-4294967296".parse()?; +/// /// // There are lots of ways to get an `Err` from this function. Here are /// // a few: /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("=")); @@ -327,7 +328,6 @@ impl FromStr for ProtoSet { /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4")); -/// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-7")); /// /// // Things which would get parsed into an _empty_ `ProtoSet` are, /// // however, legal, and result in an empty `ProtoSet`: diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index b4fb2e842..514aeffc5 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// before concluding that someone is trying to DoS us /// /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` -pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); +const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// @@ -155,6 +155,10 @@ impl ProtoEntry { supported.parse() } +pub fn len(&self) -> usize { +self.0.len() +} + pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { self.0.get(protocol) } @@ -209,8 +213,11 @@ impl FromStr for ProtoEntry { let proto_name: Protocol = proto.parse()?; proto_entry.insert(proto_name, versions); -} +if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND { +return Err(ProtoverError::ExceedsMax); +} +} Ok(proto_entry) } } @@ -723,8 +730,13 @@ mod test { } #[test] +fn test_protoentry_from_str_allowed_number_of_versions() { +assert_protoentry_is_parseable!("Desc=1-4294967294"); +} + +#[test] fn test_protoentry_from_str_too_many_versions() { -assert_protoentry_is_unparseable!("Desc=1-65537"); +assert_protoentry_is_unparseable!("Desc=1-4294967295"); } #[test] diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 11015f35b..2db01a163 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { } #[test] -#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] -// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an -// UnvalidatedProtoEntry is defined as a Hashmap. -// Because it contains the ProtoSet part, there's still *some* validation -// happening, so in this case the DoS protections in the Rust code are kicking -// in because the range here is exceeds
[tor-commits] [tor/master] rust: Implement more memory-efficient protover datatype.
commit e6625113c98c281b0a649598d7daa347c28915e9 Author: Isis Lovecruft Date: Wed Mar 21 00:43:55 2018 + rust: Implement more memory-efficient protover datatype. * ADD new protover::protoset module. * ADD new protover::protoset::ProtoSet class for holding protover versions. * REMOVE protover::Versions type implementation and its method `from_version_string()`, and instead implement this behaviour in a more rust-like manner as `impl FromStr for ProtoSet`. * MOVE the `find_range()` utility function from protover::protover to protover::protoset since it's only used internally in the implementation of ProtoSet. * REMOVE the `contract_protocol_list()` function from protover::protover and instead refactor it (reusing nearly the entire thing, with minor superficial, i.e. non-behavioural, changes) into a more rusty `impl ToString for ProtoSet`. * REMOVE the `expand_version_range()` function from protover::protover and instead refactor it into a more rusty implementation of `impl Into> for ProtoSet` using the new error types in protover::errors. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/lib.rs | 1 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 215 +- 3 files changed, 641 insertions(+), 209 deletions(-) diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 371d1ae58..483260bca 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -31,6 +31,7 @@ extern crate tor_allocate; extern crate tor_util; pub mod errors; +pub mod protoset; mod protover; pub mod ffi; diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs new file mode 100644 index 0..f94e6299c --- /dev/null +++ b/src/rust/protover/protoset.rs @@ -0,0 +1,634 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Sets for lazily storing ordered, non-overlapping ranges of integers. + +use std::slice; +use std::str::FromStr; +use std::u32; + +use protover::MAX_PROTOCOLS_TO_EXPAND; +use errors::ProtoverError; + +/// A single version number. +pub type Version = u32; + +/// A `ProtoSet` stores an ordered `Vec` of `(low, high)` pairs of ranges of +/// non-overlapping protocol versions. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// +/// use protover::errors::ProtoverError; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// +/// # fn do_test() -> Result { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,8")?; +/// +/// // We could also equivalently call: +/// let protoset: ProtoSet = "3-5,8".parse()?; +/// +/// assert!(protoset.contains(&4)); +/// assert!(!protoset.contains(&7)); +/// +/// let expanded: Vec = protoset.clone().into(); +/// +/// assert_eq!(&expanded[..], &[3, 4, 5, 8]); +/// +/// let contracted: String = protoset.clone().to_string(); +/// +/// assert_eq!(contracted, "3-5,8".to_string()); +/// # Ok(protoset) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct ProtoSet { +pub(crate) pairs: Vec<(Version, Version)>, +} + +impl Default for ProtoSet { +fn default() -> Self { +let pairs: Vec<(Version, Version)> = Vec::new(); + +ProtoSet{ pairs } +} +} + +impl<'a> ProtoSet { +/// Create a new `ProtoSet` from a slice of `(low, high)` pairs. +/// +/// # Inputs +/// +/// We do not assume the input pairs are deduplicated or ordered. +pub fn from_slice(low_high_pairs: &'a [(Version, Version)]) -> Result { +let mut pairs: Vec<(Version, Version)> = Vec::with_capacity(low_high_pairs.len()); + +for &(low, high) in low_high_pairs { +pairs.push((low, high)); +} +// Sort the pairs without reallocation and remove all duplicate pairs. +pairs.sort_unstable(); +pairs.dedup(); + +ProtoSet{ pairs }.is_ok() +} +} + +/// Expand this `ProtoSet` to a `Vec` of all its `Version`s. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// # use protover::errors::ProtoverError; +/// +/// # fn do_test() -> Result, ProtoverError> { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,21")?; +/// let versions: Vec = protoset.into(); +/// +/// assert_eq!(&versions[..], &[3, 4, 5, 21]); +/// # +/// # Ok(versions) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +/// ``` +impl Into> for ProtoSet { +fn into(self) -> Vec { +let mut versions: Vec = Vec::new(); + +for &(low, high) in self.iter() { +versions.extend(low..high + 1); +} +versions +} +}
[tor-commits] [tor/master] rust: Port all C protover_all_supported tests to Rust.
commit 6eea0dc5f186429d598edda046156afc2a93120c Author: Isis Lovecruft Date: Tue Mar 27 21:38:29 2018 + rust: Port all C protover_all_supported tests to Rust. The behaviours still do not match, unsurprisingly, but now we know where a primary difference is: the Rust is validating version ranges more than the C, so in the C it's possible to call protover_all_supported on a ridiculous version range like "Sleen=0-4294967294" because the C uses MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust uses it to count the total number of *versions* of all subprotocols. --- src/rust/protover/tests/protover.rs | 86 +++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 9f8b5a443..11015f35b 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -6,6 +6,7 @@ extern crate protover; use protover::ProtoEntry; use protover::ProtoverVote; use protover::UnvalidatedProtoEntry; +use protover::errors::ProtoverError; #[test] fn parse_protocol_with_single_proto_and_single_version() { @@ -320,9 +321,88 @@ fn protover_compute_vote_may_exceed_limit() { } #[test] -fn protover_all_supported_should_include_version_we_actually_do_support() { +fn protover_all_supported_should_exclude_versions_we_actually_do_support() { let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap(); -let _result: String = proto.all_supported().unwrap().to_string(); +let result: String = proto.all_supported().unwrap().to_string(); -assert_eq!(_result, "Link=3-999".to_string()); +assert_eq!(result, "Link=6-999".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() { +let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=345-666".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex2() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer() { +let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-2147483648".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer_max_versions() { +let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-4294967294".to_string()); +} + +#[test] +// C_RUST_DIFFERS: The C will return true (e.g. saying "yes, that's supported") +// but set the msg to NULL (??? seems maybe potentially bad). The Rust will +// simply return a None. +fn protover_all_supported_should_return_empty_string_for_weird_thing() { +let proto: UnvalidatedProtoEntry = "Fribble=".parse().unwrap(); +let result: Option = proto.all_supported(); + +assert!(result.is_none()); +} + +#[test] +fn protover_unvalidatedprotoentry_should_err_entirely_unparseable_things() { +let proto: Result = "Fribble".parse(); + +assert_eq!(Err(ProtoverError::Unparseable), proto); +} + +#[test] +fn protover_all_supported_over_maximum_limit() {
[tor-commits] [tor/master] rust: Refactor protover::compute_for_old_tor().
commit fc2a42cc49fdcab9bbc0d90212c3c7bebda238b8 Author: Isis Lovecruft Date: Tue Mar 27 02:41:25 2018 + rust: Refactor protover::compute_for_old_tor(). During code review and discussion with Chelsea Komlo, she pointed out that protover::compute_for_old_tor() was a public function whose return type was `&'static CStr`. We both agree that C-like parts of APIs should: 1. not be exposed publicly (to other Rust crates), 2. only be called in the appropriate FFI code, 3. not expose types which are meant for FFI code (e.g. `*mut char`, `CString`, `*const c_int`, etc.) to the pure-Rust code of other crates. 4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called from pure-Rust, not even from other modules in its own crate (i.e. do not call `protover::ffi::*` from anywhere in `protover::protoset::*`, etc). With that in mind, this commit makes the following changes: * CHANGE `protover::compute_for_old_tor()` to be visible only at the `pub(crate)` level. * RENAME `protover::compute_for_old_tor()` to `protover::compute_for_old_tor_cstr()` to reflect the last change. * ADD a new `protover::compute_for_old_tor()` function wrapper which is public and intended for other Rust code to use, which returns a `&str`. --- src/rust/protover/ffi.rs | 2 +- src/rust/protover/protover.rs | 54 +++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 44c65114e..2dfeda87b 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -231,6 +231,6 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const Err(_) => return empty.as_ptr(), }; -supported = compute_for_old_tor(&version); +supported = compute_for_old_tor_cstr(&version); supported.as_ptr() } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 5492f7458..b4fb2e842 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -600,8 +600,8 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { supported_versions.contains(vers) } -/// Older versions of Tor cannot infer their own subprotocols -/// Used to determine which subprotocols are supported by older Tor versions. +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. /// /// # Inputs /// @@ -615,34 +615,70 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { /// "HSDir=1-1 LinkAuth=1" /// /// This function returns the protocols that are supported by the version input, -/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS. +/// only for tor versions older than `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS` +/// (but not older than 0.2.4.19). For newer tors (or older than 0.2.4.19), it +/// returns an empty string. /// -/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` -pub fn compute_for_old_tor(version: &str) -> &'static CStr { +/// # Note +/// +/// This function is meant to be called for/within FFI code. If you'd +/// like to use this code in Rust, please see `compute_for_old_tor()`. +// +// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` +pub(crate) fn compute_for_old_tor_cstr(version: &str) -> &'static CStr { let empty: &'static CStr = cstr!(""); if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) { return empty; } - if c_tor_version_as_new_as(version, "0.2.9.1-alpha") { return cstr!("Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2"); } - if c_tor_version_as_new_as(version, "0.2.7.5") { return cstr!("Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2"); } - if c_tor_version_as_new_as(version, "0.2.4.19") { return cstr!("Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2"); } - empty } +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. +/// +/// # Inputs +/// +/// * `version`, a string comprised of "[0-9a-z.-]" +/// +/// # Returns +/// +/// A `Result` whose `Ok` value is an `&'static str` encoding a list of protocol +/// names and supported versions. The string takes the following format: +/// +/// "HSDir=1-1 LinkAuth=1" +/// +/// This function returns the protocols that are supported by the version input, +/// only for tor versions older than `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS`. +/// (but not older than 0.2.4.19). For n
[tor-commits] [tor/master] rust: Add new protover::ProtoEntry type which uses new datatypes.
commit 88b2f170e451567a3b0095a420544a675a5826b0 Author: Isis Lovecruft Date: Wed Mar 21 01:18:02 2018 + rust: Add new protover::ProtoEntry type which uses new datatypes. This replaces the `protover::SupportedProtocols` (why would you have a type just for things which are supported?) with a new, more generic type, `protover::ProtoEntry`, which utilises the new, more memory-efficient datatype in protover::protoset. * REMOVE `get_supported_protocols()` and `SupportedProtocols::tor_supported()` (since they were never used separately) and collapse their functionality into a single `ProtoEntry::supported()` method. * REMOVE `SupportedProtocols::from_proto_entries()` and reimplement its functionality as the more rusty `impl FromStr for ProtoEntry`. * REMOVE `get_proto_and_vers()` function and refactor it into the more rusty `impl FromStr for ProtoEntry`. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 160 -- 1 file changed, 75 insertions(+), 85 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 30d9e0a89..d507336cc 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -1,20 +1,20 @@ // Copyright (c) 2016-2017, The Tor Project, Inc. */ // See LICENSE for licensing information */ -use std::str; -use std::str::FromStr; +use std::collections::HashMap; +use std::collections::hash_map; use std::ffi::CStr; use std::fmt; -use std::collections::{HashMap, HashSet}; -use std::ops::Range; +use std::str; +use std::str::FromStr; use std::string::String; -use std::u32; use tor_log::{LogSeverity, LogDomain}; use external::c_tor_version_as_new_as; use errors::ProtoverError; use protoset::Version; +use protoset::ProtoSet; /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. @@ -130,100 +130,90 @@ pub(crate) fn get_supported_protocols_cstr() -> &'static CStr { Relay=1-2") } -/// Get a string representation of current supported protocols. -/// -/// # Returns -/// -/// An `&'a str` whose value is the existing protocols supported by tor. -/// Returned data is in the format as follows: -/// -/// "HSDir=1-1 LinkAuth=1" -pub fn get_supported_protocols<'a>() -> &'a str { -let supported_cstr: &'static CStr = get_supported_protocols_cstr(); -let supported: &str = match supported_cstr.to_str() { -Ok(x) => x, -Err(_) => "", -}; - -supported -} +/// A map of protocol names to the versions of them which are supported. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProtoEntry(HashMap); -pub struct SupportedProtocols(HashMap); - -impl SupportedProtocols { -pub fn from_proto_entries(protocol_strs: I) -> Result -where -I: Iterator, -S: AsRef, -{ -let mut parsed = HashMap::new(); -for subproto in protocol_strs { -let (name, version) = get_proto_and_vers(subproto.as_ref())?; -parsed.insert(name, version); -} -Ok(SupportedProtocols(parsed)) +impl Default for ProtoEntry { +fn default() -> ProtoEntry { +ProtoEntry( HashMap::new() ) } +} -/// Translates a string representation of a protocol list to a -/// SupportedProtocols instance. -/// -/// # Examples -/// -/// ``` -/// use protover::SupportedProtocols; -/// -/// let supported_protocols = SupportedProtocols::from_proto_entries_string( -/// "HSDir=1-2 HSIntro=3-4" -/// ); -/// ``` -pub fn from_proto_entries_string( -proto_entries: &str, -) -> Result { -Self::from_proto_entries(proto_entries.split(" ")) +impl ProtoEntry { +/// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. +pub fn iter(&self) -> hash_map::Iter { +self.0.iter() } /// Translate the supported tor versions from a string into a -/// HashMap, which is useful when looking up a specific +/// ProtoEntry, which is useful when looking up a specific /// subprotocol. -/// -fn tor_supported() -> Result { -Self::from_proto_entries_string(get_supported_protocols()) +pub fn supported() -> Result { +let supported_cstr: &'static CStr = get_supported_protocols_cstr(); +let supported: &str = supported_cstr.to_str().unwrap_or(""); + +supported.parse() +} + +pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { +self.0.get(protocol) +} + +pub fn insert(&mut self, key: Protocol, value: ProtoSet) { +self.0.insert(key, value); +} + +pub fn remove(&mut self, key: &Protocol) -> Option { +self.0.remove(key) +} + +pub fn is_empty(&self) -> bool { +self.0.is_empty() } } -/// Par
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_compute_vote().
commit 6f252e098637bbac87b4ad99ece7fa4a7e404cdb Author: Isis Lovecruft Date: Wed Mar 21 03:05:56 2018 + rust: Refactor Rust impl of protover_compute_vote(). This includes a subtle difference in behaviour to the previous Rust implementation, where, for each vote that we're computing over, if a single one fails to parse, we skip it. This now matches the current behaviour in the C implementation. * REFACTOR `protover::ffi::protover_compute_vote()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 7f8d86d04..780fe6963 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -161,6 +161,8 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char { /// Provide an interface for C to translate arguments and return types for /// protover::compute_vote +// +// Why is the threshold a signed integer? âisis #[no_mangle] pub extern "C" fn protover_compute_vote( list: *const Stringlist, @@ -175,10 +177,19 @@ pub extern "C" fn protover_compute_vote( // Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. let data: Vec = unsafe { (*list).get_list() }; +let hold: usize = threshold as usize; +let mut proto_entries: Vec = Vec::new(); -let vote = compute_vote(data, threshold); +for datum in data { +let entry: UnvalidatedProtoEntry = match datum.parse() { +Ok(x) => x, +Err(_) => continue, +}; +proto_entries.push(entry); +} +let vote: UnvalidatedProtoEntry = ProtoverVote::compute(&proto_entries, &hold); -allocate_and_copy_string(&vote) +allocate_and_copy_string(&vote.to_string()) } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Implement more memory-efficient protover datatype.
commit 9925d2e68709aa7346f4c5bc98ea1349df6741f3 Author: Isis Lovecruft Date: Wed Mar 21 00:43:55 2018 + rust: Implement more memory-efficient protover datatype. * ADD new protover::protoset module. * ADD new protover::protoset::ProtoSet class for holding protover versions. * REMOVE protover::Versions type implementation and its method `from_version_string()`, and instead implement this behaviour in a more rust-like manner as `impl FromStr for ProtoSet`. * MOVE the `find_range()` utility function from protover::protover to protover::protoset since it's only used internally in the implementation of ProtoSet. * REMOVE the `contract_protocol_list()` function from protover::protover and instead refactor it (reusing nearly the entire thing, with minor superficial, i.e. non-behavioural, changes) into a more rusty `impl ToString for ProtoSet`. * REMOVE the `expand_version_range()` function from protover::protover and instead refactor it into a more rusty implementation of `impl Into> for ProtoSet` using the new error types in protover::errors. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/lib.rs | 1 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 214 +- 3 files changed, 641 insertions(+), 208 deletions(-) diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 2455d5081..9a9f92ed7 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -35,6 +35,7 @@ extern crate tor_util; extern crate tor_log; pub mod errors; +pub mod protoset; mod protover; pub mod ffi; diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs new file mode 100644 index 0..f94e6299c --- /dev/null +++ b/src/rust/protover/protoset.rs @@ -0,0 +1,634 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Sets for lazily storing ordered, non-overlapping ranges of integers. + +use std::slice; +use std::str::FromStr; +use std::u32; + +use protover::MAX_PROTOCOLS_TO_EXPAND; +use errors::ProtoverError; + +/// A single version number. +pub type Version = u32; + +/// A `ProtoSet` stores an ordered `Vec` of `(low, high)` pairs of ranges of +/// non-overlapping protocol versions. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// +/// use protover::errors::ProtoverError; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// +/// # fn do_test() -> Result { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,8")?; +/// +/// // We could also equivalently call: +/// let protoset: ProtoSet = "3-5,8".parse()?; +/// +/// assert!(protoset.contains(&4)); +/// assert!(!protoset.contains(&7)); +/// +/// let expanded: Vec = protoset.clone().into(); +/// +/// assert_eq!(&expanded[..], &[3, 4, 5, 8]); +/// +/// let contracted: String = protoset.clone().to_string(); +/// +/// assert_eq!(contracted, "3-5,8".to_string()); +/// # Ok(protoset) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct ProtoSet { +pub(crate) pairs: Vec<(Version, Version)>, +} + +impl Default for ProtoSet { +fn default() -> Self { +let pairs: Vec<(Version, Version)> = Vec::new(); + +ProtoSet{ pairs } +} +} + +impl<'a> ProtoSet { +/// Create a new `ProtoSet` from a slice of `(low, high)` pairs. +/// +/// # Inputs +/// +/// We do not assume the input pairs are deduplicated or ordered. +pub fn from_slice(low_high_pairs: &'a [(Version, Version)]) -> Result { +let mut pairs: Vec<(Version, Version)> = Vec::with_capacity(low_high_pairs.len()); + +for &(low, high) in low_high_pairs { +pairs.push((low, high)); +} +// Sort the pairs without reallocation and remove all duplicate pairs. +pairs.sort_unstable(); +pairs.dedup(); + +ProtoSet{ pairs }.is_ok() +} +} + +/// Expand this `ProtoSet` to a `Vec` of all its `Version`s. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// # use protover::errors::ProtoverError; +/// +/// # fn do_test() -> Result, ProtoverError> { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,21")?; +/// let versions: Vec = protoset.into(); +/// +/// assert_eq!(&versions[..], &[3, 4, 5, 21]); +/// # +/// # Ok(versions) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +/// ``` +impl Into> for ProtoSet { +fn into(self) -> Vec { +let mut versions: Vec = Vec::new(); + +for &(low, high) in self.iter() { +versions.extend(low..high + 1); +} +versions +} +} + +im
[tor-commits] [tor/master] rust: Add new protover::UnknownProtocol type.
commit 811178434eae16fd673b67ebda06b630aa27dec4 Author: Isis Lovecruft Date: Wed Mar 21 01:07:18 2018 + rust: Add new protover::UnknownProtocol type. * ADD new type, protover::UnknownProtocol, so that we have greater type safety and our protover functionality which works with unsanitised protocol names is more clearly demarcated. * REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new protover::UnknownProtocol type name. * ADD a utility conversion of `impl From for UnknownProtocol` so that we can easily with known protocols and unknown protocols simultaneously (e.g. doing comparisons, checking their version numbers), while not allowing UnknownProtocols to be accidentally used in functions which should only take Protocols. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/ffi.rs | 30 src/rust/protover/protover.rs | 53 +++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index a9d5013c6..5ecbc2969 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -9,27 +9,29 @@ use libc::{c_char, c_int, uint32_t}; use std::ffi::CStr; use std::ffi::CString; -use protover::*; use smartlist::*; use tor_allocate::allocate_and_copy_string; +use errors::ProtoverError; +use protover::*; + /// Translate C enums to Rust Proto enums, using the integer value of the C -/// enum to map to its associated Rust enum +/// enum to map to its associated Rust enum. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -fn translate_to_rust(c_proto: uint32_t) -> Result { +fn translate_to_rust(c_proto: uint32_t) -> Result { match c_proto { -0 => Ok(Proto::Link), -1 => Ok(Proto::LinkAuth), -2 => Ok(Proto::Relay), -3 => Ok(Proto::DirCache), -4 => Ok(Proto::HSDir), -5 => Ok(Proto::HSIntro), -6 => Ok(Proto::HSRend), -7 => Ok(Proto::Desc), -8 => Ok(Proto::Microdesc), -9 => Ok(Proto::Cons), -_ => Err("Invalid protocol type"), +0 => Ok(Protocol::Link), +1 => Ok(Protocol::LinkAuth), +2 => Ok(Protocol::Relay), +3 => Ok(Protocol::DirCache), +4 => Ok(Protocol::HSDir), +5 => Ok(Protocol::HSIntro), +6 => Ok(Protocol::HSRend), +7 => Ok(Protocol::Desc), +8 => Ok(Protocol::Microdesc), +9 => Ok(Protocol::Cons), +_ => Err(ProtoverError::UnknownProtocol), } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index ae6931d05..30d9e0a89 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -32,8 +32,8 @@ pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -#[derive(Hash, Eq, PartialEq, Debug)] -pub enum Proto { +#[derive(Clone, Hash, Eq, PartialEq, Debug)] +pub enum Protocol { Cons, Desc, DirCache, @@ -46,7 +46,7 @@ pub enum Proto { Relay, } -impl fmt::Display for Proto { +impl fmt::Display for Protocol { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self) } @@ -56,26 +56,51 @@ impl fmt::Display for Proto { /// Error if the string is an unrecognized protocol name. /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` -impl FromStr for Proto { +impl FromStr for Protocol { type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { -"Cons" => Ok(Proto::Cons), -"Desc" => Ok(Proto::Desc), -"DirCache" => Ok(Proto::DirCache), -"HSDir" => Ok(Proto::HSDir), -"HSIntro" => Ok(Proto::HSIntro), -"HSRend" => Ok(Proto::HSRend), -"Link" => Ok(Proto::Link), -"LinkAuth" => Ok(Proto::LinkAuth), -"Microdesc" => Ok(Proto::Microdesc), -"Relay" => Ok(Proto::Relay), +"Cons" => Ok(Protocol::Cons), +"Desc" => Ok(Protocol::Desc), +"DirCache" => Ok(Protocol::DirCache), +"HSDir" => Ok(Protocol::HSDir), +"HSIntro" => Ok(Protocol::HSIntro), +"HSRend" => Ok(Protocol::HSRend), +"Link" => Ok(Protocol::Link), +"LinkAuth" => Ok(Protocol::LinkAuth), +"Microdesc" => Ok(Protocol::Microdesc), +"Relay" => Ok(Protocol::Relay), _ => Err(ProtoverError::UnknownProtocol), } } } +/// A protocol string which is not one of the `Protocols` we currently know +/// about. +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct UnknownProtocol(String); + +impl fmt::Display for UnknownProtocol { +fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Resu
[tor-commits] [tor/master] rust: Refactor protover::is_supported_here().
commit aa241e99dedeadf92ca72e29d16ed3e31a439392 Author: Isis Lovecruft Date: Wed Mar 21 02:09:04 2018 + rust: Refactor protover::is_supported_here(). This changes `protover::is_supported_here()` to be aware of new datatypes (e.g. don't call `.0` on things which are no longer tuple structs) and also changes the method signature to take borrows, making it faster, threadable, and easier to read (i.e. the caller can know from reading the function signature that the function won't mutate values passed into it). * CHANGE the `protover::is_supported_here()` function to take borrows. * REFACTOR the `protover::is_supported_here()` function to be aware of new datatypes. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 6f234c61a..572a13c0f 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -579,26 +579,25 @@ impl ProtoverVote { /// /// # Examples /// ``` -/// use protover::*; +/// use protover::is_supported_here; +/// use protover::Protocol; /// -/// let is_supported = is_supported_here(Proto::Link, 10); +/// let is_supported = is_supported_here(&Protocol::Link, &10); /// assert_eq!(false, is_supported); /// -/// let is_supported = is_supported_here(Proto::Link, 1); +/// let is_supported = is_supported_here(&Protocol::Link, &1); /// assert_eq!(true, is_supported); /// ``` -pub fn is_supported_here(proto: Proto, vers: Version) -> bool { -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(result) => result.0, +pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { +let currently_supported: ProtoEntry = match ProtoEntry::supported() { +Ok(result) => result, Err(_) => return false, }; - -let supported_versions = match currently_supported.get(&proto) { +let supported_versions = match currently_supported.get(proto) { Some(n) => n, None => return false, }; - -supported_versions.0.contains(&vers) +supported_versions.contains(vers) } /// Older versions of Tor cannot infer their own subprotocols ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_list_supports_protocol_or_later().
commit 0a5494b81df684109986354a3d31051b6f8d0bec Author: Isis Lovecruft Date: Wed Mar 21 02:59:25 2018 + rust: Refactor Rust impl of protover_list_supports_protocol_or_later(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol_or_later()` to use new types and methods. --- src/rust/protover/ffi.rs | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 0d0376e52..7f8d86d04 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -138,13 +138,15 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( Err(_) => return 0, }; -let is_supported = protover_string_supports_protocol_or_later( -protocol_list, -protocol, -version, -); +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, +Err(_) => return 1, +}; -return if is_supported { 1 } else { 0 }; +if proto_entry.supports_protocol_or_later(&protocol.into(), &version) { +return 1; +} +0 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`.
commit 26bafb3c335241c01e1f165c64d4167d26acfcc6 Author: Isis Lovecruft Date: Wed Mar 21 01:44:59 2018 + rust: Add macro for `impl ToString for {Unvalidated}ProtoEntry`. This implements conversions from either a ProtoEntry or an UnvalidatedProtoEntry into a String, for use in replacing such functions as `protover::write_vote_to_string()`. * ADD macro for implementing ToString trait for ProtoEntry and UnvalidatedProtoEntry. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 21 + 1 file changed, 21 insertions(+) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index db9f5154f..9ec29a26d 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -215,6 +215,27 @@ impl FromStr for ProtoEntry { } } +/// Generate an implementation of `ToString` for either a `ProtoEntry` or an +/// `UnvalidatedProtoEntry`. +macro_rules! impl_to_string_for_proto_entry { +($t:ty) => ( +impl ToString for $t { +fn to_string(&self) -> String { +let mut parts: Vec = Vec::new(); + +for (protocol, versions) in self.iter() { +parts.push(format!("{}={}", protocol.to_string(), versions.to_string())); +} +parts.sort_unstable(); +parts.join(" ") +} +} +) +} + +impl_to_string_for_proto_entry!(ProtoEntry); +impl_to_string_for_proto_entry!(UnvalidatedProtoEntry); + /// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just /// the supported ones enumerated in `Protocols`. The protocol versions are /// validated, however. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] protover: Change protover_all_supported() to return only unsupported.
commit 6e353664dd7c7a362be82af578954797e348d298 Author: Isis Lovecruft Date: Tue Mar 27 16:59:49 2018 + protover: Change protover_all_supported() to return only unsupported. Previously, if "Link=1-5" was supported, and you asked protover_all_supported() (or protover::all_supported() in Rust) if it supported "Link=3-999", the C version would return "Link=3-999" and the Rust would return "Link=6-999". These both behave the same now, i.e. both return "Link=6-999". --- src/or/protover.c| 77 +++- src/test/test_protover.c | 17 ++- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/or/protover.c b/src/or/protover.c index cb168085c..6532f09c2 100644 --- a/src/or/protover.c +++ b/src/or/protover.c @@ -671,7 +671,9 @@ int protover_all_supported(const char *s, char **missing_out) { int all_supported = 1; - smartlist_t *missing; + smartlist_t *missing_some; + smartlist_t *missing_completely; + smartlist_t *missing_all; if (!s) { return 1; @@ -684,7 +686,8 @@ protover_all_supported(const char *s, char **missing_out) return 1; } - missing = smartlist_new(); + missing_some = smartlist_new(); + missing_completely = smartlist_new(); SMARTLIST_FOREACH_BEGIN(entries, const proto_entry_t *, ent) { protocol_type_t tp; @@ -696,26 +699,86 @@ protover_all_supported(const char *s, char **missing_out) } SMARTLIST_FOREACH_BEGIN(ent->ranges, const proto_range_t *, range) { + proto_entry_t *unsupported = tor_malloc_zero(sizeof(proto_entry_t)); + proto_range_t *versions = tor_malloc_zero(sizeof(proto_range_t)); uint32_t i; + + unsupported->name = tor_strdup(ent->name); + unsupported->ranges = smartlist_new(); + for (i = range->low; i <= range->high; ++i) { if (!protover_is_supported_here(tp, i)) { - goto unsupported; + if (versions->low == 0 && versions->high == 0) { +versions->low = i; +/* Pre-emptively add the high now, just in case we're in a single + * version range (e.g. "Link=999"). */ +versions->high = i; + } + /* If the last one to be unsupported is one less than the current + * one, we're in a continous range, so set the high field. */ + if ((versions->high && versions->high == i - 1) || + /* Similarly, if the last high wasn't set and we're currently + * one higher than the low, add current index as the highest + * known high. */ + (!versions->high && versions->low == i - 1)) { +versions->high = i; +continue; + } +} else { + /* If we hit a supported version, and we previously had a range, + * we've hit a non-continuity. Copy the previous range and add it to + * the unsupported->ranges list and zero-out the previous range for + * the next iteration. */ + if (versions->low != 0 && versions->high != 0) { +proto_range_t *versions_to_add = tor_malloc(sizeof(proto_range_t)); + +versions_to_add->low = versions->low; +versions_to_add->high = versions->high; +smartlist_add(unsupported->ranges, versions_to_add); + +versions->low = 0; +versions->high = 0; + } } } + /* Once we've run out of versions to check, see if we had any unsupported + * ones and, if so, add them to unsupported->ranges. */ + if (versions->low != 0 && versions->high != 0) { +smartlist_add(unsupported->ranges, versions); + } + /* Finally, if we had something unsupported, add it to the list of + * missing_some things and mark that there was something missing. */ + if (smartlist_len(unsupported->ranges) != 0) { +smartlist_add(missing_some, (void*) unsupported); +all_supported = 0; + } else { +proto_entry_free(unsupported); +tor_free(versions); + } } SMARTLIST_FOREACH_END(range); continue; unsupported: all_supported = 0; -smartlist_add(missing, (void*) ent); +smartlist_add(missing_completely, (void*) ent); } SMARTLIST_FOREACH_END(ent); + /* We keep the two smartlists separate so that we can free the proto_entry_t + * we created and put in missing_some, so here we add them together to build + * the string. */ + missing_all = smartlist_new(); + smartlist_add_all(missing_all, missing_some); + smartlist_add_all(missing_all, missing_completely); + if (missing_out && !all_supported) { -tor_assert(0 != smartlist_len(missing)); -*missing_out = encode_protocol_list(missing); +tor_assert(smartlist_len(missing_all) != 0); +*missing_out = encode_protocol_list(missing_all); } - smartlist_free(missing); + SMARTLIST_FOREACH(missing_some, proto_entry_t *, ent, proto_entry_free(ent)); + smar
[tor-commits] [tor/master] rust: Refactor protover tests with new methods; note altered behaviours.
commit 15e59a1fedf47e7e689e06e5649849552d8b8c0d Author: Isis Lovecruft Date: Wed Mar 21 02:13:40 2018 + rust: Refactor protover tests with new methods; note altered behaviours. Previously, the rust implementation of protover considered an empty string to be a valid ProtoEntry, while the C version did not (it must have a "=" character). Other differences include that unknown protocols must now be parsed as `protover::UnknownProtocol`s, and hence their entries as `protover::UnvalidatedProtoEntry`s, whereas before (nearly) all protoentries could be parsed regardless of how erroneous they might be considered by the C version. My apologies for this somewhat messy and difficult to read commit, if any part is frustrating to the reviewer, please feel free to ask me to split this into smaller changes (possibly hard to do, since so much changed), or ask me to comment on a specific line/change and clarify how/when the behaviours differ. The tests here should more closely match the behaviours exhibited by the C implementation, but I do not yet personally guarantee they match precisely. * REFACTOR unittests in protover::protover. * ADD new integration tests for previously untested behaviour. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/protover.rs | 214 +- src/rust/protover/tests/protover.rs | 355 2 files changed, 285 insertions(+), 284 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 572a13c0f..5492f7458 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -650,154 +650,118 @@ mod test { use super::*; +macro_rules! assert_protoentry_is_parseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_ok(), format!("{:?}", protoentry.err())); +) +} + +macro_rules! assert_protoentry_is_unparseable { +($e:expr) => ( +let protoentry: Result = $e.parse(); + +assert!(protoentry.is_err()); +) +} + #[test] -fn test_versions_from_version_string() { -use std::collections::HashSet; +fn test_protoentry_from_str_multiple_protocols_multiple_versions() { +assert_protoentry_is_parseable!("Cons=3-4 Link=1,3-5"); +} -use super::Versions; +#[test] +fn test_protoentry_from_str_empty() { +assert_protoentry_is_unparseable!(""); +} -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("a,b")); -assert_eq!(Err("invalid protocol entry"), Versions::from_version_string("1,!")); +#[test] +fn test_protoentry_from_str_single_protocol_single_version() { +assert_protoentry_is_parseable!("HSDir=1"); +} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -assert_eq!(versions, Versions::from_version_string("1").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -assert_eq!(versions, Versions::from_version_string("1,2").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(3); -assert_eq!(versions, Versions::from_version_string("1-3").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(2); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1-2,5").unwrap().0); -} -{ -let mut versions: HashSet = HashSet::new(); -versions.insert(1); -versions.insert(3); -versions.insert(4); -versions.insert(5); -assert_eq!(versions, Versions::from_version_string("1,3-5").unwrap().0); -} +#[test] +fn test_protoentry_from_str_unknown_protocol() { +assert_protoentry_is_unparseable!("Ducks=5-7,8"); } #[test] -fn test_contains_only_supported_protocols() { -use super::contains_only_supported_protocols; - -assert_eq!(false, contains_only_supported_protocols("")); -assert_eq!(true, contains_only_supported_protocols("Cons=")); -assert_eq!(true, contains_only_supported_protocols("Cons=1")); -assert_eq!(false, contains_only_supported_protocols("Cons=0")); -assert_eq!(false, contains_only_supported_protocols("Cons=0-1")); -assert_eq!(false, contains_only_supported_protocols("Cons=5")); -assert_eq!(false, contains_only_supported_protocols("Cons=1-5")); -assert_eq!(false, contains_only_supporte
[tor-commits] [tor/master] rust: Implement error types for Rust protover implementation.
commit b6059297d7cb76f0e00e2098e38d6677d3033340 Author: Isis Lovecruft Date: Wed Mar 21 00:24:46 2018 + rust: Implement error types for Rust protover implementation. This will allow us to do actual error handling intra-crate in a more rusty manner, e.g. propogating errors in match statements, conversion between error types, logging messages, etc. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/errors.rs | 43 +++ src/rust/protover/lib.rs | 3 +++ src/rust/protover/protover.rs | 6 -- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs new file mode 100644 index 0..56473d12e --- /dev/null +++ b/src/rust/protover/errors.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Various errors which may occur during protocol version parsing. + +use std::fmt; +use std::fmt::Display; + +/// All errors which may occur during protover parsing routines. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] // See Display impl for error descriptions +pub enum ProtoverError { +Overlap, +LowGreaterThanHigh, +Unparseable, +ExceedsMax, +ExceedsExpansionLimit, +UnknownProtocol, +ExceedsNameLimit, +} + +/// Descriptive error messages for `ProtoverError` variants. +impl Display for ProtoverError { +fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +match *self { +ProtoverError::Overlap +=> write!(f, "Two or more (low, high) protover ranges would overlap once expanded."), +ProtoverError::LowGreaterThanHigh +=> write!(f, "The low in a (low, high) protover range was greater than high."), +ProtoverError::Unparseable +=> write!(f, "The protover string was unparseable."), +ProtoverError::ExceedsMax +=> write!(f, "The high in a (low, high) protover range exceeds u32::MAX."), +ProtoverError::ExceedsExpansionLimit +=> write!(f, "The protover string would exceed the maximum expansion limit."), +ProtoverError::UnknownProtocol +=> 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."), +} +} +} diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index d1d49d2a5..2455d5081 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -22,6 +22,8 @@ //! protocols to develop independently, without having to claim compatibility //! with specific versions of Tor. +#[deny(missing_docs)] + extern crate libc; extern crate smartlist; extern crate external; @@ -32,6 +34,7 @@ extern crate tor_util; #[macro_use] extern crate tor_log; +pub mod errors; mod protover; pub mod ffi; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index fd1f41d78..01d17ac8b 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -13,6 +13,8 @@ use std::u32; use tor_log::{LogSeverity, LogDomain}; use external::c_tor_version_as_new_as; +use errors::ProtoverError; + /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// @@ -54,7 +56,7 @@ impl fmt::Display for Proto { /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` impl FromStr for Proto { -type Err = &'static str; +type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { @@ -68,7 +70,7 @@ impl FromStr for Proto { "LinkAuth" => Ok(Proto::LinkAuth), "Microdesc" => Ok(Proto::Microdesc), "Relay" => Ok(Proto::Relay), -_ => Err("Not a valid protocol type"), +_ => Err(ProtoverError::UnknownProtocol), } } } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] tests: Make inline comments in test_protover.c more accurate.
commit 22c65a0e4bc7da08e216a769bba1b91f7b2a0ca1 Author: Isis Lovecruft Date: Tue Mar 27 21:34:00 2018 + tests: Make inline comments in test_protover.c more accurate. The DoS potential is slightly higher in C now due to some differences to the Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs. Also, the comment about "failing at the splitting stage" in Rust wasn't true, since when we split, we ignore empty chunks (e.g. "1--1" parses into "(1,None),(None,1)" and "None" can't be parsed into an integer). Finally, the comment about "Rust seems to experience an internal error" is only true in debug mode, where u32s are bounds-checked at runtime. In release mode, code expressing the equivalent of this test will error with `Err(ProtoverError::Unparseable)` because 4294967295 is too large. --- src/test/test_protover.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 95cc5f083..e7e17efe3 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -273,7 +273,7 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001"); tor_free(msg); - /* CPU/RAM DoS loop: Rust only */ + /* We shouldn't be able to DoS ourselves parsing a large range. */ tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg)); tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); tor_free(msg); @@ -546,8 +546,6 @@ test_protover_vote_roundtrip(void *args) { "Link=1,9-8,3", NULL }, { "Faux=-0", NULL }, { "Faux=0--0", NULL }, -// "These fail at the splitting stage in Rust, but the number parsing -// stage in C." { "Faux=-1", NULL }, { "Faux=-1-3", NULL }, { "Faux=1--1", NULL }, @@ -556,9 +554,9 @@ test_protover_vote_roundtrip(void *args) /* Large range */ { "Sleen=1-501", "Sleen=1-501" }, { "Sleen=1-65537", NULL }, -/* CPU/RAM DoS Loop: Rust only. */ +/* Both C/Rust implementations should be able to handle this mild DoS. */ { "Sleen=0-2147483648", NULL }, -/* Rust seems to experience an internal error here. */ +/* Rust tests are built in debug mode, so ints are bounds-checked. */ { "Sleen=0-4294967295", NULL }, }; unsigned u; ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust implementation of protover_is_supported_here().
commit 9766d53cf990432f25fb70916fe9b165f1d4a36d Author: Isis Lovecruft Date: Wed Mar 21 03:08:35 2018 + rust: Refactor Rust implementation of protover_is_supported_here(). It was changed to take borrows instead of taking ownership. * REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method signature on `protover::is_supported_here()`. --- src/rust/protover/ffi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 780fe6963..44c65114e 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -204,7 +204,7 @@ pub extern "C" fn protover_is_supported_here( Err(_) => return 0, }; -let is_supported = is_supported_here(protocol, version); +let is_supported = is_supported_here(&protocol, &version); return if is_supported { 1 } else { 0 }; } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Add new protover::UnvalidatedProtoEntry type.
commit b786b146edd33b025774819f54d7bba3d07bf832 Author: Isis Lovecruft Date: Wed Mar 21 01:29:36 2018 + rust: Add new protover::UnvalidatedProtoEntry type. This adds a new protover::UnvalidatedProtoEntry type, which is the UnknownProtocol variant of a ProtoEntry, and refactors several functions which should operate on this type into methods. This also fixes what was previously another difference to the C implementation: if you asked the C version of protovet_compute_vote() to compute a single vote containing "Fribble=", it would return NULL. However, the Rust version would return "Fribble=" since it didn't check if the versions were empty before constructing the string of differences. ("Fribble=" is technically a valid protover string.) This is now fixed, and the Rust version in that case will, analogous to (although safer than) C returning a NULL, return None. * REMOVE internal `contains_only_supported_protocols()` function. * REMOVE `all_supported()` function and refactor it into `UnvalidatedProtoEntry::all_supported()`. * REMOVE `parse_protocols_from_string_with_no_validation()` and refactor it into the more rusty implementation of `impl FromStr for UnvalidatedProtoEntry`. * REMOVE `protover_string_supports_protocol()` and refactor it into `UnvalidatedProtoEntry::supports_protocol()`. * REMOVE `protover_string_supports_protocol_or_later()` and refactor it into `UnvalidatedProtoEntry::supports_protocol_or_later()`. * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix another C/Rust different in compute_vote(). This fixes the unittest from the prior commit by checking if the versions are empty before adding a protocol to a vote. --- src/rust/protover/lib.rs | 3 - src/rust/protover/protover.rs | 403 ++ 2 files changed, 208 insertions(+), 198 deletions(-) diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 9a9f92ed7..ce964196f 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -31,9 +31,6 @@ extern crate tor_allocate; #[macro_use] extern crate tor_util; -#[macro_use] -extern crate tor_log; - pub mod errors; pub mod protoset; mod protover; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index d507336cc..db9f5154f 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -9,7 +9,6 @@ use std::str; use std::str::FromStr; use std::string::String; -use tor_log::{LogSeverity, LogDomain}; use external::c_tor_version_as_new_as; use errors::ProtoverError; @@ -216,221 +215,235 @@ impl FromStr for ProtoEntry { } } -/// Parses a single subprotocol entry string into subprotocol and version -/// parts, and then checks whether any of those versions are unsupported. -/// Helper for protover::all_supported -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1" -/// -/// # Returns -/// -/// Returns `true` if the protocol entry is well-formatted and only contains -/// versions that are also supported in tor. Otherwise, returns false -/// -fn contains_only_supported_protocols(proto_entry: &str) -> bool { -let (name, mut vers) = match get_proto_and_vers(proto_entry) { -Ok(n) => n, -Err("Too many versions to expand") => { -tor_log_msg!( -LogSeverity::Warn, -LogDomain::Net, -"get_versions", -"When expanding a protocol list from an authority, I \ -got too many protocols. This is possibly an attack or a bug, \ -unless the Tor network truly has expanded to support over {} \ -different subprotocol versions. The offending string was: {}", -MAX_PROTOCOLS_TO_EXPAND, -proto_entry -); -return false; -} -Err(_) => return false, -}; - -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(n) => n.0, -Err(_) => return false, -}; - -let supported_versions = match currently_supported.get(&name) { -Some(n) => n, -None => return false, -}; +/// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just +/// the supported ones enumerated in `Protocols`. The protocol versions are +/// validated, however. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnvalidatedProtoEntry(HashMap); -vers.0.retain(|x| !supported_versions.0.contains(x)); -vers.0.is_empty() +impl Default for UnvalidatedProtoEntry { +fn default() -> UnvalidatedProtoEntry { +UnvalidatedProtoEntry( HashMap::new() ) +} } -/// Determine if we support every protocol a client supports, and if not, -/// determine which protocols we do not have support for. -/// -/// # Inputs
[tor-commits] [tor/master] rust: Add new ProtoverVote type and refactor functions to methods.
commit 9abbd23df7104f1f46a172c2eaa62bec0afc6d9c Author: Isis Lovecruft Date: Wed Mar 21 01:52:41 2018 + rust: Add new ProtoverVote type and refactor functions to methods. This adds a new type for votes upon `protover::ProtoEntry`s (technically, on `protover::UnvalidatedProtoEntry`s, because the C code does not validate based upon currently known protocols when voting, in order to maintain future-compatibility), and converts several functions which would have operated on this datatype into methods for ease-of-use and readability. This also fixes a behavioural differentce to the C version of protover_compute_vote(). The C version of protover_compute_vote() calls expand_protocol_list() which checks if there would be too many subprotocols *or* expanded individual version numbers, i.e. more than MAX_PROTOCOLS_TO_EXPAND, and does this *per vote* (but only in compute_vote(), everywhere else in the C seems to only care about the number of subprotocols, not the number of individual versions). We need to match its behaviour in Rust and ensure we're not allowing more than it would to get the votes to match. * ADD new `protover::ProtoverVote` datatype. * REMOVE the `protover::compute_vote()` function and refactor it into an equivalent-in-behaviour albeit more memory-efficient voting algorithm based on the new underlying `protover::protoset::ProtoSet` datatype, as `ProtoverVote::compute()`. * REMOVE the `protover::write_vote_to_string()` function, since this functionality is now generated by the impl_to_string_for_proto_entry!() macro for both `ProtoEntry` and `UnvalidatedProtoEntry` (the latter of which is the correct type to return from a voting protocol instance, since the entity voting may not know of all protocols being voted upon or known about by other voting parties). * FIXES part of #24031: https://bugs.torproject.org/24031 rust: Fix a difference in compute_vote() behaviour to C version. --- src/rust/protover/protover.rs | 189 -- 1 file changed, 90 insertions(+), 99 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 9ec29a26d..6f234c61a 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -270,6 +270,15 @@ impl UnvalidatedProtoEntry { self.0.is_empty() } +pub fn len(&self) -> usize { +let mut total: usize = 0; + +for (_, versions) in self.iter() { +total += versions.len(); +} +total +} + /// Determine if we support every protocol a client supports, and if not, /// determine which protocols we do not have support for. /// @@ -467,120 +476,102 @@ impl From for UnvalidatedProtoEntry { } } -/// Protocol voting implementation. -/// -/// Given a list of strings describing protocol versions, return a new -/// string encoding all of the protocols that are listed by at -/// least threshold of the inputs. -/// -/// The string is sorted according to the following conventions: -/// - Protocols names are alphabetized -/// - Protocols are in order low to high -/// - Individual and ranges are listed together. For example, -/// "3, 5-10,13" -/// - All entries are unique +/// A mapping of protocols to a count of how many times each of their `Version`s +/// were voted for or supported. /// -/// # Examples -/// ``` -/// use protover::compute_vote; +/// # Warning /// -/// let protos = vec![String::from("Link=3-4"), String::from("Link=3")]; -/// let vote = compute_vote(protos, 2); -/// assert_eq!("Link=3", vote) -/// ``` -pub fn compute_vote( -list_of_proto_strings: Vec, -threshold: i32, -) -> String { -let empty = String::from(""); +/// The "protocols" are *not* guaranteed to be known/supported `Protocol`s, in +/// order to allow new subprotocols to be introduced even if Directory +/// Authorities don't yet know of them. +pub struct ProtoverVote( HashMap> ); -if list_of_proto_strings.is_empty() { -return empty; +impl Default for ProtoverVote { +fn default() -> ProtoverVote { +ProtoverVote( HashMap::new() ) } +} -// all_count is a structure to represent the count of the number of -// supported versions for a specific protocol. For example, in JSON format: -// { -// "FirstSupportedProtocol": { -// "1": "3", -// "2": "1" -// } -// } -// means that FirstSupportedProtocol has three votes which support version -// 1, and one vote that supports version 2 -let mut all_count: HashMap> = -HashMap::new(); - -// parse and collect all of the protos and their versions and collect them -for vote in list_of_proto_strings { -let this_vote: HashMap = -match parse_protocols_from_string_with_no_validation(&vote) { -
[tor-commits] [tor/master] tests: Run all existing protover tests in both languages.
commit 527a2398634ecf1c244422121653f431c8df94fc Author: Isis Lovecruft Date: Tue Mar 27 21:36:10 2018 + tests: Run all existing protover tests in both languages. There's now no difference in these tests w.r.t. the C or Rust: both fail miserably (well, Rust fails with nice descriptive errors, and C gives you a traceback, because, well, C). --- src/test/test_protover.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index e7e17efe3..7bf1471eb 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -283,23 +283,22 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Sleen=0-4294967294"); tor_free(msg); - /* If we get an unparseable list, we say "yes, that's supported." */ -#ifndef HAVE_RUST - // let's make this section unconditional: rust should behave the - // same as C here! + /* If we get a (barely) valid (but unsupported list, we say "yes, that's + * supported." */ + tt_assert(protover_all_supported("Fribble=", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Fribble", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); tor_end_capture_bugs_(); - /* This case is forbidden. Since it came from a protover_all_supported, - * it can trigger a bug message. */ + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); - tor_free(msg); tor_end_capture_bugs_(); -#endif done: tor_end_capture_bugs_(); ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3
commit 8d6b1da2e6729c0cf8331e663bdfeee5d5660237 Merge: 961d2ad59 b503df277 Author: Nick Mathewson Date: Tue Apr 3 15:29:29 2018 -0400 Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3 changes/bug24031| 13 + src/or/protover.c | 77 ++- src/rust/protover/errors.rs | 43 ++ src/rust/protover/ffi.rs| 94 ++- src/rust/protover/lib.rs|4 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 1265 --- src/rust/protover/tests/protover.rs | 421 +++- src/test/test_protover.c| 44 +- 9 files changed, 1690 insertions(+), 905 deletions(-) ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_list_supports_protocol().
commit 52c3ea50454bcbcbcf5b18e628a1490441962b33 Author: Isis Lovecruft Date: Wed Mar 21 02:52:04 2018 + rust: Refactor Rust impl of protover_list_supports_protocol(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types and methods. --- src/rust/protover/ffi.rs | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 1588b2e4d..0d0376e52 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -98,16 +98,18 @@ pub extern "C" fn protocol_list_supports_protocol( Ok(n) => n, Err(_) => return 1, }; - -let protocol = match translate_to_rust(c_protocol) { -Ok(n) => n, +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, Err(_) => return 0, }; - -let is_supported = -protover_string_supports_protocol(protocol_list, protocol, version); - -return if is_supported { 1 } else { 0 }; +let protocol: UnknownProtocol = match translate_to_rust(c_protocol) { +Ok(n) => n.into(), +Err(_) => return 0, +}; +match proto_entry.supports_protocol(&protocol, &version) { +false => return 0, +true => return 1, +} } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Refactor Rust impl of protover_list_supports_protocol().
commit 63eeda89ea11bf719ec6fddc7619994cc7f654ca Author: Isis Lovecruft Date: Wed Mar 21 02:52:04 2018 + rust: Refactor Rust impl of protover_list_supports_protocol(). This includes a subtle difference in behaviour, as in 4258f1e18, where we return (matching the C impl's return behaviour) earlier than before if parsing failed, saving us computation in parsing the versions into a protover::protoset::ProtoSet. * REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types and methods. --- src/rust/protover/ffi.rs | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index c17696803..d9365bdd7 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -101,16 +101,18 @@ pub extern "C" fn protocol_list_supports_protocol( Ok(n) => n, Err(_) => return 1, }; - -let protocol = match translate_to_rust(c_protocol) { -Ok(n) => n, +let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { +Ok(n) => n, Err(_) => return 0, }; - -let is_supported = -protover_string_supports_protocol(protocol_list, protocol, version); - -return if is_supported { 1 } else { 0 }; +let protocol: UnknownProtocol = match translate_to_rust(c_protocol) { +Ok(n) => n.into(), +Err(_) => return 0, +}; +match proto_entry.supports_protocol(&protocol, &version) { +false => return 0, +true => return 1, +} } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Refactor Rust impl of protover_all_supported().
commit c7bcca0233d1d4c9805f78da5e7186be2c1bcdca Author: Isis Lovecruft Date: Wed Mar 21 02:45:41 2018 + rust: Refactor Rust impl of protover_all_supported(). This includes differences in behaviour to before, which should now more closely match the C version: - If parsing a protover `char*` from C, and the string is not parseable, this function will return 1 early, which matches the C behaviour when protocols are unparseable. Previously, we would parse it and its version numbers simultaneously, i.e. there was no fail early option, causing us to spend more time unnecessarily parsing versions. * REFACTOR `protover::ffi::protover_all_supported()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index ce2837832..c17696803 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -59,19 +59,26 @@ pub extern "C" fn protover_all_supported( Err(_) => return 1, }; -let (is_supported, unsupported) = all_supported(relay_version); +let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() { +Ok(n) => n, +Err(_) => return 1, +}; +let maybe_unsupported: Option = relay_proto_entry.all_supported(); -if unsupported.len() > 0 { -let c_unsupported = match CString::new(unsupported) { +if maybe_unsupported.is_some() { +let unsupported: UnvalidatedProtoEntry = maybe_unsupported.unwrap(); +let c_unsupported: CString = match CString::new(unsupported.to_string()) { Ok(n) => n, Err(_) => return 1, }; let ptr = c_unsupported.into_raw(); unsafe { *missing_out = ptr }; + +return 0; } -return if is_supported { 1 } else { 0 }; +1 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Refactor Rust implementation of protover_is_supported_here().
commit fd127bfbfa13d407e5fb5d22a567f51a30af4c2e Author: Isis Lovecruft Date: Wed Mar 21 03:08:35 2018 + rust: Refactor Rust implementation of protover_is_supported_here(). It was changed to take borrows instead of taking ownership. * REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method signature on `protover::is_supported_here()`. --- src/rust/protover/ffi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 13d64821f..3632f5de8 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -218,7 +218,7 @@ pub extern "C" fn protover_is_supported_here( Err(_) => return 0, }; -let is_supported = is_supported_here(protocol, version); +let is_supported = is_supported_here(&protocol, &version); return if is_supported { 1 } else { 0 }; } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Port all C protover_all_supported tests to Rust.
commit 4b4e36a413bb5d0e2ea499dd6bc34b3d24bd3375 Author: Isis Lovecruft Date: Tue Mar 27 21:38:29 2018 + rust: Port all C protover_all_supported tests to Rust. The behaviours still do not match, unsurprisingly, but now we know where a primary difference is: the Rust is validating version ranges more than the C, so in the C it's possible to call protover_all_supported on a ridiculous version range like "Sleen=0-4294967294" because the C uses MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust uses it to count the total number of *versions* of all subprotocols. --- src/rust/protover/tests/protover.rs | 86 +++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 9f8b5a443..11015f35b 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -6,6 +6,7 @@ extern crate protover; use protover::ProtoEntry; use protover::ProtoverVote; use protover::UnvalidatedProtoEntry; +use protover::errors::ProtoverError; #[test] fn parse_protocol_with_single_proto_and_single_version() { @@ -320,9 +321,88 @@ fn protover_compute_vote_may_exceed_limit() { } #[test] -fn protover_all_supported_should_include_version_we_actually_do_support() { +fn protover_all_supported_should_exclude_versions_we_actually_do_support() { let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap(); -let _result: String = proto.all_supported().unwrap().to_string(); +let result: String = proto.all_supported().unwrap().to_string(); -assert_eq!(_result, "Link=3-999".to_string()); +assert_eq!(result, "Link=6-999".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() { +let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=345-666".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex2() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12".to_string()); +} + +#[test] +fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { +let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer() { +let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-2147483648".to_string()); +} + +#[test] +#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] +// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an +// UnvalidatedProtoEntry is defined as a Hashmap. +// Because it contains the ProtoSet part, there's still *some* validation +// happening, so in this case the DoS protections in the Rust code are kicking +// in because the range here is exceeds the maximum, so it returns an +// Err(ProtoverError::ExceedsMax). The C, however seems to parse it anyway. +fn protover_all_supported_should_not_dos_anyones_computer_max_versions() { +let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap(); +let result: String = proto.all_supported().unwrap().to_string(); + +assert_eq!(result, "Sleen=0-4294967294".to_string()); +} + +#[test] +// C_RUST_DIFFERS: The C will return true (e.g. saying "yes, that's supported") +// but set the msg to NULL (??? seems maybe potentially bad). The Rust will +// simply return a None. +fn protover_all_supported_should_return_empty_string_for_weird_thing() { +let proto: UnvalidatedProtoEntry = "Fribble=".parse().unwrap(); +let result: Option = proto.all_supported(); + +assert!(result.is_none()); +} + +#[test] +fn protover_unvalidatedprotoentry_should_err_entirely_unparseable_things() { +let proto: Result = "Fribble".parse(); + +assert_eq!(Err(ProtoverError::Unparseable), proto); +} + +#[test] +fn protover_all_supported_over_maximum_limit() {
[tor-commits] [tor/maint-0.3.3] add a missing word
commit 7ccb1c5a859873656ab074c88935865bcf4b4c38 Author: Nick Mathewson Date: Tue Apr 3 15:31:30 2018 -0400 add a missing word --- changes/bug24031 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/bug24031 b/changes/bug24031 index adffa46d8..2bb0e8309 100644 --- a/changes/bug24031 +++ b/changes/bug24031 @@ -10,4 +10,4 @@ in outside crates and avoid mistakenly passing an internal error string to C over the FFI boundary. Many tests were added, and some previous differences between the C and Rust implementations have been - remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. + remedied. Fixes bug 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.
commit c65088cb1943748412e1a390de655e20bdb28692 Author: Isis Lovecruft Date: Tue Mar 27 22:46:14 2018 + rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C. Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied in Rust to the maximum number of version (total, for all subprotocols). Whereas in C, it was being applied to the number of subprotocols that were allowed. This changes the Rust to match C's behaviour. --- src/rust/protover/protoset.rs | 14 +++--- src/rust/protover/protover.rs | 18 +++--- src/rust/protover/tests/protover.rs | 14 -- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs index f94e6299c..4afc50edf 100644 --- a/src/rust/protover/protoset.rs +++ b/src/rust/protover/protoset.rs @@ -8,7 +8,6 @@ use std::slice; use std::str::FromStr; use std::u32; -use protover::MAX_PROTOCOLS_TO_EXPAND; use errors::ProtoverError; /// A single version number. @@ -183,10 +182,6 @@ impl ProtoSet { last_high = high; } -if self.len() > MAX_PROTOCOLS_TO_EXPAND { -return Err(ProtoverError::ExceedsMax); -} - Ok(self) } @@ -317,9 +312,15 @@ impl FromStr for ProtoSet { /// assert!(protoset.contains(&5)); /// assert!(!protoset.contains(&10)); /// -/// // We can also equivalently call `ProtoSet::from_str` by doing: +/// // We can also equivalently call `ProtoSet::from_str` by doing (all +/// // implementations of `FromStr` can be called this way, this one isn't +/// // special): /// let protoset: ProtoSet = "4-6,12".parse()?; /// +/// // Calling it (either way) can take really large ranges (up to `u32::MAX`): +/// let protoset: ProtoSet = "1-7".parse()?; +/// let protoset: ProtoSet = "1-4294967296".parse()?; +/// /// // There are lots of ways to get an `Err` from this function. Here are /// // a few: /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("=")); @@ -327,7 +328,6 @@ impl FromStr for ProtoSet { /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-")); /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4")); -/// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-7")); /// /// // Things which would get parsed into an _empty_ `ProtoSet` are, /// // however, legal, and result in an empty `ProtoSet`: diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index fc89f70d4..5e5a31cd3 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// before concluding that someone is trying to DoS us /// /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` -pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); +const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16); /// Currently supported protocols and their versions, as a byte-slice. /// @@ -166,6 +166,10 @@ impl ProtoEntry { supported.parse() } +pub fn len(&self) -> usize { +self.0.len() +} + pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> { self.0.get(protocol) } @@ -220,8 +224,11 @@ impl FromStr for ProtoEntry { let proto_name: Protocol = proto.parse()?; proto_entry.insert(proto_name, versions); -} +if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND { +return Err(ProtoverError::ExceedsMax); +} +} Ok(proto_entry) } } @@ -738,8 +745,13 @@ mod test { } #[test] +fn test_protoentry_from_str_allowed_number_of_versions() { +assert_protoentry_is_parseable!("Desc=1-4294967294"); +} + +#[test] fn test_protoentry_from_str_too_many_versions() { -assert_protoentry_is_unparseable!("Desc=1-65537"); +assert_protoentry_is_unparseable!("Desc=1-4294967295"); } #[test] diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs index 11015f35b..2db01a163 100644 --- a/src/rust/protover/tests/protover.rs +++ b/src/rust/protover/tests/protover.rs @@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() { } #[test] -#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")] -// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an -// UnvalidatedProtoEntry is defined as a Hashmap. -// Because it contains the ProtoSet part, there's still *some* validation -// happening, so in this case the DoS protections in the Rust code are kicking -// in because the range here is exceeds the m
[tor-commits] [tor/master] rust: Refactor Rust impl of protover_all_supported().
commit 2f3a7376c06e39942ac3c4d447ca94759f12a5c3 Author: Isis Lovecruft Date: Wed Mar 21 02:45:41 2018 + rust: Refactor Rust impl of protover_all_supported(). This includes differences in behaviour to before, which should now more closely match the C version: - If parsing a protover `char*` from C, and the string is not parseable, this function will return 1 early, which matches the C behaviour when protocols are unparseable. Previously, we would parse it and its version numbers simultaneously, i.e. there was no fail early option, causing us to spend more time unnecessarily parsing versions. * REFACTOR `protover::ffi::protover_all_supported()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 5ecbc2969..1588b2e4d 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -56,19 +56,26 @@ pub extern "C" fn protover_all_supported( Err(_) => return 1, }; -let (is_supported, unsupported) = all_supported(relay_version); +let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() { +Ok(n) => n, +Err(_) => return 1, +}; +let maybe_unsupported: Option = relay_proto_entry.all_supported(); -if unsupported.len() > 0 { -let c_unsupported = match CString::new(unsupported) { +if maybe_unsupported.is_some() { +let unsupported: UnvalidatedProtoEntry = maybe_unsupported.unwrap(); +let c_unsupported: CString = match CString::new(unsupported.to_string()) { Ok(n) => n, Err(_) => return 1, }; let ptr = c_unsupported.into_raw(); unsafe { *missing_out = ptr }; + +return 0; } -return if is_supported { 1 } else { 0 }; +1 } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Add new protover::UnknownProtocol type.
commit 60daaa68b153cdca6d48b09f1951d6ba580609e5 Author: Isis Lovecruft Date: Wed Mar 21 01:07:18 2018 + rust: Add new protover::UnknownProtocol type. * ADD new type, protover::UnknownProtocol, so that we have greater type safety and our protover functionality which works with unsanitised protocol names is more clearly demarcated. * REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new protover::UnknownProtocol type name. * ADD a utility conversion of `impl From for UnknownProtocol` so that we can easily with known protocols and unknown protocols simultaneously (e.g. doing comparisons, checking their version numbers), while not allowing UnknownProtocols to be accidentally used in functions which should only take Protocols. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/ffi.rs | 30 src/rust/protover/protover.rs | 53 +++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 2ee0286ec..ce2837832 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -9,30 +9,32 @@ use libc::{c_char, c_int, uint32_t}; use std::ffi::CStr; use std::ffi::CString; -use protover::*; use smartlist::*; use tor_allocate::allocate_and_copy_string; use tor_util::strings::byte_slice_is_c_like; use tor_util::strings::empty_static_cstr; +use errors::ProtoverError; +use protover::*; + /// Translate C enums to Rust Proto enums, using the integer value of the C -/// enum to map to its associated Rust enum +/// enum to map to its associated Rust enum. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -fn translate_to_rust(c_proto: uint32_t) -> Result { +fn translate_to_rust(c_proto: uint32_t) -> Result { match c_proto { -0 => Ok(Proto::Link), -1 => Ok(Proto::LinkAuth), -2 => Ok(Proto::Relay), -3 => Ok(Proto::DirCache), -4 => Ok(Proto::HSDir), -5 => Ok(Proto::HSIntro), -6 => Ok(Proto::HSRend), -7 => Ok(Proto::Desc), -8 => Ok(Proto::Microdesc), -9 => Ok(Proto::Cons), -_ => Err("Invalid protocol type"), +0 => Ok(Protocol::Link), +1 => Ok(Protocol::LinkAuth), +2 => Ok(Protocol::Relay), +3 => Ok(Protocol::DirCache), +4 => Ok(Protocol::HSDir), +5 => Ok(Protocol::HSIntro), +6 => Ok(Protocol::HSRend), +7 => Ok(Protocol::Desc), +8 => Ok(Protocol::Microdesc), +9 => Ok(Protocol::Cons), +_ => Err(ProtoverError::UnknownProtocol), } } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index d74ca00c1..1ccad4af4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -56,8 +56,8 @@ pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] = /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` -#[derive(Hash, Eq, PartialEq, Debug)] -pub enum Proto { +#[derive(Clone, Hash, Eq, PartialEq, Debug)] +pub enum Protocol { Cons, Desc, DirCache, @@ -70,7 +70,7 @@ pub enum Proto { Relay, } -impl fmt::Display for Proto { +impl fmt::Display for Protocol { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self) } @@ -80,26 +80,51 @@ impl fmt::Display for Proto { /// Error if the string is an unrecognized protocol name. /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` -impl FromStr for Proto { +impl FromStr for Protocol { type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { -"Cons" => Ok(Proto::Cons), -"Desc" => Ok(Proto::Desc), -"DirCache" => Ok(Proto::DirCache), -"HSDir" => Ok(Proto::HSDir), -"HSIntro" => Ok(Proto::HSIntro), -"HSRend" => Ok(Proto::HSRend), -"Link" => Ok(Proto::Link), -"LinkAuth" => Ok(Proto::LinkAuth), -"Microdesc" => Ok(Proto::Microdesc), -"Relay" => Ok(Proto::Relay), +"Cons" => Ok(Protocol::Cons), +"Desc" => Ok(Protocol::Desc), +"DirCache" => Ok(Protocol::DirCache), +"HSDir" => Ok(Protocol::HSDir), +"HSIntro" => Ok(Protocol::HSIntro), +"HSRend" => Ok(Protocol::HSRend), +"Link" => Ok(Protocol::Link), +"LinkAuth" => Ok(Protocol::LinkAuth), +"Microdesc" => Ok(Protocol::Microdesc), +"Relay" => Ok(Protocol::Relay), _ => Err(ProtoverError::UnknownProtocol), } } } +/// A protocol string which is not one of the `Protocols` we currently know +/// about. +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct UnknownProtocol(String); + +impl fmt:
[tor-commits] [tor/maint-0.3.3] rust: Refactor Rust impl of protover_compute_vote().
commit 32638ed4a6c47a03b899cd09582888674ae3bf08 Author: Isis Lovecruft Date: Wed Mar 21 03:05:56 2018 + rust: Refactor Rust impl of protover_compute_vote(). This includes a subtle difference in behaviour to the previous Rust implementation, where, for each vote that we're computing over, if a single one fails to parse, we skip it. This now matches the current behaviour in the C implementation. * REFACTOR `protover::ffi::protover_compute_vote()` to use new types and methods. --- src/rust/protover/ffi.rs | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index e7c821116..13d64821f 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -175,6 +175,8 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char { /// Provide an interface for C to translate arguments and return types for /// protover::compute_vote +// +// Why is the threshold a signed integer? âisis #[no_mangle] pub extern "C" fn protover_compute_vote( list: *const Stringlist, @@ -189,10 +191,19 @@ pub extern "C" fn protover_compute_vote( // Dereference of raw pointer requires an unsafe block. The pointer is // checked above to ensure it is not null. let data: Vec = unsafe { (*list).get_list() }; +let hold: usize = threshold as usize; +let mut proto_entries: Vec = Vec::new(); -let vote = compute_vote(data, threshold); +for datum in data { +let entry: UnvalidatedProtoEntry = match datum.parse() { +Ok(x) => x, +Err(_) => continue, +}; +proto_entries.push(entry); +} +let vote: UnvalidatedProtoEntry = ProtoverVote::compute(&proto_entries, &hold); -allocate_and_copy_string(&vote) +allocate_and_copy_string(&vote.to_string()) } /// Provide an interface for C to translate arguments and return types for ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Refactor protover::compute_for_old_tor().
commit cd28b4c7f5cefd319d6ded635d25911b4323b50b Author: Isis Lovecruft Date: Tue Mar 27 02:41:25 2018 + rust: Refactor protover::compute_for_old_tor(). During code review and discussion with Chelsea Komlo, she pointed out that protover::compute_for_old_tor() was a public function whose return type was `&'static CStr`. We both agree that C-like parts of APIs should: 1. not be exposed publicly (to other Rust crates), 2. only be called in the appropriate FFI code, 3. not expose types which are meant for FFI code (e.g. `*mut char`, `CString`, `*const c_int`, etc.) to the pure-Rust code of other crates. 4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called from pure-Rust, not even from other modules in its own crate (i.e. do not call `protover::ffi::*` from anywhere in `protover::protoset::*`, etc). With that in mind, this commit makes the following changes: * CHANGE `protover::compute_for_old_tor()` to be visible only at the `pub(crate)` level. * RENAME `protover::compute_for_old_tor()` to `protover::compute_for_old_tor_cstr()` to reflect the last change. * ADD a new `protover::compute_for_old_tor()` function wrapper which is public and intended for other Rust code to use, which returns a `&str`. --- src/rust/protover/ffi.rs | 2 +- src/rust/protover/protover.rs | 58 +-- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 3632f5de8..a40353eb1 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -246,7 +246,7 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const Err(_) => return empty.as_ptr(), }; -elder_protocols = compute_for_old_tor(&version); +elder_protocols = compute_for_old_tor_cstr(&version); // If we're going to pass it to C, there cannot be any intermediate NUL // bytes. An assert is okay here, since changing the const byte slice diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 96e9dd582..fc89f70d4 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -611,8 +611,8 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { supported_versions.contains(vers) } -/// Older versions of Tor cannot infer their own subprotocols -/// Used to determine which subprotocols are supported by older Tor versions. +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. /// /// # Inputs /// @@ -626,24 +626,28 @@ pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { /// "HSDir=1-1 LinkAuth=1" /// /// This function returns the protocols that are supported by the version input, -/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS. +/// only for tor versions older than `FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS` +/// (but not older than 0.2.4.19). For newer tors (or older than 0.2.4.19), it +/// returns an empty string. /// -/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` -pub fn compute_for_old_tor(version: &str) -> &'static [u8] { +/// # Note +/// +/// This function is meant to be called for/within FFI code. If you'd +/// like to use this code in Rust, please see `compute_for_old_tor()`. +// +// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor` +pub(crate) fn compute_for_old_tor_cstr(version: &str) -> &'static [u8] { if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) { return NUL_BYTE; } - if c_tor_version_as_new_as(version, "0.2.9.1-alpha") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.7.5") { return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0"; } - if c_tor_version_as_new_as(version, "0.2.4.19") { return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \ Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0"; @@ -652,6 +656,44 @@ pub fn compute_for_old_tor(version: &str) -> &'static [u8] { NUL_BYTE } +/// Since older versions of Tor cannot infer their own subprotocols, +/// determine which subprotocols are supported by older Tor versions. +/// +/// # Inputs +/// +/// * `version`, a string comprised of "[0-9a-z.-]" +/// +/// # Returns +/// +/// A `Result` whose `Ok` value is an `&'static str` encoding a list of protocol +/// names and supported versions. The string takes the following format: +/// +/// "HSDir=1-1 LinkAuth=1" +/// +/// This function returns the protocols that are supported by
[tor-commits] [tor/maint-0.3.3] tests: Run all existing protover tests in both languages.
commit 6739a69c590a12af0f1cb2af62972f4305803670 Author: Isis Lovecruft Date: Tue Mar 27 21:36:10 2018 + tests: Run all existing protover tests in both languages. There's now no difference in these tests w.r.t. the C or Rust: both fail miserably (well, Rust fails with nice descriptive errors, and C gives you a traceback, because, well, C). --- src/test/test_protover.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index e7e17efe3..7bf1471eb 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -283,23 +283,22 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Sleen=0-4294967294"); tor_free(msg); - /* If we get an unparseable list, we say "yes, that's supported." */ -#ifndef HAVE_RUST - // let's make this section unconditional: rust should behave the - // same as C here! + /* If we get a (barely) valid (but unsupported list, we say "yes, that's + * supported." */ + tt_assert(protover_all_supported("Fribble=", &msg)); + tt_ptr_op(msg, OP_EQ, NULL); + + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Fribble", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); tor_end_capture_bugs_(); - /* This case is forbidden. Since it came from a protover_all_supported, - * it can trigger a bug message. */ + /* If we get a completely unparseable list, protover_all_supported should + * hit a fatal assertion for BUG(entries == NULL). */ tor_capture_bugs_(1); tt_assert(protover_all_supported("Sleen=0-4294967295", &msg)); - tt_ptr_op(msg, OP_EQ, NULL); - tor_free(msg); tor_end_capture_bugs_(); -#endif done: tor_end_capture_bugs_(); ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Implement error types for Rust protover implementation.
commit 88204f91df3e01b69e79b89ba029b42a4025d11f Author: Isis Lovecruft Date: Wed Mar 21 00:24:46 2018 + rust: Implement error types for Rust protover implementation. This will allow us to do actual error handling intra-crate in a more rusty manner, e.g. propogating errors in match statements, conversion between error types, logging messages, etc. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/errors.rs | 43 +++ src/rust/protover/lib.rs | 3 +++ src/rust/protover/protover.rs | 6 -- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs new file mode 100644 index 0..56473d12e --- /dev/null +++ b/src/rust/protover/errors.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Various errors which may occur during protocol version parsing. + +use std::fmt; +use std::fmt::Display; + +/// All errors which may occur during protover parsing routines. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[allow(missing_docs)] // See Display impl for error descriptions +pub enum ProtoverError { +Overlap, +LowGreaterThanHigh, +Unparseable, +ExceedsMax, +ExceedsExpansionLimit, +UnknownProtocol, +ExceedsNameLimit, +} + +/// Descriptive error messages for `ProtoverError` variants. +impl Display for ProtoverError { +fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +match *self { +ProtoverError::Overlap +=> write!(f, "Two or more (low, high) protover ranges would overlap once expanded."), +ProtoverError::LowGreaterThanHigh +=> write!(f, "The low in a (low, high) protover range was greater than high."), +ProtoverError::Unparseable +=> write!(f, "The protover string was unparseable."), +ProtoverError::ExceedsMax +=> write!(f, "The high in a (low, high) protover range exceeds u32::MAX."), +ProtoverError::ExceedsExpansionLimit +=> write!(f, "The protover string would exceed the maximum expansion limit."), +ProtoverError::UnknownProtocol +=> 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."), +} +} +} diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index fe8c0f9bb..371d1ae58 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -22,12 +22,15 @@ //! protocols to develop independently, without having to claim compatibility //! with specific versions of Tor. +#[deny(missing_docs)] + extern crate libc; extern crate smartlist; extern crate external; extern crate tor_allocate; extern crate tor_util; +pub mod errors; mod protover; pub mod ffi; diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index e5dc69b9a..8ce182cf1 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -13,6 +13,8 @@ use std::u32; use tor_util::strings::NUL_BYTE; +use errors::ProtoverError; + /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// @@ -78,7 +80,7 @@ impl fmt::Display for Proto { /// /// C_RUST_COUPLED: src/or/protover.c `PROTOCOL_NAMES` impl FromStr for Proto { -type Err = &'static str; +type Err = ProtoverError; fn from_str(s: &str) -> Result { match s { @@ -92,7 +94,7 @@ impl FromStr for Proto { "LinkAuth" => Ok(Proto::LinkAuth), "Microdesc" => Ok(Proto::Microdesc), "Relay" => Ok(Proto::Relay), -_ => Err("Not a valid protocol type"), +_ => Err(ProtoverError::UnknownProtocol), } } } ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] rust: Implement more memory-efficient protover datatype.
commit e6625113c98c281b0a649598d7daa347c28915e9 Author: Isis Lovecruft Date: Wed Mar 21 00:43:55 2018 + rust: Implement more memory-efficient protover datatype. * ADD new protover::protoset module. * ADD new protover::protoset::ProtoSet class for holding protover versions. * REMOVE protover::Versions type implementation and its method `from_version_string()`, and instead implement this behaviour in a more rust-like manner as `impl FromStr for ProtoSet`. * MOVE the `find_range()` utility function from protover::protover to protover::protoset since it's only used internally in the implementation of ProtoSet. * REMOVE the `contract_protocol_list()` function from protover::protover and instead refactor it (reusing nearly the entire thing, with minor superficial, i.e. non-behavioural, changes) into a more rusty `impl ToString for ProtoSet`. * REMOVE the `expand_version_range()` function from protover::protover and instead refactor it into a more rusty implementation of `impl Into> for ProtoSet` using the new error types in protover::errors. * FIXES part of #24031: https://bugs.torproject.org/24031. --- src/rust/protover/lib.rs | 1 + src/rust/protover/protoset.rs | 634 ++ src/rust/protover/protover.rs | 215 +- 3 files changed, 641 insertions(+), 209 deletions(-) diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs index 371d1ae58..483260bca 100644 --- a/src/rust/protover/lib.rs +++ b/src/rust/protover/lib.rs @@ -31,6 +31,7 @@ extern crate tor_allocate; extern crate tor_util; pub mod errors; +pub mod protoset; mod protover; pub mod ffi; diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs new file mode 100644 index 0..f94e6299c --- /dev/null +++ b/src/rust/protover/protoset.rs @@ -0,0 +1,634 @@ +// Copyright (c) 2018, The Tor Project, Inc. +// Copyright (c) 2018, isis agora lovecruft +// See LICENSE for licensing information + +//! Sets for lazily storing ordered, non-overlapping ranges of integers. + +use std::slice; +use std::str::FromStr; +use std::u32; + +use protover::MAX_PROTOCOLS_TO_EXPAND; +use errors::ProtoverError; + +/// A single version number. +pub type Version = u32; + +/// A `ProtoSet` stores an ordered `Vec` of `(low, high)` pairs of ranges of +/// non-overlapping protocol versions. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// +/// use protover::errors::ProtoverError; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// +/// # fn do_test() -> Result { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,8")?; +/// +/// // We could also equivalently call: +/// let protoset: ProtoSet = "3-5,8".parse()?; +/// +/// assert!(protoset.contains(&4)); +/// assert!(!protoset.contains(&7)); +/// +/// let expanded: Vec = protoset.clone().into(); +/// +/// assert_eq!(&expanded[..], &[3, 4, 5, 8]); +/// +/// let contracted: String = protoset.clone().to_string(); +/// +/// assert_eq!(contracted, "3-5,8".to_string()); +/// # Ok(protoset) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct ProtoSet { +pub(crate) pairs: Vec<(Version, Version)>, +} + +impl Default for ProtoSet { +fn default() -> Self { +let pairs: Vec<(Version, Version)> = Vec::new(); + +ProtoSet{ pairs } +} +} + +impl<'a> ProtoSet { +/// Create a new `ProtoSet` from a slice of `(low, high)` pairs. +/// +/// # Inputs +/// +/// We do not assume the input pairs are deduplicated or ordered. +pub fn from_slice(low_high_pairs: &'a [(Version, Version)]) -> Result { +let mut pairs: Vec<(Version, Version)> = Vec::with_capacity(low_high_pairs.len()); + +for &(low, high) in low_high_pairs { +pairs.push((low, high)); +} +// Sort the pairs without reallocation and remove all duplicate pairs. +pairs.sort_unstable(); +pairs.dedup(); + +ProtoSet{ pairs }.is_ok() +} +} + +/// Expand this `ProtoSet` to a `Vec` of all its `Version`s. +/// +/// # Examples +/// +/// ``` +/// use std::str::FromStr; +/// use protover::protoset::ProtoSet; +/// use protover::protoset::Version; +/// # use protover::errors::ProtoverError; +/// +/// # fn do_test() -> Result, ProtoverError> { +/// let protoset: ProtoSet = ProtoSet::from_str("3-5,21")?; +/// let versions: Vec = protoset.into(); +/// +/// assert_eq!(&versions[..], &[3, 4, 5, 21]); +/// # +/// # Ok(versions) +/// # } +/// # fn main() { do_test(); } // wrap the test so we can use the ? operator +/// ``` +impl Into> for ProtoSet { +fn into(self) -> Vec { +let mut versions: Vec = Vec::new(); + +for &(low, high) in self.iter() { +versions.extend(low..high + 1); +} +versions +} +}
[tor-commits] [tor/maint-0.3.3] rust: Refactor protover::is_supported_here().
commit 35b86a12e60a8696b512261e96dc6f1c75ecebfc Author: Isis Lovecruft Date: Wed Mar 21 02:09:04 2018 + rust: Refactor protover::is_supported_here(). This changes `protover::is_supported_here()` to be aware of new datatypes (e.g. don't call `.0` on things which are no longer tuple structs) and also changes the method signature to take borrows, making it faster, threadable, and easier to read (i.e. the caller can know from reading the function signature that the function won't mutate values passed into it). * CHANGE the `protover::is_supported_here()` function to take borrows. * REFACTOR the `protover::is_supported_here()` function to be aware of new datatypes. * FIXES part of #24031: https://bugs.torproject.org/24031 --- src/rust/protover/protover.rs | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 6f1ad768e..247166c23 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -590,26 +590,25 @@ impl ProtoverVote { /// /// # Examples /// ``` -/// use protover::*; +/// use protover::is_supported_here; +/// use protover::Protocol; /// -/// let is_supported = is_supported_here(Proto::Link, 10); +/// let is_supported = is_supported_here(&Protocol::Link, &10); /// assert_eq!(false, is_supported); /// -/// let is_supported = is_supported_here(Proto::Link, 1); +/// let is_supported = is_supported_here(&Protocol::Link, &1); /// assert_eq!(true, is_supported); /// ``` -pub fn is_supported_here(proto: Proto, vers: Version) -> bool { -let currently_supported = match SupportedProtocols::tor_supported() { -Ok(result) => result.0, +pub fn is_supported_here(proto: &Protocol, vers: &Version) -> bool { +let currently_supported: ProtoEntry = match ProtoEntry::supported() { +Ok(result) => result, Err(_) => return false, }; - -let supported_versions = match currently_supported.get(&proto) { +let supported_versions = match currently_supported.get(proto) { Some(n) => n, None => return false, }; - -supported_versions.0.contains(&vers) +supported_versions.contains(vers) } /// Older versions of Tor cannot infer their own subprotocols ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] tests: Make inline comments in test_protover.c more accurate.
commit f769edd148bfbb381a48217e9016902f036b9ed8 Author: Isis Lovecruft Date: Tue Mar 27 21:34:00 2018 + tests: Make inline comments in test_protover.c more accurate. The DoS potential is slightly higher in C now due to some differences to the Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs. Also, the comment about "failing at the splitting stage" in Rust wasn't true, since when we split, we ignore empty chunks (e.g. "1--1" parses into "(1,None),(None,1)" and "None" can't be parsed into an integer). Finally, the comment about "Rust seems to experience an internal error" is only true in debug mode, where u32s are bounds-checked at runtime. In release mode, code expressing the equivalent of this test will error with `Err(ProtoverError::Unparseable)` because 4294967295 is too large. --- src/test/test_protover.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/test_protover.c b/src/test/test_protover.c index 95cc5f083..e7e17efe3 100644 --- a/src/test/test_protover.c +++ b/src/test/test_protover.c @@ -273,7 +273,7 @@ test_protover_all_supported(void *arg) tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001"); tor_free(msg); - /* CPU/RAM DoS loop: Rust only */ + /* We shouldn't be able to DoS ourselves parsing a large range. */ tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg)); tt_str_op(msg, OP_EQ, "Sleen=0-2147483648"); tor_free(msg); @@ -546,8 +546,6 @@ test_protover_vote_roundtrip(void *args) { "Link=1,9-8,3", NULL }, { "Faux=-0", NULL }, { "Faux=0--0", NULL }, -// "These fail at the splitting stage in Rust, but the number parsing -// stage in C." { "Faux=-1", NULL }, { "Faux=-1-3", NULL }, { "Faux=1--1", NULL }, @@ -556,9 +554,9 @@ test_protover_vote_roundtrip(void *args) /* Large range */ { "Sleen=1-501", "Sleen=1-501" }, { "Sleen=1-65537", NULL }, -/* CPU/RAM DoS Loop: Rust only. */ +/* Both C/Rust implementations should be able to handle this mild DoS. */ { "Sleen=0-2147483648", NULL }, -/* Rust seems to experience an internal error here. */ +/* Rust tests are built in debug mode, so ints are bounds-checked. */ { "Sleen=0-4294967295", NULL }, }; unsigned u; ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
[tor-commits] [tor/maint-0.3.3] changes: Add changes file for #24031.
commit b503df2775d22cff0b74740357121ba5195e4a12 Author: Isis Lovecruft Date: Tue Apr 3 19:19:40 2018 + changes: Add changes file for #24031. (cherry picked from commit 5a8cdec3f8617920f19e3ab7707233ad3f02424f) --- changes/bug24031 | 13 + 1 file changed, 13 insertions(+) diff --git a/changes/bug24031 b/changes/bug24031 new file mode 100644 index 0..adffa46d8 --- /dev/null +++ b/changes/bug24031 @@ -0,0 +1,13 @@ + o Major bugfixes (protover, voting): +- Revise Rust implementation of protover to use a more memory-efficient + voting algorithm and corresponding data structures, thus avoiding a + potential (but small impact) DoS attack where specially crafted protocol + strings would expand to several potential megabytes in memory. In the + process, several portions of code were revised to be methods on new, + custom types, rather than functions taking interchangeable types, thus + increasing type safety of the module. Custom error types and handling + were added as well, in order to facilitate better error dismissal/handling + in outside crates and avoid mistakenly passing an internal error string to + C over the FFI boundary. Many tests were added, and some previous + differences between the C and Rust implementations have been + remedied. Fixes 24031; bugfix on 0.3.3.1-alpha. ___ tor-commits mailing list tor-commits@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits