[MIR] trace-cmd

 % lintian --pedantic
W: trace-cmd source: newer-standards-version 4.6.2 (current is 4.6.0.1)
P: trace-cmd source: no-homepage-field

Hi,
info for Ioanna and Paul.

While I was looking at libtracefs I thought about testing. This is a stack of libtracefs (bug 2051925) libtraceevent (bug 2051916) and trace-cmd.

IMHO the libs are well tested on the level of being "just libs", but on this level we should require to set up some good autopkgtests that help us to ensure this full stack is really working well on e.g. changed kernels.

Review for Source Package: trace-cmd

[Summary]
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.

Although the package can pass our security check-list, trace-cmd itself runs as root
to setup events in ftrace kernel subsystem.
This combined with the use of setuid/setgid in trace-record.c (change_user()) and
extensive use of malloc/sprintf, I think are good enough reasons for a sec review.

This does need a security review, so I'll assign ubuntu-security, after the required TODOs
are addressed.

List of specific binary packages to be promoted to main: trace-cmd, libtracecmd1,libtracecmd-dev
Specific binary packages built, but NOT to be promoted to main:<None>

Notes:
Required TODOs:
1. Other dependies to MIR:
   a. libtracefs - https://bugs.launchpad.net/ubuntu/+source/libtracefs/+bug/2051925
   b. libtraceevent - https://bugs.launchpad.net/ubuntu/+source/libtraceevent/+bug/2051916
2. No tests during build, please add tests.
3. No autopackage tests present. This TODO incorporates Christain's feedback (comment #2)
   "but on this level we should require to set up some good autopkgtests that help us to
    ensure this full stack is really working well on e.g. changed kernels."
    Please add autopackage tests.

Recommended TODOs:
4. The output of 'lintian --pendatic --tag-display-limit 0' yields many warnings
   some of them segfaults. Not sure if this is a problem of the package or troff
   but please take a look. (https://pastebin.ubuntu.com/p/JYGrJ7wnJz/)
5. There a few warning (unused return values) during build
6. The package should get a team bug subscriber before being promoted

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

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

Problems:
- other Dependencies to MIR due to this

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have unexpected Built-Using entries
OK:
- 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 deamon 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.
- 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:
- this doe...

I reviewed trace-cmd 3.2-1 as checked into noble. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

> TRACE-CMD: The front-end application to Ftrace. The back-end application to KernelShark.

- CVE History
  - none
- Build-Depends
  - most are for docs
  - libtrace* mirs are ack'd
  - note the d/control suggestion for installing kernelshark
    - trace-cmd is the backend for kernelshark
    - https://git.kernel.org/pub/scm/utils/trace-cmd/kernel-shark.git/
- pre/post inst/rm scripts
  - none
- init scripts
  - none
- systemd units
  - none
- dbus services
   - none
- setuid binaries
  - none
- binaries in PATH
  - root owned ./usr/bin/trace-cmd
- sudo fragments
  - none
- polkit files
  - none
- udev rules
  - none
- cron jobs
  - none
- unit tests / autopkgtests
  - needs tests, see MIR team's requirements
- Build logs
  - -Walloc-size-larger-than=
  - -Wformat-overflow=
  - -Wunused-result
  - please do not use in production environments

- Processes spawned
  - moderate use, as expected by nature of program
  - root user privileges are expected when using this tool
  - checked uses and attempts looks okay
  - in traceinput.c, regexec() is controlled by root unprivileged user
  - note that arbitrary commands can be specified to run based on tracing triggers
- Memory management
  - extremely heavy use
  - this code is unlikely safe to be used in production. this is meant for development.
    - we should never suggest usecases that input is untrusted
      - e.g., network traffic from untrusted sources
- File IO
  - heavy use
- Logging
  - some use of tracecmd_debug(), mostly perror()
- Environment variable usage
  - TRACECMD_PLUGIN_DIR, HOME, USER, LOGNAME, PATH
  - mostly used to run commands as another user
- Use of privileged functions
  - setuid, setgid, ioctl, initgroups
  - used to run arbitrary commands as an abitrary user by record_trace_command()
  - ioctl used to get the local context id of a vm socket
    - hardcoded to use Linux Kernel constant 0x7b9 +1
    - see https://github.com/mdlayher/vsock/blob/main/fd_linux.go and past ioctl_linux.go iteration
- Use of cryptography / random number sources etc
  - none
- Use of temp files
  - safe use of mkstemp
- Use of networking
  - yes, heavy socket use
- Use of WebKit
  - none
- Use of PolicyKit
  - none

- Any significant cppcheck and Coverity results
  - many results, most are likely false-positives
  - potential memory leaks caused by jumps
  - treating these as bugs in a _development tool_
    - this is not meant for _production_
  - checked OOB reports are false-positives
- Any significant shellcheck results
  - none
- Any significant bandit results
  - none
- Any significant govulncheck results
  - none
- Any significant Semgrep results
  - none
  - noisy rule complains about strtok v. strtok_r
    - see tracecmd/trace-cmd.c:53
    - proper use is understood

Security is content to review this as a _development tool_. Extreme caution should be taken if used in production.

Security team ACK for promoting trace-cmd to main.

Thank you for the security review, most of the other open requests are still open AFAICS (we said in the team meeting that we wanted to re-check all cases):

Required:
1. Other dependies to MIR:
WIP a. libtracefs - https://bugs.launchpad.net/ubuntu/+source/libtracefs/+bug/2051925
DONE b. libtraceevent - https://bugs.launchpad.net/ubuntu/+source/libtraceevent/+bug/2051916

2. No tests during build, please add tests.
   => This depends how doable or not that is in the build environment,
      but I've so far not seen an update or discussion for this yet

3. No autopackage tests present.
   => No additions yet AFAICS, especially as this covers the full stack

Recommended TODOs:
4. The output of 'lintian --pendatic --tag-display-limit 0' yields many warnings
   some of them segfaults. Not sure if this is a problem of the package or troff
   but please take a look. (https://pastebin.ubuntu.com/p/JYGrJ7wnJz/)
5. There a few warning (unused return values) during build
6. The package should get a team bug subscriber before being promoted

=> While those are optional, there was no update for them either so far.

This might all be still fine as, after all, you might tackle them one by one.
But that means, for now, this is still incomplete waiting for you to provide these aspects before full approval.

I am now working on this because Paul had other obligations.

This bug was fixed in the package trace-cmd - 3.2-1ubuntu1

---------------
trace-cmd (3.2-1ubuntu1) noble; urgency=medium

  * Address autopkgtest TODOs for MIR (LP: #2051850)
    - debian/tests: add simple autopkgtest using man page examples
    - debian/tests/control: run trace-utest as autopkgtest
    - debian/patches: fallback to using /usr/bin/trace-cmd in trace-utest

 -- Nick Rosbrook <email address hidden> Fri, 05 Apr 2024 16:36:49 -0400

I uploaded trace-cmd with some new autopkgtests. The trace-utest tests are not done at build time because these require root, or at least the ability to read /sys/kernel/tracing/. Hence, unfortunately we cannot reasonably add build-time tests right now.

The autopkgtests I added are looking good[1], except for infra issues on amd64, and it looks like I will need to do a quick follow-up to make trace-utest skippable so it does not run on armhf.

[1] https://autopkgtest.ubuntu.com/packages/trace-cmd

Actually, I think this is ready, pending libtracefs MIR (bug #2051925).

- Security ACK in comment #4
- build-time tests (requirement #2) rejected, due to build-env incompatibilities
- autopkgtests (requirement #3) added.
- already seeded
- foundations-bugs subscribed

Override component to main
trace-cmd 3.2-1ubuntu2 in noble: universe/devel -> main
libtracecmd-dev 3.2-1ubuntu2 in noble amd64: universe/libdevel/optional/100% -> main
libtracecmd-dev 3.2-1ubuntu2 in noble arm64: universe/libdevel/optional/100% -> main
libtracecmd-dev 3.2-1ubuntu2 in noble armhf: universe/libdevel/optional/100% -> main
libtracecmd-dev 3.2-1ubuntu2 in noble ppc64el: universe/libdevel/optional/100% -> main
libtracecmd-dev 3.2-1ubuntu2 in noble riscv64: universe/libdevel/optional/100% -> main
libtracecmd-dev 3.2-1ubuntu2 in noble s390x: universe/libdevel/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble amd64: universe/libs/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble arm64: universe/libs/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble armhf: universe/libs/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble ppc64el: universe/libs/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble riscv64: universe/libs/optional/100% -> main
libtracecmd1 3.2-1ubuntu2 in noble s390x: universe/libs/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble amd64: universe/devel/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble arm64: universe/devel/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble armhf: universe/devel/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble ppc64el: universe/devel/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble riscv64: universe/devel/optional/100% -> main
trace-cmd 3.2-1ubuntu2 in noble s390x: universe/devel/optional/100% -> main
Override [y|N]? y
19 publications overridden.