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.4 for Isoo (2022-02-05) #202

Closed
8 of 9 tasks
haobinnan opened this issue Aug 24, 2021 · 32 comments
Closed
8 of 9 tasks

shim-15.4 for Isoo (2022-02-05) #202

haobinnan opened this issue Aug 24, 2021 · 32 comments
Labels
extra review wanted Initial review(s) look good, another review desired superseded Vendor has added a new review which makes this obsolete

Comments

@haobinnan
Copy link

haobinnan commented Aug 24, 2021

Make sure you have provided the following information:

  • link to your code branch cloned from rhboot/shim-review in the form user/repo@tag
    https://github.com/haobinnan/shim-review/tree/isoo-shim-20220205
  • 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 do vendor_db ( if you use vendor_db and have hashes allow-listed )
  • 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
  • a Dockerfile to reproduce the build of the provided shim EFI binaries
What organization or people are asking to have this signed:
  • Qinhuangdao Yizhishu Software Development Co., Ltd.
  • Isoo is a software developer for data recovery, disk utilities and system backup. https://isoo.com/
  • Managing Director: Hao Binnan
What product or service is this for:
  • This is Isoo’s Linux-based operating system. We are going to develop some function based on the OS, such as resize partition, back up & restore operating system, etc.
Please create your shim binaries starting with the 15.4 shim release tar file:
https://github.com/rhboot/shim/releases/download/15.4/shim-15.4.tar.bz2
This matches https://github.com/rhboot/shim/releases/tag/15.4 and contains
the appropriate gnu-efi source.
Please confirm this as the origin your shim.
  • This is based on shim 15.4
What's the justification that this really does need to be signed for the whole world to be able to boot it:
  • Isoo wants to employ Secure Boot for building a trusted operating system from Shim to GRUB to the kernel to signed filesystem partitions. Secure Boot is the first step for this.
  • Isoo would like customers to be able to run Isoo’s Linux-based system on any amd64(64Bit) and x86(32Bit) device without disabling Secure Boot.
How do you manage and protect the keys used in your SHIM?
  • They're in an HSM
Do you use EV certificates as embedded certificates in the SHIM?
  • No.
If you use new vendor_db functionality, are any hashes allow-listed, and if yes: for what binaries ?
  • We don't use vendor_db.
Is kernel upstream commit 75b0cea7bf307f362057cc778efe89af4c615354 present in your kernel, if you boot chain includes a Linux kernel ?
  • Yes.
if SHIM is loading GRUB2 bootloader, are CVEs CVE-2020-14372,
CVE-2020-25632, CVE-2020-25647, CVE-2020-27749, CVE-2020-27779,
CVE-2021-20225, CVE-2021-20233, CVE-2020-10713, CVE-2020-14308,
CVE-2020-14309, CVE-2020-14310, CVE-2020-14311, CVE-2020-15705,
( July 2020 grub2 CVE list + March 2021 grub2 CVE list )
and if you are shipping the shim_lock module CVE-2021-3418
fixed ?
  • Yes.
"Please specifically confirm that you add a vendor specific SBAT entry for SBAT header in each binary that supports SBAT metadata
( grub2, fwupd, fwupdate, shim + all child shim binaries )" to shim review doc ?
Please provide exact SBAT entries for all SBAT binaries you are booting or planning to boot directly through shim
  • SBAT for shim:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,1,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.isoo,1,Isoo,shim,15.4,https://www.isoo.com/

  • SBAT for grub2:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,1,Free Software Foundation,grub,2.04,https://www.gnu.org/software/grub/
grub.ubuntu,1,Ubuntu,grub2,2.04-1ubuntu46,https://www.ubuntu.com/
grub.isoo,1,Isoo,grub2,2.04-isoo,https://www.isoo.com/

Were your old SHIM hashes provided to Microsoft ?
  • Yes.
Did you change your certificate strategy, so that affected by CVE-2020-14372, CVE-2020-25632, CVE-2020-25647, CVE-2020-27749,
CVE-2020-27779, CVE-2021-20225, CVE-2021-20233, CVE-2020-10713,
CVE-2020-14308, CVE-2020-14309, CVE-2020-14310, CVE-2020-14311, CVE-2020-15705 ( July 2020 grub2 CVE list + March 2021 grub2 CVE list )
grub2 bootloaders can not be verified ?
  • Yes.
