Ctrl IQ, Inc Shim 15.7 for x64 & ia32 #339
Comments
THS-on
added
new vendor
|
I know there are a lot of things going on in the committee and we appreciate all of your hard work. Can we get an update on when our shim would start the process? When can we expect verification challenge and such? Thank you in advance! |
aronowski
added
the
bug
|
Jason, as far as I'm aware, it would be optimal to start the process once the aforementioned binutils-related bug has been fixed. Back then I did not add any label since I didn't have access to such an action, but as the current status is, the application simply wouldn't get accepted.
If there's something that requires help, clarification, porting assistance, etc. from my side, then please, let me know - I'll be happy to make things go faster here for you and your team. |
|
Kamil, Thank you for the guidance! I was able to update the our shim src repo and the reproducible build repo with the buggy binutil patch. I also corrected the check marks as advised. The output to objdump output as well: |
|
Thanks for the update! Moving on with the review. Jason, your public key seems to be valid for less than 2 years. Will that be alright? I mean that once it expires, a new contact verification will be needed. As far as I can see, Michael's public key has expired on September 27, 2023. Therefore, the contact verification can't proceed until this is resolved.
I believe this section should also mention the buggy binutils patch too. I know it was already mentioned earlier, but still. Oh, this is the one section that was historically being misunderstood and for a good reason - it's a bit unclear and confusing. I filed a proposal here in the Improving the document for a review application paragraph to make it more straightforward in the future. In most cases the answer should be: yes, it has been set to 3, because that's what there is in Rocky, am I right? Try this command and see what happens in regard to the entry that mentions Free Software Foundation - is that line the same as in the listing below: Historically the If something changed and I'm not aware of that, then please, point me to the exact location, where I can find this commit being introduced in CIQ's kernel - we're all here to learn, myself included. As far as I can see this is the old checksum that referred to the shim binary with the buggy It needs to be updated to the current one(s). I've confirmed that the reproducibility is right there, and that's a great thing! The Does Private key for CA mean the private part for the CA certificate, which can be found at How is the vault being secured and how the security measures prevent exfiltration through covert channels, or by malware that can bridge air gaps, among others? Are backups stored in a way that prevents destruction of everything in one fell swoop by a fire accident, a rogue employee, etc. - I mean if they are stored in, at the very least, a physically distinct place? Please, paste the exact entries, which your binaries include, right here in this section. They are required for the application to be reviewed. Also, a hint: watch out to provide the entries straight from the binaries, rather than e.g. GRUB2 containing the You can get them by running
I'll check this out thoroughly in the future - please, ping me on this once new changes are introduced to the current application. There were some ongoing discussions in this regard, e.g. if it's possible to I kindly request @mheese to help me out with this question. |
I highly doubt that these problems exist with the RHEL upstream Linux kernels. Essentially, there are two things one needs to confirm with the kernels:
|
|
Concerning our public keys: |
Question about commit eadb2f47a3ced5c64b23b90fd2a3463f63726066After some searching I was able to find CVE-2022-21499. The CVE "kernel: possible to use the debugger to write zero into a location of choice" was applied to RHEL kernels. |
|
@mheese is their a list of secure boot patches? Are the 3 patches in the questionnaire the patches you are referring to? Or are there other patches? Asking so we can verify if we have all the secure boot patches applied to our kernels. Is their a patch for the "kexec" issue? Just trying to see how we can ensure we remedy the issue. |
Unfortunately, not that I am aware of. I really wish there would be. The questionnaire is pretty much just referring to previous vulnerabilities and that they are fixed. I'm going to raise that point with the committee.
No, I just found out this week that the Debian 12 (bookworm) kernel which is based on 6.1.x still has some additional secure boot related patches which are not in upstream yet. I will include them now myself in my own builds - because my kernel is not based on a RHEL or Debian upstream version. In your case, you are most likely okay as you are using a RHEL upstream version. fyi, these patches is what I'm talking about (from http://deb.debian.org/debian/pool/main/l/linux/linux_6.1.55-1.debian.tar.xz):
I know, we really need to come up with a checklist of some sort.
That is pretty much just a kernel configuration setting. You should enable lockdown (one of the patches above actually will enforce it anyways if secure boot is enabled), and require signatures for linux images with kexec (CONFIG_KEXEC_SIG_FORCE if I recall correctly). |
|
We have updated the README.md questionnaire with the requested information. |
|
Thanks for the updates!
As far as I can see, only a public subkey's date got extended. The public key, to which that subkeys is bound to, is still expired. That's good, thanks! The update announcement is OK, but the SBAT entry looks suspicious - more on this in a moment.
Thanks, investigating it. Checksums fixed, OK! Explained in detail, thanks! At least the GRUB2-related SBAT entries seem wrong. Not saying that they are, but I need to dive deeper into this: the However, to give a detailed answer on this, I need to study the history of the changes; what was happening back then and why certain values are used today. This applies at least to the component names and generation numbers. While the I'll leave the bug label until the GPG-related thing gets resolved. In the meantime, please, provide as much information as possible to make my investigation easier. Especially when it comes to the |
|
Thank you for your time and guidance! We will definiately look into the pub key issue. I was looking into the sbat issue and you you can see that the sbat from rocky grub2 repo lists grub.rhel8 I hope that will clear things up |
|
After looking at the sbat more closely I see what you are saying about the grub.rhel8 entry. I will look into remedy the issue. |
|
I am uploading an image of the objcopy output from rocky grub on my test system. You can see that the rhel line lists rocky as well.
|
I wonder what I am missing. I just ran the following inside of a container: Thank you for your help. |
Actually that's a good point and I think it's a good area to improve the documentations abit, as far as I recall, when Rocky did apply for shim the name of RHEL binary was grub.rhel8, we were asked to include it in our SBAT and then it changed later "We didn't update our SBAT entry for grub2 since then but we are aware of this for our next grub release", however if any other vendors are rebuilding packages from Rocky as upstream for some reason, I assume they should have the 3 entries here for grub2 and fwupd:
I think an entry example should be something like this: sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md PS caps letter can and should be changed to match the correct values Even if it just a rebuild, one of those vendors might start carrying different patches in grub or fwupd or whatever EFI binary that might be allowed to load in the future. I can't think of any other reasons to include upstream vendor's binary's other than this and maybe prevents loading vulnerable version of upstream EFI binaries that doesn't match the correct SBAT level for that upstream. Not sure if I can make my point across though, it's too early in the morning here , at least for me 😄 thoughts? |
|
@elguero, the process now works fine for me - confirmed with your listing. Thank you. I'll send verification emails soon. @SherifNagy, thanks for the clarification and the example. I'd opt for keeping up with upstream entires, but I suspect some even more clarifications shall be made in the future. PR #348 is a good starting point. The example has one error - there's no generation number after In regard to the build process of GRUB2, I'd like to clarify what happens. The Therefore, let's keep in mind that the build process will replace upstream versions with downstream versions, which will be incorrect, whenever a downstream distro appends their own disttag. Most Enterprise Linux distros do so to distinguish between packages with downstream branding or other changes. As a hint, I'd suggest basing the implementation on the one by AlmaLinux. See for yourself, how the great folks behind this project did it: |
|
Verification emails sent. |
equiomnipotent |
|
We have been investigating the grub sbat issues @aronowski brought to our attention. The resolution was to update the sbat.csv.in and SPEC file to generate the appropriate versions for the upstream lines. We also added our own ciq line to the sbat. Below you can see our latest output. If the output is correct we will update the questionnaire. sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md |
cotidal flectionless gurglet whauve trinitrate diapir Kaimo bronchocele |
|
The words match the ones I sent - contacts have been verified! @jason-rodri, Almost there! There are some curiosities that I'd like to address, though. For context, let's focus on Rocky's 15.6 application, in particular on the SBAT section. For the Red Hat, Inc. entry I'd preserve the one they currently ship, that is so the whole hierarchy stays faithfully represented. Though for that I'd need to contact folks from RESF, I think. @SherifNagy, please, help me out with this. Now, when it comes to Rocky's and Ctrl IQ's entries: While right here Rocky's entry is fine, at least the Ctrl IQ's generation number is wrong - it should be Unless I'm wrong and it's justified, e.g. with number Furthermore, is there a need to use |
aronowski
removed
the
contact verification needed
|
Lets try this again: Here is the updated sbat from the grub binary We replicated the rhel line to preserve the original grub sbat. |
|
While I am not an official reviewer, here are my comments "after all the back and forth with @aronowski " :) :
I just have couple of questions for clarification, I see in your answers, you mention RHEL / Rocky 8 + 9 kernel version and grub version couple of times, are you planning to release this SHIM for both releases? second question might be more for @aronowski "since I am still learning myself" , since CIQ's is going full secure boot chain away from Rocky, I think the fwupd-efi SBAT entry will require a CIQ entry if I am not mistaken, is that correct? Other thank this, everything looks good to me |
|
For preserving upstream's SBAT entries in fwupd, I got two answers:
So I guess we can leave this up to the distros? still need confirmation about this. Now, MSFT just released a exception for NX support https://techcommunity.microsoft.com/t5/hardware-dev-center/nx-exception-for-shim-community/ba-p/3976522#M147 which means, I don't think they will sign a SHIM that has NX flag enabled while the rest of the stack isn't "that's just my guess" also need someone else to confirm this One question remaining, is about the intention of using this shim review for modified Rocky 8 and 9 or just 8 |
|
Thank you for reviewing our submission! We are very focused on a modified Rocky 8. We'd like to be approved for 9 as well (as a Rocky derivative, similar to 8 ). But if that takes another submission, that's fine - we at CIQ are good either way. We will leave the NX support in for now and see what guidance comes down from Committee or development group. |
|
If we have to generate a shim without NX support should that be done in this review or will need to generate an additional review? |
|
I have update our submission with a shim that does not have the NX patch. I have also updated/removed the references to the NX patch in the README.md Please let me know if this is not the correct protocol. Thank you in advance |
|
Hi @aronowski , Thank you for all the assistance that has been provided on the initial review. Is there anything that we need to provide or do on our side to help get to the next stage of review? Thanks! |
|
@elguero, I believe the best bet is to ping other reviewers from the committee. It's worth to try and I wish people respond sooner than later, but considering how much is going on at the moment, both in professional (the NX support thread) and personal (obligations outside of jobs and the volunteer work) settings, I can't promise this. I tried to analyze things thoroughly and did my best, but can't duplicate myself to start either from scratch or from what already has been posted. For instance, the fact that I'm replying here 5 days after being pinged is not out of malice, but instead because last week I've been getting little to no sleep and got overwhelmed with traveling across the country to clean up certain legal things, therefore volunteer work in these conditions is not optimal, to say the least. Furthermore, considering the latest news, there may soon be the need to remove the NX compatibility patch. For that, replying to @jason-rodri, I think there's no need to open up another application, but instead, simply modify this one. The shim binary checksum will change, but the rest of the review should be fine. Maybe just mention at the very bottom of the application, what's the deal with no NX compatibility - so when people from Microsoft take a look at it, they will immediately know, what's going on. |
@aronowski Thank you for your response, it seems like we have made you our shim review lifeline. You are just one of few, thank you again for all your hard work and dedication. I completely empathize with the issues you listed and wish you the best of luck. Thank you for the suggestion of tagging other reviewers. |
|
We have updated our submission by removing the nx patch from the shim build. The removal of the NX patch was prompted by the lack of NX support from our grub and kernel offerings. The nx patch removal was also triggered by the nx exception issued by Microsoft and shim development group recommendation. @aronowski has reviewed and approved our submission. Can we get a second set of eyes on our submission. |
|
@jason-rodri, I can see the work is being done on the The binary reproduces fine, but there are still the old checksums written in the What is the SHA256 hash of your final SHIM binary? section. Please, update these and let me know, so I can double-check if everything's OK - once it is, simply tag the final revision appropriately and update the GitHub issue initial post. |
|
@aronowski Thank again! I have updated the hashes of the shims in the "What is the SHA256 hash of your final SHIM binary". |
|
@jason-rodri, it looks like only the original post received this update. I can't see the change being introduced to the Bitbucket repository. |
|
@aronowski Thank you again, The hashes documented in the Readme.md and in the issue should match the checksums in the shim_rpmbuild.log now. |
|
Great job, the checksums match! Now, you'll need to wait for another official review, while keeping an eye on the news, in case something like the NX support was to change again. I can't duplicate myself to write another review, but can try my best to answer questions regarding these aforementioned news, in case some arise. |
|
@THS-on @steve-mcintyre @dennis-tseng99 , Happy new year. Can we get an estimate on when we will be assigned an additional reviewer? @aronowski has completed their review and everything seems to be in order. Thank you in advance! |
|
My schedule is currently pretty full. I can likely have a look it after FOSDEM in February. |
|
@THS-on Thank you for the reply. Appreciated all the hard work you contribute to review process! Looking forward to your review inputs. |
|
I want to make sure the review team was not waiting on anything from us, since The question tag was still applied. |
|
Not anymore. ;-) At least to my current understanding, that is. If the requirements change, another one will be added. |
aronowski
added
the
superseded
|
I suppose we can close this one, as there's the updated 15.8 application? |
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://bitbucket.org/ciqinc/ciq-shim-build/src/ctrliq-shim-x64-ia32-20231222/
Repo migrated to
https://github.com/ctrliq/ciq-shim-build/tree/ctrliq-shim-x64-ia32-20231222
What is the SHA256 hash of your final SHIM binary?
25aaf786950ab589de771097a87995a114d11df5aa195a2049ab88894028b28c /shimx64.efi
09841f7081a79084d993deeb3716f44537e5eb42297a0b62f8e43c56768e44ed /shimia32.efi
What is the link to your previous shim review request (if any, otherwise N/A)?
N/A. This is our first submission.
The text was updated successfully, but these errors were encountered: