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.6 for GOOROOM-20220814 #270

Closed
8 tasks done
ozun215 opened this issue Aug 14, 2022 · 12 comments
Closed
8 tasks done

shim 15.6 for GOOROOM-20220814 #270

ozun215 opened this issue Aug 14, 2022 · 12 comments
Labels
bug Problem with the review that must be fixed before it will be accepted contact verification needed Contact verification is needed for this review new vendor This is a new vendor

Comments

@ozun215
Copy link

ozun215 commented Aug 14, 2022

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 )
  • 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 is the link to your tag in a repo cloned from rhboot/shim-review?


https://github.com/ozun215/shim-review/tree/gooroom-shim-amd64-20220817-4


What is the SHA256 hash of your final SHIM binary?


cfa3cf6ac47e7714622a3f2bbedd00d12b455593e583edb27752becbedb1a55b shimx64.efi

@frozencemetery
Copy link
Member

Presumably blocked on #265

@steve-mcintyre steve-mcintyre added bug Problem with the review that must be fixed before it will be accepted question Reviewer(s) waiting on response new vendor This is a new vendor contact verification needed Contact verification is needed for this review labels Aug 16, 2022
@steve-mcintyre
Copy link
Collaborator

Assuming this is meant to supersede #265 (please confirm that!)

Docker build fails here:

Step 7/20 : RUN git clone https://github.com/ozun215/shim-review.git
 ---> Running in e472ef3e5f42
Cloning into 'shim-review'...
Removing intermediate container e472ef3e5f42
 ---> 76c05e040721
Step 8/20 : WORKDIR /shim-review
 ---> Running in 6e243825970f
Removing intermediate container 6e243825970f
 ---> c147eec00edb
Step 9/20 : RUN git checkout https://github.com/ozun215/shim-review/tree/gooroom-shim-amd64-20220814
 ---> Running in 1e9d7c5185e8
error: pathspec 'https://github.com/ozun215/shim-review/tree/gooroom-shim-amd64-20220814' did not match any file(s) known to git
The command '/bin/sh -c git checkout https://github.com/ozun215/shim-review/tree/gooroom-shim-amd64-20220814' returned a non-zero code: 1

@ozun215
Copy link
Author

ozun215 commented Aug 16, 2022

Yes it is superseded #265

We are working on dockerfile to be excitable soon

thanks for your patience

@ozun215
Copy link
Author

ozun215 commented Aug 17, 2022

We have fixed some bugs and update Readme.md and Dockerfile

New release is https://github.com/ozun215/shim-review/tree/gooroom-shim-amd64-20220817-4

new sha256 hash is

cfa3cf6ac47e7714622a3f2bbedd00d12b455593e583edb27752becbedb1a55b shimx64.efi

Thank you

@steve-mcintyre
Copy link
Collaborator

So finally I can reproduce this build. However, there are still issues in your submission

  • It's very light on detail. I don't know what Gooroom is, and you've given us no clues.
  • "To secure boot working on Gooroom OS" tells us nothing useful. Why should we care that Gooroom boots using Secure Boot?
  • Contact 1 is just "Gooroom", which tells us nothing
  • A large chunk of your README.md is copied directly from what I wrote in the Debian submission Debian GNU/Linux 12 shim-15.6-1 x64, ia32 and aarch64 #267, even where I don't think it makes sense.

You're clearly deriving from the Debian shim package (which is fine if it's done OK) and you've added your own key. However looking at your scripting around your key (debian/make-certs-gooroom) I'm really worried about how you're managing keys. You appear to be generating a key at build time, and it has the CA bit set. You're giving it a very generic name "Gooroom VENDOR CERT" and it's missing a lot of the key settings that I'd normally expect. You mention "a specially hardened machine that is in our build environment" - how exactly are you generating and managing keys?

You're still only using Debian SBAT data and not adding your own.

This is still nowhere near ready. Go and read the docs at https://github.com/rhboot/shim/wiki/submitting

@ozun215
Copy link
Author

ozun215 commented Aug 31, 2022

We have fixed some bugs and update Readme.md(include SBAT)

New release is https://github.com/ozun215/shim-review-1/tree/gooroom-shim-amd64-20220831-1

new sha256 hash is

cecaac151d3e06a1a70494d4bd66e13dbfa948858af80aaab94719176352f6ec shimx64.efi

Thank you

@ozun215
Copy link
Author

ozun215 commented Sep 19, 2022

Is there a problem with the current issue? Please let me know if you need any modifications.

@ozun215
Copy link
Author

ozun215 commented Oct 4, 2022 via email

@steve-mcintyre
Copy link
Collaborator

Sorry for leaving you for quite a while - I'm a volunteer and I've been really busy on other work in Debian.

Could you please respond to my issues listed above(5adc7524bbd7774d8711d57037320852bc73af67) , explaining what you've done in each case? I'm looking through your updated shim and shim repos already, but I'd like you to explain things please.

@ozun215
Copy link
Author

ozun215 commented Nov 21, 2022

Hello~!!
Nice to meet you!!!

We thought a lot about your comment. and discussed so many times then follow things we modified

  1. We have changed the contact information of the representative technician, including the introduction of our operating system and the contents of the organization.
  2. A storage plan for keys
  3. SBAT Related Contents
  4. Module list
  5. Grub version info
  6. Certification management way
  7. Also we added the answers to the questions we didn't understand before because it was our first application.

As already mentioned, we are trying this work for the first time in our country, so please understand if there is anything lacking.
We have scant information for reference

thank you and please review our issue again.

@frozencemetery
Copy link
Member

Please note #307

@frozencemetery frozencemetery removed the question Reviewer(s) waiting on response label Feb 16, 2023
@ozun215 ozun215 mentioned this issue Apr 18, 2023
8 tasks
@steve-mcintyre
Copy link
Collaborator

Superseded by #331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem with the review that must be fixed before it will be accepted contact verification needed Contact verification is needed for this review new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

3 participants