What exact implementation of Secureboot in grub2 ( if this is your bootloader ) you have ?
* Upstream grub2 shim_lock verifier or * Downstream RHEL/Fedora/Debian/Canonical like implementation ?
  • Downstream RHEL/Fedora/Debian/Canonical like implementation
What is the origin and full version number of your bootloader (GRUB or other)?
If your SHIM launches any other components, please provide further details on what is launched
  • Our shim does not load any other components.
If your GRUB2 launches any other binaries that are not Linux kernel in SecureBoot mode,
please provide further details on what is launched and how it enforces Secureboot lockdown
  • It doesn't
If you are re-using a previously used (CA) certificate, you
will need to add the hashes of the previous GRUB2 binaries
exposed to the CVEs to vendor_dbx in shim in order to prevent
GRUB2 from being able to chainload those older GRUB2 binaries. If
you are changing to a new (CA) certificate, this does not
apply. Please describe your strategy.
  • We switched to a new certificate after boothole2
How do the launched components prevent execution of unauthenticated code?
  • Grub2 include common secure boot patches so they will only load appropriately signed binaries.
Does your SHIM load any loaders that support loading unsigned kernels (e.g. GRUB)?
  • No, our grub does not allow loading unsigned kernels when secure boot is enabled.
What kernel are you using? Which patches does it includes to enforce Secure Boot?
  • Linux Kernel: 5.11.22
  • It has the usual lockdown patches applied.
What changes were made since your SHIM was last signed?
  • Bug and security fixes.
  • Changelog (since version 15-4).

fix-import_one_mok_state.patch
fix-broken-ia32-reloc.patch
MOK-BootServicesData.patch
Don-t-call-QueryVariableInfo-on-EFI-1.10-machines.patch
relax_check_for_import_mok_state.patch
fix_arm64_rela_sections.patch

What is the SHA256 hash of your final SHIM binary?
  • shimia32.efi.sha256sum: 38cd8f5c80083e85ba9aa06241e29a18b24a48273a22a8d776e4f99cf89d82c4
  • shimx64.efi.sha256sum: a9324449f127e6cb6a37b61ae8a6d1b12cf8a9a3ef464937f67486b06ca25021
@haobinnan
Copy link
Author

My previously accepted SHIM:
#192

@haobinnan
Copy link
Author

Can the submission be reviewed
@steve-mcintyre

@haobinnan haobinnan changed the title shim-15.4 for Isoo (2021-08-24) shim-15.4 for Isoo (2021-08-25) Aug 25, 2021
@haobinnan
Copy link
Author

The shim submitted this time uses patches provided by Debian and adds blacklist dbx.

@haobinnan
Copy link
Author

Can the submission be reviewed

@frozencemetery frozencemetery added bug Problem with the review that must be fixed before it will be accepted incomplete This submission is missing required bits and removed bug Problem with the review that must be fixed before it will be accepted labels Oct 11, 2021
@realnickel
Copy link

  • SBAT sections for shim and grub2 match to submitted;
  • 20 years vendor_cert which didn't change since last accepted submission;
  • shimx64.efi and shimia32.efi binaries are rebuildable by provided Dockerfile and sha256 sums match to submitted;
  • Base Docker image is an official ubuntu:focal.
  • vendor_dbx parts for x64 and i32 are incorporated into respective binaries.

Still, I guess, as you provide vendor_dbx hashes, you also should have provided mentioned binaries as checklist requires...

@haobinnan, IMHO it would be more convenient for reviewers if rebuild artifacts were exported from container or container remained undeleted for manual review.

@haobinnan haobinnan changed the title shim-15.4 for Isoo (2021-08-25) shim-15.4 for Isoo (2021-10-12) Oct 12, 2021
@haobinnan
Copy link
Author

vendor_dbx binaries have been provided, and they can be found here: https://github.com/haobinnan/shim-review/tree/isoo-shim-20211012
Also, rebuild artifacts are remained undeleted.

@frozencemetery
@realnickel

@julian-klode
Copy link
Collaborator

@frozencemetery You added the incomplete tag a month ago without a comment, do you think this is complete now?

@frozencemetery
Copy link
Member

Right, I added it because the checklist at the top was not filled out all the way. It looks like binaries may have been provided in #202 (comment) and the checklist just wasn't updated

@frozencemetery frozencemetery removed the incomplete This submission is missing required bits label Nov 12, 2021
@julian-klode
Copy link
Collaborator

