shim 15.6 for SUSE

[jsegitz]

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?

---

https://github.com/jsegitz/shim-review/tree/SUSE-SLES-shim-x86_aarch64-20220712

---

What is the SHA256 hash of your final SHIM binary?

---

f83baf7da3d799c33806765facd86671ec3426a747f3219d2689958679aa1d32 shim-sles_aarch64.efi
28c14c4e40eb76d226703ac3765dec2226bdbc343d1467893126fc5615a798cb shim-sles_x86_64.efi

[jsegitz]

I'm leaving for a prolonged holiday end of July. It would be great if I could get this done (incl. the signing part by Microsoft) before that

[jsegitz]

Ping. Is there anything blocking this review? Can I help somehow?

[msmeissn]

Chenxi, note this ticket is about the SUSE SLES shim, not the SUSE EULER shim.

[frozencemetery]

• Your readme mentions SBAT for fwupdate; are you still using fwupdate for things? If so, are you aware that fwupdate has been subsumed into fwupd?
• You appear to have several downstream patches to shim. Can you speak to your plans for upstreaming them?
• Your README mentions shim 15.4; I assume this should refer to 15.6?
• README.md specifies a command to build (podman build --build-arg ARCHITECTURE=x86_64 -t sles_shim:15.4) which I believe is missing the trailing dot. Please fix.
• Also on the above, is 15.4 correct? Seems like it should be 15.6?
• Would you be willing to make podman build . without arguments do the right thing? Currently it fails.

[msmeissn]

• we use fwupdated on older SUSE Linux Enterprise Distributions, we also ship fwupd on newers SLES 15 SP2 onwards. (but we did not remove fwupdate-efi from newer distributions yet.)
• patches:

description first:

• remove_build_id.patch
  It's for reproducible builds.

• shim-arch-independent-names.patch
  Base on the patch description, it is not for reproducible builds. It's more convenient if we can use the same script to handle(install) shim on different architectures. So we removed the architecture from the name of shim binary. We can send it to upstream with a build option.

• shim-bsc1177315-verify-eku-codesign.patch
  It's a SUSE specific function for NIAP OS_PP certification. For implementation, this patch copies VerifyEKUsInPkcs7Signature() function from EDK2 and also with a 21f984cedec edk2 patch to fix used-after-free bug in this function. We can also send this patch to shim upstream with a build option. If anyone also wants to check codesign eku in shim.

• shim-bsc1177789-fix-null-pointer-deref-AuthenticodeVerify.patch
  This patch is from EDK2 (the upstream of Cryptlib) for CVE-2019-14584. We will send it to shim upstream against CVE-2019-14584.

• shim-change-debug-file-path.patch
  Yes, this is a SUSE specific patch. We have no plan to send it to upstream.

• shim-disable-export-vendor-dbx.patch
  Some machines doesn't have enough space for variable. This patch can decrease the possibility of crash. This patch did not send to upstream and didn't merge yet. (the patch on shim/pull/369 is a different patch. It's 3f327f546c in shim git) We will send shim-disable-export-vendor-dbx.patch to upstream with a build option.

Summary:

Will send to upstream (4):
shim-arch-independent-names.patch
shim-bsc1177315-verify-eku-codesign.patch
shim-bsc1177789-fix-null-pointer-deref-AuthenticodeVerify.patch
shim-disable-export-vendor-dbx.patch

SUSE specific (2):
shim-change-debug-file-path.patch

• We include shim 15.6, sorry for the 15.4 typos.
• The README.md will be fixed by our engineer @jsegitz who will return this week.
• the -t should be changed to sles_shim:15.6 , yes.
• @jsegitz will answer if he can just make podman build . work , i think it should be doable.

[jsegitz]

I assume Marcus answered the first two questions.

As for 15.4: There's one place where this was wrong (What patches are being applied and why). The 15.4 in the podman commands etc. refer to our product SLES 15.4. I know that's a bit confusing so I'll change it to 15.6 too, since it doesn't really matter for us

I added the trailing dot, thanks

We can't drop the arguments to podman (apart from the naming for the image) since the Dockerfile is for two architectures that need slightly different commands.By setting ARCHITECTURE this is done. the alternative would be to provide separate Dockerfiles per architecture. If that's better then I can do that.

[jsegitz]

This seems stuck. Is there something we can do to help this review?

[jsegitz]

ping. Anything we can do to help?

[jsegitz]

and again. Don't want to be annoying but we need this approved please

[jsegitz]

can someone please reach out to me to discuss when/how this will get reviewed?

[frozencemetery]

As you may know, shim reviews are a community effort. The idea is that distros review each other to ensure quality. You are at this point intimately aware that the number of reviews is exceeding reviewer capacity. The best way to expedite your own review is to review submissions from other entities to reduce the pile.

Regarding our earlier conversation about building, I'm still not sure I understand why you need an architecture argument. If the intent is to make cross-arch building work, it doesn't:

$ $ uname -m
x86_64
$ podman build --build-arg ARCHITECTURE=aarch64 -t sles_shim:15.6 .
...
STEP 11/19: RUN zypper -n in /pesign-obs-integration-10.1-13.3.1.$ARCHITECTURE.rpm
Refreshing service 'container-suseconnect-zypp'.
Warning: Skipping service 'container-suseconnect-zypp' because of the above error.
Problem retrieving the repository index file for service 'container-suseconnect-zypp':
[container-suseconnect-zypp|file:/usr/lib/zypp/plugins/services/container-suseconnect-zypp] 
Loading repository data...
Reading installed packages...
'_tmpRPMcache_:pesign-obs-integration=0:10.1-13.3.1' not found in package names. Trying capabilities.
No provider of '_tmpRPMcache_:pesign-obs-integration=0:10.1-13.3.1' found.
Error: error building at STEP "RUN zypper -n in /pesign-obs-integration-10.1-13.3.1.$ARCHITECTURE.rpm": error while running runtime: exit status 104

I have access to aa64 hardware and reproduced this there, so I'm not concerned enough to block the review on it, but please fix or remove this for next submission.

[jsegitz]

Thank you for the review.

I'm more than happy to help out with the reviews. I tried to get this going in
#84
but without being able to actually tag submissions as accepted this never went anywhere. Maybe we can create an appointment where bigger stakeholders (e.g. Oracle, SUSE) can discuss how to help out here.

As for cross-arch building: This is there because of this and it should work. I'll look into the issue, didn't happen for me. I think this is a probelm with our repos that didn't happen when I worked through this