[MIR] edk2

Team subscriber: formally, this should be ubuntu-server, alongside qemu and seabios. In practice, this package sees somewhat active maintenance from other quarters (Foundations, Server Enablement, Launchpad) due to its rather central position in our cloud-on-arm64 story.

Is this needed for 16.04?

On Fri, Apr 15, 2016 at 05:37:25PM -0000, Tyler Hicks wrote:
> Is this needed for 16.04?

It would be lovely if we could get it done for 16.04, so that the qemu
package would have the correct dependencies out of the box. If it doesn't
get done for 16.04, this package is still going to be pulled into all
ARM64-based clouds everywhere, as the one and only possible firmware option
for VM guests; it's just a question of how far up the stack the thing is
that does the pulling, and whether it's pulling from universe or main.

You probably already know that you are not going to find this an enjoyable
security review, because edk2 source embeds a copy of openssl source in
order to implement Secure Boot.

The Security Team did not have enough time to review edk2 for 16.04. It will need a farily indepth audit to determine if we can support it since it uses and implements crypto, uses and implements networking, uses and implements TPM interfaces, etc.

There is a lot of code in the edk2 package and it may not be easy to support years from now. Upstream does not cut new releases very often and while they do have service pack releases, they don't release them very often, either. The service pack release notes are vague and some of the descriptions of bugs fixed sound like they may be CVE worthy. It isn't clear to me if upstream is proactive regarding CVE requests for issues.

We will perform a post-16.04 review of edk2 to determine supportability and look at the potentially problematic areas of the code mentioned above.

Status changed to 'Confirmed' because the bug affects multiple users.

@Tyler: is a security team review still in plan?

This review is still in plan. It hasn't completed due to its large scope and relatively low priority.

This is still on our "would be nice to get it" list.
There is no huge business case for it yet, but as UEFI grows this would be useful.

Furthermore Debian now recommends that, so that will soon become Delta to be maintained.
For all of that giving this a (slight) bump would be nice.

Thanks for bump.

This (perhaps) makes for a useful time to point out that while there are still not formal releases, EDK2 now makes quarterly-ish "stable tags". So far we have edk2-stable201808 and edk2-stable201811, next one will be edk2-stable201903.

My goal is for this to move to proper releases, longer-term. For now it's more of a valve on the fire hose, and the introduction of some level of process/discipline on the timing for applying non-bugfixes.

As for openssl, don't know if that makes a huge difference, but that has now moved to a git submodule instead of being manually imported into the tree.

fyi, we are using the stable tags as a base now.

\o/

Excellent, glad to hear it.
I'll make sure to let the others know.

On Wed, Jan 09, 2019 at 05:52:44PM -0000, dann frazier wrote:
> fyi, we are using the stable tags as a base now.
>
> --
> You received this bug notification because you are subscribed to edk2 in
> Ubuntu.
> https://bugs.launchpad.net/bugs/1570617
>
> Title:
> [MIR] edk2
>
> Status in edk2 package in Ubuntu:
> Confirmed
>
> Bug description:
> [Availability]
> Pending upload of 0~20160408 upstream snapshot release (in progress: http://anonscm.debian.org/cgit/pkg-qemu/edk2.git)
>
> [Rationale]
> edk2 provides EFI ROM images to be used by KVM VMs. On x86 this can be used as an alternative to seabios. On arm64, however, there is no "seabios" option - edk2 is the *only* way to boot an arm64 guest using the kernel/ramdisk configured in the image. Without edk2, users have to manually extract the kernel/ramdisk/cmdline from the cloud image and provide them externally to qemu. This obviously isn't a reasonable way to e.g. manage OpenStack instances, as the kernel/ramdisk would have to be re-extracted and reconfigured on every kernel security update. We'd therefore like to be able to have qemu-system-x86 and qemu-system-arm depend on the corresponding edk2 binary for the architecture.
>
> [Security]
> edk2 had a couple of CVEs assigned in 2014: CVE-2014-4859, CVE-2014-4860, but it looks like those were the only 2.
>
> [Quality assurance]
> The binary packages don't require any configuration - they're basically just providing data files that can optionally be used by QEMU.
>
> [Dependencies]
> All build deps are in main, and all produced binary packages have no runtime dependencies.
>
> [Standards compliance]
> EDK2 is the sample implementation for UEFI from Intel - de facto standards-compliant.
>
> [Maintenance]
> Canonical supports OpenStack on arm64, for which this is a key component. Linaro is actively maintaining the virt bits upstream, and Canonical's HWE and Foundations team are maintaining the package for that purpose.
>
> [Background information]
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1570617/+subscriptions

Here's a reduced set of cppcheck results -- I've removed results in Python, Lua, OpenSSL, and maybe a few more paths out of the results. I believe upstream need to add cppcheck into their processes.

Every one I've inspected so far represents a real bug. The bugs may not be easy to hit in practice, but that's the way bugs are. The easy ones are found by developers and hard ones are found by users.

Is there a good way to ask EDK2 upstream to add cppcheck to their workflow? Filing a few dozen bugs for all these in each project individually doesn't sound fun.

Thanks

There's a lintian error and a warning that probably deserve investigation:

 E: edk2 source: source-contains-unsafe-symlink EmulatorPkg/Unix/Host/X11IncludeHack
 W: edk2 source: unknown-field-in-dsc build-indep-architecture

https://lintian.debian.org/tags/source-contains-unsafe-symlink.html
https://lintian.debian.org/tags/unknown-field-in-dsc.html

Thanks

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

On Fri, Apr 12, 2019 at 9:54 PM Seth Arnold <email address hidden> 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.

Can you point me to them?

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

Happy to answer questions or demo it for you - just let me know.

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

Correct

> - 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 built-in one, but see below...

> - No cron jobs
> - Build logs use -Werror -- the logs give misleading impression.
>
> - It's difficult to tell if subprocesses are spawned

You can certainly run EFI apps in it, so in a sense....

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

While I consider it more of a QEMU vector than edk2 itself, it is
possible for this firmware, or EFI apps it runs, to access devices on
the host if QEMU allows it. For example, we have seen bugs[1][2] where
shim in a guest corrupts the guest's firmware image on the host. But
that's only possible because QEMU was given write access to that
image.

[1] https://bugs.launchpad.net/ubuntu/+source/shim-signed/+bug/1811901
[2] https://bugs.launchpad.net/ubuntu/+source/shim-signed/+bug/1811722

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

One security-relevant thing to note is that edk2 will be responsible
for the firmware-aspect of SecureBoot for QEMU guests booted in
SecureBoot mode.

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

On Wed, Apr 17, 2019 at 10:08:51PM -0000, dann frazier wrote:
> > It appears there's roughly 20 CVEs for sources in this package. We've
> > accidentally mis-filed eight of them.
>
> Can you point me to them?

Here's the CVEs that we triaged against edk2:

CVE-2014-4859
CVE-2014-4860
CVE-2018-12178
CVE-2018-12179
CVE-2018-12180
CVE-2018-12181
CVE-2018-12182
CVE-2018-12183
CVE-2018-3613
CVE-2018-3630
CVE-2019-0160
CVE-2019-0161

And the corresponding list of URLs for these issues in our database:

https://people.canonical.com/~ubuntu-security/cve/CVE-2014-4859
https://people.canonical.com/~ubuntu-security/cve/CVE-2014-4860
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12178
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12179
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12180
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12181
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12182
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-12183
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-3613
https://people.canonical.com/~ubuntu-security/cve/CVE-2018-3630
https://people.canonical.com/~ubuntu-security/cve/CVE-2019-0160
https://people.canonical.com/~ubuntu-security/cve/CVE-2019-0161

Here's the CVEs that we may have mis-triaged:

CVE-2017-14872 # Tianocore EDK2 sources
CVE-2017-14893 # Tianocore EDK2 sources
CVE-2017-15824 # Android EDK2 bootloader
CVE-2017-18158 # Android EDK2 flashing
CVE-2017-18159 # Android EDK2 StrHwPlatform handling
CVE-2018-5887 # Android edk2 usb
CVE-2018-5888 # Android edk2 Qcom boot
CVE-2018-5889 # Android edk2 Qcom boot

And corresponding links to Debian's CVE database (often much more useful
than ours when we don't have the issue in our database).

https://security-tracker.debian.org/tracker/CVE-2017-14872
https://security-tracker.debian.org/tracker/CVE-2017-14893
https://security-tracker.debian.org/tracker/CVE-2017-15824
https://security-tracker.debian.org/tracker/CVE-2017-18158
https://security-tracker.debian.org/tracker/CVE-2017-18159
https://security-tracker.debian.org/tracker/CVE-2018-5887
https://security-tracker.debian.org/tracker/CVE-2018-5888
https://security-tracker.debian.org/tracker/CVE-2018-5889

> > This package provides a boot environment; I must admit that I don't
> > understand it well.
>
> Happy to answer questions or demo it for you - just let me know.

Thanks! This would be wonderful.

> > - It's difficult to tell if subprocesses are spawned
>
> You can certainly run EFI apps in it, so in a sense....

The concern is mostly around applications that just use system(3) or
similar string-based mechanisms to execute new processes. The clear
majority of applications that use system(3) introduce security flaws or
reliability problems. execve(2) and similar array-based mechanisms are the
safe approach to executing new processes.

I don't know if EFI offers a similar API that is so easily misused. I
expect it's a vastly different environment to the point that this isn't
really an issue.

My "difficult to tell" is that in the typical unix-ish sources vendored
into the codebase are *loads* of process execution tools, and tracking
...

ubuntu-server is now subscribed to bug mail.

I've gone ahead and sync'd edk2 into Ubuntu:

https://launchpad.net/ubuntu/+source/edk2/0~20190606.20d2e5a1-1ubuntu1

This has the identified embedded source copies removed. Upstream has actually moved most of them out of the source tree - the only source we're additionally removing is BrotliCompress. I've also added some basic autopkgtests. They just make sure each image can boot to an EFI shell in (non-KVM) QEMU instance. I'd like to add tests that involve booting real OS's, and maybe using KVM when available - that will take some time, but at least I've got the groundwork laid.

Please let me know if there's anything else you require from me to process this MIR.

Thanks Dann, I believe this package is ready to go from the Security Team's point of view:

- ubuntu-server is subscribed to bugs, and presumably ready and willing for triage, testing, etc as needed

- the autopkgtests clearly demonstrate at least simple use of the tool. More detailed testing help to avoid regressing users would be wonderful, but this can provide us with a starting point for "this must work" for preparing updates.

Security team ACK for promoting edk2 to main.

Thanks

FYI: Asked Dannf on IRC how exactly we want to pull it in (seed/dependency change).

If having qemu-system-arm recommend qemu-efi-aarch64, qemu-efi-arm as Debian does is sufficient, then that is my suggestion.

On Thu, Jul 11, 2019 at 05:32:44PM -0000, dann frazier wrote:
> If having qemu-system-arm recommend qemu-efi-aarch64, qemu-efi-arm as
> Debian does is sufficient, then that is my suggestion.

+1

While we're here, what's this do? :)

# Bumping this up to >= GCC5 will require the replacing the pre-compiled
# liblto*.a binaries that we strip from our orig.tar.xz.
EDK2_TOOLCHAIN = GCC49

from debian/rules

Thanks

On Thu, Jul 11, 2019 at 12:59 PM Seth Arnold <email address hidden> wrote:
>
> While we're here, what's this do? :)
>
> # Bumping this up to >= GCC5 will require the replacing the pre-compiled
> # liblto*.a binaries that we strip from our orig.tar.xz.
> EDK2_TOOLCHAIN = GCC49
>
> from debian/rules

What it does /not/ do is change which compiler we actually use - we
build with the default Ubuntu gcc. Rather, GCC49, GCC5, CLANG35, etc
are toolchain templates. The build system uses templates to tweak
parameters to suit the toolchain in-use. In the GCC5 recipe, upstream
started taking advantage of link-time optimization. But doing so
required adding some "glue binaries" to the source tree:

https://github.com/tianocore/edk2/commit/e1458aaded8e34b0c74c1a17ac1dc3765d97c082

We strip those binaries out, so we need to disable LTO, and specifying
the GCC49 template is an easy way to accomplish that.

  -dann

Dann, aha, I see! That makes sense.

Thanks

I'll upload [1] to Ubuntu once the current builds are out of proposed.
I already proposed the change to Debian so that we can drop it on a subsequent merge.

In a perfect world this should be done by now, but ppc64el slow tests and builds are a known inhibitor for a while it seems.
Assigning to myself to trigger the component mismatch with the change in Eoan once things are ready.

[1]: https://salsa.debian.org/qemu-team/qemu/merge_requests/7

Ok the former things in Eoan completed.
Debian added ovmf/qemu-efi-aach64 (and qemu-efi which is onla y transitional) back in [1]
We had them as suggests until now, the change will make them recommends.

