Shim 15.8 for SonicWall

soniccore-snwl commented 9 months ago

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

[x] completed README.md file with the necessary information
[x] shim.efi to be signed
[x] public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
[x] binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
[x] any extra patches to shim via your own git tree or as files
[x] any extra patches to grub via your own git tree or as files
[x] build logs
[x] 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/sonicwall/shim-review/tree/SonicWall-shim-x86_64-20240412

What is the SHA256 hash of your final SHIM binary?

c39bfbe2c93325bc3de07273308e8fc096e3eae99720945c5ff99c2dc0d8c574

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

N/A

soniccore-snwl commented 9 months ago

Hi - could you confirm if the contact verification has been sent, I do not see anything in my email with "shim-review".

Thanks

THS-on commented 9 months ago

Contact verification will be likely send with the initial review or some time after.

MuthuvelKuppusamy commented 9 months ago

15.7 with recommended patch(0001-Enable-the-NX-compatibility-flag-by-default.patch) only.
Build and sha256sum is good. shimx64.efi : 440f9d7f0922f7c0c75042d18ba774de6ca08c2a91b5a17d5928d03099867657
Contents of section .sbat: d5000 73626174 2c312c53 42415420 56657273 sbat,1,SBAT Vers d5010 696f6e2c 73626174 2c312c68 74747073 ion,sbat,1,https d5020 3a2f2f67 69746875 622e636f 6d2f7268 ://github.com/rh d5030 626f6f74 2f736869 6d2f626c 6f622f6d boot/shim/blob/m d5040 61696e2f 53424154 2e6d640a 7368696d ain/SBAT.md.shim d5050 2c332c55 45464920 7368696d 2c736869 ,3,UEFI shim,shi d5060 6d2c312c 68747470 733a2f2f 67697468 m,1,https://gith d5070 75622e63 6f6d2f72 68626f6f 742f7368 ub.com/rhboot/sh d5080 696d0a73 68696d2e 736f6e69 6377616c im.shim.sonicwal d5090 6c2c312c 536f6e69 6357616c 6c2c7368 l,1,SonicWall,sh d50a0 696d2c31 352e372c 68747470 733a2f2f im,15.7,https:// d50b0 736f6e69 6377616c 6c2e636f 6d0a sonicwall.com.
Contents of section .sbatlevel: 88000 00000000 08000000 22000000 73626174 ........"...sbat 88010 2c312c32 30323230 35323430 300a6772 ,1,2022052400.gr 88020 75622c32 0a007362 61742c31 2c323032 ub,2..sbat,1,202 88030 32313131 3530300a 7368696d 2c320a67 2111500.shim,2.g 88040 7275622c 330a00 rub,3..
Cert.der Validity: Not Before: Nov 3 08:11:26 2023 GMT Not After : Oct 29 08:11:26 2043 GMT

aronowski commented 8 months ago

The review is very well-written! Just one question out of curiosity:

Since I'm not familiar with OpenEmbedded, I assume you just modified the upstream Wind River meta-secure-core layer in your local setup, so the meta-efi-secure-boot/recipes-bsp/grub/grub-efi/sbat.csv file uses grub.4 rather than grub.3 in your build process?

This assumption comes from the fact that I couldn't find any other references apart from this OpenEmbedded Layer Index. In the meantime, I've requested to integrate the number update here.

If you have a different build process, please let me know. I'm here to learn new things as well.

@THS-on, please let me know the status of the verification emails.

THS-on commented 8 months ago

@aronowski I've sent out the emails a minute ago :)

soniccore-snwl commented 8 months ago

fusees Americans crowdfunding collectivism's specialist inefficacy's bubblegum scales traders berry

sonicwall-jma commented 8 months ago

moderation perfumes airbag fabric fog assemble greetings sitcom dragons lisp

sonicwall-jma commented 8 months ago

The review is very well-written! Just one question out of curiosity:

Since I'm not familiar with OpenEmbedded, I assume you just modified the upstream Wind River meta-secure-core layer in your local setup, so the meta-efi-secure-boot/recipes-bsp/grub/grub-efi/sbat.csv file uses grub.4 rather than grub.3 in your build process?

This assumption comes from the fact that I couldn't find any other references apart from this OpenEmbedded Layer Index. In the meantime, I've requested to integrate the number update here.

If you have a different build process, please let me know. I'm here to learn new things as well.

@THS-on, please let me know the status of the verification emails.

Thank you @aronowski for your review.

We have a different build process. We extended the grub-efi_2.04.bb recipe from upstream openembedded-core (not from upstream Wind River meta-secure-core) with the following commits from upstream grub

