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

Shim 15.8 for TeraByte #369

Closed
8 tasks done
TBOpen opened this issue Feb 3, 2024 · 21 comments
Closed
8 tasks done

Shim 15.8 for TeraByte #369

TBOpen opened this issue Feb 3, 2024 · 21 comments
Labels
accepted Submission is ready for sysdev

Comments

@TBOpen
Copy link

TBOpen commented Feb 3, 2024

Updated Feb 23, 2024:

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 ) (Note: N/A)
  • 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 - (NOTE: docker file will generate them.)
  • 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/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
Copy link
Author

TBOpen commented Feb 16, 2024

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 aronowski self-assigned this Feb 23, 2024
@aronowski
Copy link
Collaborator

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.

@aronowski aronowski added question Reviewer(s) waiting on response incomplete This submission is missing required bits contact verification needed Contact verification is needed for this review labels Feb 23, 2024
@TBOpen
Copy link
Author

TBOpen commented Feb 23, 2024

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

TBOpen commented Feb 23, 2024

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

TBOpen commented Feb 23, 2024

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

TBOpen commented Feb 29, 2024

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

@TBOpen
Copy link
Author

TBOpen commented Mar 4, 2024

Did you get the updated .zip ?

@TBOpen
Copy link
Author

TBOpen commented Mar 6, 2024

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

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

Verification email sent.

@aronowski aronowski removed question Reviewer(s) waiting on response incomplete This submission is missing required bits labels Mar 7, 2024
@TBOpen
Copy link
Author

TBOpen commented Mar 7, 2024

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

@aronowski aronowski removed the contact verification needed Contact verification is needed for this review label Mar 8, 2024
@aronowski
Copy link
Collaborator

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?

@aronowski aronowski added the question Reviewer(s) waiting on response label Mar 11, 2024
@TBOpen
Copy link
Author

TBOpen commented Mar 11, 2024

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

aronowski commented Mar 11, 2024

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.

@aronowski aronowski added extra review wanted Initial review(s) look good, another review desired and removed question Reviewer(s) waiting on response labels Mar 11, 2024
@aronowski aronowski removed their assignment Mar 11, 2024
@TBOpen
Copy link
Author

TBOpen commented Mar 11, 2024

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

@TBOpen
Copy link
Author

TBOpen commented Mar 19, 2024

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

@dennis-tseng99
Copy link
Collaborator

Reviewing

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

  • Binaries are producible based on tag TeraByte-Shim15.8-x64-20240310 (ok)
    shim_build.log file is generated.

  • Hash value is matched (ok)
    $ sha256sum shimx64.efi
    02c4bf81a5f359213d80ec365366d8be35b02bf84e522802c5fc8ee8694c8e05 shimx64.efi

  • NX flag is disable: (ok)
    $ objdump -x shimx64.efi | grep -E 'SectionAlignment|DllCharacteristics'
    SectionAlignment 00001000
    DllCharacteristics 00000000

  • shim sbat (ok)
    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

  • There are 2 patches needed to be digged in.


What patches are being applied and why:
shim-15.8.patch
grub-2.12-tbu.patch

  • Certificate Validity: (ok)
    openssl x509 -in shim.cer -inform der -noout -text
    Certificate:
    Data:
    Version: 3 (0x2)
    Serial Number:
    0d:75:dc:e4:ee:cc:7f:f3:23:a2:50:d0:ee:42:f7:ed:e6:58:33:3c
    Signature Algorithm: sha256WithRSAEncryption
    Issuer: CN = TeraByte UEFI SB 2024, O = Terabyte Inc, L = Las Vegas, ST = Nevada, C = US
    Validity
    Not Before: Feb 3 00:55:05 2024 GMT
    Not After : Jan 31 00:55:05 2034 GMT
    Subject: CN = TeraByte UEFI SB 2024, O = Terabyte Inc, L = Las Vegas, ST = Nevada, C = US
    Subject Public Key Info:
    Public Key Algorithm: rsaEncryption
    RSA Public-Key: (2048 bit)

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.

  • Suggestion:
    For the next submission, please specify more details about the contact information for your product when you specify vendor_url field, or just use email instead. For example, you might change to:
    shim.terabyte,1,TeraByte,UEFI shim,2,https://www.terabyteunlimited.com/packages/shim

  • Still digging into your patch files. Any expert's help is welcome.

@TBOpen
Copy link
Author

TBOpen commented Mar 20, 2024

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

Understood. Let's accept it.

@dennis-tseng99 dennis-tseng99 added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired labels Mar 20, 2024
@TBOpen
Copy link
Author

TBOpen commented Mar 20, 2024

Thanks!

@TBOpen
Copy link
Author

TBOpen commented Mar 29, 2024

Completed.

@TBOpen TBOpen closed this as completed Mar 29, 2024
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

3 participants