shim-15.7 for Isoo (20231226)

[haobinnan]

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/haobinnan/shim-review/tree/isoo-shim-20231226

---

What is the SHA256 hash of your final SHIM binary?

---

shimia32.efi.sha256sum: 7e6817db37778605193b8514661fd313242f91d3e14dfcd9c198839ef842df47
shimx64.efi.sha256sum: 39e41b55268c6409e5c95fbc551625c6f6e83555a2f7eeaf76088c330ff9df01

---

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

---

#246

[aronowski]

Hao, as far as I can see, your shim binaries have no NX support, which is mandatory; their DLL Characteristics value is set to 0.

Here I describe two ways of solving this. Please integrate the necessary changes, and I'll perform a further review.

[haobinnan]

Hi, thank you for your review! I have added the NX patch (0001-NX-compatibility-default.patch) and recompiled it, could you please review it again? Thank you!

@aronowski

[evilteq]

I was able to run the docker without any problems, the checksums match.

Certificate is valid for 30 years. 4k bits strong. Flags are a bit different than what I'm used to, probably using a different script or way to generate it, but I cannot see anything wrong.

objdump says now the NX_COMPAT bit is set.
Everything is as mainline as it can get, the only patch being the NX one. Grub is also taken directly from the ubuntu one.

Not much info about kernel other than version number.

Several non-relevant comments:

• I don't think you have to clone the shim-review again inside the docker build, you already have if you are building it.
• Quite "exotic" comment format :P
• The line to decide the number of parallel make jobs made me laugh :D Make="make -j$(cat /proc/cpuinfo | grep "cpu cores" | wc -l)" I mean, it works and it indeed produces the same output as the more traditional nproc --all, but I think something else was meant.

[haobinnan]

Can the submission be reviewed
@aronowski

[aronowski]

Thanks for the ping and the reminder. 👍

Sometime about the last week I did scratch the surface, but checking things out thoroughly proven to be just difficult. I'll try my best to post a comment this Friday or next Monday with as much information as I can figure out.

[aronowski]

Binaries are reproducible, checksums match.

.sbat and .sbatlevel sections look alright (checked also for trailing newline).

shim.isoo generation number 1 is OK as upstream entry got bumped since the last review took place.

---

PGP keys remain unchanged since the last review, which got accepted. Assuming everything's OK.

---

GRUB2 generation numbers are correct - verified with Ubuntu Jammy's one: http://archive.ubuntu.com/ubuntu/pool/main/g/grub2-signed/grub-efi-amd64-signed_1.187.6+2.06-2ubuntu14.4_amd64.deb.

---

GRUB2 modules remain unchanged since the last review, which got accepted. Assuming everything's OK.

---

LGTM, great job!

[haobinnan]

@steve-mcintyre
Can the submission be reviewed

[haobinnan]

Can the submission be reviewed

[aronowski]

Considering that there should be at least one positive review from an official reviewer apart from me, as I already posted one, I'll ping people for help here.

@THS-on
@dennis-tseng99

[dennis-tseng99]

===== review for shim-15.7 for Isoo (20230824-2) #338 =====

• Binary codes are reproducible:

=================== sha256sum ===================
16040e1ec64d3141f1d4f822567efe21e42a8198f470f6c24ab5e15059249d68  shim_result/shimia32.efi
bf29426f474c1dc6596bb4da02b05a8360c8b3a5d8449f023cca2eec5bacf6b1  shim_result/shimx64.efi
=================== sha256sum ===================
Complete.
Removing intermediate container 0c3732f028d1
 ---> 290fc836ca57
Successfully built 290fc836ca57

• extra patch:
  There is only one extra patch for NX flag setting which is good.

• NX flag is enable, 4K-Alignment is set

SectionAlignment        00001000
DllCharacteristics      00000100

000000b0  00 00 00 00 00 00 00 00  00 10 00 00 00 02 00 00  |................| <-- SectAlig=0x00001000=4096 (4K alignment)
000000c0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
000000d0  00 10 0d 00 00 04 00 00  41 88 0e 00 0a 00 00 01  |........A.......| <-- DllCharacteristics=0x0100

• sbatlevel has no binutil extra NUL character problem

objdump -j .sbatlevel -s shimx64.efi

.sbatlevel section：
 84000 00000000 08000000 22000000 73626174  ........"...sbat
 84010 2c312c32 30323230 35323430 300a6772  ,1,2022052400.gr
 84020 75622c32 0a007362 61742c31 2c323032  ub,2..sbat,1,202
 84030 32313131 3530300a 7368696d 2c320a67  2111500.shim,2.g
 84040 7275622c 330a00                      rub,3..
 
 sbat,1,2022052400
 grub,2
 sbat,1,2022111500
 shim,2
 grub,3

