search

rhboot / shim-review

Reviews of shim
66 stars 131 forks source link

Shim 15.8 for TeraByte #369

Closed TBOpen closed 7 months ago

TBOpen commented 9 months ago

Updated Feb 23, 2024:

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/TBOpen/shim-review/releases/tag/TeraByte-Shim15.8-x64-20240310

What is the SHA256 hash of your final SHIM binary?

02c4bf81a5f359213d80ec365366d8be35b02bf84e522802c5fc8ee8694c8e05 *shimx64.efi

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

https://github.com/TBOpen/shim-review/releases/tag/TeraByte-Shim15.4-x64-20210510

TBOpen commented 8 months ago

Just waiting patiently, while customers disable secure boot on their systems. FWIW, the latest Gpg4win outlook plugin no longer crashes outlook so it's in place and also added the corp email to the newer openpgp key servers rather than our really old ones on the MIT server.

aronowski commented 8 months ago

Reviewing.

[...]
- PGP key fingerprint:  No longer have an up to date one.
Anyway, looks like I still have support@terabyteunlimited.com in the PGP database.

It will be required to receive the signed binary. The latest comment says:

added the corp email to the newer openpgp key servers rather than our really old ones on the MIT server

I found one that sounds just like this:

$ gpg --list-keys corp@terabyteunlimited.com
pub   ed25519 2024-02-04 [SC] [expires: 2027-02-04]
      2891C706D99AB4B2ECF5D1BE64B1B1216F7DC24B
uid           [ unknown] TeraByte Corporate <corp@terabyteunlimited.com>
sub   cv25519 2024-02-04 [E] [expires: 2027-02-04]

Is this one the correct one? Should I look elsewhere for another one?
Please, paste the correct fingerprint in the application and ping me back. I'll perform the verification process.

*******************************************************************************
### If shim is loading GRUB2 bootloader and your previously released shim booted a version of GRUB2 affected by any of the CVEs in the July 2020, the March 2021, the June 7th 2022, the November 15th 2022, or 3rd of October 2023 GRUB2 CVE list, have fixes for all these CVEs been applied?
[...]
*******************************************************************************
The old grub version will not work with the new shim.  New certificate was created.

I don't understand how the answer relates to the question - however, I can manually check the package mentioned in the question above that one, that is grub2-unsigned-2.12~rc1-10ubuntu4.

*******************************************************************************
### Were old shims hashes provided to Microsoft for verification and to be added to future DBX updates?
### Does your new chain of trust disallow booting old GRUB2 builds affected by the CVEs?
*******************************************************************************
N/A

But I can see you did have a signed shim binary once here. So why is it N/A?

*******************************************************************************
### 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.
*******************************************************************************
yes, each major update gets a new key.  It's automatic on our build system.

An earlier answer shed some more light on the matter, that is:

as mentioned every major kernel update (e.g. 5.14 to 5.15) gets a new key.

However, each kernel rebuild shall use an ephemeral key, not only a new version update.

*******************************************************************************
### Which files in this repo are the logs for your build?
This should include logs for creating the buildroots, applying patches, doing the build, creating the archives, etc.
*******************************************************************************
All done in the Dockerfile.

Please, provide them as part of updating the application - they will be needed to make sure that certain build dependencies have been used, especially since they may change in the meantime, as Fedora 38 is still being updated. More on this at the very bottom of this comment.

******************************************************************************
### Do you add a vendor-specific SBAT entry to the SBAT section in each binary that supports SBAT metadata ( GRUB2, fwupd, fwupdate, systemd-boot, systemd-stub, shim + all child shim binaries )?
### Please provide exact SBAT entries for all SBAT binaries you are booting or planning to boot directly through shim.
### Where your code is only slightly modified from an upstream vendor's, please also preserve their SBAT entries to simplify revocation.
If you are using a downstream implementation of GRUB2 or systemd-boot (e.g.
from Fedora or Debian), please preserve the SBAT entry from those distributions
and only append your own. More information on how SBAT works can be found
[here](https://github.com/rhboot/shim/blob/main/SBAT.md).
*******************************************************************************
Yes, we append our own SBAT entries.

Please, paste them here. You can get them by running the following command:

$ objcopy --only-section .sbat -O binary shimx64.efi /dev/stdout

That's the example for the shim binary. Do so for others that make use of the SBAT mechanism.

*******************************************************************************
### Add any additional information you think we may need to validate this shim.
*******************************************************************************
NOTE: Everytime you build the shim it gets a different SHA256 because the PE
date and time fields.   If you binary compare you can see the differences is
only in the header.

I don't know if other reviewers, as well as Microsoft, are willing to accept such process, but I can play around with analyzing the binaries statically.

However, as a curiosity, Fedora 38 is still being updated until 05/14/2024 , so if the build dependencies change, the binary might become non-reproducible. If possible, prefer using the releases that don't receive updates anymore.

TBOpen commented 8 months ago

I'm not sure how to quote the comment for the comments like you did above, but here are the answers:

1 - Yes, the one you found TeraByte Corporate corp@terabyteunlimited.com is correct, publicly published.

2 - We are moving to a newer GRUB version (that has all fixes in place) and the new shim won't be able to load the old GRUB. So I guess "yes", would be the answer.

3 - It's N/A because we had no bugs in our part of the software being loaded so no need to blacklist it. I presume the SBAT entries (or NX requirement) will handle it (something is already preventing loading on updated systems). Or is it even if SHIM itself needs to be updated, we have to provide old hashes to our builds (I thought that was why SBAT was created - to prevent overloading the database).

4 - We typically only use the new key on updates from say 6.6 to 6.7 but not 6.6.1 to 6.6.2 but we can.

5 - I just figured if you build the binary and it's the same, that's all that is needed. Is there an exact process to pull them out of the docker build (do they exist or do we need to pipe the output somehow). I've seen the updates happen but the binary output is the same. If you tell me exactly what you want I can get it for you.

6 - The shim was provided and figured you would want to look what was in there, but I can post here as well (I extracted this from the .efi): sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim shim.terabyte,1,TeraByte,UEFI shim,2,https://www.terabyteunlimited.com

7 - I don't recall the prior versions doing that, but I noticed the hash was different on each build and I looked in to it, it was the PE header time/date field (plus what looks like a crc of the header). Is there a better fedora number to use, it doesn't matter to me as long as it builds okay. Is Fedora 37 a good one to use? I'll move to that one if that would be better.

Thank You.

TBOpen commented 8 months ago

I just ran a test using Fedora 37 - the nice thing is the binary hash matches each build - this is because the date/time field is 0, whereas the newer compiler/linker in Fedora 38 actually outputs the date/time (although the binary is slightly smaller too). So this board will have to deal with that at some point. I'll just revert to 37 so that I'm not having to hassle with thing because of being on the bleeding edge.

So someone just let me know exactly what log I'm supposed to get and how to get it. I don't see much docker output. If I rebuild it, you see all the installing of fedora items, if I just build where it just recompiles the shim, there is hardly any output. But the binary will match.

TBOpen commented 8 months ago

Okay, I updated the issue - New link and hash provided in original message. I also output the the build capture to the same folder the shim is output to. When you run the docker_make_shim it will overwrite the items in terabyte_shim-15.8_built so you may want to save them away if you want to compare them. I also put a copy of the shim .efi in the root of the submissions (basically copied out of terabyte_shim-15.8_built).

TBOpen commented 8 months ago

The confirmation email can be sent anytime someone is ready...

TBOpen commented 8 months ago

Did you get the updated .zip ?

TBOpen commented 8 months ago

Are you guys checking in to the policy for SBAT vs adding to DBX ? Just wondering why the delay, MS said sometimes they are communicating with you guys behind the scenes.

aronowski commented 8 months ago

I managed to scratch the surface and I'm posting the results as of today.

I still have some work to do with this application, though. I'll answer as much questions as possible too, but that will take some time.

I'm not sure how to quote the comment for the comments like you did above

I copy and paste all the text that I want to reference and format it with Markdown syntax appropriately. Then I paste it as comments

Yes, the one you found TeraByte Corporate corp@terabyteunlimited.com is correct, publicly published.

I'll soon send a verification email and post another comment after it's sent. Once received, you'll need to decrypt it and paste the random words, that I'll send, here as a new comment.

I just ran a test using Fedora 37 - the nice thing is the binary hash matches each build - this is because the date/time field is 0, whereas the newer compiler/linker in Fedora 38 actually outputs the date/time (although the binary is slightly smaller too). So this board will have to deal with that at some point. I'll just revert to 37 so that I'm not having to hassle with thing because of being on the bleeding edge.

Thanks! The build does reproduce now.

aronowski commented 8 months ago

Verification email sent.

TBOpen commented 8 months ago

The words are: rationalism anticyclones rubs workplaces tidiest searchlights tacticians gooiest immanence wicks

aronowski commented 8 months ago

2 - We are moving to a newer GRUB version (that has all fixes in place) and the new shim won't be able to load the old GRUB. So I guess "yes", would be the answer.

Alright! Just so the question about those patches is confirmed as answered positively.

3 - It's N/A because we had no bugs in our part of the software being loaded so no need to blacklist it. I presume the SBAT entries (or NX requirement) will handle it (something is already preventing loading on updated systems). Or is it even if SHIM itself needs to be updated, we have to provide old hashes to our builds (I thought that was why SBAT was created - to prevent overloading the database).

OK - this answer seems valid and I simply expected it in the application (README.md) rather than "N/A".

4 - We typically only use the new key on updates from say 6.6 to 6.7 but not 6.6.1 to 6.6.2 but we can.

If an ephemeral key is used, it shall be used per each build. So the built kernel and its specific modules are bound in a way, so it can't load others that are unrelated (unsigned or signed with an unrelated key).

Please, update the appropriate section in README.md and ping me once the change is there - the application seems OK right now, and once that one section is corrected, I'll add the "extra review wanted" label and one more review will be required - another reviewer might be pinged for extra help; may not be in the committee, as you've already had a signed shim binary in the past.

6 - The shim was provided and figured you would want to look what was in there, but I can post here as well

They're all right there - thank you.

Are you guys checking in to the policy for SBAT vs adding to DBX ? Just wondering why the delay, MS said sometimes they are communicating with you guys behind the scenes.

If something does happen behind the scenes, it naturally stays behind the scenes. ;-)

