[tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-team
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor |Version:
  Browser|   Keywords:  ff52-esr,
 Severity:  Normal   |  TorBrowserTeam201704, tbb-7.0-must-
 |  alpha
Actual Points:   |  Parent ID:
   Points:   |   Reviewer:
  Sponsor:  Sponsor4 |
-+-
 Even though we don't use any Rust in ESR 52 we should get rid of proxy
 bypassing calls for safety and to remind us for later:

 third_party/rust/url/src/host.rs has a to_socket_addrs and ToSocketAddrs
 methods.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, TorBrowserTeam201704,  |  Actual Points:
  tbb-7.0-must-alpha |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by cypherpunks):

 Are you compiling with {{{--enable-rust}}} so that those calls are
 available?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, TorBrowserTeam201704,  |  Actual Points:
  tbb-7.0-must-alpha |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by gk):

 No.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, TorBrowserTeam201704,  |  Actual Points:
  tbb-7.0-must-alpha |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by cypherpunks):

 Then what rust code is in ESR52?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, TorBrowserTeam201705R, |  Actual Points:
  tbb-7.0-must   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-
Changes (by gk):

 * status:  new => needs_review
 * keywords:  ff52-esr, TorBrowserTeam201705, tbb-7.0-must => ff52-esr,
 TorBrowserTeam201705R, tbb-7.0-must


Comment:

 `bug_21862` (https://gitweb.torproject.org/user/gk/tor-
 browser.git/commit/?h=bug_21862&id=44fc4b2675d36b79d6d0cf374d4fcc74aebb1f1f)
 has a patch for review.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by mcs):

 r=mcs
 I don't read or write Rust, but your changes look okay. Maybe we should
 ask a Rust person to look at them? Certainly your patch is good enough to
 serve as a reminder that we need to look at the Rust code more closely
 once we start compiling it.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by arthuredelstein):

 I don't know rust either, but I was curious what happens if these
 functions are ripped out. Is any code calling them?

 I tried building with `ac_add_options --enable-rust` in the mozconfig file
 and I got the follow error message:

 {{{
  2:45.75 error: the listed checksum of `/home/arthur/tor-
 browser/third_party/rust/url/src/lib.rs` has changed:
  2:45.75 expected:
 c3542aabc733f76a3b9c7d24b5c41e60e9eb84d2712660611300d1de0e7c2072
  2:45.75 actual:
 3abae55faf84f05be573d6275fa49ca2e61a4a95e3717a9059e3971b5d53101a
  2:45.75
  2:45.75 directory sources are not intended to be edited, if modifications
 are required then it is recommended that [replace] is used with a forked
 copy of the source
  2:45.76 /home/arthur/tor-browser/config/rules.mk:939: recipe for target
 'force-cargo-build' failed
  2:45.76 make[5]: *** [force-cargo-build] Error 101
  2:45.76 /home/arthur/tor-browser/config/recurse.mk:71: recipe for target
 'toolkit/library/rust/target' failed
  2:45.76 make[4]: *** [toolkit/library/rust/target] Error 2
  2:45.76 make[4]: *** Waiting for unfinished jobs
 }}}

 The "expected" hash (c3542aab...) is located in `third_party/rust/url
 /.cargo-checksum.json`. The README in the same directory says this code is
 a
 {{{
 URL library for Rust, based on the [URL
 Standard](https://url.spec.whatwg.org/).
 }}}

 So it looks to me like this is patching a "third-party library", whereas
 we should probably be ripping out something considered to be "first-party"
 gecko code.

 Another option might be just to remove the whole third-party directory or
 even all rust files from the source code.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-
Changes (by gk):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Replying to [comment:9 arthuredelstein]:
 > So it looks to me like this is patching a "third-party library", whereas
 we should probably be ripping out something considered to be "first-party"
 gecko code.
 >
 > Another option might be just to remove the whole third-party directory
 or even all rust files from the source code.

 I think that is a good reminder of what we need to think about once
 building Rust parts is mandatory as this is different from the way we
 handled a lot of our proxy-bypass-(in-depth)-defenses so far. I think just
 for the sake of having those calls out of esr52 the current approach still
 seems to be sufficient. And if you can't get the stuff even compiled right
 now, even better. :) Thus, I take it as-is.

 Applied to `tor-browser-52.1.1esr-7.0-1` and `tor-browser-52.1.0-7.0-2`
 (commit 78aa6185cd8d1d11e09495f6e4dc5cbc19e80cba and
 88c25c56d96f8fc0801359358d808f0d0e7d4b93) with a changed commit message
 pointing out that we probably need a more elaborate approach once we
 compile with Rust enabled.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by manish.earth):

 In the next ESR (and already in the current release), `--enable-rust` will
 not exist.

 When we say "third party" code it means vendored dependencies necessary
 for building Firefox that are not directly maintained in the mozilla-
 central repo. This is not something that can/should be ripped out.
 Currently, the third_party directory is only used for vendored Rust
 dependencies, however other vendored code may move there in the future.

 In fact, rust-url is maintained by Servo, which is a Mozilla project.
 However, there are other crates under third_party which are not maintained
 by Mozilla. We intend to do some auditing before shipping. The currently
 shipping crates are all maintained by the Servo or Rust projects.


 Currently (as of a recent nightly), the Rust code in mozilla-central
 consists of:

  - The mp4 parser. Enabled by default. Should not hit the network.
  - Stylo (enabled recently); servo's style system in Firefox. Built by
 default, preffed off , planned to be preffed on on 57. Can be disabled; is
 a large chunk of code. May become mandatory a few release cycles after 57.
 This should not hit the network on its own, we thread through Gecko
 networking code.
  - Webrender. Built by default, preffed off. Should not hit the network.
  - encoding-rs. I believe this is built by default and preffed on, but I
 am unsure. Should not hit the network.
  - rust-url for parsing, serialization, and manipulation of URLs. built by
 default, preffed off. This is a currently-stalled experiment which won't
 be enabled before we make it work well with everything. While this itself
 shouldn't be hitting the network (it's only for dealing with the URL data,
 not making requests), it will be audited more rigorously before being
 enabled (right now it's just there for experimentation)
  - We unconditionally use rust-url's IPV6 parsing code as of
 https://bugzilla.mozilla.org/show_bug.cgi?id=1324243 . This code shouldn't
 hit the network.


 The call to `getaddrinfo` mentioned here exists because there is a
 `ToSocketAddrs` implementation for URL; an implementation which is never
 invoked.

 However, we can probably add a build time option to remove them.

 All the Rust code in Firefox gets built into
 `$objdir/toolkit/library/$target/(debug or release)/libgkrust.a`, so
 inspecting that binary may be useful in ensuring that getaddrinfo is never
 called. Currently none of the Rust code being used should be calling it.
 However, the standard library does include a call to getaddrinfo that goes
 unused, which still turns up in that file -- I'm not sure how to
 differentiate between used and unused here -- I tried creating a test Rust
 staticlib that didn't do anything and even with `-Clto` the final
 staticlib still had an unlinked getaddrinfo symbol. If y'all know the
 correct linker args and `nm` / `objdump` invocation to use, let me know
 and I'll run it on my local all-rust-enabled Firefox build.

 Let me know if you need more info!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-

Comment (by gk):

 Thanks, that's really helpful. For now we are good. We'll take a closer
 look at it during the transition to esr59 and will get back to you in case
 we need more info.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R, tbb-no-uplift   |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-
Changes (by arthuredelstein):

 * keywords:  ff52-esr, tbb-7.0-must, TorBrowserTeam201706R => ff52-esr,
 tbb-7.0-must, TorBrowserTeam201706R, tbb-no-uplift


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21862 [Applications/Tor Browser]: Make rust code in ESR 52 proxy safe

[Tor Bug Tracker & Wiki]

#21862: Make rust code in ESR 52 proxy safe
-+-
 Reporter:  gk   |  Owner:  tbb-
 |  team
 Type:  defect   | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  ff52-esr, tbb-7.0-must,  |  Actual Points:
  TorBrowserTeam201706R  |
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor4
-+-
Changes (by arthuredelstein):

 * keywords:  ff52-esr, tbb-7.0-must, TorBrowserTeam201706R, tbb-no-uplift
 => ff52-esr, tbb-7.0-must, TorBrowserTeam201706R


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs