Policy around external dependencies

[Hainish]

Type: other

To make it easier to review PRs like #13560 it seems to me there should be a policy for adding dependencies. I was thinking something lightweight, such as a checklist for maintainers to review. I wanted to get some input from the community to help shape this policy.

• Are there other projects that have particularly good policies you could point to?
• What should we be sure not to forget in such a policy?

A couple things I'm thinking about are:

• How does the addition of new dependencies or a dependency management system affect the attack surface of the extension?
• Dependencies that only are used by developers would have a lower bar.
• How should we pin versions?
• Should we preference stable releases?

For now let's leave the technical implementation out of the discussion and focus on the policy first. Please share your feedback in the next week and then @brainwane and @wonderchook will craft a checklist.

Thanks!

[jeremyn]

@Hainish Is this discussion strictly about JavaScript dependencies that get shipped with the extension, like CodeMirror in PR #13560, or does it also include build dependencies like those added in install-dev-dependencies.sh?

[Hainish]

@jeremyn I was thinking of scoping this discussion to the JavaScript dependencies that are built into the extension. This has the most direct impact for our users. If we want to have a separate discussion about the development dependencies, I'm fine with doing that elsewhere.

[jeremyn]

(The general justification for all of these items is to improve security and maintainability.)

We should minimize the number and complexity of dependencies.

We should not bundle/pack dependencies.

We should not have different classes of dependencies, such as developer or general-purpose, with different levels of scrutiny for each. A dependency might cross classes or move from one class to another and bypass review.

We should use stable dependencies and pin to exact versions. The Ubuntu LTS/Debian stable/RHEL approach is the model here: pick one version, stick to it, and only upgrade to fix identified vulnerabilities.

We should not use dependencies that require subject matter expertise to understand. The code should be entirely understandable by any competent, generalist JavaScript developer. Issue https://github.com/EFForg/https-everywhere/issues/12087 has a discussion about using tweet-nacl.js that is relevant here.

The full code for a dependency should be checked into the repository.

Minifying dependencies should be encouraged, especially for large dependencies, as long as the minification process is repeatable with provided instructions. The best approach though would be to leave everything unminified in the repository and then minify everything at build time. See @koops76 's comment at https://github.com/EFForg/https-everywhere/pull/13560#issuecomment-344755555. We don't have that set up yet however, so per-file minification should be fine for now.

