-
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
AmZetta Technologies, LLC shim-15.7 x64 #321
Comments
|
Thank you for the review. I have corrected the SBAT file and recompiled the SHIM and updated it. Please let us know is there are any corrections needed. |
Just scratching the surface and there's one error I noticed right away.
There are some NUL characters, which shouldn't be here. You'll need to port this patch and once that's done, update the shims' checksums. Once that's done, I'll dig deeper into the review. PS: Since the README has to be updated, please format listings properly with three graves; otherwise the render of it on GitHub is harder to read. |
Thank you for the review. I have applied the "Make sbat_var.S parse right with buggy gcc/binutils #535" patch and uploaded the latest binaries. Could you please review the Shim submission and let us know if any changes are needed from our end? |
I'm into reviewing. I'll have to do it carefully, so it will take me some time. In the meantime please update the datetime appropriately in the issue question |
@aronowski Thank you for the review, I have updated the README file and updated the source code tag also. Thank you. |
If you want to help me out with reviewing, please attach a log file of building your GRUB2. I'm interested in particular with what arguments In the meantime, the formatting and datetimes are still not fixed. |
@aronowski Thank you for the review.
Please let me know if any corrections are needed. Thank you very much. |
Tag is OK. Now that we have the script for building GRUB2 and the build log, everything should be clear. Looks good to me. The README still seems broken. If you want, it can stay this way but I'm concerned if the official reviewers will like it this way. Or if you want, I can help reformat it for you (a new tag with a new datetime will then have to be provided). |
I have verified the README.md file format with Can you please help me to reformat the READMD file. I will create the new tag with the reformatted READMD.md file, Thank you so much for the review. |
I've created a patch for you that you can apply this on top of your
Then commit the changes (you can add Then edit this issue so the links point to the latest tag. |
@aronowski Thank you very much. README.md file has been uploaded after applying the patch. New Branch : AmZetta-shim-x86_64-20230510 I hope README.md file format issue has been fixed now. Could you please verify this from your end? |
The formatting looks OK now. Good job! Although I meant commiting with a message like this:
with splitting the message into two lines so it shows up properly but it's a minor thing. The review seems OK now, so let's wait for the official committee review it. Wish you all the best! PS: I pinged you in my own review so the peer review concept would be present. As a token of appreciation, you can review my review. Thanks! |
@aronowski Thank you for the quick and prompt reply, I have corrected the comments in the release. README formatting fixes Co-authored-by: Kamil Aronowski [email protected] Thank you |
@frozencemetery we have been waiting for this SHIM approval for a very long time, Can you please review our shim we were waiting for the shim approval? |
Well, we all have been waiting (@aronowski as well). It would be beneficial if @frozencemetery, @julian-klode or someone else could give us a hint on what's really going on or what to expect, whether we will be reviewed, closed due to a security incident etc. Communication is really needed as probably many commercial projects depend on this process. |
I have just a few questions before doing the full review:
|
You stated that EV certificates are embedded in the shim, but the given amzetta.der (https://github.com/amzdev0401/shim-review/blob/AmZetta-shim-x86_64-20230510/amzetta.der) is self signed. Do you want to submit with this certificate or do you have an EV certificate that you want to use instead?
You stated that no local patches are applied to the Ubuntu kernel that you use. How does your kernel differ from the one that Ubuntu provides (e.g. other build options, different embedded certificates etc.)?
|
@amzdev0401 can you update your submission to reflect that and issue an new tag? |
Could you please elaborate on which section needs to be updated in the submission? |
The following sections:
Please then also include the question about kernel module signing introduced in 1f85d85 |
@THS-on Do you use an ephemeral key for signing kernel modules?If not, please describe how you ensure that one kernel build does not load modules built for another kernel.[your text here] |
Yes please add this question and update the other ones, than create a new tag and update it in the top comment of this issue. |
I have updated the README.md file with the answer and created a new tag with the comment. https://github.com/amzdev0401/shim-review/releases/tag/AmZetta-shim-x86_64-20231011 Thank you |
@ dennis-tseng99 Can you please review our shim too? |
Hi, I have not received my contact verification E-mail. |
@amzdev0401, I'll send the verification emails soon. Looks like the earlier comment did not result in a successful pinging due to an additional space between the '@' character and the username. Furthermore, there's been some changes since I helped you out in the first half of 2023. I'll need to re-review this application, especially considering the latest news regarding NX support, among others. I'll hopefully be able to make it with peace and quiet during the holidays, when I won't be being disturbed. |
Verification emails sent. |
@aronowski Can you please verify the contents From : Loganathan Ranganathan From : Justin Bagby |
As far as I can see, only the answer on ephemeral key has been added. The rest has been clarified in this GitHub issue. OK. 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 here. |
As you suggested, I have made the following changes and created the new release label.
Thank you so much. |
Alright, thank you! However, I spotted an error - the README says that EV certificates are/will be used as being embedded in the shim binary, but the file
Please, change the answer in the |
I have corrected the README.file and recreated the release build (v1.0.4) with the updated file. |
@amzdev0401, thank you. Now, there's also the thing that the If you want, you can locally delete the earlier one, then tag the Once this is done, please ping me. |
I have created the new tag AmZetta-shim-x86_64-20231225 from AmZetta-shim-x86_64-20231224 tag to avoid a commit version mismatch. The new release version is v1.0.5. |
@amzdev0401, the tag is now OK. Thank you. The application now seems alright to me. Please, ping other reviewers from the committee and ask that the application be reviewed by them too. Though I can't guarantee, when another review will be written. I can't speak for other reviewers and their situations - I wrote a proposal on writing a document, which may clarify that this is all volunteer work here, but historically I myself was being less active due to life situations, which resulted in lack of sleep and free time. I wish you that the next review happens sooner than later. |
@aronowski can you please assign someone for extra review, we have been waiting for so long time for this SHIM approval. |
I know that feeling.
Yes, since there was no pinging as I suggested, I'll just do the assignment. However, in the meantime shim 15.8 got released and the application shall be updated, since I got a tip that shims 15.7 won't be accepted by Microsoft anymore - you can reuse this GitHub issue, only updating the binaries, the application document and pushing the new tag - mention me, once this happens, so I'll prioritize this GitHub issue. |
Sorry for losing your message due to being busy for company project. I'll do it asap. But will you still need shim-15.7 rather than shim-15.8 ? Anyway, I'll assume 15.7 is what you need. |
@dennis-tseng99 @aronowski @THS-on Since, 15.7 is not accepted by Microsoft, I will re-submit the 15.8 SHIM in the same GitHub issue as soon as possible. Thank you |
@dennis-tseng99 @aronowski @THS-on I have resubmitted the latest SHIM 15.8. I kindly request you to review this new SHIM submission. |
Superseded by #383, will review that one ASAP. |
Confirm the following are included in your repo, checking each box:
'https://github.com/amzdev0401/shim-review/blob/AmZetta-shim-x86_64-20231225/build.log'
'https://github.com/amzdev0401/shim-review/blob/AmZetta-shim-x86_64-20231225/Dockerfile'
What is the link to your tag in a repo cloned from rhboot/shim-review?
'https://github.com/amzdev0401/shim-review/tree/AmZetta-shim-x86_64-20231225'
'https://github.com/amzdev0401/shim-review/releases/tag/AmZetta-shim-x86_64-20231225'
What is the SHA256 hash of your final SHIM binary?
[0a3a19af7762d418bb325a2b6a08f22f9b6488435f61e3f67f7bd85ed9d0ff3b shimx64.efi]
What is the link to your previous shim review request (if any, otherwise N/A)?
'#280'
The text was updated successfully, but these errors were encountered: