shim 15.7 for Navix 8

[leejun9503]

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/NaverCloudPlatform/shim-review/tree/navix-shim-x86_64-20231026

---

What is the SHA256 hash of your final SHIM binary?

---

390a849f60b61591e72e44cc26a0f8194c78e2dc8e094d1da7fe1f4f49991446  shimia32.efi
367196b6c335fb25e0312cf8717875decf3460436475a15741b7e46364eb6ac6  shimx64.efi

---

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

---

N/A. This is our first shim submission.

[aronowski]

Nice to see a RHEL-based distro! There are some errors in the current application, but I'll try my best to help.

First, not every checkbox is checked in the issue. It's supposed to be a confirmation akin to "yes, I've checked if I have these or not and if I do have them, I included them". Please, edit this issue and check these.

---

I haven't checked the shim binary checksums and reproducibility, since the .sbatlevel section has the binutils bug.
Since you're rebuilding RHEL, you can just use my port of the patch that fixes this, called buggy-binutils.patch I created here.

---

*******************************************************************************
### Do you add a vendor-specific SBAT entry to the SBAT section in each binary that supports SBAT metadata ( grub2, fwupd, fwupdate, shim + all child shim binaries )?
### Please provide exact SBAT entries for all SBAT binaries you are booting or planning to boot directly through shim.
### Where your code is only slightly modified from an upstream vendor's, please also preserve their SBAT entries to simplify revocation.
*******************************************************************************
Yes.

[...]

grub2

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md  
grub,3,Free Software Foundation,grub,@@VERSION@@,https//www.gnu.org/software/grub/  
grub.rh,2,Red Hat,grub2,2.02-148.el8,mailto:[email protected]  
grub.navix,1,Navix,grub2,2.02-148.el8,mailto:[email protected]  


[...]

The final grubx64.efi binary shall not have the @@VERSION@@ variable to be interpolated. Instead, take a look at this listing from a RHEL 8 system:

# objcopy -O binary --only-section=.sbat /boot/efi/EFI/**/grubx64.efi /dev/stdout
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,3,Free Software Foundation,grub,2.02,https//www.gnu.org/software/grub/
grub.rh,2,Red Hat,grub2,2.02-148.el8_8.1,mailto:[email protected]

---

*******************************************************************************
### If your boot chain of trust includes a Linux kernel:
### Is upstream commit [1957a85b0032a81e6482ca4aab883643b8dae06e "efi: Restrict efivar_ssdt_load when the kernel is locked down"](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1957a85b0032a81e6482ca4aab883643b8dae06e) applied?
### Is upstream commit [75b0cea7bf307f362057cc778efe89af4c615354 "ACPI: configfs: Disallow loading ACPI tables when locked down"](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=75b0cea7bf307f362057cc778efe89af4c615354) applied?
### Is upstream commit [eadb2f47a3ced5c64b23b90fd2a3463f63726066 "lockdown: also lock down previous kgdb use"](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eadb2f47a3ced5c64b23b90fd2a3463f63726066) applied?
*******************************************************************************
Yes.

*******************************************************************************
### Do you build your signed kernel with additional local patches? What do they do?
*******************************************************************************
No. Kernel source is identical to RHEL.

So if the sources are identical, I suppose you made no changes to the source RPM.
This is a listing from a source RPM I just checked:

[user@localhost kernel-4.18.0-477.27.1.el8_8.src]$ grep CONFIG_KDB_DEFAULT_ENABLE *x86_64*.config
kernel-x86_64.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-x86_64-debug.config:CONFIG_KDB_DEFAULT_ENABLE=0x0

According to it, the "lockdown: also lock down previous kgdb use" patch should not be required. So am I right, that you implemented it just for safety?
If so, great job! But in that case you'll need to change an answer, so it mentions your implementation, since your SRPM is not identical to RHEL's anymore.

---

Your Dockerfile has these entries:

FROM rockylinux:8.8.20230518
[...]
RUN dnf install dnf-plugins-core rpm-build -y; \
    dnf builddep shim-unsigned-x64-15.7-1.el8.src.rpm -y

This is fine, but since that Rocky Linux image has their DNF repos set up, so packages will be continuously updated, once Rocky 8.9 comes out, it might break build reproducibility.

One workaround is to hardcode references to repos from an older release, e.g. 8.7. If you want, you can base yours on mine, though keep in mind to adapt it to version 8 rather than 9.

---

If you need help with porting or verifying some of these, or would like me to explain something more in-depth, feel free to ask.

[leejun9503]

@aronowski
Thanks for reviewing!
We will fix and update the issue you mentioned.

---

Nice to see a RHEL-based distro! There are some errors in the current application, but I'll try my best to help.

First, not every checkbox is checked in the issue. It's supposed to be a confirmation akin to "yes, I've checked if I have these or not and if I do have them, I included them". Please, edit this issue and check these.

Got it. I understanded those questions as "check the boxes if your shim submission has those features".

---

I haven't checked the shim binary checksums and reproducibility, since the .sbatlevel section has the binutils bug. Since you're rebuilding RHEL, you can just use my port of the patch that fixes this, called buggy-binutils.patch I created here.

When i compiled the shim on local/Dockerfile, binary checksum always returned same value so I thought there were no issues.
We will apply your patch. Thank you.

---

The final grubx64.efi binary shall not have the @@VERSION@@ variable to be interpolated. Instead, take a look at this listing from a RHEL 8 system:

