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

FixMeStick shim-15.7 x64 and ia32 #312

Closed
8 tasks done
coreyvelan opened this issue Jan 27, 2023 · 13 comments
Closed
8 tasks done

FixMeStick shim-15.7 x64 and ia32 #312

coreyvelan opened this issue Jan 27, 2023 · 13 comments
Labels
accepted Submission is ready for sysdev

Comments

@coreyvelan
Copy link

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


#276

@THS-on
Copy link
Collaborator

THS-on commented Feb 8, 2023

Disclaimer: I am not a not an authorized reviewer

  • FixMeStick last signed Shim is 15.6
  • They require a custom Shim to load custom kernel modules for enabling file system support without disabling Secure Boot
  • Security contacts have not changed since last review
  • Shim is reproducible using the Dockefile

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:[email protected]
  • Upstream 15.7 is used with NX patches applied

  • Embedded certificate matches the organization (has not changed since last review)

    • Serial: 5a:03:3f:9e:f9:12:b2:ab:ff:73:0e:f3:5c:69:84:38
    • Subject: Subject: serialNumber = 740160296, jurisdictionC = CA, businessCategory = Private Organization, C = CA, ST = Quebec, O = FixMeStick Technologies Inc., CN = FixMeStick Technologies Inc
    • Valid till Jul 4 23:59:59 2025 GMT (3 years)
    • Certificate is an EV certificate and code signing attribute is set
  • The key is stored on a FIPS-140-2 token

  • Shim launches only GRUB

    • GRUB is based on Debians implementation (2.06-7)
    • SBAT level is set to 4 and includes the November CVE patches
    • GRUB modules list is the one from Debian + ntfs3
  • Kernel is based on the Debian's Linux 6.1.4-1

    • Includes lockdown patches
    • Has no local patches applied

Notes:

  • Review mentions the 535 patch, but this one is not applied. Is that intended?
  • The SBAT version of shim.fixmestick is set to 3, if there was no vulnerability specific to your Shim it can be still set to 1. Same goes for GRUB only grub.debian needs to be at 4 and the upstream version at 3.

@coreyvelan
Copy link
Author

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

THS-on commented Feb 8, 2023

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

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

@THS-on
Copy link
Collaborator

THS-on commented Feb 9, 2023

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

@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:[email protected]

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

Please note #307

@frozencemetery frozencemetery added the bug Problem with the review that must be fixed before it will be accepted label Feb 16, 2023
@coreyvelan
Copy link
Author

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

@frozencemetery frozencemetery removed the bug Problem with the review that must be fixed before it will be accepted label Feb 21, 2023
@coreyvelan
Copy link
Author

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

@steve-mcintyre
Copy link
Collaborator

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

OK

  • Shim builds reproduce fine for ia32 and x64
  • Revocation story sounds OK, using VENDOR_DBX_FILE
  • Shim from 15.7 upstream, with 2 patches applied from upstream:
    • 657b2483ca6e9fcf2ad8ac7ee577ff546d24c3aa (gcc/binutils fix)
    • 7c7642530fab73facaf3eac233cfbce29e10b0ef (force NX)
      and one more patch from the issue tracker:
    • #531 (Add validation function for Microsoft signing)
  • SBAT data looks fine for both GRUB and shim
  • GRUB looks ok, borrowed from Debian
  • List of grub modules looks OK for now
  • Kernel lockdown sounds ok
  • Contact verification already performed previously in FixMeStick shim-15.6 x64 and ia32 #276
  • Embedded Sectigo codesigning cert lasts until July 2025, OK
  • Controlled in an HSM

Issues / queries / outstanding

  • Nothing, all good!

@steve-mcintyre steve-mcintyre added the accepted Submission is ready for sysdev label Sep 11, 2023
@steve-mcintyre
Copy link
Collaborator

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

@SherifNagy
Copy link
Collaborator

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

@coreyvelan
Copy link
Author

Closing as we got the signed binaries from Microsoft.

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

5 participants