util/mkimage: Remove unused code to add BSS section https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=d52f78def1b9c4f435fdbf6b24fd899208580c76

util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32() https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=1710452aca05ccdd21e74390ec08c63fdf0ee10a

util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=ae8936f9c375e1a38129e85a1b5d573fb451f288

util/mkimage: Unify more of the PE32 and PE32+ header set-up https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=a4e8936f010a8e928e973b80390c8f83ad6b8000

util/mkimage: Reorder PE optional header fields set-up https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=ba44c87e56a8bccde235ebb7d41d5aa54604d241

util/mkimage: Improve data_size value calculation https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=ff406eff25465932b97a2857ee5a75fd0957e9b9

util/mkimage: Refactor section setup to use a helper https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=f60ba9e5945892e835e53f0619406d96002f7f70

util/mkimage: Add an option to import SBAT metadata into a .sbat section https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=b11547137703bbc642114a816233a5b6fed61b06

grub-install-common: Add --sbat option https://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=bb51ee2b49fbda0f66c1fa580a33442ff578f110

And then specified the sonicwall grub sbat file during the build process.

THS-on commented 8 months ago

@aronowski contact verification was successful.

THS-on commented 6 months ago

Please update either this submission for 15.8 or create a new submission.

sonicwall-jma commented 6 months ago

Thanks for the notification. We are working on it and will update this submission.

soniccore-snwl commented 6 months ago

Shim review has been updated to 15.8 @THS-on

soniccore-snwl commented 5 months ago

Hi

Could someone help us understand why is it taking so long to get our SHIM reviewed, we see others who submitted a SHIM for review as a "new vendor" were completed before our request. Are we missing some part of the process?

Thanks

aronowski commented 5 months ago

@soniccore-snwl, thank you for raising this issue. Appreciate it.

I suppose many factors may partake in the delays - not even taking into account that most people volunteer their free time after working their jobs for a living, but in case of this application, maybe few people in the shim-review environment feel competent to learn a new framework to verify things thoroughly.

I'm not a low-level or embedded developer, but see that it took me about 19 days to only scratch the surface on how the environment related to OpenEmbedded works.

And I'm not even counting even those applications, where a mention of lesser known intermediary (second-stage) bootloaders is provided - in such cases people wait way longer.

While I can't clone myself and start reviewing this application from scratch, I think it's possible for the SonicWall team to help with reviewing other applications, making things easier for other reviewers, and having a promotion to an official reviewer in the future.

soniccore-snwl commented 4 months ago

Hi @aronowski thanks for working on our submission. We have also worked on reviewing submission https://github.com/rhboot/shim-review/issues/403

aronowski commented 4 months ago

@soniccore-snwl, thank you!

Any volunteers for this application (may not be in the committee)?

NeilHanlon commented 4 months ago

n.b., I am a volunteer reviewer and not part of the committee.

Review of SonicWall-shim-x86_64-20240223

This is a new vendor.
Contact verification was successful (above).
PGP keys are valid with 2025 expiration dates, but do not have cross-signatures (including cross signing) (at least, on the ubuntu keyservers).
Keys are stored in an HSM with restricted physical access.

Shim

Latest 15.8 source is used
SBAT entries look appropriate including (new) vendor entry.
NX Flag is not set as the chain is not ready yet
CA Certificate is self signed and valid for 10 years. 2048-bit RSA.
I am unable to reproduce the shim at this time using the current docker file:

STEP 11/18: RUN tar -xf "/work/${SHIM_PKG_FILE}"
--> 02ca86fec9e8
STEP 12/18: RUN if test -d /work/patches; then find /work/patches -type f -name '*.patch' -print0 | sort --zero-terminated | xargs -0 -I{} -- patch -p1 --directory="/work/shim-${SHIM_VER}" --input={}; fi
patching file BUILDING
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file BUILDING.rej
patching file Make.defaults
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file Make.defaults.rej
patching file Makefile
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file Makefile.rej
patching file post-process-pe.c
Error: building at STEP "RUN if test -d /work/patches; then find /work/patches -type f -name '*.patch' -print0 | sort --zero-terminated | xargs -0 -I{} -- patch -p1 --directory="/work/shim-${SHIM_VER}" --input={}; fi": while running runtime: exit status 123

Need info from Vendor

GRUB2

SBAT looks appropriate given the provenance
module list is reasonable
NTFS patches applied; SBAT set to 4.

Kernel

Ephemeral keys are used
Lockdown patches are applied or not applicable due to config disablement.
Kernel source is from yocto project and should be fine. No additional patches.

