Comment 3 for bug 2051850

Revision history for this message
Ioanna Alifieraki (joalif) wrote :

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 does not make 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 not need special HW for build or test
- no new python2 dependency

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place.
- debian/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is good
- Debian/Ubuntu update history is slow
- the current release is packaged
- 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:
- massive Lintian warnings - https://pastebin.ubuntu.com/p/JYGrJ7wnJz/

[Upstream red flags]
OK:
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- 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:
- a few warnings during the build about ignoring return values
- heavy use of malloc and sprintf
- use of setuid/setgid