rhboot / shim-review

Reviews of shim
67 stars 131 forks source link

FixMeStick shim-15.7 x64 and ia32 #312

Closed coreyvelan closed 9 months ago

coreyvelan commented 1 year ago

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


What is the link to your tag in a repo cloned from rhboot/shim-review?


https://github.com/coreyvelan/shim-review/tree/fixmestick-shim-ia32-x64-20230126


What is the SHA256 hash of your final SHIM binary?


b28d0ddfeafb068acc1eb51f496623dc0929da58660b4a3a7b5806b0b28ecbb5 /build/target/shimx64.efi 39c9bf03c09679834c34c25582a4c52d4906c099b65b6b8bf0f14215adeb26ba /build/target/shimia32.efi


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


https://github.com/rhboot/shim-review/issues/276

THS-on commented 1 year ago

Disclaimer: I am not a not an authorized reviewer

Hashes

b28d0ddfeafb068acc1eb51f496623dc0929da58660b4a3a7b5806b0b28ecbb5  /build/target/shimx64.efi
b28d0ddfeafb068acc1eb51f496623dc0929da58660b4a3a7b5806b0b28ecbb5  /build/verify/shimx64.efi
39c9bf03c09679834c34c25582a4c52d4906c099b65b6b8bf0f14215adeb26ba  /build/target/shimia32.efi
39c9bf03c09679834c34c25582a4c52d4906c099b65b6b8bf0f14215adeb26ba  /build/verify/shimia32.efi

SBAT

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.fixmestick,3,FixMeStick Technologies Inc.,shim,15.7-0,mail:security@fixmestick.com

Notes:

coreyvelan commented 1 year ago

@THS-on thank you for the review! I corrected the shim.fixmestick SBAT version to 1 and rebuilt. I also updated the grub.fixmestick SBAT version to 1. Lastly, as for patch 535, in the Dockerfile, please see these lines: ``

For Patch 535 binutils

RUN git checkout 657b2483ca6e9fcf2ad8ac7ee577ff546d24c3aa `` That checkout brings in patch 535. Is that ok?

The new tag is: https://github.com/coreyvelan/shim-review/tree/fixmestick-shim-ia32-x64-20230208 The new shim binaries are : e1dd42b83626c2050587c3233bd094bdf55922ab1bf5f306fa6b1dad6629896f /build/verify/shimx64.efi 21ca841e983028d04cc10ca42f7da720a3abc8a21d0f5c0a87ea9480c4a1d624 /build/target/shimia32.efi

THS-on commented 1 year ago

Can confirm that the SBAT levels are now correct and the new hashes are also reproducible:

e1dd42b83626c2050587c3233bd094bdf55922ab1bf5f306fa6b1dad6629896f  /build/target/shimx64.efi
e1dd42b83626c2050587c3233bd094bdf55922ab1bf5f306fa6b1dad6629896f  /build/verify/shimx64.efi
21ca841e983028d04cc10ca42f7da720a3abc8a21d0f5c0a87ea9480c4a1d624  /build/target/shimia32.efi
21ca841e983028d04cc10ca42f7da720a3abc8a21d0f5c0a87ea9480c4a1d624  /build/verify/shimia32.efi

That checkout brings in patch 535. Is that ok?

I missed that. For me this is fine, but keeping them as patches and in the same format as (proposed to) upstream makes it easier to review.

coreyvelan commented 1 year ago

@THS-on Noted for next time re the patch 535. Is there anything else I need to do before official approval?

THS-on commented 1 year ago

Looking at Debian's Shim patches, you might also want to add the one that is blocking grub.debian < 3 SBAT versions: https://github.com/steve-mcintyre/shim-review/blob/debian-12-shim-amd64-arm64-i386-20230209/patches/block-grub-sbat3-debian.patch If you have not build your GRUB from the buster branch your builds were likely not affected by this.

Otherwise I don't think so, just one of the official reviewers needs to have a look at it.

coreyvelan commented 1 year ago

@THS-on Thanks for pointing that patch out. The only signed grub we have deployed contains this SBAT,

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md 
grub,2,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/ 
grub.debian,1,Debian,grub2,2.06-3,https://tracker.debian.org/pkg/grub2
grub.fixmestick,1,FixMeStick Technologies Inc.,grub2,2.06-3,mail:security@fixmestick.com

which will be revoked by the SBAT_VAR_LATEST_REVOCATIONS before that patch you referenced since the grub version is 2

#define SBAT_VAR_LATEST_REVOCATIONS "shim,2\ngrub,3\n"

So, I think we're good as is.

frozencemetery commented 1 year ago

Please note https://github.com/rhboot/shim-review/issues/307

coreyvelan commented 1 year ago

@frozencemetery I believe I am handling #307. Note the 530.patch that I apply in the Dockerfile. Am I doing something incorrectly or did you simply miss that patch? Thanks!

coreyvelan commented 1 year ago

@frozencemetery What else is required to do in order for this to be approved? Thanks for your help.

steve-mcintyre commented 1 year ago

Review of FixMeStick shim-15.7 x64 and ia32 fixmestick-shim-ia32-x64-20230208

OK

Issues / queries / outstanding

steve-mcintyre commented 1 year ago

@THS-on thanks for your help reviewing here :-)

SherifNagy commented 11 months ago

If you got the signed shim back from Microsoft, can you close this ticket?

coreyvelan commented 9 months ago

Closing as we got the signed binaries from Microsoft.