Double-check Rust code for potential proxy bypass in ESR 60
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.
Activity
Trac:
Parent: N/A to #22176 (moved)-
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.
-
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-testsgets specified, which we do. 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/geckolibFor each of those packages, I ran
$ grep -rni -E "tcp|udp|socket|bind|connect|listener|send|recv|stream" $p(where
$pwas each directory path from above).Many of the results were false-positives. In particular,
bindmatched 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/geckolibmedia/mp4parse-rust/mp4parse_capihas 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 ofstreamare from audio (FLAC) track information.xpcom/rust/xpcom/xpcom_macroshas a occurrence of "bind" and a few instances of "stream". "bind" is related to FFI, and "stream" areTokenStreams.dom/webauthn/u2f-hid-rshas "send" and "recv", but these are methods called on astd::sync::mpsc::channel. There is another wrapper methodsendrecvthat callsU2FHIDCont::writeandU2FHIDInit::readfor 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.)
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_tomlStep 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_cratesStep 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_cratesStep 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 -RThis 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)
-
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.
-
We have kind of a double-check with tjr's work over in https://bugzilla.mozilla.org/show_bug.cgi?id=1376621. That needs to be enough for now, given that esr68 got just released and we need to prepare for that one.
Trac:
Status: new to closed
Resolution: N/A to fixed