The delays may be for many reasons, but one of them is that in most cases people volunteer their free time and energy to do the reviews. Usually after long, hard days at work. Right now in my timezone it's 5 AM as I'm writing this comment.
Some have even more responsibilities apart from jobs, like taking care of their families, or any other life situation that needs to be handled.
One way to help is to review other applications and mention curiosities and errors.

Did you get the updated .zip ?

Sorry, which .zip file is it about?

TBOpen commented 8 months ago

Sorry, which .zip file is it about?

I was referring to the the new tag link - which you obviously have.

Please, update the appropriate section in README.md and ping me once the change is there

It's updated, new tag: https://github.com/TBOpen/shim-review/releases/tag/TeraByte-Shim15.8-x64-20240310

Thanks.

aronowski commented 8 months ago

I was referring to the the new tag link - which you obviously have.

Oh, OK.
My flow is a bit different, so I got confused - I simply clone a repository representing the application and work with it directly. Helps with tracking changes across tags and whatnot.

Seems alright now - feel free to ping another reviewer. May or may not be in the committee.

TBOpen commented 8 months ago

Thanks, I don't want to bug anyone, I presume it won't take long for the regulars to check it out ...

TBOpen commented 7 months ago

@SherifNagy @vden-irm @dennis-tseng99 It would be much appreciated if someone could provide the extra review?

dennis-tseng99 commented 7 months ago

Reviewing

=== Review for Shim 15.8 for TeraByte #369 ===

shim.cer is DER format: /shim-review-TeraByte-Shim15.8-x64-20240310/cert> hexdump shim.cer 0000000 8230 b303 8230 9b02 03a0 0102 0202 0d14 0000010 dc75 eee4 7fcc 23f3 50a2 eed0 f742 e6ed 0000020 3358 303c 060d 2a09 4886 f786 010d 0b01 0000030 0005 6930 1e31 1c30 0306 0455 0c03 5415 0000040 7265 4261 7479 2065 4555 4946 5320 2042 ........

ASN.1 Tree is something like this: 30 82 03 b3 ----30 82 02 9b --------a0 03 ------------02 01 02 --------02 14 0d 75 dc e4 ee cc 7f f3 23 a2 50 d0 ee 42 f7 ed e6 58 33 3c

10 years is good. However, NIST deems Certificate RSA 2048 sufficient until 2030, but your last date of the validity is 2034. Suggest you use 4096 bits next time.

TBOpen commented 7 months ago

Thanks.

grub patch is already mainstream to be in next Debian grub 2.12-2 (and grub 2.12 itself) (except of course the title).

shim patch disables fallback, we don't need it or want it, works around compiler warning treated as error issues, and allows providing second stage filename in shim.dat file instead of hard coded or parameter in uefi boot item because that is how we do it.

dennis-tseng99 commented 7 months ago

Understood. Let's accept it.

TBOpen commented 7 months ago

Thanks!

TBOpen commented 7 months ago

Completed.