debcrafters-packages team mailing list archive

[Giampaolo Fresi Roglia <2113961@xxxxxxxxxxxxxxxxxx>]

I reviewed util-linux 2.41-4ubuntu3 as checked into questing. This
shouldn't be considered a full audit but rather a quick gauge of
maintainability. Given "All packages currently in Universe should
remain in Universe", the packages bin:lastlog2, bin:libpam-lastlog2
and bin:util-linux-extra has been excluded from the review.

util-linux is a suite of essential utilities for any Linux system. It
contains many basic utilities used for setting up partitions and basic
system infrastructure on a Linux system.

- CVE History
  - util-linux had 22 CVEs over 20 years, the last being from 2024.
    The issues has been addressed properly and on time by upstream. No
    concerns here.
- Build-Depends
  - debhelper-compat
  - dh-exec
  - dh-package-notes
  - dh-sequence-installsysusers
  - dh-sequence-zz-debputy-rrr
  - asciidoctor
  - bc
  - bison
  - flex
  - gettext
  - libaudit-dev
  - libcap-ng-dev
  - libcrypt-dev
  - libcryptsetup-dev
  - libncurses-dev
  - libpam0g-dev
  - libreadline-dev
  - libselinux1-dev
  - libsqlite3-dev
  - libsystemd-dev
  - libtool
  - libudev-dev
  - netbase
  - pkgconf
  - po-debconf
  - po4a
  - socat
  - systemd
  - systemd-dev
  - zlib1g-dev
- pre/post inst/rm scripts
  - everything seems sound
- init scripts
  - uuid-runtime installs /etc/init.d/uuidd
- systemd units
  - util-linux installs the `fstrim` service
  - uuid-runtime installs the `uidd` service
- dbus services
  - none
- setuid binaries
  - login: `/usr/bin/newgrp`
  - mount: `/usr/bin/mount` and `/usr/bin/umount`
  - util-linux: `/usr/bin/su`
- binaries in PATH
  - bsdextrautils
    - /usr/bin/col
    - /usr/bin/colcrt
    - /usr/bin/colrm
    - /usr/bin/column
    - /usr/bin/hexdump
    - /usr/bin/look
    - /usr/bin/ul
    - /usr/bin/hd
  - bsdutils
    - /usr/bin/logger
    - /usr/bin/renice
    - /usr/bin/script
    - /usr/bin/scriptlive
    - /usr/bin/scriptreplay
    - /usr/bin/wall
  - eject
    - /usr/bin/eject
  - fdisk
    - /usr/sbin/cfdisk
    - /usr/sbin/fdisk
    - /usr/sbin/sfdisk
  - login
    - /usr/bin/login
    - /usr/bin/newgrp
    - /usr/sbin/nologin
    - /usr/bin/sg
  - mount
    - /usr/bin/mount
    - /usr/bin/umount
    - /usr/sbin/losetup
    - /usr/sbin/swapoff
    - /usr/sbin/swapon
  - rfkill
    - /usr/sbin/rfkill
  - util-linux
    - /usr/bin/choom
    - /usr/bin/chrt
    - /usr/bin/dmesg
    - /usr/bin/fallocate
    - /usr/bin/findmnt
    - /usr/bin/flock
    - /usr/bin/getopt
    - /usr/bin/hardlink
    - /usr/bin/ionice
    - /usr/bin/ipcmk
    - /usr/bin/ipcrm
    - /usr/bin/ipcs
    - /usr/bin/lsblk
    - /usr/bin/lscpu
    - /usr/bin/lsipc
    - /usr/bin/lslocks
    - /usr/bin/lslogins
    - /usr/bin/lsmem
    - /usr/bin/lsns
    - /usr/bin/mcookie
    - /usr/bin/more
    - /usr/bin/mountpoint
    - /usr/bin/namei
    - /usr/bin/nsenter
    - /usr/bin/partx
    - /usr/bin/prlimit
    - /usr/bin/rename.ul
    - /usr/bin/rev
    - /usr/bin/setarch
    - /usr/bin/setpriv
    - /usr/bin/setsid
    - /usr/bin/setterm
    - /usr/bin/su
    - /usr/bin/taskset
    - /usr/bin/uclampset
    - /usr/bin/unshare
    - /usr/bin/wdctl
    - /usr/bin/whereis
    - /usr/sbin/agetty
    - /usr/sbin/blkdiscard
    - /usr/sbin/blkid
    - /usr/sbin/blkzone
    - /usr/sbin/blockdev
    - /usr/sbin/chcpu
    - /usr/sbin/chmem
    - /usr/sbin/findfs
    - /usr/sbin/fsck
    - /usr/sbin/fsfreeze
    - /usr/sbin/fstrim
    - /usr/sbin/isosize
    - /usr/sbin/ldattach
    - /usr/sbin/mkfs
    - /usr/sbin/mkswap
    - /usr/sbin/pivot_root
    - /usr/sbin/readprofile
    - /usr/sbin/rtcwake
    - /usr/sbin/runuser
    - /usr/sbin/sulogin
    - /usr/sbin/swaplabel
    - /usr/sbin/switch_root
    - /usr/sbin/wipefs
    - /usr/sbin/zramctl
    - /usr/bin/i386
    - /usr/bin/linux32
    - /usr/bin/linux64
    - /usr/bin/x86_64
    - /usr/sbin/getty
  - uuid-runtime
    - /usr/bin/uuidgen
    - /usr/bin/uuidparse
    - /usr/sbin/uuidd
