Review for Source Package: raspi-utils

[Summary]
This is a collection of offical tools from RaspberryPi Ltd. Especially,
"vcgencmd" and "dtoverlay". It replaces the "libraspberrypi-bin" package
(from src:raspberrpi-userland). The utilities are required by "rpi-eeprom"
for updating the boot EEPROM on Raspberry Pi.
This package is not shipped in Debian, but taken from the Raspbian repository,
and has a few packaging shortcomings, which should be improved (see below).

MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main:
raspi-utils-core, raspi-utils-otp, raspi-utils-dt, raspi-utils-eeprom, raspinfo
Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
#0 I'm requesting security review for the extensive use of sprintf/malloc, the
  use of sudo & LD_LIBRARY_PATH as part of the utility scripts and the
  parsing of device tree (overlay) data.

Required TODOs:
#1 Can you please clarif how rpi-eeprom did function previously, without those
  utils? Was it all part of "libraspberrypi-bin"? How is that going to be
  replaced in "main"? Does it require additional test cases, now that we have
  rpi-eeprom and raspi-utils?
#2 Please clarify: How are you going to replace the "libraspberrypi-bin" binary
  from src:raspberrypi-userland? Do we need to Conflict/Replace the old
  libraspberrypi-bin package?
#3 You mention "Due to this being a runtime dependency of rpi-eeprom [...]"
  => I could not confirm this dependency. Can you please clarify? Also, would
     that introduce a circular dependency, as there's also the Dependency of
     raspinfo->rpi-eeprom
#4 kms++-utils is listed as a Recommends but does not exist (not in main, nor in
  Debian/Ubuntu at all). Please drop it (or downgrade to Suggests at least).
#5 Please specify the exact manual test case(es), as used during ISO testing
  to verify this package and link them from the MIR bug description.
  => Maybe something from here?
     https://iso.qa.ubuntu.com/qatracker/milestones/464/builds/321949/testcases
#6 avoid usage of "sudo" in raspinfo and LD_LIBRARY_PATH in overlaycheck

Recommended TODOs:
#7 The package should get a team bug subscriber before being promoted
#8 can you run "systemd-analyze security rpi-eeprom-update.service" to find
  suggestions for isolating this functionality (I know that services is from
  a different source package, but it's related and might give good hints for
  this package, too).
#9 Please investigate if we can have some automated build-time tests. Thinking
  of the device-tree overlay merging util, this sounds like something which can
  be done offline (during tests) and validated at build-time. There might be
  other cases like this for the different raspi-utils tools.
#10 Consider fixing some of the Lintian warning listed  below in [Packaging red 
flags]
#11 debian/watch is tracking the raspberrypi.org archive. Consider if we should
  rather track the upstream GitHub repository and spin date-based snapshots
  ourselves. This would allow pulling in the latest version (including the new
  "kdtc" tool).
#12 Consider fixing the CMake warning listed below in [Upstream red flags]
#13 help fixing FTBFS with gcc 14.1, 
https://github.com/raspberrypi/utils/issues/87

[Rationale, Duplication and Ownership]
- There is no other package in main providing the same functionality.
- A team is committed to own long term maintenance of this package.
  => Foundations Architecture
- The rationale given in the report seems valid and useful for Ubuntu

[Dependencies]
OK:
- no -dev/-debug/-doc packages that need exclusion

Problems:
- other Dependencies to MIR due to this
  + rpi-eeprom (not detected by check-mir, but seems fine)
  + kms++-utils (Recommends, not packages, should be Suggests)
- dependencies in main that are only superficially tested requiring
  more tests now. (rpi-eeprom, bug 1895137)

[Embedded sources and static linking]
OK:
- no embedded source present (all included tools are part of the upstream repo)
- no static linking
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

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 expose any external endpoint (port/socket/... or similar)
- 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:
- does parse data formats (device-trees) from an untrusted source.
- this makes appropriate (for its exposure) use of established risk
  mitigation features (dropping permissions, using temporary environments,
  restricted users/groups, seccomp, systemd isolation features,
  apparmor, ...)

[Common blockers]
OK:
- does not FTBFS currently
- This does need special HW for thorough testing, but all options to
  get this covered have been exhausted and based on demonstration of
  enough investigation and proof of why there is currently no other
  option it is accepted as-is as a compromise.
  The owning team is committed and aware of the implications for
  ongoing maintenance.
- no new python2 dependency
- Python package, but using dh_python

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest
  * This does seem to need special HW for build or test so it can't be
     automatic at build or autopkgtest time. But as outlined
     by the requester in [Quality assurance - testing] there:
       - is hardware and a test plan or code:
         + Raspi hardware owned by Foundations team members
         + test cases: 
https://iso.qa.ubuntu.com/qatracker/milestones/464/builds/321949/testcases

[Packaging red flags]
OK:
- symbols tracking not applicable for this kind of code.
- Upstream update history is good
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- debian/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Ubuntu does carry a delta (not packaged in Debian)
- Debian/Ubuntu update history is sporadic (initial upload)
- debian/watch is present and looks ok (if needed, e.g. non-native)
  - tracking archive.raspberrypi.org instead of upstream GitHub (no 
tags/releases)
- the current release is packaged – there are no releases. Latest upstream 
contains a new tool "kdtc"
- should it Conflict/Replace the old libraspberrypi-bin package?
- some Lintian warnings
"""
I: raspi-utils: capitalization-error-in-description meta-package metapackage
I: raspi-utils-eeprom: extended-description-is-probably-too-short
I: raspi-utils-otp: extended-description-is-probably-too-short
I: raspi-utils source: patch-not-forwarded-upstream 
[debian/patches/0001-fix-raspiotp-manpage-groff.patch]
I: raspi-utils source: quilt-patch-missing-description 
[debian/patches/interpreter_fix.diff]
I: raspi-utils source: quilt-patch-missing-description 
[debian/patches/overlaycheck.diff]
I: raspi-utils-core: typo-in-manual-page occuring occurring 
[usr/share/man/man1/vcgencmd.1.gz:198]
X: raspinfo: package-contains-no-arch-dependent-files
X: raspi-utils source: upstream-metadata-file-is-missing
"""

[Upstream red flags]
OK:
- no Errors during the build
- no use of user nobody
- no use of setuid / setgid
- no dependency on webkit, qtwebkit or libseed
- not part of the UI for extra checks
- no translation present, but none needed for this case (user visible)?

Problems:
- lots of malloc/sprintf usage (not snprintf)
- use of sudo (raspinfo) and LD_LIBRARY_PATH (overlaycheck)
- important open bugs (FTBFS, gcc 14.1):
  https://github.com/raspberrypi/utils/issues/87
- warnings during the build:
"""
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_EXPORT_NO_PACKAGE_REGISTRY
    CMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY
    CMAKE_FIND_USE_PACKAGE_REGISTRY
    FETCHCONTENT_FULLY_DISCONNECTED
"""

** Bug watch added: github.com/raspberrypi/utils/issues #87
   https://github.com/raspberrypi/utils/issues/87

** Changed in: raspi-utils (Ubuntu)
       Status: New => Incomplete

** Changed in: raspi-utils (Ubuntu)
     Assignee: Lukas Märdian (slyon) => (unassigned)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2094805

Title:
  [MIR] raspi-utils

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/raspi-utils/+bug/2094805/+subscriptions


-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to