[pre-RFC] Security advisories as part of crates.io metadata

[untitaker]

If yank would want to satisfy all those usecases, it IMO would have to change such that, by default, it only ever prints warnings about yanked crates, and never refuses to compile. In particular “refuse to compile added dependencies if those are yanked” is problematic for security vulnerabilities, but an absolute no-starter for some of the other yank reasons you listed.

I’d say such semantics are toothless. Even with the new output format you propose I feel like all the user sees at a glance is a giant wall of yellow at the end of the output, because if yank is extended for deprecating packages for all kinds of reasons, there sure is going to be a lot of that.

You could then add options to make certain classes of yanks hard errors and suppress others, but I feel like those options are more likely to be abused than allow_vulnerable.

[alexcrichton]

The idea behind that behavior is: Better mark too many crates as vulnerable than too few.

True! This is executed pretty rarely, though, and I imagine we could solve the issue by making it easy to execute the command multiple times?

What about a hybrid?

The lack of support on Windows (this will basically never work) makes me think that we shouldn’t try to optimize this use case. It means many users will not have an editor opened automatically, so we shouldn’t optimize for that.

I feel like those situations are not comparable as deprecations are much more common (by orders of magnitude!) than security advisories.

True yeah, but I think the risk is still there. If a user once or twice starts seeing vulnerabilities they have no idea how to fix, then one might quickly enter the mode of “just always ignore” the messages.

@bascule suggested to create an option that makes warnings errors instead. Is that more acceptable? Note my follow-up questions in that thread for which I haven’t found a good answer:

I would prefer the reverse, an option to turn warnings to errors. I personally feel that breaking existing builds with a Cargo.lock, for whatever reason, is a non-starter.

I don’t know what you mean by this. Are you saying that allow-vulnerable should affect behavior of publish? This is already my intention.

Oh I just mean cargo publish --allow-vulnerable or something like that rather than modifying Cargo.toml.

[briansmith]

Drawbacks

There is a risk that users will abuse this system to mark their versions as deprecated or to call out other kinds of critical bugs such as data loss. This would make the vulnerable-flag semantically worthless.

There is another drawback: Some crate developers may not be aware of this mechanism and/or may be aware of it and choose not to use it for various reasons. Further, the cost is too expensive to keep track of vulnerabilities in every version of a crate ever published, to minimize costs it makes sense to mark everything but the newest version vulnerable, or abstain from using the mechanism at all.

Thus, while the existence of the mechanism would likely induce people to depend on it, but it isn’t something people can depend on and it would create a false sense of security.

[untitaker]

True! This is executed pretty rarely, though, and I imagine we could solve the issue by making it easy to execute the command multiple times?

What would that solve? You’d still have the risk of forgetting a bugfix release in your enumeration.

The lack of support on Windows (this will basically never work) makes me think that we shouldn’t try to optimize this use case. It means many users will not have an editor opened automatically, so we shouldn’t optimize for that.

Fair enough, I’ll change that. EDIT: https://github.com/untitaker/rfcs/commit/b0691a1e7c61c130e634f918de6d94e22cc6a5e9

True yeah, but I think the risk is still there. If a user once or twice starts seeing vulnerabilities they have no idea how to fix, then one might quickly enter the mode of “just always ignore” the messages.

Which is why I wanted to use hard errors for security vulnerabilities during testing, such that they’re harder to ignore and people rather ask questions than ignore.

I would prefer the reverse, an option to turn warnings to errors.

That’s what I said.

Oh I just mean cargo publish --allow-vulnerable or something like that rather than modifying Cargo.toml.

see here for why having a bool switch for turning off all vulnerability warnings is highly problematic. My version to disable them on a per-dependency basis is already argued to be too broad, yours is broader.

to minimize costs it makes sense to mark everything but the newest version vulnerable

For “lazier” developers this is exactly the behavior I want to nudge them into rather than not using the feature at all, by defaulting to selecting all versions by default. The idea is that you’d release a new patched version after marking all existing versions as vulnerable.

Regarding developers that simply won’t use this due to other reasons, I can’t think of an alternative design that solves this problem, but it’s surely worth mentioning as drawback. FIXME

[untitaker]

I pushed some updates:

• https://github.com/rust-lang/rfcs/commit/b11cc12ab66d7b3b5200dd67a10eab7919309e97 – Introduce advisory IDs and allow multiple advisories per version. allow_vulnerable now runs on advisory IDs too.

• https://github.com/rust-lang/rfcs/commit/b0691a1e7c61c130e634f918de6d94e22cc6a5e9 – EDITOR is no longer used

• https://github.com/rust-lang/rfcs/commit/d8aedf2a8df8d0eb3c40fbb47e471584c2491cac – cargo test is now also just warnings

Open things:

• Elaborate on drawbacks on not doing anything, and talk about risk of the false sense of security by people not doing anything.

• …anything else?

[troplin]

I think to be future-proof the dwf field should be called id or ids and should support a namespacing mechanism. Eg:

ids = ["CVE:XXXX-YYYY", "DWF:ZZZZ-WWWW", "INTERNAL:012YAZ"]

[untitaker]

I think to be future-proof the dwf field should be called id or ids and should support a namespacing mechanism. Eg:

ids = [“CVE:XXXX-YYYY”, “DWF:ZZZZ-WWWW”, “INTERNAL:012YAZ”]

Would there be an authority that manages this namespace? How about this:

ids = { cve = "...", dwf = "...."}

Also @briansmith do you have an opinion on the entire RFC, or a suggestion to countermeasure the “false sense of security”?

[untitaker]

I guess it’s time to open open an actual pull request for the RFC: https://github.com/rust-lang/rfcs/pull/1752

[vi0]

Nobody should be (newly!) depending on a crate with known sec vulnerabilities

A crate can be big and abandoned.

Big means a vulnerability can affect only small portion of it which is not used by an application (including new applications). So application author may decide to suppress this individual vulnerability.

Abandoned means there is no newer version without vulnerability (and the advisory may be actually created by some third party). It does not also mean there is non-abandoned successor.

[untitaker]

The discussion has since moved to the RFC PR.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.