- sudo fragments
  - None
- polkit files
  - None
- udev rules
  - None
- unit tests / autopkgtests
  - util-linux does have unit tests and the build fails if those tests fail.
  - the source package does have autopkgtests.
- cron jobs
  - None
- Build logs
  - Nothing to be reported

- Processes spawned
  - A lot of binaries do spawn subprocesses to provide the features
    they are supposed to.
  - Here is a non-exaustive list of those:
	- login
	- sulogin
	- script
	- newgrp
	- getty
	- agetty
	- su
	- vipw
	- eject
	- fsck
	- mkfs
- Memory management
  - heavy use of memory related functions.
  - some binaries use sprintf after allocating enough buffer space.
  - most binaries make use of a set of wrapper functions for handling
    string allocation.
  - an exhaustive check of all the code is impractical. I limited the
    manual code analysis to the most critical components and the most
    suspicious findings. Everything seems to be done properly.
- File IO
  - looks ok.
- Logging
  - Looks ok. Format functions seem to be used properly. Destination
    buffers are checked for overflow.
- Environment variable usage
  - the package makes use of a lot of env variables. Some variable
    like `HOME`, `USER`, `SHELL`, `LOGNAME`, etc, are set by utils
    like `su`, `login` and `setpriv`. Some others, including the
    already mentioned ones are read. Among those, the sensitive ones
    like `CREDENTIALS_DIRECTORY` are accessed via the wrapper
    `safe_getenv()` which uses `secure_getenv()`.
- Use of privileged functions
  - heavy use of privileged functions from tools like `newgrp`,
    `login`, `agetty`, `su`, `nsenter`, `setpriv`, etc. Their use is
    expected and the code is written reasonably well.
- Use of cryptography / random number sources etc
  - None
- Use of temp files
  - None
- Use of networking
  - uuidd: listens to unix socket
  - agetty: listens to NETLINK socket for "reload" messages.
- Use of WebKit
  - None
- Use of PolicyKit
  - None
- Any significant cppcheck results
  - false positives. most of them due to macros unknown to cppcheck.
- Any significant Coverity results
  - Coverity reported more than 1800 findings. Most of them (~1600)
    are Low severity findings due to the analyzer not being able to
    model some structures.
  - the vast majority of the Coverity results are false positives.
  - Among the High severity findings, some are plain false positives
    (Bad Free, Uninitialized pointer read), some (Resource Leak) are
    not really issues (one shot cli programs, memory gets freed at
    termination), others (Integer Overflows) do depend on data whose
    tampering would already mean the system has already been
    compromised.
  - Among the Medium severity findings we are in front of a similar
    scenario as for the High severity ones. Some are plain false
    positives, other findings are related to test files or sample
    code.
- Any significant shellcheck results
  - only in tests
- Any significant Semgrep results
  - suggests the usage of `strtok_r()` in place of `strtok()`. This is
    safe to ignore, since the tools do not make use of concurrency.

Overall, the code seems to be readable and well written. On the
maintainability part, packaging is complex but the package is very
important for Debian too, for this reason we do not have any concern.

Security team ACK.


** Changed in: util-linux (Ubuntu)
     Assignee: Ubuntu Security Team (ubuntu-security) => (unassigned)

-- 
You received this bug notification because you are a member of
Debcrafters packages, which is subscribed to util-linux in Ubuntu.
https://bugs.launchpad.net/bugs/2113961

Title:
  [MIR] util-linux

Status in util-linux package in Ubuntu:
  In Progress

