[MIR] cargo, dh-cargo

Bug #1993819 reported by Simon Chopin
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cargo (Ubuntu)
Won't Fix
High
Unassigned
dh-cargo (Ubuntu)
Fix Released
High
Unassigned
rustc (Ubuntu)
Fix Released
High
Ubuntu Foundations Bugs

Bug Description

[Availability]
The packages dh-cargo and cargo are already in Ubuntu universe.
The packages build for the architectures they are designed to work on,

They currently build and works for architectures:
  * amd64
  * arm64
  * armhf
  * i386
  * ppc64el
  * riscv64
  * s390x

Link to packages:

https://launchpad.net/ubuntu/+source/cargo
https://launchpad.net/ubuntu/+source/dh-cargo

The cargo-doc package is *not* part of the MIR.

[Rationale]
The packages cargo and dh-cargo are required in Ubuntu main as the Rust
programming language is gaining in popularity. cargo is the standard
build tool and package manager for the ecosystem, and dh-cargo is the
debhelper plugin to more easily build new packages.

Note that the huge majority of our users will not use these packages,
their purpose is to be a build-dependency for other packages. In
particular, it is not particularly expected at this stage that those of
our users that are Rust developers, which usually rely on their
toolchain being managed in their $HOME by the `rustup` tool.

[Security]
cargo has 3 security vulnerabilities recorded:

* https://nvd.nist.gov/vuln/detail/CVE-2019-16760
  A new feature to apply a local name to a dependency can lead to the
  wrong package being used when using older toolchains. This didn't
  apply to Ubuntu since we upgrade the Rust toolchain wholesale.
* https://nvd.nist.gov/vuln/detail/CVE-2022-36114
  DoS on disk space via crafted dependency (zip-bomb). Low priority
  since cargo *by design* can execute arbitrary code from dependencies
  (build scripts & procedural macros). Unpatched in current Ubuntu.
* https://nvd.nist.gov/vuln/detail/CVE-2022-36113
  Crafted dependency can lead to 2 byte overwrite of arbitrary files.
  Low priority (see above). Unpatched in current Ubuntu.

There is an official Rust Security working group that curates a database
of security issues within the Rust ecosystem, including cargo:

https://github.com/rustsec/advisory-db

There are no history of known security issues with dh-cargo.

- no `suid` or `sgid` binaries
- no executables in `/sbin` and `/usr/sbin`
- Package does not install services, timers or recurring jobs
- Packages does not open privileged ports (ports < 1024)
- Packages does not contain extensions to security-sensitive software
(filters, scanners, plugins, UI skins, ...)

Note however that in typical use outside of packaging, building a
project with cargo involves executing code that has been downloaded from
crates.io or any other configured registry: cargo builds and executes
the `build.rs` file for any pre-compilation task (a bit like a
Makefile), and any use of a proc macro implies building and running a
standalone binary to transform the input token stream. While there are
leads for sandboxing the latter (using WASM, for instance), the former
needs by definition broad access to the system, i.e. to check installed
libraries.

[Quality assurance - function/usage]
The packages work well right after install, one can easily create a
simple Rust project and run it.

[Quality assurance - maintenance]
The packages do not deal with exotic hardware we cannot support

[Quality assurance - testing]
The cargo package runs a test suite at build time, and rebuilds itself
(including its test suite) as autopkgtest.

dh-cargo doesn't have builtin tests, and only has one autopkgtest for
testing our delta (tracking vendored dependencies). However, all Rust
packages built using dh-cargo have a Test-Trigger on it and their tests
are usually a rebuild of the package.

[Quality assurance - packaging]
debian/watch is present and works, dh-cargo is a native package.

You'll find attached the build logs of src:cargo along with a lintian
run. src:cargo has an override file for the source package, for
relatively minor warnings. I chose to willingly ignore the MPL-2.0+ vs
MPL-2.0 warnings, as adding a full-blown copy of the same license for
the sake of an "or later" statement seemed overkill.

dh-cargo is lintian-clean.

These packages do not rely on obsolete or about to be demoted packages.

The packages will not be installed by default.

