Comment 2 for bug 2031406

Revision history for this message
Lukas Märdian (slyon) wrote :

Review for Source Package: libei

[Summary]
libei is a library for simulated input forwarding (client & server), which can
be used to remote control a system, similar to xdotools/libxdo. It seems to be
properly maintained upstream, but is still very new in Ubuntu (and not yet
packaged for Debian).

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.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: libeis1 (libei1, liboeffis1)
Specific binary packages built, but NOT to be promoted to main: None

Notes:
#0 This parses (simulated) keyboard/mouse input (& XML) and can remote control a
   system, so I'm requesting security review.
#1 There's an embedded copy of the "munit" test framework, but it seems to be
   used at compile/test time only, so I'm not criticizing this. Also, the desktop
   team agreed to provide updates/backports to the security team as needed.
   Also, the vendoring is properly set-up, using debian/watch components.

Required TODOs:
#2 Debian/Ubuntu update history is sporadic, will the desktop team keep track of
   new upstream versions and keep the package maintained (supporting Debian's
   "X Strike Force" team)?
#3 Please provide proper autopkgtests, see "Common blockers" below

Recommended TODOs:
#4 The package should get a team bug subscriber before being promoted
#5 Help upstream to improving the compiler warnings, see specifics below
#6 Get the package included in Debian and Ubuntu's version in sync
   (pending "inclusion in Debian under the Debian X Strike Force team")
#7 Help upstream to resolve the s390x endianess issues, so we can re-enable all tests
   https://gitlab.freedesktop.org/libinput/libei/-/issues/41

[Duplication]
There is no other package in main providing the same functionality.
There's xdotool, xautomation, ydotool, wtype that provide similar functionality,
nothing in "main", though.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - SRCPKG 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.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present (but munit testing framework)
- 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

Problems:
- embeeded "munit" test framework, but seems to be fine (not used at runtime)

[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 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 parse data formats (keyboard/mouse input, xml) from an untrusted source.
- does expose an external endpoint: AF_UNIX socket in src/libei-socket.c (EIS server)

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- no new python2 dependency
- Not a Python package
- Not a go package

Problems:
- does have a non-trivial test suite that runs as autopkgtest
  - This does not need special HW for build or test
- if a non-trivial test on this level does not make sense (the lib alone
  is only doing rather simple things), is the overall solution (app+libs)
  extensively covered i.e. via end to end autopkgtest ?
  - I couldn't find any relevant tests in mutter, using libei[s]
- "The package does not run an autopkgtest because we haven't written one yet."
  - Sorry, but this is not a good excuse. Please provide proper testing, using
    upstream's build time tests, upstream's CI tests or other custom tests. It
    seems to be doable for this kind of simulated input library.

[Packaging red flags]
OK:
- symbols tracking is in place.
- debian/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is good (approx. every 4 months)
- 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
- debian/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Ubuntu does carry a delta (going ahead of Debian)
- Debian/Ubuntu update history is sporadic
  - Only a single upload in Ubuntu, not included in Debian
- s390x dh_auto_test disabled in debian/rules
  https://gitlab.freedesktop.org/libinput/libei/-/issues/41

[Upstream red flags]
OK:
- no Errors during the build
- no incautious use of malloc/sprintf (as far as we can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside tests)
- 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:
- warnings during the build (ignoring a few in the munit subproject):
  + src/util-sources.c:311:9: warning: ignoring return value of ‘read’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  + tools/ei-demo-client.c:102:9: warning: ignoring return value of ‘read’ declared with attribute ‘warn_unused_result’ [-Wunused-result]