search

rhboot / shim-review

Reviews of shim
66 stars 128 forks source link

Shim 15.8 for Azure Linux 3.0 (x86_64 and aarch64) #427

Open ddstreetmicrosoft opened 3 months ago

ddstreetmicrosoft commented 3 months ago

Confirm the following are included in your repo, checking each box:

What is the link to your tag in a repo cloned from rhboot/shim-review?

Initial submission: https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240531

Update (2024-07-01): https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240701

What is the SHA256 hash of your final SHIM binary?

Initial submission: 0eff03502514be459b182c3cda1cef6a3762cbf10462591685a17c356e42774d shimaa64.efi b6a99795d9f3e882afa318d11bdb648dff7e547470d14bfeba03af385eb452fc shimx64.efi

Update (2024-07-01): 7905c30de3109eb4ff8a5d198f5077bceb35b0fc3559b03924cf78a96e511bd0 shimaa64.efi 83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801 shimx64.efi

What is the link to your previous shim review request (if any, otherwise N/A)?

https://github.com/rhboot/shim-review/issues/387

If no security contacts have changed since verification, what is the link to your request, where they've been verified (if any, otherwise N/A)?

Dan Streetman: https://github.com/rhboot/shim-review/issues/387#issuecomment-2042990828

Chris Co: https://github.com/rhboot/shim-review/issues/387#issuecomment-2072937510

ddstreetmicrosoft commented 2 months ago

Hi, just checking if anything else is needed for this review.

THS-on commented 2 months ago

@ddstreetmicrosoft no don't think so. I gave it just an initial read and will do a complete review in the next few days. In the meantime it really helps us to speed up the queue if submitters review also other submissions.

THS-on commented 2 months ago

There seems something wrong with the gcc installation in the Dockerfile:

 > [3/5] RUN <<-EOF (set -ex...):                                                                                                  
0.287 ++ rpm -E %_arch                                                                                                             
0.294 + export ARCH=x86_64                                                                                                         
0.294 + ARCH=x86_64                                                                                                                
0.294 + case $ARCH in                                                                                                              
0.294 + PKGNAME=shim-unsigned-x64
0.294 + SHIMDIR=/usr/share/shim/15.8/x64
0.294 + SHIM=shimx64.efi
0.294 + tdnf install -y binutils-0:2.41-2.azl3 dos2unix-0:7.5.1-1.azl3 efivar-devel-0:39-1.azl3 gcc-0:13.2.0-4.azl3 git-0:2.42.0-2.azl3 glibc-devel-0:2.38-3.azl3 kernel-headers-0:6.6.29.1-3.azl3 make-0:4.4.1-1.azl3 openssl-devel-0:3.3.0-1.azl3 pesign-0:116-3.azl3 rpm-build-0:4.18.2-1.azl3 vim-extra-0:9.0.2190-2.azl3
0.306 Loaded plugin: tdnfrepogpgcheck
0.330 Refreshing metadata for: 'Azure Linux Official Microsoft Non-Open-Source Preview 3.0 x86_64'
2.054 Refreshing metadata for: 'Azure Linux Official Preview 3.0 x86_64'
6.143 gcc-0:13.2.0-4.azl3 package not found or not installed
6.143 Error(1011) : No matching packages
------
Dockerfile:4
--------------------
   3 |     
   4 | >>> RUN <<-EOF
   5 | >>>   set -ex
   6 | >>> 
   7 | >>>   export ARCH=$(rpm -E %_arch)
   8 | >>>   case $ARCH in
   9 | >>>     x86_64)
  10 | >>>       PKGNAME=shim-unsigned-x64
  11 | >>>       SHIMDIR=/usr/share/shim/15.8/x64
  12 | >>>       SHIM=shimx64.efi
  13 | >>>       ;;
  14 | >>>     aarch64)
  15 | >>>       PKGNAME=shim-unsigned-aarch64
  16 | >>>       SHIMDIR=/usr/share/shim/15.8/aa64
  17 | >>>       SHIM=shimaa64.efi
  18 | >>>       ;;
  19 | >>>     *)
  20 | >>>       echo "Invalid arch: $ARCH"
  21 | >>>       false
  22 | >>>       ;;
  23 | >>>   esac
  24 | >>> 
  25 | >>>   tdnf install -y binutils-0:2.41-2.azl3 \
  26 | >>>                   dos2unix-0:7.5.1-1.azl3 \
  27 | >>>                   efivar-devel-0:39-1.azl3 \
  28 | >>>                   gcc-0:13.2.0-4.azl3 \
  29 | >>>                   git-0:2.42.0-2.azl3 \
  30 | >>>                   glibc-devel-0:2.38-3.azl3 \
  31 | >>>                   kernel-headers-0:6.6.29.1-3.azl3 \
  32 | >>>                   make-0:4.4.1-1.azl3 \
  33 | >>>                   openssl-devel-0:3.3.0-1.azl3 \
  34 | >>>                   pesign-0:116-3.azl3 \
  35 | >>>                   rpm-build-0:4.18.2-1.azl3 \
  36 | >>>                   vim-extra-0:9.0.2190-2.azl3
  37 | >>> 
  38 | >>>   rpm -iv /${PKGNAME}-*.src.rpm
  39 | >>>   rpmbuild -bb /usr/src/azl/SPECS/${PKGNAME}.spec
  40 | >>>   rpm -iv /usr/src/azl/RPMS/${ARCH}/${PKGNAME}-*.${ARCH}.rpm
  41 | >>>   sha256sum ${SHIMDIR}/${SHIM} /${SHIM} > /shim.sha256
  42 | >>>   cmp ${SHIMDIR}/${SHIM} /${SHIM}
  43 | >>>   objcopy -O binary -j .sbat ${SHIMDIR}/${SHIM} /shim-sbat
  44 | >>> EOF
  45 |     
--------------------
ERROR: failed to solve: process "/bin/sh -c   set -ex\n\n  export ARCH=$(rpm -E %_arch)\n  case $ARCH in\n    x86_64)\n      PKGNAME=shim-unsigned-x64\n      SHIMDIR=/usr/share/shim/15.8/x64\n      SHIM=shimx64.efi\n      ;;\n    aarch64)\n      PKGNAME=shim-unsigned-aarch64\n      SHIMDIR=/usr/share/shim/15.8/aa64\n      SHIM=shimaa64.efi\n      ;;\n    *)\n      echo \"Invalid arch: $ARCH\"\n      false\n      ;;\n  esac\n\n  tdnf install -y binutils-0:2.41-2.azl3 \\\n                  dos2unix-0:7.5.1-1.azl3 \\\n                  efivar-devel-0:39-1.azl3 \\\n                  gcc-0:13.2.0-4.azl3 \\\n                  git-0:2.42.0-2.azl3 \\\n                  glibc-devel-0:2.38-3.azl3 \\\n                  kernel-headers-0:6.6.29.1-3.azl3 \\\n                  make-0:4.4.1-1.azl3 \\\n                  openssl-devel-0:3.3.0-1.azl3 \\\n                  pesign-0:116-3.azl3 \\\n                  rpm-build-0:4.18.2-1.azl3 \\\n                  vim-extra-0:9.0.2190-2.azl3\n\n  rpm -iv /${PKGNAME}-*.src.rpm\n  rpmbuild -bb /usr/src/azl/SPECS/${PKGNAME}.spec\n  rpm -iv /usr/src/azl/RPMS/${ARCH}/${PKGNAME}-*.${ARCH}.rpm\n  sha256sum ${SHIMDIR}/${SHIM} /${SHIM} > /shim.sha256\n  cmp ${SHIMDIR}/${SHIM} /${SHIM}\n  objcopy -O binary -j .sbat ${SHIMDIR}/${SHIM} /shim-sbat\n" did not complete successfully: exit code: 243

@ddstreetmicrosoft can you have a look at it?

christopherco commented 2 months ago

@THS-on thanks for taking a look - I'll look into it and see what is going on.

christopherco commented 2 months ago

@THS-on @ddstreetmicrosoft I've identified the issue and would like to update our Azure Linux 3.0 submission to reflect the changes. New tagged submission - https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240701

7905c30de3109eb4ff8a5d198f5077bceb35b0fc3559b03924cf78a96e511bd0 shimaa64.efi
83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801 shimx64.efi

Let me know if you'd prefer a new GH issue for the submission or to handle in some other way.

Details for the submission difference:

THS-on commented 2 months ago

Review for azurelinux-shim-aa64-x64-20240701

Shim

GRUB2

Linux

Notes and Questions

ddstreetmicrosoft commented 1 month ago

Can you update top comment with the new tag and hashes?

updated; I'll let Chris respond to the other questions

christopherco commented 1 month ago

Can you clarify the GRUB2 version that is used?

Sorry about the confusion. We are releasing with 2.06 with Fedora 34 patches for invoking EFI Handover Protocol. We had initially made the move to 2.12 but encountered an unexpected regression where the TPM Event Log was not being handed over to the kernel, only when Secure Boot was enabled. We'll be sticking here with 2.06 for the foreseeable future of Azure Linux 3.0. I'll also add the SBAT entry into the grub SBAT.

How do you ensure that kernel is forced in lockdown mode when booted via Secure Boot?

All of our default images and configurations set lockdown=integrity in the kernel command line to force the kernel into lockdown mode. i.e., https://github.com/microsoft/azurelinux/blob/3.0/toolkit/tools/internal/resources/assets/grub2/grub#L5 which is used during base image creation.

We have previously mulled over bringing in the out-of-tree kernel patches to link Secure Boot to forcing lockdown mode directly, but we thus far decided to stick closer to upstream stable LTS kernel for the time being.

THS-on commented 1 month ago

@ddstreetmicrosoft @christopherco thank you for the clarifications.

Regarding the grub config. Forcing lockdown=integrity is a valid way to achieve the same, as the lockdown patches. The question is if a user can remove this option or not, i.e. is the config part of the GRUB2 binary that is signed? If the user is able to change it, than this would differ for the lockdown patches.

christopherco commented 1 month ago

Regarding the grub config. Forcing lockdown=integrity is a valid way to achieve the same, as the lockdown patches. The question is if a user can remove this option or not, i.e. is the config part of the GRUB2 binary that is signed? If the user is able to change it, than this would differ for the lockdown patches.

Ah yes, in our case, it is possible for a user that has root privileges to remove the lockdown setting since the kernel command line setting is located in our grub.cfg. I can discuss internally on pulling in the lockdown patches to cover the difference.

THS-on commented 1 month ago

Yeah you can either include the lockdown patches or build your signed kernel CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY=y which forces the lockdown mode to be enabled.

Note tho that the following lockdown patch is not upstream, so you probably want to include either way: https://salsa.debian.org/kernel-team/linux/-/blob/master/debian/patches/features/all/lockdown/mtd-disable-slram-and-phram-when-locked-down.patch?ref_type=heads

christopherco commented 1 week ago

Following up here - our 6.6.47 kernel now has the lockdown patches that will tie kernel lockdown to secure boot state

THS-on commented 23 hours ago

@christopherco great! The the question is if it makes already sense to also rotate your signing certificate for the kernel and then put the old one on the revocation list, so that older kernels cannot be booted with the new shim?