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.7 for Gooroom OS #331

Closed
8 tasks done
ozun215 opened this issue Apr 18, 2023 · 30 comments
Closed
8 tasks done

Shim 15.7 for Gooroom OS #331

ozun215 opened this issue Apr 18, 2023 · 30 comments
Labels
bug Problem with the review that must be fixed before it will be accepted new vendor This is a new vendor

Comments

@ozun215
Copy link

ozun215 commented Apr 18, 2023

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-1/tree/gooroom-shim-amd64-20230418


What is the SHA256 hash of your final SHIM binary?


7fb5db43feb7a0ade8bedd573eb044cede248501000865db2cb5745d2695ccea


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


#270

@ozun215 ozun215 closed this as completed Apr 18, 2023
@ozun215 ozun215 reopened this Apr 18, 2023
@aronowski
Copy link
Collaborator

While I'm not an official reviewer, I can see a few curiosities:

The build does not reproduce for me with this Dockerfile. I get the error:

------
Dockerfile:19
--------------------
  17 |     RUN hexdump -Cv /shim/shim*.efi > build
  18 |     RUN hexdump -Cv /shim-review-1/$(basename /shim/shim*.efi) > orig
  19 | >>> RUN diff -u orig build
  20 |     RUN sha256sum /shim/shim*.efi /shim-review-1/$(basename /shim/shim*.efi)
  21 |
--------------------
ERROR: failed to solve: process "/bin/sh -c diff -u orig build" did not complete successfully: exit code: 1

Removing this line makes the build proceed. However, there still seems to be something wrong - the artifacts in the final docker image have the following SHA-256 sums:

7fb5db43feb7a0ade8bedd573eb044cede248501000865db2cb5745d2695ccea  /shim/shimx64.efi
cecaac151d3e06a1a70494d4bd66e13dbfa948858af80aaab94719176352f6ec  /shim-review-1/shimx64.efi

I've checked and your Dockerfile has the following entries:

RUN git clone https://github.com/ozun215/shim-review-1.git
WORKDIR /shim-review-1
RUN git checkout gooroom-3.0

And I was right - the shimx64.efi artifact on the branch gooroom-3.0 has this cecaac15 checksum.

Maybe it would be a good idea to checkout the tag used for this review rather than a branch?


*******************************************************************************
### Do you add a vendor-specific SBAT entry to the SBAT section in each binary that supports SBAT metadata ( grub2, fwupd, fwupdate, 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 si
mplify revocation.
*******************************************************************************
[...]

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,3,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
grub.debian,1,Debian,grub2,2.06-3,https://tracker.debian.org/pkg/grub2
grub.gooroom,1,Gooroom,gooroom-grub,2.06-3+grm3u5,https://github.com/gooroom/gooroom-grub

[...]

Why grub.debian,1? As far as I can see Debian already has their generation number set to 4. This is the entry Debian's grubx64.efi has:

grub.debian,4,Debian,grub2,2.06-3~deb11u5,https://tracker.debian.org/pkg/grub2

*******************************************************************************
### What patches are being applied and why:
*******************************************************************************
No patch applied

This would be OK in the context of using custom patches rather than relying on upstream patches. However, for me this was misleading as there are already several Debian patches being applied. Looks like they are these:

Enable-NX.patch
Make-sbat_var.S-parse-right-with-buggy-gcc-binutils.patch
aarch64-gnuefi-old.patch
aarch64-shim-old.patch
block-grub-sbat3-debian.patch

I can also see the block-grub-sbat3-debian.patch is there and seems to deny loading the prohibited old versions of GRUB2. Earlier I wrote about a potentially improper SBAT entry for your GRUB2.

So if I'm right, then your own shim won't allow booting your own GRUB2.

I strongly suggest prototyping all this work in a dedicated environment like a libvirt virtual machine but replacing all the provided variables with your own .auths and rebuilding and signing your assets with custom certificates and keys. I can help you with that if you want.

@ozun215
Copy link
Author

ozun215 commented Apr 20, 2023

Thanks for the help

We fixed Dockerfile and SBAT. there are new release below

https://github.com/ozun215/shim-review-1/tree/gooroom-shim-amd64-20230420

We want your help if you don't mind. We are having a very difficult time doing this work.

Best regards

@aronowski
Copy link
Collaborator

I can see the changes. Good job!

I'd also update the What patches are being applied and why question as said earlier and update the link in your first post so it reflects the latest tag.

I can give some tips on prototyping the whole thing locally but I'll need help too as this is a Debian-based system and I'm more into Fedora/RHEL, so things might work differently here.

Can I contact you via email? If so, which one should I use?

@ozun215
Copy link
Author

ozun215 commented May 4, 2023

hello aronowski~

sorry for resopons delay...
I've been busy with other things for a while.

Yes I'd appreciate it if you could contact me.

My email address is "[email protected]"

wait for your message~

thank you~

@aronowski
Copy link
Collaborator

For confirmation: message sent from the address posted on my GitHub profile and signed with GPG.

@THS-on THS-on added new vendor This is a new vendor contact verification needed Contact verification is needed for this review labels Sep 26, 2023
@aronowski aronowski self-assigned this Oct 27, 2023
@aronowski
Copy link
Collaborator

I'd like to send emails for contact verification, but can't find the appropriate public keys anywhere. Please, point me to a source of truth, where I can get them.

@aronowski aronowski added the incomplete This submission is missing required bits label Oct 27, 2023
@ozun215
Copy link
Author

ozun215 commented Nov 2, 2023

I'm sorry late response.

I also don't understand what happened, So finding out what is wrong then I got a know my account is disabled.

I'm reactivate account from now. please contact verification again.

@aronowski
Copy link
Collaborator

Please, point me to the place where the public keys have been uploaded.

@ozun215
Copy link
Author

ozun215 commented Nov 6, 2023

I'm sorry but I don't understand what you mean...
Does public key mean PGP?
The PGP is written in the review document.

@aronowski
Copy link
Collaborator

I mean the complete public PGP keys. There are only fingerprints in the application.

If you need help with exporting them and uploading, let me know and I'll try my best to help you.

@es-fabricemarie
Copy link