@frozencemetery I think it's more a case of vendor_db being empty and so they ignored the checkmark (the files were for vendor_dbx; which arguably matters less).

@haobinnan
Copy link
Author

May I know if my SHIM can be reviewed and accepted?

@haobinnan
Copy link
Author

May I know if my SHIM can be reviewed and accepted?
@julian-klode
@steve-mcintyre

@haobinnan
Copy link
Author

Hello, would it be possible to review my shim? I found that there is something wrong with the shim submitted last time, so I submit a new one. Thank you very much!

@realnickel
Copy link

Hello, would it be possible to review my shim? I found that there is something wrong with the shim submitted last time, so I submit a new one. Thank you very much!

@haobinnan, do you mean you will change the submission when another "new one" is ready, or do you mean
https://github.com/haobinnan/shim-review/tree/isoo-shim-20211012
is the corrected one already?
It is not clear at least to me.

@haobinnan
Copy link
Author

@realnickel
Thank you very much for the quick response. https://github.com/haobinnan/shim-review/tree/isoo-shim-20211012 is the corrected one, and it needs to be reviewed. Thank you!

@realnickel
Copy link

@realnickel Thank you very much for the quick response. https://github.com/haobinnan/shim-review/tree/isoo-shim-20211012 is the corrected one, and it needs to be reviewed. Thank you!

You are welcome. By the way, while you are waiting for the review you might want to participate in submitters cross review as it was suggested several times before: #165 (comment)

@haobinnan
Copy link
Author

Thank you very much for your quick response. May I know if my shim can be reviewed and accepted now?
I'm interested in the submitters cross review. How do I participate in it?

@realnickel

@realnickel
Copy link

[...]
How do I participate in it?

The link provided above contained a recipe, but let me put a direct link to the manual here:
https://github.com/rhboot/shim/wiki/reviewer-guidelines

