Opened 5 months ago

Last modified 12 days ago

#27616 new task

Double-check Rust code for potential proxy bypass in ESR 60

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201902
Cc: sysrqb, manish.earth Actual Points:
Parent ID: #22176 Points:
Reviewer: Sponsor:

Description

I think we are good with respect to the Rust proxy adherence situation for ESR 60 but I think another set of (fresh) eyes could not hurt, given this is a lot of code that can in theory do nasty networking stuff and we can't rip it out as we would usually do.

Child Tickets

Change History (11)

comment:1 Changed 5 months ago by gk

Parent ID: #22176

comment:2 Changed 5 months ago by gk

See the second part of comment:15:ticket:22176 for how I tried to deal with the Rust proxy audit. I hope we find some more stream-lined approach during this ESR cycle and/or make progress on https://bugzilla.mozilla.org/show_bug.cgi?id=1376621 and related bugs.

comment:3 Changed 5 months ago by gk

FWIW and just to make a note: geckodriver is about to get built my default: https://bugzilla.mozilla.org/show_bug.cgi?id=1471281. It's not getting built during esr60 and it won't get built by default if --disable-tests gets specified, which we do.

comment:4 in reply to:  2 Changed 5 months ago by sysrqb

Replying to gk:

See the second part of comment:15:ticket:22176

Okay, I started with gk's 3) from that ticket. First, I enumerated all packages and their dependencies (not including the vendored crates). From these packages, I searched for all occurrences of "tcp", "udp", "socket", "bind", "connect", "listener", "send", "recv", and "stream". (I don't claim these are the only functions/methods that can be used for transmitting a message).

I found these are the in-tree packages (not vendored in third_party/rust):

media/mp4parse-rust/mp4parse_capi
servo/support/gecko/nsstring
xpcom/rust/nserror
netwerk/base/rust-helper
xpcom/rust/xpcom
xpcom/rust/xpcom/xpcom_macros
modules/libpref/parser
netwerk/base/rust-url-capi
dom/webauthn/u2f-hid-rs
servo/ports/geckolib

For each of those packages, I ran

$ grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" $p

(where $p was each directory path from above).

Many of the results were false-positives. In particular, bind matched many incstances of "binding" or "bindgen". So, excluding those:

$ grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" $p | grep -v -E "[bB]inding|[bB]indgen" | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream"

These directories didn't contain any matches:

servo/support/gecko/nsstring
xpcom/rust/nserror
netwerk/base/rust-helper
modules/libpref/parser
netwerk/base/rust-url-capi
servo/ports/geckolib

media/mp4parse-rust/mp4parse_capi has instances of "stream" (but that's not surprising considering it's doc comment says "Parses ISO Base Media Format aka video/mp4 streams."). All instances of stream are from audio (FLAC) track information.

xpcom/rust/xpcom/xpcom_macros has a occurrence of "bind" and a few instances of "stream". "bind" is related to FFI, and "stream" are TokenStreams.

dom/webauthn/u2f-hid-rs has "send" and "recv", but these are methods called on a std::sync::mpsc::channel. There is another wrapper method sendrecv that calls U2FHIDCont::write and U2FHIDInit::read for reading/writing the U2F device. These read/write methods specifically take a device as the first argument. Using this for making network calls seems very difficult (without digging too deep).

(to be continued.)

comment:5 Changed 5 months ago by gk

Keywords: TorBrowserTeam201810 added

comment:6 Changed 4 months ago by sysrqb

Okay. Back to this.

I took a slightly different approach.

Step 1. Find all Cargo.toml files starting from the root of the repo. These will be useful next when we must find where the vendored crate is located within the repo.

$ find . -name Cargo.toml > all_cargo_toml

Step 2. Find the package name within each Cargo.toml files - these are the crate names we'll need later. These are of the form path/to/Cargo.toml:name = "name-of-crate".

$ while read crate; do echo -n $crate:; grep -A4 '\[package\]' $crate | grep 'name ='; done < all_cargo_toml | grep 'toml:name =' > all_rust_crates

Step 3. From the list of crates, from the ones currently being used (or potentially being used)

$ while read crate; do grep "= \"$crate\"" all_rust_crates; done < rust_crates | sort > used_crates

Step 4. Search the used crates for expected proxy-bypass variables/functinos/methods/etc.

$ cut -d: -f 1 used_crates | sed 's/Cargo.toml//' | xargs grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | grep -v -E "[bB]inding|[bB]indgen" | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | less -R

This resulted in 15373 matches.

We can prevent 100 matches by excluding the directories audited in the previous comment.

$ cut -d: -f 1 used_crates | sed 's/Cargo.toml//' | xargs grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | grep -v -E "[bB]inding|[bB]indgen" | grep -v -e '^./media/mp4parse-rust/mp4parse_capi' -e '^./servo/support/gecko/nsstring' -e '^./xpcom/rust/nserror' -e '^./netwerk/base/rust-helper' -e '^./xpcom/rust/xpcom' -e '^./modules/libpref/parser' -e '^./netwerk/base/rust-url-capi' -e '^./dom/webauthn/u2f-hid-rs' -e '^servo/ports/geckolib' | grep -ni --color=always -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" | less -R

(to be to continued)

comment:7 Changed 4 months ago by gk

I realized that Mozilla imported a ton of updated crates and new ones for the 60.3.0esr release. I wondered whether our proposed audit strategy can cope with such a scenario as we ideally want to make sure that those updates don't include proxy bypasses. If not, it might be worth thinking about an audit strategy that *does* take such point release updates into account.

comment:8 Changed 3 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201810 removed

Moving our tickets to November.

comment:9 Changed 2 months ago by gk

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201811 removed

Moving our tickets to December.

comment:10 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam201901 added; TorBrowserTeam201812 removed

Moving tickets to Jan 2019.

comment:11 Changed 12 days ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201901 removed

Moving tickets to February.

Note: See TracTickets for help on using tickets.