Idea: Security advisories as part of crates.io metadata

[untitaker]

FWIW I filed an issue at https://github.com/rust-lang/crates.io/issues/406

[untitaker]

I have the work-in-progress of an RFC at https://github.com/untitaker/rfcs/blob/security-advisories/text/0000-security-advisories.md. The last three sections are not written yet, so I’ll not file a PR yet.

This is a crosspost from the crates.io issue. I’m not sure which forum is appropriate.

• I consider Summary, Motivation and Detailed Design to be complete for my standards, but I haven’t given much thought to it yet. I don’t think it has to be described how the Crates.io API should look, but I considered specifying the UX for this important. Unsure about the way I specified it though.
• Drawbacks: Danger of duplicating CVEs maybe?
• Alternatives: List similar concepts in other ecosystems: Ruby Advisory Database, npm deprecate, and why this command is specifically for vulnerabilities. Do I have to rationalize every UX decision?
• Unresolved questions:
  • How tooling around this API should look like.
    • Should crates.io notify authors of reverse dependencies per email (scraped from GitHub)?
    • Should there be an RSS feed? (IMO no, as RSS is not a reliable notification system for technical reasons: If RSS reader updates seldomly, items may be missed).
    • How do we expose this API to third-party developers? Ideally they should not need to install cargo to use the API.

[nikomatsakis]

In https://github.com/rust-lang/cargo/issues/2608, @alexcrichton described an idea to allow people to pull their own packages and give a reason – then, if cargo was building and found a yanked crate in your dependencies, it would warn you that a cargo update is a good idea. Seems like a very similar thing.

[untitaker]

Here are some reasons I think yanked packages (as they are right now) are not a solution for security vulnerabilities:

• Cargo doesn’t allow yanked packages to be used in new packages. I think people who know what they’re doing should be allowed to depend on vulnerable packages, they might use the crate in a way that poses no security problem.

• Cargo doesn’t give any advice about further procedure when yanking a package. I think in the context of security vulnerabilities this is very much needed, as few OSS maintainers are exposed to this problem regularly enough to know what they’re doing.

• Lastly, the data exposed via the Crates.io would be a lot less structured. Automatic notification like I imagine with this draft would be impossible because the overwhelming majority of yanks are for relatively uncritical things.

All of these things may of course be fixable by changing the semantics of yank accordingly: We could require people to “categorize” their yanks into security, deprecation, and whatever else comes to mind. Then, when attempting to compile such crates, Cargo could exhibit different behavior, being less “stern” when trying to compile a deprecated version, and more “stern” when compiling a version with security vulnerabilities. It could also display better advice depending on the category (such as “request a CVE for this”) and the data would obviously be more structured as well.

[Valloric]

Cargo would refuse to compile such versions without an override switch.

I like the idea of notifying users that they’re depending on an insecure library version very much, but the above suggested approach is not the way to do it. Thou shal never break another project’s build. EVER. Any idea that proposes this should be dead in the water.

Printing a warning during the build sounds fine though.

[untitaker]

This section is already revised in the linked RFC draft. It now prints a warning when compiling, and cargo test fails instead.

[mitsuhiko]

I want to add my two cents to this: I agree that using yank for this is not the best idea. It just does not have enough meta information. It would be great if you could get an overview of the sanity of your app and ideally that spits out why exact something is a security problem. In particular large packages often contain security issues in areas that are unused so you might want to just explicitly silence a particular security issue after review.

Ideally when you mark a package to have a security issue you assign CVE numbers or a local identifier which can be explicitly silenced so if something else shows up in the future you get a warning again.

[untitaker]

Good idea about assigning a identifier. Having CVE numbers as those would make using CVEs mandatory. For some reason I wanted to avoid that, but I’m not sure why.

We can totally “overload” yank to work with security stuff, the question is whether this is shoehorning too much logic into one command.

[untitaker]

Actually the more I think about it, the more I prefer:

• adding both a freeform “reason” field and a category (security|deprecated|other) to yanked versions
• allowing overrides of some form to compile yanked versions even in a new application.
• printing warnings for existing applications

Thoughts?

[mitsuhiko]

I don’t think one could make CVEs mandatory. They just recently started issuing longer numbers. Pretty sure they would not appreciate if one starts to simplify the process so that they have to issue a few orders of magnitude more. Crates could assign local numbers.

[retep998]

Extending yanking to be more descriptive of why it was yanked, and printing out warnings when using yanked crates, sounds like a good idea to me.

[untitaker]

Whenever I have time I’ll rewrite the RFC to extend yanking instead, it seems to be a much more popular option.

[hgrecco]

The RFC does not say anything about who could flag a crate as vulnerable. I guess it is implied that only the crate author can do it. However, it might be good to consider an alternative way for the (not unlikely) case in which an abandoned but commonly used crate is found vulnerable. A possibility would be that a group of people appointed by the core devs have permissions on crates.io to flag a crate as vulnerable. I understand that some would not like this for being invasive but I think it could be a good thing overall if it is done transparently.

[untitaker]

I’ll clarify this. I think intervention by “crates.io mods” in general doesn’t require an RFC and will happen anyway.

[untitaker]

I think when formulated this way, there are actually two proposals that might as well be split in two RFCs:

• Adding extended, structured metadata to yanked crates
• Changing Cargo’s behavior for yanked crates to allow using yank-ing for security announcements

Which of those even warrants a RFC?

[mitsuhiko]

I suppose one RFC would be enough particularly because it most likely will have to explain how to silence individual security warnings explicitly (would that go into Cargo.toml?)

[vi0]

Shall it also extend to non-security serious bugs that can lead to data loss?

[untitaker]

Absolutely not. I don’t want to duplicate a bugtracker into Crates.io.

[mcpherrinm]

Getting a CVE issued is not guaranteed. Anything “out of scope” will not get one, and so blocking use of this feature on a 3rd party seems troublesome.

[bascule]

I would suggest using DWF instead of CVE:

Distributed Weakness Filing

DWF was created by the guy who runs https://iwantacve.org/ to distributed the administrative overhead of acquiring CVEs.

All CVEs are valid DWFs, and a CVE can be converted into a DWF by doing s/CVE/DWF/. DWF can be thought of as a superset of the CVE space.

With DWF, https://crates.io could apply for a block of DWF identifiers, and automatically assign them when someone does cargo vuln (or, as I’d now personally prefer, cargo advisory). Or, crates that do get a CVE assigned can use that instead, but translating it to a DWF first.