Uploaded to Eoan, once it passed the rest of builds and tests it should show up as component mismatch to be resolved by an AA then.

[1]: https://salsa.debian.org/qemu-team/qemu/commit/8ea83360bdd6889534773ad432f9b75ef7f74db3

Hrm, why didn't this component-mismatch when migrating, I must have missed something:
From build [1]:

1. qemu-system-arm
Recommends: ... qemu-efi-aarch64, qemu-efi-arm
But:
 qemu-system-arm | 1:4.0+dfsg-0ubuntu4 | eoan
 qemu-efi-aarch64 | 0~20190606.20d2e5a1-1ubuntu2 | eoan/universe

2. qemu-system-x86
Recommends: ... ovmf
But:
 qemu-system-x86 | 1:4.0+dfsg-0ubuntu4 | eoan
 ovmf | 0~20190606.20d2e5a1-1ubuntu2 | eoan/universe

This is from rmadison, another view onto the same is in an eoan system of today:
root@e:~# apt-cache policy qemu-system-x86 ovmf
qemu-system-x86:
  Installed: 1:4.0+dfsg-0ubuntu4
  Candidate: 1:4.0+dfsg-0ubuntu4
  Version table:
 *** 1:4.0+dfsg-0ubuntu4 500
        500 http://archive.ubuntu.com/ubuntu eoan/main amd64 Packages
        100 /var/lib/dpkg/status
ovmf:
  Installed: 0~20190606.20d2e5a1-1ubuntu2
  Candidate: 0~20190606.20d2e5a1-1ubuntu2
  Version table:
 *** 0~20190606.20d2e5a1-1ubuntu2 500
        500 http://archive.ubuntu.com/ubuntu eoan/universe amd64 Packages
        100 /var/lib/dpkg/status
root@e:~# apt-cache show qemu-system-x86 | grep Recommends
Recommends: qemu-system-gui (= 1:4.0+dfsg-0ubuntu4), qemu-utils, ovmf, cpu-checker

Yet things migrated, [2] is in release pocket.
And it is not showing up in [3] as I'd have expected.

Sorry Dannf for the extra delay, maybe there is another hidden archive mechanic I have yet to learn :-/ - I'll need to reach out.

[1]: https://launchpad.net/ubuntu/+source/qemu/1:4.0+dfsg-0ubuntu4
[2]: https://launchpad.net/ubuntu/+source/qemu/1:4.0+dfsg-0ubuntu4
[3]: https://people.canonical.com/~ubuntu-archive/component-mismatches

Change that triggers the mismatch is in the archive and seen in [1].
The correct status for now is Fix Committed, waiting for an AA to resolve that by promoting edk2.

Also thanks to APW who fixed the script generating the list of mismatches that was unable to handle unicode in my launchpad name.

[1]: https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.svg

Now that things properly show up I FYI-subscribed AAs here to resolve by promoting edk2.

On Mon, Jul 15, 2019 at 11:54:08AM -0000, Christian Ehrhardt  wrote:
> Now that things properly show up I FYI-subscribed AAs here to resolve by
> promoting edk2.

Just FTR this is not a requirement of the process, we pick these up directly
from the component-mismatches report.

Override component to main
edk2 0~20190606.20d2e5a1-1ubuntu2 in eoan: universe/misc -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan amd64: universe/misc/extra/100% -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan arm64: universe/misc/extra/100% -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan armhf: universe/misc/extra/100% -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan i386: universe/misc/extra/100% -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan ppc64el: universe/misc/extra/100% -> main
ovmf 0~20190606.20d2e5a1-1ubuntu2 in eoan s390x: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan amd64: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan arm64: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan armhf: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan i386: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan ppc64el: universe/misc/extra/100% -> main
qemu-efi 0~20190606.20d2e5a1-1ubuntu2 in eoan s390x: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan amd64: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan arm64: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan armhf: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan i386: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan ppc64el: universe/misc/extra/100% -> main
qemu-efi-aarch64 0~20190606.20d2e5a1-1ubuntu2 in eoan s390x: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan amd64: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan arm64: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan armhf: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan i386: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan ppc64el: universe/misc/extra/100% -> main
qemu-efi-arm 0~20190606.20d2e5a1-1ubuntu2 in eoan s390x: universe/misc/extra/100% -> main
25 publications overridden.