-
Notifications
You must be signed in to change notification settings - Fork 131
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.7 for Adaptech s.r.o. #335
Comments
I am not an official reviewer. I just want to help and learn.
shim-review# openssl x509 -in Adaptech.cer -inform der -noout -text
|
Good job with the review! The technicalities have already been provided by @dennis-tseng99, but I'd like to point out, how well this review has been written. For starters, the company seems to have a deep understanding of how SBAT works. Everything, that is, the decision behind using The company proves its experience working with source code and porting patches. I primarily focus on the company's GRUB2 entry, where Gentoo is used for building and the required assets are provided in the For now only shim needs to be NX-compatible, but there's already an NX-compatible kernel provided, as well as the description of hard work being done on making GRUB2 NX-compatible. To sum it up, there has been more work done than enough! The only thing I'm worried a bit is the certificate, which expires soon, but I believe this is due to the company using an external Certificate Authority rather than generating its own. Again, good job and wish you the review goes well! |
Unfortunately I cannot reproduce the shim using the Dockerfile (switched the hexdump and sha256sum steps):
This is likely because of an compiler update. Regarding the EV certificate being expired: for the shim this still should be fine because time checks are disabled (https://github.com/rhboot/shim/blob/7ba7440c49d32f911fb9e1c213307947a777085d/Cryptlib/Pk/CryptPkcs7Verify.c#L952), but for the submission to MS you'll need an valid certificate. |
Hello @THS-on, all should be OK, first post updated. Many enhancements and fixes done including ephemeral kernel keys, our brand new EV code signing certificate and much more. Everything should be reproducible and much better organized now. |
I'll try to do a full review later this week, but here already some notes:
|
@rehakp To skip the error, I modified Dockerfile, and found your build result shimx64.efi is different from the original one:
|
To answer comments above:
|
Hello @THS-on and @dennis-tseng99, |
@rehakp Sorry for the late response due to busy project running.
I will continue reviewing the rest part. Thanks for your patience. |
===== Review for Shim 15.7 for Adaptech s.r.o. #335 ======
This release is acceptable to me, but I would like to invite @aronowski, @THS-on or other experts for help to speed up this reviewing, although reviewing job will reduce our time on our own project. |
Alright, self-assigning this one. I'll try my best and do this ASAP, despite already having some delays with older applications due to some personal issues I've been dealing with for some time, that take lots of time and energy. Let's see who can handle this application first and once it's reviewed, most likely label it as accepted (we've reviewed it back then and I wrote, how well this application has been written). |
There seems to be some confusion on the NX-related requirements and exceptions, but since we've been accepting applications that have the bit enabled, while the whole chain is not yet ready, I think we should continue on, unless an official announcement from Microsoft is there to prove that we should be doing otherwise. I've checked the application and there's just one error - since the GRUB2 global generation number has been bumped from And once that's done and no new requirements are made public, I'm tagging the application as accepted, so we can celebrate at last! |
Hi @aronowski,
1st comment updated again. |
Awesome! |
Closing the issue as we have just received signed Shim from Microsoft. |
Closing the issue as we have received signed Shim from Microsoft. |
Confirm the following are included in your repo, checking each box:
logs/
subdirectoryWhat is the link to your tag in a repo cloned from rhboot/shim-review?
https://github.com/rehakp/shim-review/tree/adaptech-shim-x86_64-20231127
What is the SHA256 hash of your final SHIM binary?
170bb326192ac637137df71a0fa1916e0a243308cb0bdfc66ff285580282bd4c
What is the link to your previous shim review request (if any, otherwise N/A)?
#248
The text was updated successfully, but these errors were encountered: