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 UOS Linux (x86_64) #431

Open
8 tasks done
kyrie-z opened this issue Jul 11, 2024 · 26 comments
Open
8 tasks done

Shim 15.8 for UOS Linux (x86_64) #431

kyrie-z opened this issue Jul 11, 2024 · 26 comments
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer bug Problem with the review that must be fixed before it will be accepted contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response

Comments

@kyrie-z
Copy link

kyrie-z commented Jul 11, 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 )
  • 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/kyrie-z/shim-review/tree/uos-shim-15.8-amd64-20240711
https://github.com/kyrie-z/shim-review/tree/uos-shim-15.8-amd64-20240806


What is the SHA256 hash of your final SHIM binary?


958987f06da4438ab43a873e2c0dd65478299b284ad6e53ca2528524e3a069dd shimx64.efi


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


[UOS shim 15.4 for x86_64 #173 ]


If no security contacts have changed since verification, what is the link to your request, where they've been verified (if any, otherwise N/A)?


N/A

@steve-mcintyre steve-mcintyre added contact verification needed Contact verification is needed for this review contact verification pending Contact verification emails have been sent, waiting on response and removed contact verification needed Contact verification is needed for this review labels Jul 15, 2024
@steve-mcintyre
Copy link
Collaborator

verification emails sent

@kyrie-z
Copy link
Author

kyrie-z commented Jul 19, 2024

@steve-mcintyre Sorry for the late reply, but I want to confirm what went wrong. I did not receive the verification email about "shim review". The contact email is [email protected].
Please confirm again whether it has been sent. Thanks!

@kyrie-z kyrie-z closed this as completed Jul 19, 2024
@kyrie-z kyrie-z reopened this Jul 19, 2024
@jclab-joseph
Copy link

jclab-joseph commented Jul 30, 2024

Review of reproducibility for uos-shim-15.8-amd64-20240711

review helper : https://github.com/jclab-joseph/other-shim-reviews/tree/master/20240730-uos-shim-15.8-amd64-20240711

shim

certificate

  • Not After: Jan 16 00:00:00 2054 GMT
  • 4096 bit cert and valid for almost 30 years
  • The key is kept in a red zone room with strict physical and network isolation mechanisms. Compliant with FIPS 140-2 Level 2 security standards

grub

  • debian's 2.06-3~deb10u4
  • ntfs module is included, sbat is grub,4.

@steve-mcintyre
Copy link
Collaborator

Re-sent verification mails to

@steve-mcintyre
Copy link
Collaborator

And also sending to [email protected]

Not sure where I found the yanbowen mail - maybe an older review. Sorry.

@kyrie-z
Copy link
Author

kyrie-z commented Jul 31, 2024

Contact verification for [email protected]:

gages schoolboy raving preview diagramming holds results cicatrix linger sulphuring

@steve-mcintyre
Copy link
Collaborator

Just waiting on the response from [email protected] now.

@Zeno-sole
Copy link

Just waiting on the response from [email protected] now.

The old key has expired. Can I use a new key?
new key:https://github.com/kyrie-z/shim-review/blob/uos-shim-15.8-amd64-20240711/key/ChenggangLi.pub

@steve-mcintyre
Copy link
Collaborator

Just waiting on the response from [email protected] now.

The old key has expired. Can I use a new key? new key:https://github.com/kyrie-z/shim-review/blob/uos-shim-15.8-amd64-20240711/key/ChenggangLi.pub

The mail I sent was encrypted to this key, which does not appear to have expired:

pub   rsa3072/B4EE92960BB8C880 2021-04-23 [SC]
      B711456DD79BDCA3100EE9B6B4EE92960BB8C880
uid                 [ unknown] lichenggang <[email protected]>
sub   rsa3072/66A6A001ED9D8D69 2021-04-23 [E]

The new key you're suggesting I use does not match the email address [email protected]:

pub   rsa4096/A757694FF3D0B626 2024-07-11 [SC]
      61AE69171770E71B39D842F1A757694FF3D0B626
uid                 [ unknown] lichenggang <[email protected]>
sub   rsa4096/0EC1F8845EC8DD6B 2024-07-11 [E]

Please fix this.

Could you also please explain for us: what is the relationship between:

  • UnionTech Software Technology (uniontech.com)
  • UOS (chinauos.com)
  • Deepin (deepin.org)

Some consistency in UIDs and keys here is necessary.

@kyrie-z
Copy link
Author

kyrie-z commented Aug 2, 2024

Could you also please explain for us: what is the relationship between:

  • UnionTech Software Technology (uniontech.com)
  • UOS (chinauos.com)
  • Deepin (deepin.org)

I apologize for any confusion regarding the names. Please allow me to clarify:
Deepin Technology Co., Ltd. ("Deepin Technology") is a wholly-owned subsidiary of UnionTech Software Technology Co., Ltd. ("UnionTech Software").
Deepin Technology owns the product "deepin" (product website: https://www.deepin.org/), while UnionTech Software owns the product "UOS" (product website: https://www.chinauos.com/).

@kyrie-z
Copy link
Author

kyrie-z commented Aug 2, 2024

@steve-mcintyre I have updated the secondary contact email address to keep the email address consistent with the UID. Please use the new key for contact verification. Looking forward to hearing from you, thanks!
https://github.com/kyrie-z/shim-review/blob/uos-shim-15.8-amd64-20240711/README.md#who-is-the-secondary-contact-for-security-updates-etc

@steve-mcintyre
Copy link
Collaborator

Mail on the way. As you've updated your submission in git, please also add a new tag and update the issue here with that new tag.

@kyrie-z
Copy link
Author

kyrie-z commented Aug 6, 2024

I have created a new tag and updated the issue.
New tag: https://github.com/kyrie-z/shim-review/tree/uos-shim-15.8-amd64-20240806

@kyrie-z
Copy link
Author

kyrie-z commented Aug 6, 2024

By the way, the tags uos-shim-15.8-amd64-20240711 and uos-shim-15.8-amd64-20240806 are associated with the same commit (kyrie-z/shim-review@02e5eb2). I believe that jclab-joseph's review #431 (comment) is very useful, so I'm mentioning this to avoid duplicate review efforts.
I hope this helps with your review. Thank you.

@Zeno-sole
Copy link

Contact verification for [email protected]:

puritan segregate expatriating Alnitak homily daffodils Avalon bountiful blurted Hecuba

@Zeno-sole
Copy link

Contact verification for [email protected]:

puritan segregate expatriating Alnitak homily daffodils Avalon bountiful blurted Hecuba

@steve-mcintyre hello, Can you help review

@steve-mcintyre steve-mcintyre added contacts verified OK Contact verification is complete here (or in an earlier submission) and removed contact verification pending Contact verification emails have been sent, waiting on response labels Sep 8, 2024
@evilteq
Copy link

evilteq commented Oct 2, 2024

Pretty clean and by-the-book.

  • Reproduced ok: 958987f06da4438ab43a873e2c0dd65478299b284ad6e53ca2528524e3a069dd shimx64.efi
  • CA is valid until 2054, 4k RSA, Subject: C = CN, O = Uniontech, OU = Uniontech OS, CN = Uniontech UEFI Bootloader Publisher 2024
  • No patches, verified tarball downloaded from mainline github repo.
  • sbat included looks ok and matches readme's
  • keys are secure: they are in a "red zone room"!!!!!!1 ;)

Not much else to review from our side, 👍

@costinchen
Copy link

costinchen commented Oct 25, 2024

I'm not an official reviewer, but I want to help speed up reviewing.

  • Build is reproducible using Dockerfile and the sha256sum matches:
    958987f06da4438ab43a873e2c0dd65478299b284ad6e53ca2528524e3a069dd shimx64.efi
  • Revoked certs in dbx - None, last submission is shim 15.4, it can be revoked by sbat
  • Embedded cert is CA cert:
    • Subject: C = CN, O = Uniontech, OU = Uniontech OS, CN = Uniontech UEFI Bootloader Publisher 2024
    • Valid begin: Jan 16 03:11:23 2024 GMT, until: Jan 16 00:00:00 2054 GMT (30 years)
    • 4096 bit RSA key
    • Key is kept in a red zone room, compliant with FIPS 140-2 Level 2 security standards
  • SBAT sections look reasonable and matches the section in shimx64.efi :
shim:
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.uos,1,Uos,shim,15.8,mail:[email protected]

grub:
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,4,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
grub.debian,4,Debian,grub2,2.06-3~deb10u4,https://tracker.debian.org/pkg/grub2
grub.uos,1,Uos,grub2,2.06.1-1,mail:[email protected]
  • NX bit is disabled: DllCharacteristics 00000000
  • Build with official shim tarball with no patches, pretty clean

All looks good from my perspective!👍

@THS-on
Copy link
Collaborator

THS-on commented Nov 4, 2024

Review for uos-shim-15.8-amd64-20240806

General

  • Had a shim signed before
  • One security contact changed and was verified
  • Need custom kernel and Grub for their distribution UOS

Shim

  • Shim is reproducible via Dockerfile (after removing the copying on non-existent patches)
  • 958987f06da4438ab43a873e2c0dd65478299b284ad6e53ca2528524e3a069dd
  • NX bit is not set
  • Embedded certificate is not a CA with codesiging and digital signature key usage set
    • Valid till Jan 16 00:00:00 2054 GMT (fine)
    • Any reason that you not include an intermediate CA?

GRUB

  • Based on the 2.06-3 version from Debian
  • SBAT looks fine
  • What is the deepin_gfxmode module?
  • Do you really need the following modules: minix minix2 reiserfs?

Kernel

Notes and comments

  • Please make it possible to build your container using docker build . --progress plain --no-cache and not via wrapper script next time
  • There is an ongoing effort of reducing the non HSM key usages, as the guidelines given by MS require it. As you had an accepted shim before, they might be fine with the current key storage situation, but I would consider investing in moving to an HSM based approach.
  • Looking at the GRUB SBAT entry you are basing on Debian 10 which LTS is already EOL. On which Debian version is UOS based?
  • We added some more questions to the submission text since this submission was opened. Can you include the new questions?
  • Please describe how you enforce lockdown when Secure Boot is enabled

@THS-on THS-on added the question Reviewer(s) waiting on response label Nov 4, 2024
@costinchen
Copy link

costinchen commented Nov 5, 2024

  • Embedded cert is CA cert:

    • Subject: C = CN, O = Uniontech, OU = Uniontech OS, CN = Uniontech UEFI Bootloader Publisher 2024
    • Valid begin: Jan 16 03:11:23 2024 GMT, until: Jan 16 00:00:00 2054 GMT (30 years)
    • 4096 bit RSA key
    • Key is kept in a red zone room, compliant with FIPS 140-2 Level 2 security standards

Sorry for my carelessness when review the certificate in #431 (comment) , I missed the simple CA:FALSE field which means it's not a CA certificate. Here are the complete X509v3 extensions filed in your certificate :

X509v3 extensions:
    X509v3 Key Usage: critical
        Digital Signature
    X509v3 Extended Key Usage: 
        Code Signing
    X509v3 Basic Constraints: critical
        CA:FALSE
    X509v3 Subject Key Identifier: 
        2D:D8:CD:70:0A:34:9E:1B:2B:52:4F:87:D3:B1:24:D1:C7:B9:6B:0D
    X509v3 Authority Key Identifier: 
        D0:5D:4C:E6:4E:1B:9D:C0:C5:85:7B:C0:17:C6:51:0C:7B:5C:CB:17
    X509v3 Certificate Policies: 
        Policy: 1.2.156.115230.9.8.7.1
          CPS: http://www.uosca.cn/policy/
        Policy: 1.2.156.115230.9.8.7.1
          CPS: https://pki.uniontech.com/ca/cps

@kyrie-z
Copy link
Author

kyrie-z commented Nov 6, 2024

Embedded certificate is not a CA with codesiging and digital signature key usage set

  • Valid till Jan 16 00:00:00 2054 GMT (fine)
  • Any reason that you not include an intermediate CA?

Here is an end-entity certificate, which is used solely for signing verification of the Grub and kernel code. It is not intended to issue certificates, so it does not have CA signing authority.

@kyrie-z
Copy link
Author

kyrie-z commented Nov 6, 2024

  • What is the deepin_gfxmode module?

The deepin_gfxmode module is used to accurately obtain the resolution at which Grub runs, helping the system better adjust the GRUB display settings.

  • Do you really need the following modules: minix minix2 reiserfs?

Previously, many modules were added to ensure Grub had the latest features and support, but now it appears that some of these modules are outdated. We plan to gradually remove them in the future.

@steve-mcintyre steve-mcintyre added Accredited review needed Needs a successful review by an accredited reviewer 1 review needed Needs 1 (additional) successful review before being accepted and removed extra review wanted labels Nov 13, 2024
@kyrie-z
Copy link
Author

kyrie-z commented Nov 14, 2024

  • Please make it possible to build your container using docker build . --progress plain --no-cache and not via wrapper script next time

Thanks for the reminder. we'll take it on board next.

  • Looking at the GRUB SBAT entry you are basing on Debian 10 which LTS is already EOL. On which Debian version is UOS based?

UOS was based on Debian 10 before Debian 10 reached EOL. After that, it transitioned to being based on the Deepin system. It is currently using GRUB version 2.06, and GRUB will be updated to version 2.12 in the future.

  • Please describe how you enforce lockdown when Secure Boot is enabled

The two functions of secure boot and lockdown are independent of each other. Enabling secure boot does not force lockdown.

@THS-on
Copy link
Collaborator

THS-on commented Nov 17, 2024

The two functions of secure boot and lockdown are independent of each other. Enabling secure boot does not force lockdown.

Technically these are two separately features, but not enabling lockdown when booting in secure boot allows the user to circumvent the protections assumed by secure boot easily. The consensus is that no kernels should be signed that not force lockdown when booted with secure boot enabled.

@THS-on THS-on added the bug Problem with the review that must be fixed before it will be accepted label Nov 17, 2024
@kyrie-z
Copy link
Author

kyrie-z commented Nov 26, 2024

Thanks for your suggestion, @THS-on . We will try to incorporate CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT into the kernel and ensure that no kernel without this feature will be signed.

@THS-on
Copy link
Collaborator

THS-on commented Dec 16, 2024

@kyrie-z once you incorporated this, can you update the submission, so that we can have another look?

Please also include a strategy that older kernels cannot be booted with the new shim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer bug Problem with the review that must be fixed before it will be accepted contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response
Projects
None yet
Development

No branches or pull requests

7 participants