On Mon, Jul 2, 2018 at 4:05 PM, Lars Bergstrom <larsb...@mozilla.com> wrote:
> Cool! I was just worried, especially since hyper and tokio/mio are > *already* vendored in-tree (though openssl is not yet). It appears they're > used for network access in some test tools and an audio ipc server > prototype. So we're not talking about a bunch of code "appearing" in > third-party, but rather people knowing that they can use certain components > in one part of the tree (presumably test/non-product code?) and not in > another. > Indeed. Since it probably won't be possible to ban such crates from being included for non-product code, I think it's going to be difficult to enforce rules around not using imported crates (or C++ libraries for that matter) for things like network access at the build system level... I think it's reasonable to expect reviewers to look for such issues. > This topic also isn't entirely academic, as I was in a conversation with > some folks at the SF all hands unaware of the issues with using a different > (Rust) network & TLS stack in some new code they are writing in their > Firefox modules. > - Lars > > > On Mon, Jul 2, 2018 at 1:23 PM, Bobby Holley <bobbyhol...@gmail.com> > wrote: > > > I think the key distinction here is that, unlike other rust-based > projects > > (i.e. Servo), Firefox vendors all cargo dependencies into the tree, so > it's > > more obvious to reviewers when a patch indirectly pulls in a new large > > dependency. In Servo the reviewer would have to look carefully at the > > Cargo.lock diff, where it's easier to miss. > > > > This does mean, however, that reviewers should _not_ just rubber stamp > any > > changes in third_party Rust. A line-by-line review may be too much to ask > > in some cases, but the reviewer should at least look at what's added and > > removed and make sure it's sane. > > > > On Sun, Jul 1, 2018 at 7:24 PM Eric Rescorla <e...@rtfm.com> wrote: > > > > > On Sun, Jul 1, 2018 at 4:56 PM, Xidorn Quan <m...@upsuper.org> wrote: > > > > > > > On Mon, Jul 2, 2018, at 9:03 AM, Eric Rescorla wrote: > > > > > On Sat, Jun 30, 2018 at 9:35 AM, Lars Bergstrom < > > larsb...@mozilla.com> > > > > > wrote: > > > > > > > > > > > On Fri, Jun 29, 2018 at 8:33 AM, Tom Ritter <t...@mozilla.com> > > wrote: > > > > > > > > > > > > > > > > > > > > I know that enumerating badness is never a comprehensive > > solution; > > > > but > > > > > > > maybe there could be a wiki page we could point people to for > > > things > > > > that > > > > > > > indicate something is doing something scary in Rust? This > might > > > let > > > > us > > > > > > > crowd-source these reviews in a safer manner. For example, what > > > > would I > > > > > > > look for in a crate to see if it was: > > > > > > > - Adjusting memory permissions > > > > > > > - Reading/writing to disk > > > > > > > - Performing unsafe C/C++ pointer stuff > > > > > > > - Performing network connections of any type > > > > > > > - Calling out to syscalls or other kernel functions > (especially > > > > > > win32k.sys > > > > > > > functions on Windows) > > > > > > > - (whatever else you can think of...) > > > > > > > <https://lists.mozilla.org/listinfo/dev-platform> > > > > > > > > > > > > > > > > > > > Building on that, is there a list of crates that should *never* > be > > > > > > included in Firefox that you could scan for? Such as, anything > that > > > is > > > > not > > > > > > nss (openssl bindings) or necko (use of a different network stack > > > that > > > > > > might not respect proxies, threading concerns, etc.)? > > > > > > > > > > Is this a crate-specific issue? Suppose that someone decided to > land > > > > > a new C++ networking stack, that would presumably also be bad but > > > > > should be caught in code review, no? > > > > > > > > The point is that adding a new crate dependency is too easy > > accidentally, > > > > and it is very possible for reviewers to overlook that. So it may > make > > > > sense to introduce a blacklist-ish thing to avoid that to happen. > > > > > > > > > > But in order for it to have an effect other than just cluttering up the > > > build, it would have to be wired into Firefox. And if reviewers don't > > > notice that, we have bigger problems. > > > > > > -Ekr > > > > > > - Xidorn > > > > _______________________________________________ > > > > dev-platform mailing list > > > > dev-platform@lists.mozilla.org > > > > https://lists.mozilla.org/listinfo/dev-platform > > > > > > > _______________________________________________ > > > dev-platform mailing list > > > dev-platform@lists.mozilla.org > > > https://lists.mozilla.org/listinfo/dev-platform > > > > > _______________________________________________ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > -- Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