Bug description:
  [Availability]
  The package src:util-linux is already in Ubuntu main.
  The package src:util-linux build for the architectures it is designed to work on.
  It currently builds and works for architectures: amd64, arm64, armhf, i386, ppc64el, riscv64, s390x
  Link to package https://launchpad.net/ubuntu/+source/util-linux

  [Rationale]

  See previous rational below for what actually sparked this MIR.
  Now that `bin:liblastlog2-2` has been promoted and everything is unblocked, the
  rationale becomes as simple as an ask for a re-review for one of the `Essential`
  packages, shipping, among other things, a few `suid` binaries in absolutely
  every form Ubuntu can take.
  https://canonical-ubuntu-project.readthedocs-hosted.com/MIR/mir-rereview/#opt-in-re-review

  Original rationale:
  Okay, it seems the MIR template doesn't apply well for this use-case, because
  it more or less assumes that the MIR is about a source package that is currently
  in universe. In the current situation, only an existing binary package needs to be
  promoted, from a source package already in main. I'll do my best to adapt the
  template and provide a good rational.

  - bin:liblastlog2-2 is provided by src:util-linux, and was already there in
    plucky/universe.
  - The package src:util-linux is generally useful for a large part of
    our user base: it provides the bin:util-linux package, that is even flagged as
    `Essential: yes`.
    This is the package providing, among many other things, the `su`, `fsck`,
    `flock`, or `mkswap` binaries, all mostly essential to any system (random
    selection of important commands to give a quick example).
  - The package bin:liblastlog2-2 is a new runtime dependency of package
    bin:util-linux that we already support.
  - The binary packages liblastlog2-2 needs to be in main to have the latest merge
    of util-linux migrate from questing-proposed to questing.
  - All other binary packages currently in universe built by src:util-linux should
    remain in universe.
  - The package bin:liblastlog2-2 is required in Ubuntu main no later than
    somewhere in July due to some partners requiring patches to be SRU'd to Noble,
    and thus needing the package to migrate from -proposed (even though it's not a
    hard block from the SRU team, according to what I've red on Matrix recently).

  [Security]
  - Obviously, util-linux has had some security issues in the past (although not
    that much):
    - https://ubuntu.com/security/cves?package=util-linux
    - https://security-tracker.debian.org/tracker/source-package/util-linux
  - Those issues seems to be handled correctly in both Ubuntu and Debian:
    - https://ubuntu.com/security/CVE-2024-28085
    - https://security-tracker.debian.org/tracker/CVE-2024-28085
    - https://security-tracker.debian.org/tracker/CVE-2021-37600

  - There are countless binaries in sbin, but I'm fairly confident taking them out
    is a big plan of its own to still have a working system.
  - There are just a couple systemd units:
    - fstrim.{service,timer}: Discard unused filesystem blocks once a week
    - lastlog2-import.service: Import lastlog data into lastlog2 database - run
      only once in some particular situations to handle a data migration

  - About common isolation/risk-mitigation:
    - I'm not sure anything in util-linux is opening privileged ports.
    - I know some binaries are dropping privileges.
    - Going much further on that topic would be a full audit, for which I
      unfortunately don't really have time and competency for. I hope that's okay.

  - Packages does not contain extensions to security-sensitive software
  (filters, scanners, plugins, UI skins, ...)

  [Quality assurance - function/usage]
  - The package works well right after install

  [Quality assurance - maintenance]
  - The package is maintained well in Debian/Ubuntu/Upstream and does
    not have too many, long-term & critical, open bugs
    - Ubuntu https://bugs.launchpad.net/ubuntu/+source/util-linux/+bugs?orderby=-importance&start=0
    - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=util-linux
    - Upstream https://github.com/util-linux/util-linux/issues
    - Obviously this package has tons of bugs opened, but at the same time, it has
      a lot of activity, and is well maintained upstream, in Debian, and in
      Ubuntu, just because of its central position in any Linux system.
  - The package does not deal with exotic hardware we cannot support

  [Quality assurance - testing]
  - The package runs a test suite on build time, if it fails
    it makes the build fail, link to build log TBD
  - The package runs an autopkgtest, and is currently passing on
    all architectures but i386: https://autopkgtest.ubuntu.com/packages/util-linux
  - The package does have not failing autopkgtests right now

  [Quality assurance - packaging]
  - debian/watch is present and works
  - debian/control defines a correct Maintainer field

  - This package does not yield massive lintian Warnings, Errors
  - Recent build: https://launchpad.net/ubuntu/+source/util-linux/2.41-4ubuntu2/+build/30908305
  - Lintian overrides are present, but ok because most are well commented, and the rest is pretty obvious, like highly privileged binaries.

  - This package does not rely on obsolete or about to be demoted
  packages.

  - The package will be installed by default, but does not ask debconf
    questions higher than medium

  - Packaging is quite complex, but I'm not sure how much of a choice we have.
    Good thing is that this package is equally important in Debian, so it will very
    likely keep being maintained.

  [UI standards]
  - Application is end-user facing, Translation is present, via standard
    intltool/gettext. See `configure` for `libintl` and `gettext`.

  - End-user applications without desktop file, not needed because it only ships
  CLI tools.

  [Dependencies]
  - No further depends or recommends dependencies that are not yet in main

  [Standards compliance]
  - This package correctly follows FHS and Debian Policy.

  [Maintenance/Owner]
  - The owning team will be debcrafters-packages and I have their acknowledgement for
    that commitment
  - The future owning team is already subscribed to the package.

  - This does not use static builds.
  - This does not use vendored code
  - This package is not rust based

  - The package has been built within the last 3 months in the archive
  - Build link on launchpad: https://launchpad.net/ubuntu/+source/util-linux/2.41-4ubuntu2

  [Background information]
  The Package description explains the package well
  Upstream Name is `util-linux`
  Link to upstream project: https://github.com/util-linux/util-linux/

  This package has been in main since the very early beginning of Ubuntu, so never
  got the chance to get a proper MIR.
  This was sparked when the `bin:util-linux` has started to depend on
  `bin:liblastlog2-2`, which was in Universe. `liblastlog2-2` was nicely
  handled by @paelzer under the "Renamed or re-organized sources" condition.
  This MIR still makes sense to me, given that `util-linux` provides many
  very important binaries, among which many of them are `suid`, and is one the
  `Essential` packages shipped in absolutely every form Ubuntu can take.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/2113961/+subscriptions