objdump -s -j .sbat shimx64.efi

.sbat section：
 d0000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 d0010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 d0020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 d0030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 d0040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 d0050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 d0060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 d0070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 d0080 696d0a73 68696d2e 69736f6f 2c312c49  im.shim.isoo,1,I
 d0090 736f6f2c 7368696d 2c31352e 372c6874  soo,shim,15.7,ht
 d00a0 7470733a 2f2f7777 772e6973 6f6f2e63  tps://www.isoo.c
 d00b0 6f6d2f0a                             om/.
 
 sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
 shim,3,UEFI shim,shim,1,https://github.com/rhboot/shim
 shim.isoo,1,Isoo,shim,15.7,https://www.isoo.com/ <----------

In sbat section, may I suggest you append more subtree to specify the contact of web-site ? e.g. shim.isoo,1,Isoo,shim,15.7,https://www.isoo.com/xxxxx/shim
Same suggestion for grub2.

• Certificate Validity: 30 years is ok

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            18:14:56:86:42:3d:7c:f2:f0:b5:da:98:00:9c:69:7a:a5:3a:d5:6c
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = CN, ST = Hebei, L = Qinhuangdao, O = "Isoo Software Development Co., Ltd.", CN = "Isoo Software Development Co., Ltd. CA"
        Validity
            Not Before: Jun 13 06:09:33 2022 GMT
            Not After : Jun 12 06:09:33 2052 GMT
        Subject: C = CN, ST = Hebei, L = Qinhuangdao, O = "Isoo Software Development Co., Ltd.", CN = "Isoo Software Development Co., Ltd. CA"

• Conclusion:
  -- This release has been reviewed by aronowski and evilteq successfully.
  -- Minor enhancement needed for sbat section of shim and grub2.
  -- Others are acceptable to me. After a couple of days, if no other expert's comments, I might put "accepted" tag on it.

[aronowski]

@haobinnan, it's been over 2 weeks since the last positive review, but no "Accepted" label being attached, as @dennis-tseng99 said:

After a couple of days, if no other expert's comments, I might put "accepted" tag on it.

The application is great, but just recently the NX requirements have changed and most likely you'll need to remove the NX-compatibility patch for Microsoft to sign your binaries - we've had a discussion to make this venue more user-friendly here - I suggested some hints there as well, to prevent confusion.

Please, remove the NX support patch, recompile the shim binary, update the checksums in the README and in this thread's opening post, push the changes and ping me - I'll then review this application with high priority, as you've been waiting very long for the acceptance.

[dennis-tseng99]

Hi @haobinnan, would you please also enhance the 2nd item like I said in my conclusion. Many thanks.
Thank @aronowski's help.

[haobinnan]

@aronowski

[aronowski]

• Binaries are reproducible.
• No NX compatibility bit set, as the whole chain is not yet NX compatible, according to the latest requirements.
• README updated to the latest version, new questions have been answered and earlier ones updated

---

While the request

In sbat section, may I suggest you append more subtree to specify the contact of web-site ?

has not been realized, I think of it just as a suggestion, since historically we had applications labeled as accepted, when there was a similar implementation as it is right now (only the main website specified as the contact) - like here. Therefore, I think it's safe to accept it.

---

@dennis-tseng99, please write, what you think about it - is there's really the necessity to recompile the binary and to review the application again.
If there's no activity for the next few days, I'm accepting this one.

[dennis-tseng99]

Agree. Like I said above, it is just a minor enhancement. I would accept this one. But, I still want to list some examples for your next release:

shim.itrenew,1,ITrenew Inc.,shim,15.7,mail:[email protected]
shim.lux,1,LUX,shim,15.7,mail:[email protected]
shim.ipxe,1,iPXE,shim,1,https://github.com/ipxe/shim
grub.fedora,1,The Fedora Project,grub2,2.06-29.fc36,https://src.fedoraproject.org/rpms/grub2
grub,1,Free Software Foundation,grub,2.04,https://www.gnu.org/software/grub/
grub.rhel,1,Red Hat Enterprise Linux,grub2,2.02-0.34.el7_2,mail:[email protected]
grub.debian,1,Debian,grub2,2.04-12,https://packages.debian.org/source/sid/grub2
grub.acme,1,Acme Corporation,grub,1.96-8191,https://acme.arpa/packages/grub

I wish each product has a best behavior, and not lose something similar to my miss check in #320

[haobinnan]

Thank you very much for your quick response and review!
@dennis-tseng99
@aronowski

[THS-on]

@haobinnan did you get a signed shim back? If not can you create a new submission for 15.8?