Comment 2 for bug 2052813

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Review for Source Package: bpfcc

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

I'm unsure if this needs a security review, I'll be back with an update
on that once I've consulted security.

List of specific binary packages to be promoted to main: libbpfcc

Specific binary packages built, but NOT to be promoted to main: python3-bpfcc,
bpfcc-tools, libbpf-tools, bpfcc-introspection, bpfcc-lua
Those are only excluded for not beeing needed in the dependency chain right now.
They are ok to be promoted later as needed and especially the collection
of tools can be quite handy for users to start with bpf.

Notes:
- #3 I'll need to check with the security team if they think this needs
  their review as I'm unsure about that detail.

Required TODOs:
- #1 Please get Doko or Samir to confirm that it is ok in their
  POV to also promote libclang-cpp17 as part of this.
- #5 Please have a look if some of the tests could run at build time (chance is
  low due to the kernel dependency.
- #6 Please do wrap the provided tests/* into an autopkgtest against a system
  and kernel and dependencies of the same release. (more below)
- #7 There is no versioned library package nor symbols tracking. It isn't meant
  to be stable as shown on soname 0.x, but adding this to main means we might
  patch things along the way and some certainty if we by accident break API/ABI
  is a nice feature. Please have a look at adding that, symbols tracking for
  sure and considering proper soname versioned package for the lib at lower
  priority and in sync with Debian later on.
  If you believe symbols tracking isn't the way, have a look at
  abi-compliance-check or abigail or anything else that will solve the same.

Recommended TODOs:
- #2 while none is super critical, the list as of today is still of reasonable
  size. This would be your chance to triage all of the open bugs
  so that you can from now on consider it fully under control with ongoing
  triage when it is added to main.
  So please consider doing a full triage (and fix those worth) run of
  https://bugs.launchpad.net/ubuntu/+source/bpfcc/+bugs

- #4 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.
  This is not in conclict with libbpf, it is using libbpf to provide
  the next layer of value on top.
  It is somewhat related to gcc-bpf, but libbpfcc is what the ecosystem
  seems to rely on more.
  Further there is overlap with perf-tools-unstable which inspired some
  tools now found here, but perf-tools-unstable is not in main either and
  less maintained these days, hence chosing bpfcc is the right choice AFICS.

- A team is committed to own long term maintenance of this package.
  => Foundations

- The rationale given in the report seems valid and useful for Ubuntu
  BPF use cases are useful in testing and analysis and slowly replace older
  tools by being more efficient or able to do things not possible without BPF.
  This completes the suite of programs and packages aiming for this to be
  less hacky and instead more accesible and reliable.

[Dependencies]
OK:
- There is a -dev package, but it does not bring further dependencies and
  therefore needs no auto-exclude
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems:
- other Dependencies to MIR due to this:
  This will also need libclang-cpp17 to be promoted to main.
  The llvm related MIRs are quite old and follow none of todays standards.
  - https://bugs.launchpad.net/ubuntu/+source/llvm/+bug/391375
  - https://bugs.launchpad.net/ubuntu/+source/llvm-2.9/+bug/790204
  Since then things have been carried forward, and as of today in noble
  bin:libclang-cpp17 is from src:llvm-toolchain-17
  Now llvm-toolchain-17 is in main based on the old case, but we did not
  yet clearly state what is and what isn't in scope.
  This should be fine, and gladly the owning team for that is foundations
  as well, so we do not need to convince anyone else to add more.
  But you should get Doko or Samir to please confirm that it is ok in their
  POV to also promote libclang-cpp17 as part of this.

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking (some wrap libs a different way, but not 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

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- 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:
- does not run a daemon as root (but runs programs in the kernel scope which
  is ... worse ... is it?)
- does not parse data formats (files [images, video, audio,
  xml, json, asn.1], network packets, structures, ...) from
  an untrusted source - it parses data from the kernel though which
  might have been tampered. Imagine an attack that can only modify
  an unimportant thing, but then bpf tools read from there and load
  new code based on that. Might be too theoretical, but ... ?!?
- 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, ...).
  Each tool actually has a quite small function and could easily be confined,
  and it runs as root having a lots of power, but then it only handles data
  from the kernel, so what attack would we think of?
- While the history of CVEs does not look concerning (there are plenty with
  BPF but they are kernel CVEs ususally). To load BPF programs you need
  root / CAP_SYS_ADMIN which gives you everything already, so one might argue
  if there is any extra exposure.
  Without root permissions all you can do is have some of the code do a little
  bit to then be denied with some "Operation not permitted" issue.
- One could say you are all safe as you already have the keys to the castle
  if you can use it. But OTOH inserting code into the kernel is quite an
  attack vector to make more possible which otherwise would not be possible.

=> I'm not sure, I think due to and despite all of the above this will not
   need security review, but we said we want to err on the side of caution.
   I'll run this by the team to be sure.

[Common blockers]
OK:
- does not FTBFS currently
- This does not need special HW for build or test
- no new python2 dependency
- includes a Python package, but using dh_python

Problems:
- does not have a test suite that runs at build time
  weirdly the d/rules mentioned dh_auto_test but it isn't seen in the build log
- does have a non-trivial test suite that runs as autopkgtest
  There are tests in tests/* for many subcomponents.
  Those tests are even built.
  But I see no execution of the tests.
  There is ./.github/workflows/bcc-test.yml which outlines automated
  tests on Ubuntu focal so chances are high that things work, but e.g. it
  misses 22.04 or even 24.04 and also llvm newer than v15
  It would bre great to leverage those existing tests to always run the package
  content against the releases default kernel and dependencies.
  The build environment isn't the best for that, but this sounds like a great
  opportunity to spawn a small guest of the same release and run those in there.
  Added to required todo's.

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under
  control
- 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
  maintained the package
- no massive Lintian warnings
  I'm ok with some overrides for manpages, those that exist are delivered
- debian/rules is rather clean
- It is not on the lto-disabled list

Problems:
- symbols tracking is not in place.
  But then the library isn's yet having a clear soname either, is is on 0.x
  and not meant to be stable.
  There are three consumers so far. bpftrace and golang-github-iovisor-gobpf-dev
  are pretty much developed in sync and there also is the more independent
  oci-seccomp-bpf-hook.
  It is no strict urgent requirement, but should this get versioned lib
  packages and symbol tracking, I think it would help to ensure quality.
  Especially once in main and e.g. a security fix comes in it would help
  to check if by accident we broke API/ABI in any way.
  Adding to required TODOs.

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (this just works different)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests) => root only execution anyway
- 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 (admin/dev only)

Problems: None