Comment 6 for bug 1912371

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [MIR] flashrom

[Summary]
MIR Team Ack under the constraint to implement the all Required (and as much
Recommended as possible) of the lists below.

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

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

Required TODOs:
- src:libftdi1 is required as well, adding a task and assigning mclemenceau to
  decide if he wants to own that as well.
- Future owners (=Foundation) needs to check the interaction with fwupd
  if that is safe to not trigger any of the flashrom warnings to brick laptops.
  => https://flashrom.org/Laptops
  Once you have taken an explicit look at that please state here that it was
  functionally reviewed and considered safe.
- get the existing self-tests in play at build time. We can't set up VMs for
  flash emulation in autopkgtests - but the build time tests that exist should
  be added to the meson build. Or dh_auto_test runs partially on makefiles?
  => https://bugs.launchpad.net/ubuntu/+source/flashrom/+bug/1912371/comments/3

Recommended TODOs:
- this is no 1.0 so maybe one should acknowledge that stability. Please consider
  dropping the commandline syntax change warning fromt he man page as well as
  think about adding a .symbols file to auto-detect API breakage.

[Duplication]
There is no other package in main providing the same functionality.
There is ftdi-eeprom and xc3sprog, but both cover smaller subsets of use-cases
and both are not in main. But ftdi-eeprom will come back below.

[Dependencies]
OK:
- There is a -dev which will be auto-promoted, but it brings in no further
  dependencies on top of what we need anyway.

Problems:
- other Dependencies to MIR due to this. "flashrom" as well as "libflashrom1"
  depend on src:libftdi1 this will have to be owned/prepared/reviewed as well
  to be able to complete this promotion of src:flashrom.

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking

[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 open a port
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

Problems:
- does parse data formats
- does process arbitrary web content (depending on where you get your rom files)
  For both issues it is more the permission level of such roms that makes them
  security relevant. If one can sneak in odd code into the early boot that is
  potentially later undetectable. Also while gladly not a daemon, executing
  this will run at the highest possible permissions as those are needed to
  program the hardware.

[Common blockers]
OK:
- does not FTBFS currently
- The package has an intended team bug subscriber
- no translation present, but none needed for this case (user visible)?
- not a python/go package, no extra constraints to consider in that regard

Problems:
- does not have a test suite that runs at build time
- does not have a test suite that runs as autopkgtest
Here as outlined by Carl-Daniel it might need some dev effort to leverage the
existing self-tests in meson build. Totally without testing this feels a bit
too powerful.
=> https://bugs.launchpad.net/ubuntu/+source/flashrom/+bug/1912371/comments/3

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- d/watch is present and looks ok
- Upstream update history is slow but ok
- Debian/Ubuntu update history is ok
- 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
- d/rules is rather clean
- Does not have Built-Using

Problems:
- symbols tracking is not in place

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu or Upstream
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks

Problems:
- the Readme and man page is full of warnings how destructive this can be
  on laptops that interact badliy with an embedded controller. It goes
  through some lengths to detect laptop environments and NOT run there unless
  the user forces it.
  But this MIR was requested to extend "fwupd" which myself and thousands of
  other users use for exactly that - keeping laptop firmware up to date.
  I'd want the future package owner to take a deeper look and ensure that we
  don't need further extra protection from bricking users by accident.
- Another warning there is about the commandline syntax before 1.0. We are on
  1.2 now, maybe time to drop this statement and consider things stable now?