Comment 43 for bug 1854362

I reviewed tcmu 1.5.2-5build1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

tcmu is the userspace side of the kernel's LIO TCM in userspace backstore,
which allows backstores for LIO (the kernel's SCSI target) to live outside
of the kernel and to be implemented in userspace. It consists of a daemon
(tcmu-runner) which communicates with the kernel side via the TCM-USER
generic netlink family. Handlers for SCSI commands are implemented as modules
that run inside tcmu-runner - currently built and insalled are "File-backed
optical", "ZBC emulation", "QCOW image file" and "Ceph RBD" handlers. Handlers
process SCSI commands from a ring buffer shared between the kernel and
userspace.

- Some limited CVE history - a mixture of DoS and information leaks, all fixed
  in the current version:
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000198
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000199
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000200
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-1000201
- Upstream is active, and we had a positive interaction with them whilst I was
  working on this MIR (see https://github.com/open-iscsi/tcmu-runner/issues/613).
- Build-depends all in main (cmake, kmod, glib2.0, libnl3, ceph, pkg-config,
  zlib)
  - It build-depends on libkmod-dev because it consumes symbols from libkmod2,
    but what does it need the kmod binaries for at build time?
- tcmu-runner has postinst/prerm/postrm scripts just containing autogenerated
  snippets from dh_installinit and dh_installsystemd.
  - It appears to be missing cleanup hooks for /etc/tcmu/tcmu.conf though.
- Contains an init script and systemd unit that starts the tcmu-runner daemon,
  which is started by default as part of the multi-user target.
  - The daemon runs as root.
  - Doesn't listen on any ports.
- Also installs a dbus service to activate the tcmu-runner daemon.
- It exposes a couple of dbus interfaces:
  - org.kernel.TCMUService1 which provides a single CheckConfig method and is
    exported for each registered type (with a type specific object path). None
    of the internal plugins appear to implement check_config, so I'd imagine it
    just always returns success for these.
  - org.kernel.TCMUService1.HandlerManager1 which allows external handlers for
    a type to be registered (via the RegisterHandler method). Calls to
    org.kernel.TCMUService1.CheckConfig are then proxied to the external handler
    process, which owns a type-specific well-known name on the system bus.
  - The dbus policy allows all users to call
    org.kernel.TCMUService1.HandlerManager1.RegisterHandler, which doesn't seem
    desirable. I don't think there is a direct security impact from this, as
    external handlers need to be privileged in order to own the type-specific
    well-known name on the system bus, and the call will return an error if
    called before that name is owned. But I think this should only be callable
    as the root user.
- No setuid binaries.
- One binary in PATH (/usr/bin/tcmu-runner).
- No sudo fragments.
- No polkit files.
- No udev rules.
- There don't appear to be any unit tests or autopkgtests.
- No cron jobs.
- Build logs appear to be mostly clean.
  - Note, there is a patch that removes -Werror, which appears to be unnecessary.
  - There are some dpkg-shlibdeps warnings because the modules don't link
    against libtcmu2.
  - tcmu-runner has a "dbus-policy-without-send-destination" lintian warning which
    should probably be fixed.
- No subprocesses spawned.
- Memory management seems to generally be cautious - it takes care to handle
  heap allocation failures, and makes use of calloc to avoid arithmetic overflow
  issues in a lot of places.
- Seems to handle errors when performing file I/O operations. It uses pread
  and pwrite quite a bit and in most cases checks that the expected number of
  bytes are read / written.
  - There is a usage of pwritev in qcow_pwritev that checks for an error
    condition (ret < 0), but doesn't appear to check that the correct number of
    bytes are written or expose the number of bytes written for callers to
    handle.
- It appears to be fairly defensive when reading metadata from files on disk.
- Has some logging helpers, which log to a file and syslog.
  - Writing to the syslog happens on a dedicated thread.
  - There is some locking which allows the logging functions to be used from
    multiple threads - tcmu-runner has a dedicated thread for each internal
    device handler, which is why this is necessary.
  - Log levels are set by a config file, and the default is "info (3)". The
    highest log level (5) logs SCSI command sequences, but only to the
    dedicated log file rather than syslog. I didn't find any indication that
    sensitive data might be logged, although perhaps SCSI command sequences
    could be considered sensitive at the highest log level.
  - I didn't find any format string issues.
  - The file logging has proper EINTR handling around write().
- The only environment usage is reading the default log file directory
  (TCMU_LOGDIR).
- No use of privileged functions.
- No crypto.
- No tmpfile usage.
- No direct networking usage.
- No webkit.
- No policykit usage.
- All toolchain hardening measures are on.
- Coverity found what look like a couple of genuine infinite loops in qcow.c.
  - It also found a couple of TOCTOU issues (one in fbo_open and one in
    zbc_open_backstore, but these don't look security relevant).
- cppcheck found one uninitialized variable issue, but it looks spurious.
- No relevant shellcheck issues.

Tentative security team ACK for promoting tcmu to main. I'd like to see the
"dbus-policy-without-send-destination" lintian warning fixed, and the dbus
policy tightened to only allow calls to
org.kernel.TCMUService1.HandlerManager1.RegisterHandler method from the root
user, unless there is a need for that to be callable by unprivileged users.