[Bug 1942394] Re: [MIR] mdevctl 1.0.0 (rust switch)

Mon Aug 1 10:19:24 UTC 2022

Review for Package: src:mdevctl

[Summary]
This is the 1st Rust package to be MIRed, using newly adopted MIR rules.
The package looks rather clean and under control, except for all of the vendored
dependencies/sources, which can be overwhelming. But we have the Sever team's
written consent to stay on top of them for the lifetime of the package.
Lot's of "Recommended TODOs" have been stated below, which should be improved
upon step by step, but nothing critical blocking the promotion IMO.

MIR team ACK

This does need a security review, so I'll assign ubuntu-security
That is because it's 1st the Rust package and we want the security-team's input
on this. Doesn't seem super security senstive otherwise (maybe the vendored libc
bindings).

List of specific binary packages to be promoted to main: mdevctl
Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
- This is the 1st Rust package to be MIRed, using newly adopted MIR rules.
- embedded source present
- static linking in use
- handles data formats (JSON output) via serde-rs JSON library

Required TODOs:
- None

Recommended TODOs:
#1:
- dh_auto_test does not recognize the 'nocheck' DEB_BUILD_OPTION
  I: mdevctl source: override_dh_auto_test-does-not-check-DEB_BUILD_OPTIONS [debian/rules:22]
  => This should be fixed according to lintian:
     ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))

#2:
- does NOT have a non-trivial test suite that runs as autopkgtest
  => as lined out in the bug description, this should be fixed mid/long term.

#3.1: Lintian warnings/errors, some that feel worth mentioning:
- E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-i686-pc-windows-gnu/lib/*.a
- E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-x86_64-pc-windows-gnu/lib/*.a
  => Seems to be OK, as those are only Windows libraries, unused in Ubuntu.

#3.2:
- I: mdevctl source: out-of-date-standards-version 4.6.0 (released 2021-08-18) (current is 4.6.1.0)
  => can be fixed with the next version bump

#3.3:
- P: mdevctl source: very-long-line-length-in-source-file
  => lots of those, maybe they should be muted.
     It's false positives as it is complaining about .a windows binaries

#3.4: (some lintian recommendations)
  X: mdevctl source: prefer-uscan-symlink filenamemangle s/.+\/v?(\d\S+)\.tar\.gz/mdevctl-$1\.tar\.gz/ [debian/watch:3]
  X: mdevctl source: upstream-metadata-file-is-missing

#4: upstream build-time warnings (in vendored code). could be double checked
    and reported to the upstream developers
- warnings during the build (vendorized deps); some deprecations
  warning: `ansi_term` (lib) generated 12 warnings
  warning: `vec_map` (lib) generated 3 warnings
  warning: `log` (lib) generated 1 warning
  warning: `regex-syntax` (lib) generated 2 warnings
  warning: `aho-corasick` (lib) generated 3 warnings
  warning: `clap` (lib) generated 77 warnings
  warning: `proc-macro-error` (lib) generated 1 warning

[Duplication]
There is no other package in main providing the same functionality.
Except the previous (non-rust) version of mdevctl itself.

[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:
- not a go package, no extra constraints to consider in that regard
- vendoring is used, but the reasoning is sufficiently explained
  - rust: static builds are used, the team confirmed their commitment
    to the additional responsibilities implied by static builds.
- Rust package that has all dependencies vendored. It does neither
  have *Built-Using (after build). Nor does the build log indicate
  built-in sources that are missed to be reported as Built-Using.
- rust package using dh_cargo (dh ... --buildsystem cargo)
- Includes vendored code, the package has documented how to refresh this
  code at debian/README.source
- does not have unexpected Built-Using entries (only "rustc" toolchain)

Notes:
- embedded source present
- static linking in use

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 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, ...)

Notes:
- handles data formats (JSON output) via serde-rs JSON library

Problems: None

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- no new python2 dependency

Problems:
- dh_auto_test does not recognize the 'nocheck' DEB_BUILD_OPTION
- does NOT have a non-trivial test suite that runs as autopkgtest

[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 and looks ok (if needed, e.g. non-native)
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- d/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Lintian warnings/errors, some that feel worth mentioning:
  E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-i686-pc-windows-gnu/lib/*.a
  E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-x86_64-pc-windows-gnu/lib/*.a
  => Seems to be OK, as those are only Windows libraries unused in Ubuntu.
  I: mdevctl source: override_dh_auto_test-does-not-check-DEB_BUILD_OPTIONS [debian/rules:22]
  I: mdevctl source: out-of-date-standards-version 4.6.0 (released 2021-08-18) (current is 4.6.1.0)
  P: mdevctl source: very-long-line-length-in-source-file (lots of those, maybe they should be muted. It's false positives as it is complaining about .a windows binaries)
  X: mdevctl source: prefer-uscan-symlink filenamemangle s/.+\/v?(\d\S+)\.tar\.gz/mdevctl-$1\.tar\.gz/ [debian/watch:3]
  X: mdevctl source: upstream-metadata-file-is-missing

[Upstream red flags]
OK:
- no incautious use of malloc/sprintf (as far as we can check it)
  Rust helps with that :)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this case (user visible)?

Problems:
- warnings during the build (vendorized deps); some deprecations
  warning: `ansi_term` (lib) generated 12 warnings
  warning: `vec_map` (lib) generated 3 warnings
  warning: `log` (lib) generated 1 warning
  warning: `regex-syntax` (lib) generated 2 warnings
  warning: `aho-corasick` (lib) generated 3 warnings
  warning: `clap` (lib) generated 77 warnings
  warning: `proc-macro-error` (lib) generated 1 warning

** Changed in: mdevctl (Ubuntu)
     Assignee: Lukas Märdian (slyon) => Ubuntu Security Team (ubuntu-security)

-- 
You received this bug notification because you are a member of Ubuntu
Package Archive Administrators, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1942394

Title:
  [MIR] mdevctl 1.0.0 (rust switch)

To manage notifications about this bug go to:
https://bugs.launchpad.net/mdevctl/+bug/1942394/+subscriptions