Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
8 tasks done
ddstreetmicrosoft opened this issue Jun 6, 2024 · 26 comments
Open
8 tasks done

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

ddstreetmicrosoft opened this issue Jun 6, 2024 · 26 comments
Assignees
Labels
contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer extra review wanted Initial review(s) look good, another review desired question Reviewer(s) waiting on response

Comments

@ddstreetmicrosoft
Copy link

ddstreetmicrosoft commented Jun 6, 2024

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

  • completed README.md file with the necessary information
  • shim.efi to be signed
  • public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
  • binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
  • any extra patches to shim via your own git tree or as files
  • any extra patches to grub via your own git tree or as files
  • build logs
  • a Dockerfile to reproduce the build of the provided shim EFI binaries

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

Update (2024-09-23):
https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240923

Update (2024-09-30):
https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240930
Another Update, sorry (2024-09-30):
https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240930.1


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


#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: #387 (comment)

Chris Co: #387 (comment)

@THS-on THS-on added the contacts verified OK Contact verification is complete here (or in an earlier submission) label Jun 9, 2024
@ddstreetmicrosoft
Copy link
Author

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

@THS-on
Copy link
Collaborator

THS-on commented Jun 28, 2024

@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 THS-on self-assigned this Jun 28, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jun 30, 2024

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?

@THS-on THS-on added the bug Problem with the review that must be fixed before it will be accepted label Jul 1, 2024
@christopherco
Copy link
Contributor

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

@christopherco
Copy link
Contributor

christopherco commented Jul 2, 2024

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

  • This shim-review is for a new Azure Linux 3.0 release that is not fully released for production use. The packages are currently part of our "preview" package repository.
  • The packages used to build shim, including gcc, were updated, and as part of that update we also removed previous versions of the packages from our package server, so the dockerfile could not fetch those packages. I've updated the dockerfile with the updated versions that are present.
  • In this case, our gcc was updated to remove a specific default compiler optimization change which has also altered the shim binaries we were submitting. So I have rebuilt the shim binaries with the latest toolchain and the docker build now matches our new binary submissions. This change to gcc is also the main reason we purged our "preview" package repository and re-uploaded all of the packages built with the updated gcc.
  • I've also updated the submission's docker build logs to match the new build and the README and ISSUE_TEMPLATE files with the new shim binary hashes

@THS-on
Copy link
Collaborator

THS-on commented Jul 8, 2024

Review for azurelinux-shim-aa64-x64-20240701

Shim

  • Shim based on 15.8 without any patches
  • NX is disabled
  • SBAT looks fine
  • CA is valid till 2038, 4096bit RSA (ok)
  • Is reproducible
#8 0.271 83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801  /usr/share/shim/15.8/x64/shimx64.efi
#8 0.271 83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801  /shimx64.efi
#8 0.261 7905c30de3109eb4ff8a5d198f5077bceb35b0fc3559b03924cf78a96e511bd0  /usr/share/shim/15.8/aa64/shimaa64.efi
#8 0.261 7905c30de3109eb4ff8a5d198f5077bceb35b0fc3559b03924cf78a96e511bd0  /shimaa64.efi

GRUB2

Linux

Notes and Questions

  • Can you update top comment with the new tag and hashes?
  • Can you clarify the GRUB2 version that is used?
  • How do you ensure that kernel is forced in lockdown mode when booted via Secure Boot?

@THS-on THS-on added the question Reviewer(s) waiting on response label Jul 8, 2024
@ddstreetmicrosoft
Copy link
Author

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

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

@christopherco
Copy link
Contributor

christopherco commented Jul 24, 2024

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
Copy link
Collaborator

THS-on commented Jul 24, 2024

@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
Copy link
Contributor

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
Copy link
Collaborator

THS-on commented Jul 25, 2024

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
Copy link
Contributor

christopherco commented Aug 26, 2024

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

@THS-on
Copy link
Collaborator

THS-on commented Sep 8, 2024

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

@christopherco
Copy link
Contributor

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

@THS-on This shim actually has a new certificate that has not been used to sign any boot artifacts for Azure Linux yet. Once this shim is approved, we will be updating our signing certificate in our official Azure Linux 3.0 builds to sign with the new signing cert. So, we had already planned in a certificate rotation as part of this new shim.

@THS-on
Copy link
Collaborator

THS-on commented Sep 13, 2024

@christopherco thanks for the clarification. I saw that the current certificate was already included in the other Shims DB, but now also saw checked that the Grub in 2.0 is still signed with the other cert (/C=US/ST=Washington/L=Redmond/O=Microsoft Corporation/CN=Microsoft Testing PCA 2010).

LGTM, just needs another reviewer.

@THS-on THS-on added extra review wanted Initial review(s) look good, another review desired easy to review This submission might be a good place to start for an inexperienced reviewer and removed bug Problem with the review that must be fixed before it will be accepted question Reviewer(s) waiting on response labels Sep 13, 2024
@NeilHanlon
Copy link

I'm taking the extra review here, but have run into some trouble.

The July version of the review (https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240701) is unable to be built, as the container registry's DNS no longer resolves. Additionally, I cannot find the specified tag on the mcr.microsoft.com registry which now holds these images.

I tried to use the 3.0 tag from mcr.microsoft.com for azurelinux, but the repositories no longer have the versions of packages that the Dockerfile is expecting.

Pausing review until this issue is resolved, though the rest of procedural things look OK.

@steve-mcintyre steve-mcintyre added the question Reviewer(s) waiting on response label Sep 23, 2024
@christopherco
Copy link
Contributor

I'm taking the extra review here, but have run into some trouble.

The July version of the review (https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240701) is unable to be built, as the container registry's DNS no longer resolves. Additionally, I cannot find the specified tag on the mcr.microsoft.com registry which now holds these images.

I tried to use the 3.0 tag from mcr.microsoft.com for azurelinux, but the repositories no longer have the versions of packages that the Dockerfile is expecting.

Pausing review until this issue is resolved, though the rest of procedural things look OK.

@NeilHanlon Thanks for doing the extra review and apologies about the trouble.
The container registry location and package versions & server location were moved between the July tag and now. We have updated our submission collateral and created a new tag - https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240923

Note: the shim binary hashes did not change between the July tag and this new tag.

7905c30de3109eb4ff8a5d198f5077bceb35b0fc3559b03924cf78a96e511bd0  shimaa64.efi
83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801  shimx64.efi

Thanks @gjswalling for making the Dockerfile changes on our side!

@steve-mcintyre
Copy link
Collaborator

Very tempted to NACK this review. Neither docker nor podman will work for me here.

@lorddoskias
Copy link

Very tempted to NACK this review. Neither docker nor podman will work for me here.

If run with docker buildx build . it actually work and reproduces for x86 at least.

@steve-mcintyre
Copy link
Collaborator

We say "docker build ." in the docs, no mention of buildx.

@ddstreet
Copy link
Contributor

We say "docker build ." in the docs, no mention of buildx.

As noted here, we created the single Dockerfile using a here-doc, which requires use of buildx instead of build. I did this to simplify the work of reviewers, so that a single Dockerfile could be used for both architectures, as I saw no way to craft a single Dockerfile that would allow building the shim on both architectures.

As shown using the Fedora docker command, the old build is actually deprecated and the new buildx (BuildKit) is recommended, e.g.:

$ grep PRETTY_NAME /etc/os-release 
PRETTY_NAME="Fedora Linux 40 (Cloud Edition)"
$ docker build .
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            Install the buildx component to build images with BuildKit:
            https://docs.docker.com/go/buildx/

However, it looks like Fedora doesn't provide buildx until release 41:
https://src.fedoraproject.org/rpms/docker-buildx

I had been testing this using Ubuntu which has provided docker buildx as an option in current stable releases, so I didn't realize this would add difficulty to reproducing the docker build on Fedora.

In any case, we can certainly split the Dockerfile into two per-arch Dockerfiles to make it easier for you to review, if that's the easiest path forward.

@ddstreet
Copy link
Contributor

split the Dockerfile into two per-arch Dockerfiles to make it easier for you to review

I split the Dockerfile into per-arch files, located at Dockerfile-x86_64/Dockerfile and Dockerfile-aarch64/Dockerfile. I did not change the existing Dockerfile. The split per-arch Dockerfiles are the same as the main Dockerfile with the here-doc removed and use of per-arch variables replaced with actual arch strings (e.g. x86_64, x64, aarch64, aa64); and they also require the shim EFI and src.rpm to be located in their dir, so I copied those files into the per-arch Dockerfile dirs.

Using Fedora 40, I can run docker build Dockerfile-x86_64 successfully.

Using Fedora 41, because it switches the docker backend builder over to the newer BuildKit, using either file will work, i.e. docker build . will work just as well as docker build Dockerfile-x86_64

Obviously, using the arch-specific Dockerfile requires actually running on the correct arch (i.e. use Dockerfile-x86_64 only while running on a x86_64 system).

This is included in this updated tag:
https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240930

@steve-mcintyre
Copy link
Collaborator

steve-mcintyre commented Sep 30, 2024

I'm using Debian 12 (Bookworm, current stable release) here - Fedora doesn't help...

It would be nice if Docker provided stable interfaces and useful isolation. But hey, moon on a stick...

@ddstreet
Copy link
Contributor

One more minor update, sorry:
https://github.com/microsoft/shim-review/tree/azurelinux-shim-aa64-x64-20240930.1

This only fixes the aarch64-specific Dockerfile, which had some of the lines using aa64 instead of aarch64; it should be correct now and should be docker-buildable on aarch64 systems.

I'm using Debian 12 (Bookworm, current stable release) here - Fedora doesn't help...

Unfortunately Debian Bookworm doesn't seem to provide any package for docker buildx, but the arch-specific Dockerfile does build correctly:

ddstreet@d-bookworm:~/shim-review$ docker build Dockerfile-x86_64
...
Successfully built be5e10cb2919

@lorddoskias
Copy link

lorddoskias commented Oct 1, 2024

  • x86 build reproduces:
        #14 [10/11] RUN cat /shim.sha256
        #14 0.320 83c927eada08e0811adbaab47323841808d79e51d11aa8072c552bfbf80af801  /usr/share/shim/15.8/x64/shimx64.efi
  • shim is indeed based on 15.8 tar.bz2.
  • AFAIU this shim is being imported from Fedora 40, yet their sbat entry is not retained. This is based off the changelog in the .spec file for the shim:
    Initial CBL-Mariner import from Fedora 40 (license: MIT).
  • HSM is used for key security
  • The shim is signed with a certificate which expires in 2038 (14 years) and is a CA
  • Grub2 modules look ok

@christopherco
Copy link
Contributor

@lorddoskias thanks for the review!
Regarding the sbat entry, while our previous releases / shim-reviews had shims from custom specs we authored, with Azure Linux 3.0 we rebased our 3.0 shim spec onto the fedora 40 version.

We'll include the fedora SBAT entries in addition to our own and update our submission here.

shim.rh,3,The Fedora Project,shim,15.8,https://src.fedoraproject.org/rpms/shim-unsigned-x64
shim.redhat,3,The Fedora Project,shim,15.8,https://src.fedoraproject.org/rpms/shim-unsigned-x64
shim.fedora,3,The Fedora Project,shim,15.8,https://src.fedoraproject.org/rpms/shim-unsigned-x64

https://src.fedoraproject.org/rpms/shim-unsigned-x64/blob/f40/f/sbat.redhat.csv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer extra review wanted Initial review(s) look good, another review desired question Reviewer(s) waiting on response
Projects
None yet
Development

No branches or pull requests

7 participants