Skip to content
Snippets Groups Projects
  1. Apr 12, 2018
  2. Apr 11, 2018
  3. Apr 10, 2018
  4. Apr 09, 2018
  5. Apr 06, 2018
  6. Apr 05, 2018
  7. Apr 04, 2018
  8. Apr 03, 2018
  9. Apr 02, 2018
    • Isis Lovecruft's avatar
      rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C. · c65088cb
      Isis Lovecruft authored
      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.
      Unverified
      c65088cb
    • Isis Lovecruft's avatar
      rust: Port all C protover_all_supported tests to Rust. · 4b4e36a4
      Isis Lovecruft authored
      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.
      Unverified
      4b4e36a4
    • Isis Lovecruft's avatar
      tests: Run all existing protover tests in both languages. · 6739a69c
      Isis Lovecruft authored
      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).
      Unverified
      6739a69c
    • Isis Lovecruft's avatar
      tests: Make inline comments in test_protover.c more accurate. · f769edd1
      Isis Lovecruft authored
      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.
      Unverified
      f769edd1
    • Isis Lovecruft's avatar
      protover: Change protover_all_supported() to return only unsupported. · ad369313
      Isis Lovecruft authored
      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".
      Unverified
      ad369313
    • Isis Lovecruft's avatar
      rust: Refactor protover::compute_for_old_tor(). · cd28b4c7
      Isis Lovecruft authored
      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`.
      Unverified
      cd28b4c7
    • Isis Lovecruft's avatar
      rust: Refactor Rust implementation of protover_is_supported_here(). · fd127bfb
      Isis Lovecruft authored
      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()`.
      Unverified
      fd127bfb
    • Isis Lovecruft's avatar
      rust: Refactor Rust impl of protover_compute_vote(). · 32638ed4
      Isis Lovecruft authored
      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.
      Unverified
      32638ed4
Loading