Update container version to v28

[stefano-garzarella]

Summary of the PR

This new version contains alsa and pipewire libraries to build vhost-device-sound audio backends.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

• All commits in this PR are signed (with git commit -s), and the commit
  message has max 60 characters for the summary and max 75 characters for each
  description line.
• All added/changed functionality has a corresponding unit/integration
  test.
• All added/changed public-facing functionality has entries in the "Upcoming
  Release" section of CHANGELOG.md (if no such section exists, please create one).
• Any newly added unsafe code is properly documented.

[stefano-garzarella]

cargo audit hangs indefinitely, reported it here: rust-vmm/rust-vmm-container#89 (comment)

[stefano-garzarella]

Looking at https://crates.io/crates/cargo-audit they call cargo generate-lockfile before cargo audit so it looks like it is a prerequisite (since when, I don't know).

Indeed, locally it fixed the issue, so should we change

rust-vmm-ci/.buildkite/test_description.json

Line 74 in 9751aaa

"command": "cargo audit -q --deny warnings",

prepending cargo generate-lockfile or cargo build?

[epilys]

cargo generate-lockfile. Would committing Cargo.lock break something else?

[roypat]

Looking at https://crates.io/crates/cargo-audit they call cargo generate-lockfile before cargo audit so it looks like it is a prerequisite (since when, I don't know).

Indeed, locally it fixed the issue, so should we change

rust-vmm-ci/.buildkite/test_description.json

Line 74 in 9751aaa

"command": "cargo audit -q --deny warnings",

prepending cargo generate-lockfile or cargo build?

Apparently since 4 years ago: https://github.com/rustsec/rustsec/blame/main/cargo-audit/README.md. I think we're running into rustsec/rustsec#1032, which seems to be the commit at which cargo audit started using the cargo lock (and I guess it does not account for the scenario where it does not exist?). So the generate-lockfile fixing it makes sense to me, let's add it :)

[stefano-garzarella]

cargo generate-lockfile. Would committing Cargo.lock break something else?

The problem I see is that cargo generate-lockfile also updates the dependencies, so for those crates that have Cargo.lock committed, we're going to audit not the versions we're actually using.

[epilys]

Well here are some solutions I can think of right now:

• do [ -e Cargo.lock ] || cargo generate-lockfile, which I don't like because it's a workaround of Cargo.lock missing
• Fail CI if Cargo.lock is missing and update all crates to include it.
• Did not test it, but maybe cargo generate-lockfile --offline does not update dependencies?

[roypat]

cargo generate-lockfile. Would committing Cargo.lock break something else?

The problem I see is that cargo generate-lockfile also updates the dependencies, so for those crates that have Cargo.lock committed, we're going to audit not the versions we're actually using.

Mh, but "versions we're actually using" is not really a concept for a library target, only for a binary. You can build a library against any versions of its dependencies that are semver compatible. How exactly do we actually expect cargo audit to work here? Maybe "fail if any semver allowed version has a known vulnerability" would make sense (so we can prevent downstream user from using our crates together with vulnerable versions of our dependencies, but I don't think we should do this kind of policing - I know some other rust crates recently came under fire for similar things). But I don't think cargo audit does that. I feel like it can only give meaningful information to a binary target, where everything is pinned to a specific patch version, and we can only make sure that we always support the newest versions of our dependencies, to ensure downstream users can always upgrade should vulnerabilities be found 🤔

[epilys]

@roypat I agree. The Rust Cargo team now recommends libraries pin dependencies by committing the lockfile and has some arguments for it: https://doc.rust-lang.org/nightly/cargo/faq.html#why-have-cargolock-in-version-control

[stefano-garzarella]

@roypat so we should do something like [ -e Cargo.lock ] && cargo audit ?

[roypat]

@roypat so we should do something like [ -e Cargo.lock ] && cargo audit ?

I'd be fine with that. When we start checking in Cargo.lock files (which I think we should, @epilys has linked some good arguments. Especially the whole "CI fails because some new version of external dependency got pulled in" thing caused us quite some pain in the past - heck, a different facet of that problem got us into the mess we're in right now!), it'll also start running on those again, for whatever infinitesimal gain that gives us, without us needing to modify the ci package again, so that's a plus. If we really want to main the questionable status quo of "pick random versions of dependencies and then run cargo audit on the result", we can add a || cargo generate-lockfile && cargo audit to it (or simplify to [ ! -e Cargo.lock ] && cargo generate-lockfile; cargo audit).

[stefano-garzarella]

@roypat so we should do something like [ -e Cargo.lock ] && cargo audit ?

I'd be fine with that. When we start checking in Cargo.lock files (which I think we should, @epilys has linked some good arguments. Especially the whole "CI fails because some new version of external dependency got pulled in" thing caused us quite some pain in the past - heck, a different facet of that problem got us into the mess we're in right now!), it'll also start running on those again, for whatever infinitesimal gain that gives us, without us needing to modify the ci package again, so that's a plus. If we really want to main the questionable status quo of "pick random versions of dependencies and then run cargo audit on the result", we can add a || cargo generate-lockfile && cargo audit to it (or simplify to [ ! -e Cargo.lock ] && cargo generate-lockfile; cargo audit).

I also don't have a strong opinion, perhaps a little more toward the latter possibility (which @epilys also suggested). I found that cargo metadata generates Cargo.lock if it's not there, or leave what's there, but it seems too hacky to use.

I'll open another PR for this and we can discuss it there. I'll rebase this on that one.

[stefano-garzarella]

v2:

• Rebased on Generate Cargo.lock before call cargo audit #139

[stefano-garzarella]

v3:

• wrong rebase

v4:

• rebased on main since Generate Cargo.lock before call cargo audit #139 is now merged