Migrate to V3 advisory format

[tarcieri]

The V3 advisory format uses Markdown with TOML frontmatter. It was originally proposed in #240 and implemented (with some changes from the original proposal) in RustSec/rustsec-crate#167.

The new format allows us to see renderings of Markdown-formatted descriptions in PRs and when viewing advisories on GitHub. Barring any unforeseen circumstances, this will be the final stable advisory format, and will unlock releasing 1.0 versions of the various RustSec crates.

In #240, we suggested attempting a migration today, October 1st, 2020 (i.e beginning of Q3), which this issue is for tracking.

Example advisory:

https://raw.githubusercontent.com/RustSec/rustsec-crate/master/tests/support/example_advisory_v3.md

Rendered:

[advisory]
id = "RUSTSEC-2001-2101"
package = "base"
date = "2001-02-03"
url = "https://www.youtube.com/watch?v=jQE66WA2s-A"
categories = ["code-execution", "privilege-escalation"]
keywords = ["how", "are", "you", "gentlemen"]
aliases = ["CVE-2001-2101"]
cvss = "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H"

[versions]
patched = [">= 1.2.3"]
unaffected = ["0.1.2"]

[affected]
arch = ["x86"]
os = ["windows"]
functions = { "base::belongs::All" = ["< 1.2.3"] }

All your base are belong to us

You have no chance to survive. Make your time.

[Shnatsel]

Do we have a format specification listing all the fields (like a schema) and specifying how to parse this format? For example, the handling of "```" occurring in the TOML body or in the description can be ambiguous.

I feel that ease of parsing is a necessary constraint for the format, and I'm not comfortable stabilizing it until the proper way to parse it is specified and is found feasible to implement in third-party tooling.

[tarcieri]

There are two ways to parse it:

• Parse the whole file as Markdown and extract the contents of the first code block, which the document MUST begin with and MUST be TOML
• Ensure the file begins with ```toml, split its contents at the next encountered ``` delimiter, treat the beginning as TOML and the end as Markdown. This fairly simple and closer to the method the RustSec parser uses (although it would be interesting to eventually transition to something closer to above).

Also per #240 I feel it's a little late to be voicing concerns here. If there's a serious issue we can revert.

[tarcieri]

One snag: the linter needs to be updated, since it parses the raw TOML as a toml::Value

[Shnatsel]

Yeah, I'm always late with these to the point of being unhelpful 😞

[tarcieri]

WIP PR to translate the database here: #420

[tarcieri]

All advisories have been translated into the new format in #420.

We still have the option of rolling back if there are any showstoppers, but so far everything seems to be working normally on versions of cargo-audit which support the new format (v0.12.0+)

[tarcieri]

The only lingering issue right now, I think is this may have broken assign-ids. I'm trying to fix part of it here:

#421

[tarcieri]

A big problem I just noticed is the advisory template needs to be updated:

https://github.com/RustSec/advisory-db#advisory-format

I'm wondering if we should split it out into its own file so it can be easily copied/pasted without the comment annotations.

[andreeaflorescu]

All advisories have been translated into the new format in #420.

We still have the option of rolling back if there are any showstoppers, but so far everything seems to be working normally on versions of cargo-audit which support the new format (v0.12.0+)

I have a hunch that because of this new format change we are no longer able to use cargo audit in our pipeline. Our builds fails with error loading advisory database: parse error: unexpected character found: ``` at line 1 column 1. We're using cargo audit version 0.11.2 and rust version 1.44.1.

Would it be possible that future updates to the database take into consideration older versions of cargo audit? We're in the situation where we need to disable the cargo audit test so we can merge PRs in our pipeline. It would be really nice if these changes would be backwards compatible because we believe cargo audit is really useful and we'd like to continue to use it.

This problem affects both Firecracker & rust-vmm projects.

[tarcieri]

@andreeaflorescu we did. There was (confirmed) advance warning that cargo-audit would be breaking via this advisory:

#415

Prior to getting a hard error, this should've surfaced as at least a warning message emitted by cargo-audit which looks like this:

warning: this copy of cargo-audit has known advisories!

Crate:  rustsec
Title:  Obsolete versions of the `rustsec` crate do not support the new V3 advisory format
Date:   2020-10-01
URL:    https://rustsec.org/advisories/RUSTSEC-2020-0051
warning: upgrade cargo-audit to the latest version: cargo install --force cargo-audit

Is there a particular reason you can't upgrade to cargo-audit v0.12?

[andreeaflorescu]

@tarcieri we will be able to update to cargo-audit only after we update the rust version as well. Updating the rust toolchain is problematic for us because we offer security releases where we expect to have as little changes as possible. Basically in patch releases we only want to have changes that are related to the security vulnerability, and updating the Rust version comes with additional changes that we do not think should be incorporated in such releases.

We can have workarounds such as installing the newest Rust toolchain and only use that for running cargo audit (and keep the old toolchain for the build), but generally speaking we would like to know beforehand when breaking changes happen so that we can address them before the CI fails.

Are you planning on adding these warnings before breaking the compatibility for older versions? And if yes, what automated check would you recommend that we can implement such that we catch these warnings before they turn into errors?

[tarcieri]

This is hopefully going to be the last major breaking change to the advisory database format of this nature.

Did you not get a warning message, or was it not noticed because --deny-warnings wasn't enabled?

...we will be able to update to cargo-audit only after we update the rust version as well.

You said you're using Rust 1.44.1. The MSRV for cargo-audit v0.12 is Rust 1.40, so I'm confused why you're unable to update.

[andreeaflorescu]

This is hopefully going to be the last major breaking change to the advisory database format of this nature.

That'd be really nice. Also, is it not feasible for the database to also be versioned?

Did you not get a warning message, or was it not noticed because --deny-warnings wasn't enabled?

I am not sure here, I'll have to double check with my colleagues. In rust-vmm the test was not run for 2 weeks or so, and then it suddenly failed today.

...we will be able to update to cargo-audit only after we update the rust version as well.

You said you're using Rust 1.44.1. The MSRV for cargo-audit v0.12 is Rust 1.40, so I'm confused why you're unable to update.

There was some confusion related to the supported Rust version. It works indeed, thanks!

[alxiord]

You said you're using Rust 1.44.1. The MSRV for cargo-audit v0.12 is Rust 1.40, so I'm confused why you're unable to update.

There was some confusion related to the supported Rust version. It works indeed, thanks!

The combination that doesn't compile is Rust 1.44.1 and cargo-audit v0.12.0, due to rust-lang/rust#52000.

   Compiling smol_str v0.1.17
error[E0658]: `while` is not allowed in a `const fn`
  --> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/smol_str-0.1.17/src/lib.rs:58:9
   |
58 | /         while i < text.len() {
59 | |             buf[i] = text.as_bytes()[i];
60 | |             i += 1
61 | |         }
   | |_________^
   |
   = note: see issue #52000 <https://github.com/rust-lang/rust/issues/52000> for more information

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
error: could not compile `smol_str`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: failed to compile `cargo-audit v0.12.0`, intermediate artifacts can be found at `/tmp/cargo-installTWjG71`

Rust 1.44.1 + cargo-audit v0.12.1 compiles just fine.

[tarcieri]

As of #440 automatic ID assignment is fixed.

The linter will also reject the legacy V2 format.

The migration to the new format is complete.