Dependencies for distinct projects should not be grouped into the same package.json or similar. Each project should have its own package.json. Also, it would be nice if we could use the new security alerts feature on GitHub. For that to work, it looks like we have to put dependencies into a top-level package.json. (All of this argues for moving the various projects currently in utils/ into separate repositories. This would have the added benefit of simplifying licensing, see my comment at https://github.com/EFForg/https-everywhere/pull/13062#issuecomment-341272152.)

[ghost]

We should not bundle/pack dependencies.

👎

[ghost]

We should not use dependencies that require subject matter expertise to understand.

This severely limits what dependencies we can have. Auditing the source code is harder than simply reading it, see the underhanded C code contest. Also does that include independently audited code like tweet-nacl.js?

[jeremyn]

Pinging @gloomy-ghost @J0WI so they are aware of this issue.

[ghost]

The best approach though would be to leave everything unminified in the repository and then minify everything at build time.

👍. Definitely we should have a policy against minified/obfuscated code in the repo.

[ghost]

Dependencies for distinct projects should not be grouped into the same package.json or similar. Each project should have its own package.json. Also, it would be nice if we could use the new security alerts feature on GitHub. For that to work, it looks like we have to put dependencies into a top-level package.json. (All of this argues for moving the various projects currently in utils/ into separate repositories. This would have the added benefit of simplifying licensing, see my comment at #13062 (comment).)

👍

[jeremyn]

@koops76

This severely limits what dependencies we can have. Auditing the source code is harder than simply reading it, see the underhanded C code contest. Also does that include independently audited code like tweet-nacl.js?

We shouldn't need that many dependencies anyway. The project has been around for years without having any.

We should be capable enough, "in-house", of understanding and reacting to security problems in our dependencies. Generally an audit can only tell us that a particular version of a dependency is okay. If we are forced to trust an audit, then if a new version comes out, we have to wait for, and possibly pay for, a new audit, even if the new version claims to fix a critical vulnerability.

Another problem with having to trust an audit is that the security question becomes whether we can trust the auditors, not whether we can trust the code. The security of this project shouldn't depend on whether a handful of us make the right decision about whether to trust some particular small security company, unless there is absolutely no other option. (In issue https://github.com/EFForg/https-everywhere/issues/12087 we had at least two other options: use the in-browser crypto as-is, or drop the sign-rulesets functionality until the in-browser crypto improved.)

[ghost]

@jeremyn The choice is whether we want to trust the people who may be better than us or to trust ourselves to do the same job auditing the code.

[Bisaloo]

Dependencies for distinct projects should not be grouped into the same package.json or similar. Each project should have its own package.json. Also, it would be nice if we could use the new security alerts feature on GitHub. For that to work, it looks like we have to put dependencies into a top-level package.json. (All of this argues for moving the various projects currently in utils/ into separate repositories.

I was thinking we could use the new alerts as well.

But I don't think we need a top level package.json for it to work. It looks like it can find package.json files in subdirectory just fine.

https://github.com/EFForg/https-everywhere/network/dependencies

[jeremyn]

@Bisaloo Good catch, thanks.

[brainwane]

Should we consider using something like https://dependencyci.com/ ?

[jeremyn]

https://dependencyci.com seems more useful for projects that use a lot of dependencies, which have their own dependencies etc, and which are updated frequently. In our case however, if we follow my approach in https://github.com/EFForg/https-everywhere/issues/13600#issuecomment-345557230, we'll have minimal dependencies that change infrequently, so it seems unnecessary.

[Hainish]

Minimizing the number of dependencies, and also the complexity of the dependencies that we pull in, is important from a security perspective. See https://github.com/EFForg/https-everywhere/issues/12087#issuecomment-325520253 for some discussion on dependency complexity & attack surface.

Sometimes the dependency management system itself can present a threat to the build environment. This was no more apparent than in the recent attacks on npm via typosquatting, and the relative lack of verifiability in the npm namespace. Pinning to a specific package version hash is a partial mitigation against this, at least until a vulnerability is found with that version. Then we're forced to upgrade again, most likely to another unaudited dependency, since reliable audits are so rare in the software world.

All this tends to exacerbate the underlying attack surface.

---

On the question of audits, we should in general favor audited code. Yes, an audit is not a future-proof trust mechanism, a new version can introduce new vulnerabilities. Yes, there is also the question of auditor reliability. However, there are a number of reputable 3rd party software auditors that we can rely on, such as Cure53 and NCC Group. Do their audits guarantee that no vulnerabilities exist in the code? No, of course not. But scrupulous audits do flesh out major vulnerabilities, and their reports can be illuminating as to the quality of code that a project contains.

A project that has undergone multiple audits with positive results over a period of years can reasonably be judged as more trustworthy than an equivalent project which hasn't undergone the same amount of evaluation. This is one of the best mechanisms we have for judging the long-term security of a project.

This being said, as I've stated above, it's relatively rare to see dependencies which themselves are not security-focused with any security audits under their belt. Holding ourselves to a standard of only-3rd-party-audited dependencies may be an untenable goal. We may be forced in a security-vs-security situation. For instance, the delivery of out-of-band rulesets provides a security assurance to users of the Tor Browser. However, the sign-rulesets branch is also dependent on a compression library which is itself unaudited. In these situations, other metrics must be relied upon, such as author reputation and if they've written security-focused software before.

---

We should not have different classes of dependencies, such as developer or general-purpose, with different levels of scrutiny for each. A dependency might cross classes or move from one class to another and bypass review.

Scrutinizing where dependencies are used and when should be an essential part of the review process. For instance, the complexity that CodeMirror brings to the project is significant, but I deem it acceptable. Most users will never encounter this code. If some malicious ruleset was able to exploit a vulnerability in CodeMirror, it could affect developers when they enter this into their debugging panel, but it would not affect end-users when they click the popup. Compromise of developer machines is serious, to be sure. But we have protections in place to ensure that this does not automatically lead to the compromise of end-user devices, which would be disastrous, given the number of users who rely on our software.

[jeremyn]

The audit question was about code that is so specialized that we can't really tell whether it's safe to modify. An example might be the code that generates the prime numbers used for some cryptographic operation. (This is the source of the recent vulnerability with some Infineon products, https://nvd.nist.gov/vuln/detail/CVE-2017-15361 .) If we had a dependency with code like that, we would need to rely on an audit for its initial security and further audits for updates, because we (presumably) are not qualified to update it ourselves. Contrast this with something like CodeMirror where, while it might be tedious, we should be able to understand any vulnerability it has and, if needed, securely patch it ourselves.

For classes of dependencies, the situation is that a developer who sees a dependency used in one area of the code will probably feel free to include it somewhere else without much thought. For example someone who sees the pako compression library used in the ruleset signing code might decide to bring it over to the ruleset debugger code. If we have different "tiers" of security then we might accidentally bypass some necessary review.

By the way, do you really need pako for the sign-rulesets stuff? Can't you just rely on browser/server gzipping? I haven't looked into how you're using it so there might be some very good reason I just don't know about.

[Hainish]

By the way, do you really need pako for the sign-rulesets stuff? Can't you just rely on browser/server gzipping? I haven't looked into how you're using it so there might be some very good reason I just don't know about.

I may not have exhausted all the options, but I did look into various compression mechanisms, without luck. It's worth a revisit once I'm able to return to the branch, though.

[jeremyn]

I mean more like, can't you just rely on the browser requesting, and the server sending, gzipped content using https://en.wikipedia.org/wiki/HTTP_compression ?

[Hainish]

@jeremyn I'll have to modify the AJAX request to see if I can send the Accept-Encoding header, and have the server respond in a gzipped format. I'm pretty sure I tried that previously and it wasn't working for some reason. I'll revisit this.

[zoracon]

In response to this thread, set a yml defined dependency file that takes into consideration of auditing NPM dependencies. You can review here: https://github.com/EFForg/https-everywhere/commit/270328af49708118f889754a6bd8311896e049b0 Trying to move forward with the "policy defined in software" approach with this repo. I have also done this already in a mild form with this file: https://github.com/EFForg/https-everywhere/blob/master/SECURITY.md

Of course that does not cover CodeMirror and Pako: https://github.com/EFForg/https-everywhere/blob/master/chromium/external/README.md

I am going to attempt to simply bring these into package.json, unless there was a particular reason we locally hosted these that I am missing.