Using this guidelines you may provide a preliminary review to any of the open submissions (https://github.com/rhboot/shim-review/issues) not marked as "accepted" the way I provided it for yours before: #202 (comment)

You are expected to check step-by-step and post your results as a comment to the reviewed submission.

Many of us are waiting here for more than 2 or 3 months and I guess only active cross-reviewing may help to reduce the growing queue as review-board members seem to be overwhelmed.

@haobinnan
Copy link
Author

Can the submission be reviewed
@steve-mcintyre

@frozencemetery
Copy link
Member

link to your code branch cloned from rhboot/shim-review in the form user/repo@tag

Link in #c0 is not a tag. Furthermore, no tags have been provided in comments. There have been many tags in that repo; I'm going to assume you mean isoo-shim-20211012 but please be clear about this in the future.

Dockerfile calls an external shell script. It would be nicer not to do this. But can you explain what "AtomLinux" is? There's also a fair amount of dead code it would be nice to prune. Nonetheless, I have verified the build reproduces.

fix_arm64_rela_sections.patch: this resembles 34e3ef205c5d65139eacba8891fa773c03174679 but the Makefile changes do not match. Is this a mistake?

Submission does not answer all questions in submission checklist, but this information is available. (Please fill out the checklist completely in the future.)

Cert doesn't match submitting org - submission says "Qinhuangdao Yizhishu Software Development Co., Ltd." while cert says "Isoo Software Development Co., Ltd.". Can you clarify the relationship between these two entities?

It is not clear to me what grub and kernel you are using.

@frozencemetery frozencemetery added the question Reviewer(s) waiting on response label Dec 13, 2021
@haobinnan
Copy link
Author

Thank you very much for the comments. #192 has problem, so I submit #202.

  1. Yes, I mean isoo-shim-20211012 and the link https://github.com/haobinnan/shim-review/tree/isoo-shim-20211012 does direct to the tag isoo-shim-20211012.
  2. AtomLinux is a small system environment built by myself, and it is built based on source codes. You can refer to this link to get more information about AtomLinux: https://github.com/haobinnan/atomlinux
    At to dead code, I don't know what exactly "dead code" means, and cannot find "dead code". I only built the x86 shim, 32bit and 64bit. Thank you for verifying the build reproduces.
  3. fix_arm64_rela_sections.patch and all other shim patches are from debian. I did not build arm64 shim.
  4. Thank you for the kind reminder. The only question not answered in checklist is "binaries, for which hashes are added do vendor_db ( if you use vendor_db and have hashes allow-listed )". My submission does not involve this question, so I didn't answer it. If submissions have something to do with this question in the future, I'll surely answer it.
  5. "Qinhuangdao Yizhishu Software Development Co., Ltd." is the name used to buy the EV Code Certificate. The "Qinhuangdao Yizhishu" is the pinyin form of our company name (The name in Chinese is "秦皇岛易之数"). "Isoo Software Development Co., Ltd." is the literally translation version of the Chinese name "秦皇岛易之数软件开发有限公司"。Simply put, "Qinhuangdao Yizhishu Software Development Co., Ltd." and "Isoo Software Development Co., Ltd." mean the same company, they are just written in different forms.
    "Isoo Software Development Co., Ltd." is the name embedded in the shim certificate. I'll use "Isoo Software Development Co., Ltd." to sign grub and kernel.
    "
    Do you use EV certificates as embedded certificates in the SHIM?
    No.
    "
  6. I've mentioned the grub and kernel in my submission, and you can refer to following information as well.
    Grub:
    GRUB2 ubuntu version: import/2.04-1ubuntu46
    https://git.launchpad.net/ubuntu/+source/grub2/tag/?h=import/2.04-1ubuntu46
    No extra patches

Kernel:
Linux Kernel: 5.11.22
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/?h=v5.11.22
No extra patches

@frozencemetery

@frozencemetery frozencemetery removed the question Reviewer(s) waiting on response label Dec 14, 2021
@haobinnan
Copy link
Author

I found that you removed the "question" tag. May I know if my submission can be accepted?
@frozencemetery

@haobinnan
Copy link
Author

Can the submission be reviewed

@haobinnan
Copy link
Author

Can the submission be reviewed

@frozencemetery
Copy link
Member

frozencemetery commented Feb 3, 2022

  1. Existing vendor
  2. Submission reproduces
  3. Submission is built from rhboot/shim with patches that appear reasonable
  4. All questions in README were answered (though not template)
  5. Explanation of cert org name makes sense
  6. Certificate does not use HSM. I am unsure if this meets our criteria.
  7. Cert validity is 20 years.
  8. SBAT matches, and looks reasonable.
  9. Submission uses grub
  10. grub is from ubuntu
  11. kernel claims unpatched upstream
  12. N/A - not a revocation
  13. Submission appears to be normal shim->grub2->linux

So the only pending question I have is about access control (6), which I'll defer to those who have been around longer.

@frozencemetery frozencemetery added the extra review wanted Initial review(s) look good, another review desired label Feb 3, 2022
@julian-klode
Copy link
Collaborator

@frozencemetery You wrote "Ceritifcate does not use HSM", but the submission says "They're in an HSM"

@frozencemetery
Copy link
Member

Ah, yeah. I was reading from README.md where it says "My certificate is stored in a db created by pesign, which is stored on a separate computer that is only used for signing and only few trusted administrators can access it. Linux kernel, grubia32.efi and grubx64.efi are signed using this certificate."

@julian-klode
Copy link
Collaborator

@haobinnan Please address the conflicting information. I vaguely recall something, but I don't remember really.

@haobinnan haobinnan changed the title shim-15.4 for Isoo (2021-10-12) shim-15.4 for Isoo (2022-02-05) Feb 5, 2022
@haobinnan
Copy link
Author

@haobinnan
Copy link
Author

Ah, yeah. I was reading from README.md where it says "My certificate is stored in a db created by pesign, which is stored on a separate computer that is only used for signing and only few trusted administrators can access it. Linux kernel, grubia32.efi and grubx64.efi are signed using this certificate."


We only embed out CA certificate. This CA is used to sign further signing certificates which are used for signing the binaries. No other hashes are [CertFile.cer]

@haobinnan
Copy link
Author

Do you still have any questions? Could you please specify the problem?

@haobinnan
Copy link
Author

@frozencemetery
Copy link
Member

shim reviews are intended to be collaboratively done amongst distro vendors. The idea is that we provide a check on each other to ensure quality. This is not a service that "you" pay "us" for: there is no separation between the two. Most of us are overworked, and badgering those who can find time to do reviews at all only punishes us for trying - without offering any assistance of your own. If you want your submission done sooner, the best thing to do is make sure it is of good quality, and to
review those of others.

@julian-klode julian-klode added the superseded Vendor has added a new review which makes this obsolete label Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extra review wanted Initial review(s) look good, another review desired superseded Vendor has added a new review which makes this obsolete
Projects
None yet
Development

No branches or pull requests

4 participants