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 SonicWall #352

Open
8 tasks done
soniccore-snwl opened this issue Nov 10, 2023 · 26 comments
Open
8 tasks done

Shim 15.8 for SonicWall #352

soniccore-snwl opened this issue Nov 10, 2023 · 26 comments
Assignees
Labels
accepted Submission is ready for sysdev

Comments

@soniccore-snwl
Copy link

soniccore-snwl commented Nov 10, 2023

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/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

@THS-on THS-on added new vendor This is a new vendor contact verification needed Contact verification is needed for this review labels Nov 10, 2023
@soniccore-snwl
Copy link
Author

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

THS-on commented Nov 15, 2023

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

@MuthuvelKuppusamy
Copy link

  1. 15.7 with recommended patch(0001-Enable-the-NX-compatibility-flag-by-default.patch) only.

  2. Build and sha256sum is good.
    shimx64.efi : 440f9d7f0922f7c0c75042d18ba774de6ca08c2a91b5a17d5928d03099867657

  3. 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.

  4. 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..

  5. Cert.der Validity:
    Not Before: Nov 3 08:11:26 2023 GMT
    Not After : Oct 29 08:11:26 2043 GMT

@aronowski
Copy link
Collaborator

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.

@aronowski aronowski added the extra review wanted Initial review(s) look good, another review desired label Nov 29, 2023
@THS-on
Copy link
Collaborator

THS-on commented Nov 29, 2023

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

@soniccore-snwl
Copy link
Author

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

@sonicwall-jma
Copy link

moderation
perfumes
airbag
fabric
fog
assemble
greetings
sitcom
dragons
lisp

@sonicwall-jma
Copy link

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

THS-on commented Nov 30, 2023

@aronowski contact verification was successful.

@THS-on THS-on removed the contact verification needed Contact verification is needed for this review label Nov 30, 2023
@THS-on
Copy link
Collaborator

THS-on commented Feb 20, 2024

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

@sonicwall-jma
Copy link

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

@soniccore-snwl soniccore-snwl changed the title Shim 15.7 for SonicWall Shim 15.8 for SonicWall Feb 23, 2024
@soniccore-snwl
Copy link
Author

Shim review has been updated to 15.8 @THS-on

@soniccore-snwl
Copy link
Author

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

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

Hi @aronowski thanks for working on our submission. We have also worked on reviewing submission #403

@aronowski
Copy link
Collaborator

@soniccore-snwl, thank you!

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

@NeilHanlon
Copy link

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

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

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

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

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

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

@dennis-tseng99
Copy link
Collaborator

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:

@dennis-tseng99 dennis-tseng99 added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired new vendor This is a new vendor labels Apr 12, 2024
@aronowski aronowski removed their assignment Apr 16, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jun 17, 2024

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

@sonicwall-jma
Copy link

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

@THS-on
Copy link
Collaborator

THS-on commented Sep 11, 2024

@soniccore-snwl has your issue been solved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Submission is ready for sysdev
Projects
None yet
Development

No branches or pull requests

7 participants