NeilHanlon commented 4 months ago

n.b. i tried to repro this while at the airport, will try again later today with a stable and trusted internet connection.

dennis-tseng99 commented 4 months ago

Thank @aronowski and @NeilHanlon.

@soniccore-snwl I'm sorry to say I'm also failed to reproduce the codes based on the tag SonicWall-shim-x86_64-20240223.

Besides, I saw a typo in the 1st line of Dockerfile saying FROM ubuntu:jammy-20230816. I think it should be jammy-2024 or something like that.

Also, I guess your patch is used to enable NX flag which is not what we expected. And the line number in this patch conflicts with Make.default and Makefile files of shim-15.8. This is why build is failed. Thanks.

shim-15.8.tar.bz2: OK
--> c10662e6e39
STEP 11/18: RUN tar -xf "/work/${SHIM_PKG_FILE}"
--> 8f571f0a2c6
STEP 12/18: RUN if test -d /work/patches; then find /work/patches -type f -name '*.patch' -print0 | sort --zero-terminated | xargs -0 -I{} -- patch -p1 --directory="/work/shim-${SHIM_VER}" --input={}; fi
patching file BUILDING
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file BUILDING.rej
patching file Make.defaults
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file Make.defaults.rej
patching file Makefile
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file Makefile.rej
patching file post-process-pe.c
Error: error building at STEP "RUN if test -d /work/patches; then find /work/patches -type f -name '*.patch' -print0 | sort --zero-terminated | xargs -0 -I{} -- patch -p1 --directory="/work/shim-${SHIM_VER}" --input={}; fi": error while running runtime: exit status 123

sonicwall-jma commented 4 months ago

Thanks @dennis-tseng99, @NeilHanlon and @aronowski.

We have updated the application in tag SonicWall-shim-x86_64-20240412.

The patch was removed locally but not committed. We have removed it in our latest tag. The sha256sum of the shim binary remains the same as the binary was built with the patch removed locally. Our answer to the question what patches are being applied and why also remains the same.

As for ubuntu:jammy-20230816, we are trying to pin the base image to a specific version, which we know it'll work, to prevent "bit-rot". We do the same to the gcc package as well. If it's required we can update the base image to the latest jammy version.

aronowski commented 4 months ago

The build does reproduce now and the checksum matches!

Huge thanks to @dennis-tseng99 and @NeilHanlon for the help. I kindly request further reviews, so we can have the application accepted, as people have been waiting for a long time.

dennis-tseng99 commented 4 months ago

I will take care of it after going back from hospital.

dennis-tseng99 commented 4 months ago

review based on tag SonicWall-shim-x86_64-20240412

code can be reproduced now
NX flag is disabled SectionAlignment 00001000 DllCharacteristics 00000000
Hash is matched Newly built EFI binary hashes c39bfbe2c93325bc3de07273308e8fc096e3eae99720945c5ff99c2dc0d8c574 /work/shim-install-x64/boot/efi/EFI/sonicwall/shimx64.efi Shim review submission EFI binary hashes c39bfbe2c93325bc3de07273308e8fc096e3eae99720945c5ff99c2dc0d8c574 /work/artifacts/shimx64.efi

README.md: c39bfbe2c93325bc3de07273308e8fc096e3eae99720945c5ff99c2dc0d8c574 shimx64.efi

sbat looks good
ephemeral key is used
Certificate Validity: 20 years is a little bit longer for key length=2048 bits only. This is because NIST deems RSA 2048 sufficient until 2030, but your expired year is 2043. Just hope you could improve it next time. Thanks.

/shim-review/work# openssl x509 -inform der -in cert.der -text -noout

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            7a:10:07:1e:ca:38:64:0d:62:bb:8c:53:7b:51:9a:48:20:6e:85:bb
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = US, ST = California, L = Milpitas, O = SonicWall Inc., CN = SonicWall SHIM ID=SCXSHIM_COMMON_000000000000
        Validity
            Not Before: Nov  3 08:11:26 2023 GMT
            Not After : Oct 29 08:11:26 2043 GMT
        Subject: C = US, ST = California, L = Milpitas, O = SonicWall Inc., CN = SonicWall SHIM ID=SCXSHIM_COMMON_000000000000
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (**2048 bit**)
                Modulus:

THS-on commented 2 months ago

@soniccore-snwl did you get a signed shim back?

sonicwall-jma commented 1 month ago

@soniccore-snwl did you get a signed shim back?

Hi @THS-on, we are currently trying to resolve an issue with our signing account. We'll get back to you as soon as we get a signed shim back. Thanks.

rhboot / shim-review