Comment 2 for bug 2051916

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Review for Source Package: libtraceevent

[Summary]
There are quite some details to clarify before giving a MIR ACK or NACK. Some were already coming from the previous MIR and are not addressed before opening this new MIR. If we want to be able to maintain it properly, we really need to address them beforehand.

Also, this does need a security review, so I'll assign ubuntu-security once the other issues are fixed: there are a lot of malloc’s use and it’s working with trusted kernel space. I think a security review will help here.
List of specific binary packages to be promoted to main: libtraceevent-dev, libtraceevent-doc, libtraceevent1, libtraceevent1-plugin

Some parts are going to paraphrase what was written in the previous MIR request.

Required TODOs:
- It does not have a test suite that runs at build time: upstream's unit tests
  from utest/ are being compiled via `make test` but don't seem to be executed.
  I cannot spot anything outside of the compilation content output in the build log.
- Similarly, as written by the report, it does not have a non-trivial test suite that runs
  as autopkgtest: it checks compilation only, I think similarly than the previous request, we should
  at least test the unit tests against the installed version.
- there are a lot of "#MISSING: ..." entries in .symobls tracking. Can we get a comment on any of those?
- The package should get a team bug subscriber before being promoted

Recommended TODOs:
- One warning is repeated throughout the build (install target) of the software:
  `install: WARNING: ignoring --strip-program option as -s option was not specified`.
  Can we fix it so that it doesn’t obfuscate other warnings?
- The package is shipping a .a file, we generally strip those in ubuntu, do we really need this static archive?

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

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - libtraceevent checked with `check-mir`
  - all dependencies can be found in `seeded-in-ubuntu` (already in main)
  - none of the (potentially auto-generated) dependencies (Depends
    and Recommends) that are present after build are not in main
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

[Embedded sources and static linking]
OK:
- no embedded source present
- 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

[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 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, ...)
- 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
- no new python2 dependency

Problems:
- It does not have a test suite that runs at build time: upstream's unit tests
  from utest/ are being compiled via `make test` but don't seem to be executed.
  I cannot spot anything outside of the compilation content output in the build log.
- Similarly, as written by the report, it does not have a non-trivial test suite that runs
  as autopkgtest: it checks compilation only, I think similarly than the previous request, we should
  at least test the unit tests against the installed version.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place (but see in problems below)
- debian/watch is present and looks ok
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
- no massive Lintian warnings
- debian/rules is rather clean
- It is not on the lto-disabled list
  (fix, or the workaround should be directly in the package,
   see https://launchpad.net/ubuntu/+source/lto-disabled-list)

Problems:
- There are a lot of "#MISSING: ..." entries in .symobls tracking. Can we get a comment on any of those?
- The package is shipping a .a file, we generally strip those in ubuntu, do we really need this static archive?

[Upstream red flags]
OK:
- no Errors during the build (see about warning below)
- no incautious use of malloc/sprintf (as far as we can check it) but many use of them (see note below)
- no use of user nobody
- no use of setuid / setgid
- 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:
- One warning is repeated throughout the build (install target) of the software:
  `install: WARNING: ignoring --strip-program option as -s option was not specified`.
  Can we fix it so that it doesn’t obfuscate other warnings?
- There are a lot of malloc’s use and it’s working with trusted kernel space. I think a security review will help here.