dh-cargo's packaging is fairly straightforward.
src:cargo's packaging is more complex. The rules file itself is fairly
easy to grap, but the very tricky part is the vendor tarball generation:

https://git.launchpad.net/~canonical-foundations/ubuntu/+source/cargo/tree/debian/rules?h=merge-0.62
https://git.launchpad.net/~canonical-foundations/ubuntu/+source/cargo/tree/debian/README.source?h=merge-0.62

Because of this, security patching of the vendored dependencies should
be done as a quilt patch to src:cargo rather than attempting to
regenerate the vendored deps with a point-release version of the
dependency.

[UI standards]
I do not believe there's a need for translation for these applications
given the stated purpose for having them in main.

[Dependencies]
All the packages dependencies are either in main or are the subject of
their own MIRs:

https://bugs.launchpad.net/ubuntu/+source/libssh2/+bug/1991650
https://bugs.launchpad.net/ubuntu/+source/libgit2/+bug/1990655

[Standards compliance]
cargo violates the Debian Policy on vendored dependencies but is
otherwise fairly conform.
dh-cargo conforms to the Debian Policy.

[Maintenance/Owner]
Owning Team will be Foundations
Team is subscribed to all packages.

cargo uses static linking for the Rust dependencies, but otherwise links
against system dependencies on the devel release. It is however possible
that some of its dependencies (notably libgit2) might be re-vendored
when backporting new versions to previous releases, as is already the
case for Jammy and before, as newer versions regularly bump their
bindings requirements and backporting those isn't always
straightforward.

Regarding the Rust dependencies, the version in the archive currently
does *not* track them in either Cargo.lock or the
XS-Vendored-Sources-Rust field but an upload is pending to remedy that
(using the Sources field). Waiting on the archive reopening, this new
version is available in a PPA:

https://launchpad.net/~schopin/+archive/ubuntu/test-ppa/+sourcepub/14008184/+listing-archive-extra

[Background information]

The Package descriptions explains the package well.
Upstream is developed by the
Cargo team, under the umbrella of the Rust Foundation

Link to upstream project: https://www.rust-lang.org/

[Previous work]
There was a previous MIR opened against these packages along with rustc.
Given the high volume of discussion for that first package and the time
passed, I opted to open a fresh one instead. The previous MIR can be
found there:

https://bugs.launchpad.net/ubuntu/+source/rustc/+bug/1957932

In particular, a previous MIR review for cargo was done by didrocks:
https://bugs.launchpad.net/ubuntu/+source/rustc/+bug/1957932/comments/6

Most comments were addressed AFAICT, except for the copyright ones.
Regarding the libgit2-sys licensing, it pertains to the bindings, not
the libgit2 library itself, hence the different licensing. At the time,
the libgit2 sources were also embedded, hence the warning (and the error
in d/copyright).

Tags: sec-1397
Revision history for this message
Simon Chopin (schopin) wrote :
Changed in cargo (Ubuntu):
importance: Undecided → High
Changed in dh-cargo (Ubuntu):
importance: Undecided → High
Revision history for this message
Dan Bungert (dbungert) wrote :

Rewrapped text for 72 column, no content changes.

description: updated
Lukas Märdian (slyon)
Changed in cargo (Ubuntu):
assignee: nobody → Didier Roche-Tolomelli (didrocks)
tags: added: sec-1397
Changed in dh-cargo (Ubuntu):
assignee: nobody → Christian Ehrhardt  (paelzer)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (3.6 KiB)

Review for Package: dh-cargo

[Summary]
it was to be expected for the kind of package that this is, but even
checking in detail did not show any problems on this.

MIR team ACK

This does not need a security review

List of specific binary packages to be promoted to main: dh-cargo
Specific binary packages built, but NOT to be promoted to main: n/a

Notes:
- Just curious, is there a build generating a .lock file somewhere?
  I didn't see one referenced from the bug. I'm Interested to see it
  and maybe convert mdevctl to use it for its embedded source.

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
- not a rust package, no extra constraints to consider in that regard
  (this is perl and shell, not rust itself)

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not parse data formats (files [images, video, audio,
  xml, json, asn.1], network packets, structures, ...) from
  an untrusted source (only builds which are untrusted, but
  in a save env).
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems: None