Ok, we will update grub's SBAT section.

---

So if the sources are identical, I suppose you made no changes to the source RPM. This is a listing from a source RPM I just checked:
...
According to it, the "lockdown: also lock down previous kgdb use" patch should not be required. So am I right, that you implemented it just for safety? If so, great job! But in that case you'll need to change an answer, so it mentions your implementation, since your SRPM is not identical to RHEL's anymore.

We will check this issue again and update our answer accordingly.

---

This is fine, but since that Rocky Linux image has their DNF repos set up, so packages will be continuously updated, once Rocky 8.9 comes out, it might break build reproducibility.

One workaround is to hardcode references to repos from an older release, e.g. 8.7. If you want, you can base yours on mine, though keep in mind to adapt it to version 8 rather than 9.

We used Rocky because we can't release our os right now. Thanks for your suggestion. We will fix the base image.

[aronowski]

We used Rocky because we can't release our os right now.

That's OK. The distros from the Enterprise Linux family shall be binary-compatible with each other, so you can stay with Rocky.

I'm just worried about updates to the software stack they provide, that can result in outputting a different binary some time in the future. An idea of mitigating this is to use an older minor release that no longer gets updates - back then I was not aware of this happening and fixed my Dockerfile overnight as soon as possible.

I'm also aware that an application can be reviewed quickly so it's not that big of an issue for a reviewer at the time of reviewing, but can't speak for other entities that may want to reproduce a build in late future, after an application has been accepted and a signed shim has been in production use for a long time.

[leejun9503]

We updated our repository and fixed issues mentioned by aronowski.
Also updated original issue's link and shim checksums.

https://github.com/NaverCloudPlatform/shim-review/tree/navix-shim-x86_64-20231025

[aronowski]

Thanks for the update!

Things look generally fine and I'll send you verification emails soon.

However, the thing that bothers me is the fact that simply changing the image name to rockylinux:8.6 won't ensure that packages from version 8.6 will be in use. The most recent major version 8 packages are being downloaded, just as earlier. Therefore, while the binaries will be reproducible for some time, the process will break some time in the future - maybe even once version 8.9 gets a stable release.

I'll leave the bug label for now until this is resolved - if I was to remove it now, someone would soon report that the build does not reproduce and I'd have to add it again.

If you need help with enforcing the long-term stability of the buildroot, feel free to ask here.

[aronowski]

Verification emails sent.

[leejun9503]

One workaround is to hardcode references to repos from an older release, e.g. 8.7. If you want, you can base yours on mine, though keep in mind to adapt it to version 8 rather than 9.

Now I understand what you said. We edited our Dockerfile to reference only Rocky 8.6 vault repository so the build can be reproducible as time passes.
Again, we updated issues with new release and shim checksums to reflect these changes.

https://github.com/NaverCloudPlatform/shim-review/tree/navix-shim-x86_64-20231026

And here is our verification words.

JunYeong Lee

hayloft fluorescence astrologers tartars sullied Delawarean perked seem
retaliates prevented

Hwaseop Keum

Candice husbanding vinyl middle Oceania surprisings faddish overpopulate
chiaroscuro transliterates

[aronowski]

The words match the ones I sent - contacts have been verified!

---

Now I understand what you said. We edited our Dockerfile to reference only Rocky 8.6 vault repository so the build can be reproducible as time passes.

Oh... :-(
Next time tell me upfront to rewrite my answers with simpler words or to clarify things.

The binaries are reproducible. The solution with using the rocky-86.repo file is much more elegant than the one I suggested. Great job!

---

The review looks alright at the first glance!

[SherifNagy]

While I am not an official reviewer, here are my comments "looking at latest tag: https://github.com/NaverCloudPlatform/shim-review/tree/navix-shim-x86_64-20231026 ":

• SHIM sources within the SRPM matches the release hash
• SHIM has two patches, one to enable NX by default patch "the full stack doesn't support NX yet" the other buggy-binutils as suggested by @aronowski
• SHIM's CA valid for 10 years
• SHIM binary reproducible correctly
• Security contacts has been validated by @aronowski
• Contacts GPG keys looks good
• CA and certs protection story looks good
• Kernel patches and lockdown configurations looks good "Kernel is RHEL"
• Grub modules looks good, matches RHEL "source is borrowed from RHEL"
• SBAT entries looks good, still need confirmation about fwupd-efi SBAT entry

Now, For fwupd-efi SBAT entry, I had same comment on #339 and I got two answers regarding preserving upstream / RHEL SBAT entry:

• Probably yes
• The distros won't benefit from it as much as preserving in case of grub2

So I guess we can leave this up to the distros? still need confirmation about this.

Regarding NX, MSFT just released a exception for NX support https://techcommunity.microsoft.com/t5/hardware-dev-center/nx-exception-for-shim-community/ba-p/3976522#M147 which means, I don't think they will sign a SHIM that has NX flag enabled while the rest of the stack isn't "that's just my guess, could be totally wrong" but you may disable the NX comparability flag and submit for signing availing this exception, also need someone else to confirm this

I think we need one more official reviewer to look at this submission, other than this, best of luck with the submission :)

[leejun9503]

Do I have to update shim to 15.8 and resubmit the issue with updated shim-review template?

[aronowski]

I myself don't mind either updating this issue or opening a new one.

If you open a new one, you can link to this one regarding contact verification and ask me to re-review the parts that changed.

[THS-on]

I see that a new submission for 15.8 was created, closing.