Comment 14 for bug 1570617

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed edk2 version 0~20181115.85588389-2ubuntu1 as checked into
disco. This review barely scratches the surface of the package.

It appears there's roughly 20 CVEs for sources in this package. We've
accidentally mis-filed eight of them.

This package provides a boot environment; I must admit that I don't
understand it well.

- Build-Depends: debhelper, uuid-dev, iasl, gcc-multilib [i386], nasm,
  python, gcc-aarch64-linux-gnu, gcc-arm-linux-gnueabihf, genisoimage,
  qemu-utils, qemu-system-x86, python3

- Embeds OpenSSL cryptographic library
- Probably doesn't daemonize
- Probably doesn't listen on the network
  - However, it includes many network client services, likely to use
    during PXE booting or similar
- No pre/post inst/rm
- No init scripts
- No systemd unit files
- No dbus services
- No binaries in PATH
- No sudo fragments
- No udev rules
- Probably no test suite
- No cron jobs
- Build logs use -Werror -- the logs give misleading impression.

- It's difficult to tell if subprocesses are spawned
- Memory management varies; cppcheck finds many faults, but they may not
  be in code that matters
- Extensive file IO, it's difficult to tell how much would happen in the
  context of a host vs a guest
- Extensive logging, spot checks showed no unsafe logging
- Extensive cryptography, entirely unchecked
- I didn't discover privileged vs unprivileged portions of code
- Some privileged operations in the code but many appear to be available
  as platform services
- One instance of an unsafe write to /tmp/col in Oniguruma regular
  expression engine. It is probably ifdef'd out.
- No WebKit
- No PolicyKit
- Many cppcheck results reporting real bugs.

This codebase is huge. I'm still not clear on what most of it is used for,
or what threat model is assumed.

Code quality looked mixed: cppcheck reported a lot more results than I'd
like to see, but the code I studied via the uaudit greps was high quality.

In order to properly support edk2, the security team will need help from
other teams. So, the security team is providing a provisional ACK for
promoting edk2 to main once the following conditions are met:

First, Dann has volunteered to strip out the Pythons, Lua, pccts, and
perhaps other smaller pieces as testing indicates that they can be
removed. We'd like these stripped before moving the package to main.

Second, someone needs to commit to working with the security team to
properly triage edk2 issues as they arise.

Third, someone needs to commit to testing packages that the security team
prepares for our stable releases, for whatever lifetime is appropriate.
(If, in six years, edk2 is included in 20.04 LTS's ESM offering due to
customer interest, this team will need to commit to helping test 20.04's
EDK2 until 2030, or whenever 20.04 LTS's ESM support offering comes to
a close, as well as all future LTS releases that include edk2 in main.)

Please comment in this bug to sign up for these obligations.

Thanks