[Common blockers]
OK:
- does not FTBFS currently
- No special HW needed for build/test
- A non-trivial test on this level does not make much sense (for the build
  helper itself alone) but the overall things it helps building are put to
  many tests.

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest
=> I'd consider both facts ok for the special situation this package is in.
   It is more to facilitate builds and tests of other code than itself.
   The code itself is complex, but not too big yet.
   While it would be nice to have some self-tests on expected behavior
   I consider it ok to have no exhausting tests right now.
   Thanks for adding tests that cover our delta!

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under
  control
- symbols tracking not applicable for this kind of code.
- d/watch is present not present (ok = native)
- Upstream update history is N/A
- Debian/Ubuntu update history is ok (not many changes recently though)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean (couldn't be less)
- It is not on the lto-disabled list

Proble...

Read more...

Changed in dh-cargo (Ubuntu):
status: New → In Progress
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Note that this is a followup on https://bugs.launchpad.net/ubuntu/+source/rustc/+bug/1957932/comments/6 as stated on the description. I’m not going to repost the whole content as most of the issues are now fixed or addressed in the current bug description like the copyright explanation. Thanks for those!

I do have a few remaining questions before the finale MIR team ack:
* Do you mind explaining if the state on the testing strategy since 1.58 is better in Ubuntu now? IIRC, you mentioned previously that we were making the build failing only if too many tests are failing. However, with the comitment to main from the foundation team, the goal was to have a better story here and fix/remove flaky tests (as they are useless anyway: are they flaky because of the tests themselves, or is it due to a flaky behaviour, so in the production code itself? Before investigating, there is no way to know about it).
* I still find there are a lot of warnings during package build (I would say ~300 of valid ones, once you remove the deprecation tests), which is weird for one of the official Rust tooling. Do you know anything we can help to make this better? (the reasoning being: if you got one warning, and another warning, then the next one is just an additional one on the list, which can be a valid issue and go unnotice).
* Finally, at the time of the previous MIR, we got a provisional ACK from the security team, do we need more official approvals there?

Ofc, once the MIR ack is there, we still need to remaining dependencies not fully ACKed yet to be processed.

Changed in cargo (Ubuntu):
assignee: Didier Roche-Tolomelli (didrocks) → nobody
status: New → Incomplete
Revision history for this message
Zixing Liu (liushuyu-011) wrote :

Hi there,

I will take over this MIR in place of Simon and continue what we left off.

> * Do you mind explaining if the state on the testing strategy since 1.58 is better in Ubuntu now? IIRC, you mentioned previously that we were making the build failing only if too many tests are failing. However, with the comitment to main from the foundation team, the goal was to have a better story here and fix/remove flaky tests (as they are useless anyway: are they flaky because of the tests themselves, or is it due to a flaky behaviour, so in the production code itself? Before investigating, there is no way to know about it).

This is because Rust upstream has platform tiering. Some tests are not guaranteed to pass on certain operating systems + CPU combinations. For instance, upstream tests may include float-point arithmitics behavior validations that might not pass on architectures where float-point calculation does not conform fully to IEEE 754 (e.g. MIPS).
Rust upstream only gates the release depending on whether all the Tier 1 platforms pass the tests (see https://doc.rust-lang.org/rustc/platform-support.html for the Tiering, Tier 1 is considered to be both Tier 1 with Host Tools and Tier 1 without Host Tools).

> * I still find there are a lot of warnings during package build (I would say ~300 of valid ones, once you remove the deprecation tests), which is weird for one of the official Rust tooling. Do you know anything we can help to make this better? (the reasoning being: if you got one warning, and another warning, then the next one is just an additional one on the list, which can be a valid issue and go unnotice).

This is because Rust upstream adds new warnings to the compiler nearly every release. Cargo in the Debian tarball is usually pinned to a release branch and does not receive the fixes unless the fix is critical (and in Debian's case, some dependencies are probably not vendored correctly, thus producing warnings, but this will be changed in Ubuntu hopefully very soon).

> * Finally, at the time of the previous MIR, we got a provisional ACK from the security team, do we need more official approvals there?

I am not very sure about the process, I will ask security team about that.

Revision history for this message
Seth Arnold (seth-arnold) wrote :

The security team's provisional ACK on the previous MIR bug was for rustc alone, though some of the questions raised may have been better aimed at the cargo MIR. (It's hard to tell; separating rust from cargo is difficult at best.)

Thanks

tags: added: foundations-todo
Revision history for this message
Mark Esler (eslerm) wrote :

Please note that cargo is currently integrating gitoxide which will, eventually, replace the dependencies libgit2 and http-parser.

https://github.com/Byron/gitoxide/discussions/694

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

[17:01] <eslerm> at next meeting, I would like to discuss the status of LP#1993819

Revision history for this message
Zixing Liu (liushuyu-011) wrote :

> Please note that cargo is currently integrating gitoxide which will, eventually, replace the dependencies libgit2 and http-parser.

Do note that this is not going to happen anytime soon. We are still going to need `libgit2` for the foreseeable future.

Revision history for this message
Mark Esler (eslerm) wrote :

Thanks to dh-cargo and a Foundations Team spec (FO055) to add the Vendored-Sources-Rust field to Rust packages, the Security Team is ready to review cargo and other Rust packages. Many thanks to Simon \o/

Changed in cargo (Ubuntu):
assignee: nobody → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

> This is because Rust upstream has platform tiering. Some tests are not guaranteed to pass on certain operating systems + CPU combinations. For instance, upstream tests may include float-point arithmitics behavior validations that might not pass on architectures where float-point calculation does not conform fully to IEEE 754 (e.g. MIPS).
Rust upstream only gates the release depending on whether all the Tier 1 platforms pass the tests (see https://doc.rust-lang.org/rustc/platform-support.html for the Tiering, Tier 1 is considered to be both Tier 1 with Host Tools and Tier 1 without Host Tools).

This seems to be a little random. Shouldn't the tests which ought to pass on a given architecture be really identified as such? It seems that it’s an open gate for unnoticed regressions, or is there a some kind of manifest of "this tests passed in the previous build on that architecture, and so, it should pass again with the new build'?

> This is because Rust upstream adds new warnings to the compiler nearly every release. Cargo in the Debian tarball is usually pinned to a release branch and does not receive the fixes unless the fix is critical (and in Debian's case, some dependencies are probably not vendored correctly, thus producing warnings, but this will be changed in Ubuntu hopefully very soon).

So, what’s our strategy to read and ensure the warnings that are produced are under controlled and "expected"?

Revision history for this message
Zixing Liu (liushuyu-011) wrote :

> This seems to be a little random. Shouldn't the tests which ought to pass on a given architecture be really identified as such? It seems that it’s an open gate for unnoticed regressions, or is there a some kind of manifest of "this tests passed in the previous build on that architecture, and so, it should pass again with the new build'?

The upstream only setup CI testing for Tier 1 platforms (due to technical and resource constraints). So Tier 2 and Tier 3 platforms are untested (Tier 3 platforms are not even guaranteed to be able to build, as there are some very exotic targets in there).

> So, what’s our strategy to read and ensure the warnings that are produced are under controlled and "expected"?

Rust upstream has a mechanism to record warnings and compare the warnings with each run to see what warnings are new.
Since the Rust toolchain needs to be able to compile under the older compiler version, some warnings are likely unavoidable without breaking the build with older compiler (which we need to bootstrap the newer version).

Revision history for this message
Zixing Liu (liushuyu-011) wrote :

Since rustc and cargo are merged into one source package, they share the same vendored dependency tree.

I am unsure if this would impact the MIR and security reviewing process...?

Revision history for this message
Mark Esler (eslerm) wrote :

@schopin, could you weigh in?

https://launchpad.net/ubuntu/+source/rustc/1.68.0+dfsg0ubuntu1-0ubuntu1 appears to (accidentally) circumvent this MIR

How have the MIRs for cargo dependencies been affected? https://bugs.launchpad.net/ubuntu/+source/http-parser/+bug/1990655

Revision history for this message
Simon Chopin (schopin) wrote :

First off, my bad, I didn't think of this MIR when I sponsored the upload, I should have.

From our perspective, it doesn't change much regarding the MIR, as the goal is to have the cargo binary package in main. Just because it's now built from src:rustc rather than src:cargo doesn't mean it magically was seeded, so the MIR is still needed.

AFAIK, the MIRs for the dependencies are still valid, the final cargo binary is pretty much the same as it was with the split source package (with minor differences that Zixing is in a better position to explain).

The source merge actually makes the toolchain more robust as it enforces using the same versions as the ones usually consumed together in the wild, reducing the possibilities of us encountering rare bugs due to version mismatch between cargo and rustc.

Now, that's from our perspective. However, there are two problems that I didn't see coming with the merge:

1/ It makes your life harder in terms of review, because now you need to figure out which part of the code is actually involved in cargo vs other, non-MIRed tools
2/ It blows up the vendored sources data, filling it with a much bigger set of dependencies that aren't relevant to the rustc binary (IOW the package that's *currently* in main)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I think no one is opposed to the bundling, but all the code and the way it is supposed to be built, maintained, and tested need to be approved before adding it. No matter if as individual source or as vendored code in rustc.

The problem is that "doesn't mean it magically was seeded" is sadly untrue.
rustc is in main, if this would migrate then all the cargo code would be in main and thereby "magically seeded" bypassing the process and quality control.

I'd ask to set block-proposed to hold it back and coordinate with security and the MIR team in which form you want this to get reviewed, approved and then go into Ubuntu.

Changed in dh-cargo (Ubuntu):
assignee: Christian Ehrhardt  (paelzer) → nobody
Revision history for this message
Lukas Märdian (slyon) wrote (last edit ):

Tagging block-proposed (and a rustc bug-task) until this is resolved.

PS: I see that there are also new (universe) dependencies from the now uploaded (merged) rustc package (rust-doc binary), which need to be resolved, namely:
- fonts-nanum
- fonts-open-sans
- highlight.js

Or maybe the rust-doc binary should be demoted to universe instead..

Changed in rustc (Ubuntu):
assignee: nobody → Ubuntu Foundations Bugs (foundations-bugs)
tags: added: block-proposed
Revision history for this message
Zixing Liu (liushuyu-011) wrote :

I think both rust-doc and cargo-doc could be demoted to the universe if necessary.

For the newly merged source package, rustc and cargo do share the same vendored tree, but they do not share the business logic.

Revision history for this message
Łukasz Zemczak (sil2100) wrote :
Download full text (11.5 KiB)

As per our discussion, I have temporarily rustc in mantic-proposed to universe:

$ change-override -c universe -S rustc -s mantic-proposed
Override component to universe
rustc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic: main/misc -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic amd64: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic arm64: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic armhf: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic i386: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic ppc64el: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic riscv64: universe/devel/extra/100% -> universe
cargo 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic s390x: universe/devel/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic amd64: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic arm64: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic armhf: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic i386: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic ppc64el: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic riscv64: main/doc/extra/100% -> universe
cargo-doc 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic s390x: main/doc/extra/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic amd64: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic arm64: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic armhf: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic i386: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic ppc64el: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic riscv64: main/libs/optional/100% -> universe
libstd-rust-1.68 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic s390x: main/libs/optional/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic amd64: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic arm64: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic armhf: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic i386: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic ppc64el: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic riscv64: main/libdevel/extra/100% -> universe
libstd-rust-dev 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic s390x: main/libdevel/extra/100% -> universe
rust-all 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic amd64: universe/devel/optional/100% -> universe
rust-all 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic arm64: universe/devel/optional/100% -> universe
rust-all 1.68.0+dfsg0ubuntu1-0ubuntu1 in mantic armhf: universe/devel/optional/100% -> universe
rust-all 1.68.0+dfsg0ubuntu1-...

Revision history for this message
Lukas Märdian (slyon) wrote :

In order to untangle this process, we decided to demote src:rust to universe, until all of rustc and cargo is clear from MIR and security POV. Then, the new (combined) package can be re-promoted into "main".

In the meantime, Zixing can continue working on rustc in universe to satisfy the Firefox and Kernel needs.

Removing the "block-proposed" tag.

tags: removed: block-proposed
Simon Chopin (schopin)
summary: - MIR: cargo, dh-cargo
+ [MIR] cargo, dh-cargo
Revision history for this message
Paulo Flabiano Smorigo (pfsmorigo) wrote :
Download full text (4.3 KiB)

I reviewed cargo 0.68.0+ds0ubuntu1-0ubuntu1 as checked into lunar (mantic
version was not available at the time for this review). This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

Cargo is the Rust package manager. Cargo downloads your Rust package's
dependencies, compiles your packages, makes distributable packages, and uploads
them to crates.io, the Rust community's package registry. Cargo has been
written in Rust and it's an important tool for any program that uses this
language.

- CVE History
  - CVE-2022-36113 (low): Fixed in 0.67.0
  - CVE-2022-36114 (low): Fixed in 0.67.0
  - CVE-2022-46176 (medium): Fixed in 0.67.0
  - CVE-2019-16760 (medium): Fixed in 0.26.0
- Build-Depends
  - Building tools: debhelper, dpkg-dev, pkg-config
  - Rust dependencies: rustc, libstd-rust-dev, dh-cargo
  - Python 3 for some helper scripts
  - Other dependencies: libcurl, libssh, libgit, libhttp-parser, libssl, zlib1g
- pre/post inst/rm scripts
  - none
- init scripts
  - none
- systemd units
  - none
- dbus services
  - none
- setuid binaries
  - none
- binaries in PATH
  - cargo executable in /usr/bin
  - scripts in /usr/share/cargo/scripts
- sudo fragments
  - none
- polkit files
  - none
- udev rules
  - none
- unit tests / autopkgtests
  - Good amount of integration tests located in the testsuite directory
- cron jobs
  - none
- Build logs
  - Some warnings or unused import or variable
  - Some intra-doc link fails that cause some "warning: unresolved link to..."
  - Some warnings about warning future incompatibility:
    "this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!"
  - No errors logs
- Processes spawned
  - none
- Memory management
  - Rust has the memory managed through a system of ownership with a set of
    rules that the compiler checks. The only exception is when the code uses
 the unsafe approach, witch happens a few time (see semgrep). On the good
 side, all those cases seems fine, without any major concern.
  - During the research an open issue was found about a possible memory leak
    while using cargo inside a docker container. It seems to be a problem with
 libgit and there is a work around for it:
 https://github.com/rust-lang/cargo/issues/10583
- File IO
  - Seems safe. Nothing concerning found.
- Logging
  - Seems safe. It uses an external library for logging (inside vendor/log).
- Environment variable usage
  - Seems safe.
- Use of privileged functions
  - none
- Use of cryptography / random number sources etc
  - Cargo relies on RustCrypto (https://github.com/RustCrypto/utils) to
    do the cryptography. Seems to be a safe and well maintaned library.
- Use of temp files
  - Low usage. Seems safe.
- Use of networking
  - Use mainly curl (vendor/{curl,curl-sys,git2-curl}) to handle packages
    downloads.
  - Also use an external library to handle sockets (vender/socket2).
  - Other vendor, like libc, winapi, rustix, io-lifetimes, url,
    linux-raw-sys, have network functions but could identify if they
 are used by cargo.
- Use of WebKit
  - none
- Use of PolicyKit
  - none

- Any significant cppcheck results
  - none
- Any significant Coverity results
  ...

Read more...

Changed in cargo (Ubuntu):
status: Incomplete → In Progress
Revision history for this message
Zixing Liu (liushuyu-011) wrote :

I believe there are some minor issues with the report:

> - CVE History
I would recommend taking a look at https://github.com/rust-lang/cargo/security. Rust upstream will publish all the CVE information here.

> - Other vendor, like libc, winapi, rustix, io-lifetimes, URL,
> linux-raw-sys, have network functions but could identify if they
> are used by cargo.
In newer version of Cargo, it also has gitoxide, destined to replace libgit2. This library obviously also connects to the Internet.

> Some usage of "unsafe" method in the main tree and also in vendors. It
> seems necessary to do those kind of operations but also disable rust
> protections that avoids memory leaks out out of bound problems

This is a common misconception of what `unsafe` means. In Rust, `unsafe` only allows you to deference raw pointers and call to other `unsafe` functions (e.g. functions from across the FFI boundary). All the other smart pointers or safe constructs are still under the compiler's supervision.

Out-of-bound (OOB)/double-free/use-after-free (UAF) issues in `unsafe` usually occur outside of Rust territory (may be caused by external functions).

In safe Rust, memory leaks can also occur due to cyclic memory references (when using referenced pointers like `Rc` or `Arc`).

In `unsafe` Rust, assuming semantic soundness is proved for all the `unsafe` blocks, the whole program is still sound. Please see https://dl.acm.org/doi/10.1145/3158154 for more information on how this could be provable.

Revision history for this message
Lukas Märdian (slyon) wrote :

Looks like we got the security team's +1 for src:cargo \o/

So dh-cargo seems to be fine as per comment #3

I'd like to ask @didrocks to double-check if his concerns are addressed by Zixing's latest comments, in order to unblock src:cargo.

Changed in cargo (Ubuntu):
status: In Progress → Incomplete
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Indeed, the latests comments from Zixing helps understanding and addressing my last comments, so +1 from the MIR side.

Changed in cargo (Ubuntu):
status: Incomplete → Fix Committed
Changed in rustc (Ubuntu):
importance: Undecided → High
Revision history for this message
Steve Langasek (vorlon) wrote :

My understanding is the last comment is that this MIR is approved, but the 'fix committed' task was on the cargo source package which is the one being obsoleted in favor of rustc so this did not show up on https://ubuntu-archive-team.ubuntu.com/component-mismatches-proposed.html as ready to promote. I'm promoting rustc to main now based on this understanding.

Revision history for this message
Steve Langasek (vorlon) wrote :
Download full text (7.7 KiB)

Override component to main
rustc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic: universe/misc -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic amd64: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic arm64: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic armhf: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic i386: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic ppc64el: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic riscv64: universe/devel/extra/100% -> main
cargo 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic s390x: universe/devel/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic amd64: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic arm64: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic armhf: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic i386: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic ppc64el: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic riscv64: universe/doc/extra/100% -> main
cargo-doc 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic s390x: universe/doc/extra/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic amd64: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic arm64: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic armhf: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic i386: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic ppc64el: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic riscv64: universe/libs/optional/100% -> main
libstd-rust-1.71 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic s390x: universe/libs/optional/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic amd64: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic arm64: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic armhf: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic i386: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic ppc64el: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic riscv64: universe/libdevel/extra/100% -> main
libstd-rust-dev 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic s390x: universe/libdevel/extra/100% -> main
rust-all 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic amd64: universe/devel/optional/100% -> main
rust-all 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic arm64: universe/devel/optional/100% -> main
rust-all 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic armhf: universe/devel/optional/100% -> main
rust-all 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic i386: universe/devel/optional/100% -> main
rust-all 1.71.1+dfsg0ubuntu1-0ubuntu2 in mantic ppc64el: universe/devel/optional/100% ->...

Read more...

Changed in rustc (Ubuntu):
status: New → Fix Released
Changed in cargo (Ubuntu):
status: Fix Committed → Won't Fix
Revision history for this message
Mark Esler (eslerm) wrote :

Please see LP#1957932 and promote cargo/rustc dependencies to main.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

dh-cargo was ACKED in commented #3, status was not changed, doing so now.

Changed in dh-cargo (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Matthias Klose (doko) wrote :

Override component to main
dh-cargo 30ubuntu1 in mantic: universe/rust -> main
dh-cargo 30ubuntu1 in mantic amd64: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic arm64: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic armhf: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic i386: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic ppc64el: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic riscv64: universe/devel/optional/100% -> main
dh-cargo 30ubuntu1 in mantic s390x: universe/devel/optional/100% -> main
8 publications overridden.

Changed in dh-cargo (Ubuntu):
status: Fix Committed → Fix Released
Benjamin Drung (bdrung)
tags: removed: foundations-todo
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.