include loopback and squash4 modules in EFI binary

[Steve Langasek]

[SRU Justification]
Development versions of snappy Ubuntu Core leverage grub's squashfs support to load kernels and initramfs directly from the kernel snap (which is a squashfs-format archive). This requires the loopback and the squash4 grub modules to be loaded.

Currently, neither of these modules is included in the signed EFI binaries, therefore this boot strategy is not compatible with SecureBoot.

We should verify that the loopback and squash4 modules are suitable for inclusion in the signed binary, and include them.

[Test case]
1. Grab the snappy image from https://people.canonical.com/~mvo/all-snaps/amd64-all-snap.img.xz and uncompress it.
2. Install grub-efi-amd64-signed from xenial-updates.
3. Use kpartx to loop mount /dev/mapper/loopNp2.
4. Replace boot/efi/BOOT/BOOTX64.EFI in the boot partition with /usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed.
5. Unmount the boot partition.
6. Boot the image in a VM using UEFI firmware (not BIOS)
7. Confirm that the image fails to boot with an error about the loopback command not found.
8. Shut down the VM.
9. Install grub-efi-amd64-signed from xenial-proposed.
10. Mount the boot partition again.
11. Replace boot/efi/BOOT/BOOTX64.EFI in the boot partition with /usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed.
12. Unmount the boot partition and remove the kpartx mapping.
13. Boot the image in a VM again, using UEFI firmware.
14. Confirm that the image boots successfully.

[Regression potential]
Minimal. This SRU adds two additional modules to the UEFI boot images, which add a new command and a new filesystem driver respectively. Users who do not have the 'loopback' command in their grub.cfg, and who do not have any squashfs filesystems as raw disks or partitions, should not see any behavior difference. The added modules slightly increase the size of the grub images, from ~1.1MiB to ~1.2MiB. This should not affect the usability of these bootloader images.

[Seth Arnold (seth-arnold)]

./grub-core/fs/squash4.c :

There may be data format mismatches between grub2 and the linux kernel's idea of squashfs:

These are the structures from grub2 for file and long_file:
grub_uint32_t block_size[0];

These are the structures from the linux kernel for squashfs_reg_inode and squashfs_lreg_inode:
__le16 block_list[0];

squash_mount() checks grub_errno immediately after calling grub_disk_read(), before checking the return code. C idiom is to check "errno"-style variables only if an error is returned -- and also to set "errno"-style variables to 0 immediately before an operation if there's no error-return mechanism in place to avoid errors from previous operations mistakenly linger. This is probably not a security issue.

There are many calls to grub_malloc() with an arithmetic expression; normally these are better replaced with calloc(3)-alike wrappers which can check for integer wraparounds. I don't think any here are exploitable but I could have made a mistake.

grub_squash_iterate_dir() has extensive memory leaks in the reading or memory allocation error cases -- probably there's no recovery possible if the system is out of memory when running grub2, but I figured I'd mention it all the same. This is probably not a security issue.

./grub-core/disk/loopback.c :

grub_loopback_open() looks like it might handle gigantic sparse files poorly; a file that's within GRUB_DISK_SECTOR_SIZE bytes of 2^64 may set disk->total_sectors to a too-small value. This is probably not a security issue.

Now that grub is part of a security boundary the grub_malloc() calls with expressions should probably all be converted to using calloc(3)-style wrappers. It probably isn't worth blocking this specific change on this conversion though.

Thanks

[Steve Langasek (vorlon)]

Thanks for the review, Seth.

> There may be data format mismatches between grub2 and the linux kernel's
> idea of squashfs:

> These are the structures from grub2 for file and long_file:
> grub_uint32_t block_size[0];

> These are the structures from the linux kernel for squashfs_reg_inode and
> squashfs_lreg_inode:
> __le16 block_list[0];

I wasn't sure if you were raising this as a possible security issue or not. I'm also not clear what an array of 0 elements is supposed to do here. But you were comparing this with the Linux kernel headers, and the Linux kernel never writes squashfs, it only reads it... which means the inconsistency could be a bug in Linux rather than grub.

Here's what squashfs-tools has to say:

struct squashfs_reg_inode_header {
        unsigned short inode_type;
        unsigned short mode;
        unsigned short uid;
        unsigned short guid;
        int mtime;
        unsigned int inode_number;
        unsigned int start_block;
        unsigned int fragment;
        unsigned int offset;
        unsigned int file_size;
        unsigned int block_list[0];
};

struct squashfs_lreg_inode_header {
        unsigned short inode_type;
        unsigned short mode;
        unsigned short uid;
        unsigned short guid;
        int mtime;
        unsigned int inode_number;
        squashfs_block start_block;
        long long file_size;
        long long sparse;
        unsigned int nlink;
        unsigned int fragment;
        unsigned int offset;
        unsigned int xattr;
        unsigned int block_list[0];
};

This shows a 32-bit int for the block_list, which matches grub rather than the kernel. So it doesn't look like this is going to cause corruption due to a long read with a genuine squashfs. And anyway, grub's handling of the block_size field seems well-guarded to me. Based on this, I conclude that there are no blockers here for secureboot signing of this code.

[Launchpad Janitor (janitor)]

This bug was fixed in the package grub2 - 2.02~beta2-36ubuntu9

---------------
grub2 (2.02~beta2-36ubuntu9) yakkety; urgency=medium

  * Add loopback and squash4 modules to the signed EFI images. LP: #1604499.

 -- Steve Langasek <email address hidden> Tue, 19 Jul 2016 11:43:28 -0700

[Adam Conrad (adconrad)]

Hello Steve, or anyone else affected,

Accepted grub2 into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/grub2/2.02~beta2-36ubuntu3.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Adam Conrad (adconrad)]

Hello Steve, or anyone else affected,

Accepted grub2-signed into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/grub2-signed/1.66.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Ubuntu Foundations Team Bug Bot (crichton)]

As a part of the Stable Release Updates quality process a search for Launchpad bug reports using the version of grub2-signed from xenial-proposed was performed and bug 1609350 was found. Please investigate this bug report to ensure that a regression will not be created by this SRU. In the event that this is not a regression remove the "verification-failed" tag from this bug report and add the tag "bot-stop-nagging" to bug 1609350 (not this bug). Thanks!

[Mathieu Trudel-Lapierre (cyphermox)]

I've looked at bug 1609350, and it does not appear to be a regression introduced by this upload.

[Mathieu Trudel-Lapierre (cyphermox)]

Package looks good: I could replace the efi image in /EFI/BOOT, and successfully boot from that point.

Since I didn't install the full package (but the change is effectively just adding the modules in the generated EFI image), grub loaded from shim and displayed the command-line, from which point I could type in the contents of /EFI/BOOT/grub.cfg to get it to look again at the system, and load the menu with "writable" as the only entry, which boots correctly to the system.

[Launchpad Janitor (janitor)]

This bug was fixed in the package grub2 - 2.02~beta2-36ubuntu3.2

---------------
grub2 (2.02~beta2-36ubuntu3.2) xenial; urgency=medium

  * Add loopback and squash4 modules to the signed EFI images. LP: #1604499.

 -- Steve Langasek <email address hidden> Fri, 22 Jul 2016 15:29:24 -0700

[Brian Murray (brian-murray)]

The verification of the Stable Release Update for grub2 has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package grub2-signed - 1.66.2

---------------
grub2-signed (1.66.2) xenial; urgency=medium

  * Rebuild against grub2 2.02~beta2-36ubuntu3.2. (LP: #1604499)

 -- Steve Langasek <email address hidden> Fri, 22 Jul 2016 15:33:06 -0700