@ozun215 you need to:

  1. cross-sign your GPG/PGP keys (see https://gist.github.com/F21/b0e8c62c49dfab267ff1d0c6af39ab84 for some examples)
  2. upload your cross-signed GPG/PGP public key to one or more key servers (for example https://keyserver.ubuntu.com/)

This applies to the persons you have mentioned in your submission:

(both Mr Park and Mr Woo need to perform the steps 1. and 2. above).

@ozun215
Copy link
Author

ozun215 commented Nov 30, 2023

Our PGP public key is here

[email protected]

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: FlowCrypt Email Encryption 8.3.1
Comment: Seamlessly send and receive encrypted email

xjMEYv7n3BYJKwYBBAHaRw8BAQdA9IkfKpZjblWACr+S8HIocQZUTckzKkkt
6ZlGbpgEqHTNJ0pvbmdreXVuZyBXb28gPGpvbmdreXVuZy53b29AZ21haWwu
Y29tPsKPBBAWCgAgBQJi/ufcBgsJBwgDAgQVCAoCBBYCAQACGQECGwMCHgEA
IQkQUq2A3dNxIdsWIQTia7cLzK0DqXZC1E1SrYDd03Eh23ksAQDKMhqsqhQY
NP195lUieP8VlX9Boc784nyjbP3PKuAG/AD9GhrjYEgW8po9Qb5lnVbflbMh
JfSWZa71dNPid68L1w3OOARi/ufcEgorBgEEAZdVAQUBAQdAv6P8+AG4Hds2
2+GNocue1oG2qOOPAzS7j0TsV3JrZx8DAQgHwngEGBYIAAkFAmL+59wCGwwA
IQkQUq2A3dNxIdsWIQTia7cLzK0DqXZC1E1SrYDd03Eh2zDLAQCAQ06uGi0m
74wnXbJKCjXI86PmfXZnNITbg6I9YFi5EwD/fXU+3haKqUBresOTuk8YZnVM
ntl3wWXgTVYOX81Ujwc=
=qKH1
-----END PGP PUBLIC KEY BLOCK-----

[email protected]

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: FlowCrypt Email Encryption 8.3.1
Comment: Seamlessly send and receive encrypted email

xjMEYv7n3BYJKwYBBAHaRw8BAQdA9IkfKpZjblWACr+S8HIocQZUTckzKkkt
6ZlGbpgEqHTNJ0pvbmdreXVuZyBXb28gPGpvbmdreXVuZy53b29AZ21haWwu
Y29tPsKPBBAWCgAgBQJi/ufcBgsJBwgDAgQVCAoCBBYCAQACGQECGwMCHgEA
IQkQUq2A3dNxIdsWIQTia7cLzK0DqXZC1E1SrYDd03Eh23ksAQDKMhqsqhQY
NP195lUieP8VlX9Boc784nyjbP3PKuAG/AD9GhrjYEgW8po9Qb5lnVbflbMh
JfSWZa71dNPid68L1w3OOARi/ufcEgorBgEEAZdVAQUBAQdAv6P8+AG4Hds2
2+GNocue1oG2qOOPAzS7j0TsV3JrZx8DAQgHwngEGBYIAAkFAmL+59wCGwwA
IQkQUq2A3dNxIdsWIQTia7cLzK0DqXZC1E1SrYDd03Eh2zDLAQCAQ06uGi0m
74wnXbJKCjXI86PmfXZnNITbg6I9YFi5EwD/fXU+3haKqUBresOTuk8YZnVM
ntl3wWXgTVYOX81Ujwc=
=qKH1
-----END PGP PUBLIC KEY BLOCK-----

Do I have to change the contents of the application?

thank you

@es-fabricemarie
Copy link

  • See my comment 1. and 2. above on how to cross-sign and submit your key to a keyserver
  • The two PGP keys you pasted are identical and belong to [email protected]

@ozun215
Copy link
Author

ozun215 commented Nov 30, 2023

I'm sorry it is my mistake

[email protected] pgp public key
받은메일 표시

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: FlowCrypt Email Encryption 8.5.2
Comment: Seamlessly send and receive encrypted email

xjMEYuOgXhYJKwYBBAHaRw8BAQdA+uAyiGN/l5p7W3K/VJVzYm9nw+RsOLOG
dRQh7fokGJDNHuuwleyYgeykgCA8enVubkBlYWN0aXZlLmNvLmtyPsKPBBAW
CgAgBQJi46BeBgsJBwgDAgQVCAoCBBYCAQACGQECGwMCHgEAIQkQxSrSP9Xm
yukWIQRNOymeJcvFxX/LKAHFKtI/1ebK6YteAP9xLD60duecJ/GBg0ntP4v5
IM+m0cSQK4TJhbXn86w59AD/QW647PceCzBaEnUg5FmkvOcnBl6ePip360ZU
ZgHY0AzOOARi46BeEgorBgEEAZdVAQUBAQdANOzRNHxkyv8TKymK1AIfCRcH
P3VXNYoyT37yWonW8ncDAQgHwngEGBYIAAkFAmLjoF4CGwwAIQkQxSrSP9Xm
yukWIQRNOymeJcvFxX/LKAHFKtI/1ebK6VQfAQCy2m4f/SS6OFG4VYlNE8MZ
qXYfA0RKXcO+b6jRRMeiQwD/UGk9ZF/2jEH4rpKL7WhehUQ0cKfjR2Tcj25Z
56dsbw8=
=RJbh
-----END PGP PUBLIC KEY BLOCK-----

Thank you

@ozun215
Copy link
Author

ozun215 commented Dec 1, 2023

I'm Sorry about these issue
I just realized what the problem was...
It's my first time, so everything is confusing. I ask for your understanding.
Fixed PGP finger print and I registered keys in keyserver.ubuntu.com and checked my search.

new release below

https://github.com/ozun215/shim-review-1/tree/gooroom-shim-amd64-20231201

thank you all about this

@es-fabricemarie
Copy link

https://keyserver.ubuntu.com/pks/lookup?search=jongkyung.woo%40gmail.com&fingerprint=on&op=index
key 52AD80DDD37121DB: <[email protected]> 
fingerprint matches: E26BB70BCCAD03A97642D44D52AD80DDD37121DB

https://keyserver.ubuntu.com/pks/lookup?search=zunn%40eactive.co.kr&fingerprint=on&op=index
key C52AD23FD5E6CAE9: <[email protected]> 
fingerprint matches: 4D3B299E25CBC5C57FCB2801C52AD23FD5E6CAE9
  • ✗ keys are not signed by others or cross-signed
  • ✗ keys have no expiry
  • ✓ fingerprint matches for both keys
  • verification email sent to both

@ozun215
Copy link
Author

ozun215 commented Dec 4, 2023

@es-fabricemarie
For the contact verification, here are the words:


beam
shame
costume
beef
decline
linen
betray
franchise
secure
dialogue
liberal
feel
dynamic
arrest
shout

@aronowski aronowski removed the incomplete This submission is missing required bits label Dec 4, 2023
@aronowski
Copy link
Collaborator

aronowski commented Dec 4, 2023

@ozun215,

It's my first time, so everything is confusing. I ask for your understanding.

I know that feeling, but no worries - we're here to learn together.

I'll try to make the process more accessible, but it will take time, and after working hours I have some private things going on.


Quite some time passed after I initially reviewed the application on my side, and I suppose some thing have changed throughout this period. From an organizational standpoint, may I ask @es-fabricemarie to review the current revision (gooroom-shim-amd64-20231201)?

@es-fabricemarie
Copy link

@ozun215 ([email protected]) GPG+contact verified. Please ask Mr Woo to check his email and do the same verification.

@aronowski I'll start a review tomorrow as best I can, sure.

@jongkyung
Copy link

@es-fabricemarie
For the contact verification, here are the words:


publish
mutter
realism
change
sound
charity
background
deck
rhythm
cultural
consumption
or
save
canvas
ready

@es-fabricemarie
Copy link

@jongkyung ([email protected]) GPG+contact verified.

@ozun215
Copy link
Author

ozun215 commented Dec 13, 2023

@aronowski @es-fabricemarie @jongkyung
Do we have to cross-signnig process??

@aronowski
Copy link
Collaborator

@ozun215, while cross-signing is recommended, we don't mandate it right now.


Regarding the verification of contacts, I wrote the following in this application:

Regarding the verification of contacts, I believe only the ones by authorized reviewers count. The same when it comes to labeling issues and, as far as I understand, letting Microsoft know on the updates.

[...] I believe the best we can do is to [...] help with the peer review so the official reviewers have less work to do manually - especially if there are some hard-to-notice errors, which can be fixed in the meantime.

Therefore, I'll need to perform this verification again, despite @es-fabricemarie already helping me out, what I'm grateful for.

I thought a review will be made by then, but it looks like I'm not the only one, who has a lot of things ongoing and barely having time and energy for even more work, which already requires patience, focus and multiple checks.

I'll send the emails soon.

@aronowski
Copy link
Collaborator

Verification emails sent - attempt no. 2.

@jongkyung
Copy link

plains nautilus westbound recommences Minuit judiciary stepdaughters
lathing Nyasa drafting

@ozun215
Copy link
Author

ozun215 commented Dec 19, 2023

@aronowski

Message is

kindly infatuation chirp backpedal audibles succincter adopting careen
shirring chests

@aronowski aronowski removed the contact verification needed Contact verification is needed for this review label Dec 22, 2023
@aronowski
Copy link
Collaborator

Alright, reviewing.

There are some things that need fixing. I recommend doing them in the following order:

1

Due to CVE-2023-4692 and CVE-2023-4693 GRUB2 vulnerabilities described at https://lists.gnu.org/archive/html/grub-devel/2023-10/msg00028.html, it's been agreed to update the required GRUB2 SBAT global generation number in its binaries to 4: #348

I can see in your GRUB2 repository that you still use grub,3. Please, update the code, so your binaries have the grub,4 SBAT entry.

Once that's done, update the answer in your application: the entry at Please provide exact SBAT entries for all SBAT binaries you are booting or planning to boot directly through shim. shall be updated from grub,3 to grub,4.

2

Once the above has been done, the question

If these fixes have been applied, have you set the global SBAT generation on your GRUB binary to 3?

no longer applies - it's been updated to

If these fixes have been applied, is the upstream global SBAT generation in your GRUB2 binary set to 4?
The entry should look similar to: `grub,4,Free Software Foundation,grub,GRUB_UPSTREAM_VERSION,https://www.gnu.org/software/grub/`

so you'll need to replace it and answer:

*******************************************************************************
### If these fixes have been applied, is the upstream global SBAT generation in your GRUB2 binary set to 4?
The entry should look similar to: `grub,4,Free Software Foundation,grub,GRUB_UPSTREAM_VERSION,https://www.gnu.org/software/grub/`
*******************************************************************************
Yes

Remember, first you need to update the code in your GRUB2 repository, so your binaries have the grub,4 SBAT entry.

3

Recently the NX requirements have changed and most likely you'll need to remove the NX-compatibility patch for Microsoft to sign your binaries - we've had a discussion to make this venue more user-friendly here - I suggested some hints there as well, to prevent confusion.

Please, remove the NX support patch (the file ./debian/patches/Enable-NX.patch in the repository https://github.com/gooroom/shim), recompile the shim binary, update the checksums in the README and in this thread's opening post, and push the changes.

4

Preferably all the attached .patch files shall be described in the What patches are being applied and why question. Do this after removing the NX compatibility patch, so it's no longer there. You can base on the descriptions used in the actual patches, like I tried below, and if you like it, just copy this listing and paste it to your application:

- [block-grub-sbat3-debian.patch](https://github.com/gooroom/shim/blob/gooroom-3.0/debian/patches/block-grub-sbat3-debian.patch)

Debian's grub.3 update was broken - some binaries included the SBAT data update but not the security patches.

This patch denies loading binaries with `grub.debian,3`.

- [aarch64-shim-old.patch](https://github.com/gooroom/shim/blob/gooroom-3.0/debian/patches/aarch64-shim-old.patch)

shim 15.6 onwards needs newer binutils to build on aarch64. That works
better, but we don't have that binutils update in older Debian
releases. Undo the build changes here so that we can build for aarch64
on older stable releases. We're not going to sign them, but we need
the binaries for aarch64.

- [aarch64-gnuefi-old.patch](https://github.com/gooroom/shim/blob/gooroom-3.0/debian/patches/aarch64-gnuefi-old.patch)

The same reason as above.

- [Make-sbat_var.S-parse-right-with-buggy-gcc-binutils.patch](https://github.com/gooroom/shim/blob/gooroom-3.0/debian/patches/Make-sbat_var.S-parse-right-with-buggy-gcc-binutils.patch)

In https://github.com/rhboot/shim/issues/533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the 
.sbatlevel section in shim binaries.

This patch adds test cases that will cause the build to fail if this has 
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.

5

Once you've pushed the updates and tagged the new revision, edit the opening post in this GitHub issue, so the link to your repository is up to date.

Furthermore, ping me and another official reviewer, that's been active recently, to speed up the reviewing process.

@aronowski aronowski added the bug Problem with the review that must be fixed before it will be accepted label Dec 24, 2023
@ozun215
Copy link
Author

ozun215 commented Feb 14, 2024

@aronowski

I'm sorry too late

It takes a lot of time understand what your orders

new release below

https://github.com/ozun215/shim-review-1/tree/gooroom-shim-amd64-20240214

thank you all of these

@aronowski aronowski mentioned this issue Feb 15, 2024
8 tasks
@THS-on
Copy link
Collaborator

THS-on commented Feb 20, 2024

@ozun215 can you either update your submission to 15.8 or close this one and create a new one?

@ozun215
Copy link
Author

ozun215 commented Feb 22, 2024

@THS-on @jongkyung @es-fabricemarie @aronowski We will create a new one as soon as possilbe. thank you

@aronowski aronowski removed their assignment Feb 23, 2024
@THS-on THS-on closed this as completed Feb 24, 2024
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 new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

5 participants