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 for Virtuozzo Linux #406

Closed
8 tasks done
AVasiljev opened this issue Apr 1, 2024 · 44 comments
Closed
8 tasks done

Shim for Virtuozzo Linux #406

AVasiljev opened this issue Apr 1, 2024 · 44 comments
Assignees
Labels
accepted Submission is ready for sysdev contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer new vendor This is a new vendor

Comments

@AVasiljev
Copy link

AVasiljev commented Apr 1, 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/virtuozzo/shim-review/releases/tag/virtuozzo-shim-x86_64-20240401


What is the SHA256 hash of your final SHIM binary?


fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c shimx64.efi


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


N/A

@es-fabricemarie
Copy link

I'm not an official reviewer, I just want to reduce some of the workload of the official reviewers:

  • build uses official shim 15.8 tarball in a source rpm. shasum of tarball matches official one. build does not use patches.

  • build is reproducible

  • shim shasum matches: e9e1d688ddd3924179a143e2714c64a058a60ee0699b964779b56b7ba42d66fc shimx64.efi

  • certificate:

    • valid for 10 years, 2048 bit RSA
    • self signed CA issued to Subject: C = CH, ST = Zurich, L = Zurich, CN = Virtuozzo Secure Boot CA, emailAddress = security@virtuozzo.com
  • shim sbat section looks appropriate:

    objcopy -j .sbat -O binary shimx64.efi /dev/stdout
    
    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.virtuozzo,1,Virtuozzo,shim,15.8,security@virtuozzo.com
    
  • grub and other sbat sections listed look appropriate

  • NX compat is disabled

    objdump -p review/shimx64.efi  | grep DllCharacteristics
    
    DllCharacteristics      00000000
    
  • signing keys:

    • at the question How do you manage and protect the keys used in your SHIM? you mention HashiCorp Vault.
    • are you using the enterprise version that has HSM support?

@AVasiljev
Copy link
Author

AVasiljev commented Apr 9, 2024

  • at the question How do you manage and protect the keys used in your SHIM? you mention HashiCorp Vault.
  • are you using the enterprise version that has HSM support?

No we are not it is installed on the encrypted VM in protected perimeter. And only limited personel have access to it.

@SherifNagy
Copy link
Collaborator

SherifNagy commented Apr 12, 2024

  • at the question How do you manage and protect the keys used in your SHIM? you mention HashiCorp Vault.
  • are you using the enterprise version that has HSM support?

No we are not it is installed on the encrypted VM in protected perimeter. And only limited personel have access to it.

Can you explain more in details? what is the security process, anything FIPS 140-2 level2 certified? what's the role of HashiCorp? put as much details as you can please.

seems like you are building based on RHEL9, do you provide UKI kernel? if so, what SBAT entries are in the efi?

@SherifNagy SherifNagy added question Reviewer(s) waiting on response new vendor This is a new vendor labels Apr 12, 2024
@AVasiljev
Copy link
Author

To provide secured access to private keys we have installed the VM with software defined encryption on its disk.
HashiCorp Vault is installed on that machine just to manage the keys. Certificates itself are freely accessible in artifacts storage.
Vault provide access to keys only to dedicated koji nodes during the building process.
Hope that is clarifies the situation with Vault.

As for UKI image we are still investigating whether it is necessary(It is built but not signed yet). However in case we do it would look like(based on what kernel guys now working on):

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
systemd,1,The systemd Developers,systemd,252,https://systemd.io/
systemd.rhel,1,Red Hat Enterprise Linux,systemd,252-18.el9,https://bugzilla.redhat.com/
systemd.virtuozzo,1,Virtuozzo,systemd,systemd-252-18.vl9,https://bugzilla.redhat.com/
linux,1,Red Hat,linux,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
linux.rhel,1,Red Hat,linux,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
linux.virtuozzo,1,Virtuozzo,5.14.0-362.18.1.vz9,https://bugzilla.redhat.com/
kernel-uki-virt.rhel,1,Red Hat,kernel-uki-virt,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
kernel-uki-virt.virtuozzo,1,Virtuozzo,kernel-uki-virt,5.14.0-362.18.1.vz9,https://bugzilla.redhat.com/

@SherifNagy
Copy link
Collaborator

@AVasiljev so you aren't using HSM to store the keys?

@AVasiljev
Copy link
Author

Currently not. LEt me know if it is must.

@SherifNagy
Copy link
Collaborator

Currently not. LEt me know if it is must.

It is actually https://techcommunity.microsoft.com/t5/hardware-dev-center/updated-uefi-signing-requirements/ba-p/1062916

@AVasiljev
Copy link
Author

Got. It. we will move to some YUbiKey based then(less convenient but whatever needed). Should I consider the previously used keys as compromised and reiisue the new ones and then resubmit or it is OK if I will just move existing keys to YUBIKEY?

@SherifNagy
Copy link
Collaborator

I would reissue new keys and new CA cert and keys in a secure environment just to be sure that nothing is compromised, I don't use hashicorp, but I think it does support using HSM so you can still build on top of you existing infrastructure, however, this is also up to you as long as you show a deep understanding of the security concepts applied along side knowing how bad this would be if it's compromised.

Some personal pointers " nothing here you -should-" do , just more of guide:

  • You can use the cheap yubikey and use multi certs " one cert for kernel, one for grub, one for uki ,etcc...." look into using the "retired PIV slots for this"
  • You can get the slightly expensive yubiHSM2, which works perfect, I have this working with sbsigntools, however I am sure there might be ways to get this to work with pesign since it still uses pkcs11 interface
  • You can use different certs per architecture, " set of certs for x86 and another set for aarch64 for example "
  • Access to the "securebuild sign instances " should be limited to whoever can build the secureboot packages"
  • Audit is important
  • Some vendors splits the CA's key between multiple parties, so no one single person can generate new certs/keys

However, make sure that whatever HSM you use is the FIPS certified one " will be slightly more expensive and in case of yubihsm2 you need to enable the FIPS mode"

MSFT requirements states that the whole environment / operation should be FIPS certified, however, not everyone can achieve this! and the feedback I got , do as much as you can to secure the keys and the build environment with the FIPS HSM , locked environment, monitored is the bare minimum

Hope this helps, let me know if you need any clarification

@AVasiljev
Copy link
Author

That is exactly what I have planned to do. Speaking frankly the idea of Hashicorp was to provide some kind of personless access to the tokens however since all the infrastructure is virtual in our company(we are hypervisor developers) probably a USB device connected directly to the building environment would something more appropriate. Probably I will need a bunch of days/weeks to reinvent signing process for us to ensure no person will have a signgle person access.

Is this OK I will continue that submission or we should open new one once the HSM will be implemented?

@SherifNagy
Copy link
Collaborator

Make sure that the secureboot building instances have proper security measure in place.

You can keep updating the same shim review, make sure to update the hash in the issue and the readme file, take your time, once you are really, we will pick up this review again for review

@AVasiljev
Copy link
Author

I'm lucky bastard. Our IT dept. did had the FIPS enabled Yubikey 4 in stock. Anyways I have requested more recent YubikeyHSM2 for next iteration. Now it is connected directly to the secure environment building grub/kernel/shim/etc. Reissued all the certificates and added those to Yubikey storage. Updated the submission repo. So if I haven't screwed up anywhere we can continue sir.

@SherifNagy
Copy link
Collaborator

Just to confirm, your new CA's key is stored in FIPS key, that shouldn't be easily accessible unless you want to generate new certs / keys for signing .efi files? in other words, I hope that the CA's key is not hosted in the same yubikey 4 that is plugged into the secure build env.

@AVasiljev
Copy link
Author

Nope that is different Yubikey which is not connected anywhere now.

@SherifNagy SherifNagy removed the question Reviewer(s) waiting on response label Apr 22, 2024
@SherifNagy
Copy link
Collaborator

@AVasiljev just to make things clear and avoid any confusion, could you tag the latest modification? seems like you modifications are in main without a tag at the moment, once you retag, update the issue with the tag you would it like it to be reviewed

@SherifNagy SherifNagy self-assigned this Apr 23, 2024
@AVasiljev
Copy link
Author

@SherifNagy done. Sorry missed this thing.

@SherifNagy
Copy link
Collaborator

Here I am again coming with more questions / notes:

  • Main security contacts keys aren't cross signed between each others
  • Second security contact "Denis" is RSA2048, which is a bit outdated
  • What's the relation of Denis to the Virtuozzo company?
  • Just to keep things tidy, please update the UKI entries into the issue template and retag

Would difficult would be to sort out 1st and 2nd note?

@SherifNagy SherifNagy added the question Reviewer(s) waiting on response label Apr 26, 2024
@AVasiljev
Copy link
Author

Hey @SherifNagy Will certainly do. Denis is head of Kernel/HyperVisor development. So will reach him once he will come back from PTO(early May is always time for vacations here).

@AVasiljev
Copy link
Author

@SherifNagy Denis renewed his key. NOt often uses the signature AFAIK. And of course we crossigned the keys and published to ubuntu server. Is that OK?

@SherifNagy
Copy link
Collaborator

@AVasiljev great, can you just update and retag the review? don't forget to add the UKI info as well. Once this is done, I will run a quick contact verification and go ahead with the review.

One clarification about Denis's relationship toe Virtuozzo, when you said Head of kernel/HyperVisor development, you mean he is already employed by virtuozzo?

@AVasiljev
Copy link
Author

@SherifNagy I have added my estimation for SBAT section to Readme but currently it is not built.

@AVasiljev
Copy link
Author

As for Denis - he is one of co-founders of the company and working for company since it was only OpenVZ product like early 2000s. Now we have both commercial Virtuozzo and OpenVZ in place. And he is still main developer for Kernel/Hypervisor.

@AVasiljev
Copy link
Author

@SherifNagy Just to ensure You have seen that. I have retagged the proper commit. I'm not sure if my UKI answer suits. But that is the state of art now. If necessary to build this immediately signed - please let me know.

@SherifNagy
Copy link
Collaborator

Saw it, just a bit swamped with other things, will get back to you hopefully later in the day

@AVasiljev
Copy link
Author

No worries, just wanted to be sure no action is expected from me. That is my first submission and I'm pretty unsure on if I'm doing proper stuff sir.

@AVasiljev
Copy link
Author

Yikes. Will tommorow. Our new SafeNet FIPS key will be installed tommorow.

@AVasiljev
Copy link
Author

@SherifNagy I have reissued thecertificates and those are now stored on the newly requested SafeNet key. I have created 10 years valid cert so should be no issue now.

@steve-mcintyre steve-mcintyre added the contact verification needed Contact verification is needed for this review label May 27, 2024
@steve-mcintyre
Copy link
Collaborator

@AVasiljev Denis' key is not capable of encryption, which means we can't do the encrypted verification step. Please ask him to regenerate again, or add an encryption-capable subkey,

@AVasiljev
Copy link
Author

@steve-mcintyre We have issued the subkey. https://keyserver.ubuntu.com/pks/lookup?search=0BAEAA87D3020ADC1150E51F5E0771B6CB666CAB&fingerprint=on&op=index

@steve-mcintyre
Copy link
Collaborator

Verification mails sent now

@steve-mcintyre steve-mcintyre added 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 May 29, 2024
@AVasiljev
Copy link
Author

Artem's Vasiliev:

aspire scull adulthood elicit Severus kittenish circumcising homesickness decays assuring

@dlunev
Copy link

dlunev commented May 30, 2024

Denis V. Lunev:
intranet wordings granulates garrulously linen Lanka pompoms gorgeously structures

@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 May 30, 2024
@SherifNagy
Copy link
Collaborator

SherifNagy commented Jun 3, 2024

Review of virtuozzo-shim-x86_64-20240401

  • Virtuozzo has their own RHEL like kernel and signed kernels in the paste
  • Security contacts looks good, however, better crossing would be ideal
  • Keys are stored in HSM and exposed with HashiCorp Vault

Shim

  • Uses upstream 15.8 and source hashes matches original hashes
  • SBAT entries from shim looks fine
  • Vendor SBAT entry is at 1
  • Binaries are reproducible using the container image, however, There is a copy / past error, the Issue here using an older hash while the tag uses the correct hash
fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  ./usr/share/shim/15.8-4.vl9/x64/shimx64.efi
fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  /shimx64.efi

I think MSFT do review the sha256sum hashes of the binaries thought " Vendor needs to update the issue to match the readme "

  • NX flag is not set, because the chain is not yet ready
  • Self signed 4096 bit cert and valid for 10 years

GRUB2

  • SBAT looks fine (keeps upstream RHEL grub2)
  • Version currently does not include NTFS patches, but the signed versions also not include the NTFS module
  • Module list sound fine
  • Grub patches looks fine to me, however another set of eyes on those would be great!

Kernel

LGTM, we will need one more reviewer

@SherifNagy SherifNagy added extra review wanted Initial review(s) look good, another review desired easy to review This submission might be a good place to start for an inexperienced reviewer labels Jun 3, 2024
@SherifNagy
Copy link
Collaborator

Actually since this is new vendor, we will need two extra reviews including one of the trusted reviewers

@AVasiljev
Copy link
Author

@SherifNagy Should we remove the BUG flag?

@SherifNagy
Copy link
Collaborator

@AVasiljev You need to edit the issue and fix the shim hash, it doesn't match what's inside the README in the tag as I mentioned

Binaries are reproducible using the container image, however, There is a copy / past error, the Issue here using an older hash while the tag uses the correct hash

@AVasiljev
Copy link
Author

AVasiljev commented Jun 14, 2024

@SherifNagy Ouch. Fixed. Sorry.

@SherifNagy SherifNagy removed the bug Problem with the review that must be fixed before it will be accepted label Jun 17, 2024
@steve-mcintyre
Copy link
Collaborator

Review of Shim for Virtuozzo Linux

OK

  • Contact verification done - OK
  • Builds from 15.8 upstream, with no patches - OK
  • NX bit not set - OK
  • Shim has not been signed before, so no need for any revocations here.
  • shim build reproduces here for x64 - OK
  fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  shimx64.efi
  • key management using an HSM - OK
  • Kernel modules signed with ephemeral key - OK
  • Includes a CA cert, expiring in May 2034 - OK
  Serial Number:
      17:58:2a:07:bb:70:5c:7a:7e:1f:96:db:8c:a3:ec:9f:57:03:9f:a9
  Signature Algorithm: sha256WithRSAEncryption
  Issuer: C = CH, ST = Zurich, L = Zurich, CN = Virtuozzo Secure Boot CA, emailAddress = security@virtuozzo.com
  Validity
      Not Before: May 25 01:54:15 2024 GMT
      Not After : May 23 01:54:15 2034 GMT
  Subject: C = CH, ST = Zurich, L = Zurich, CN = Virtuozzo Secure Boot CA, emailAddress = security@virtuozzo.com
  • SBAT data looks fine for shim, GRUB and (maybe future) UKI - OK
  • Using GRUB 2.06-70 straight from RHEL 9, so looks OK for fixes and
    patches and module list - OK.
  • kernel 5.14.0-425.vz9 with lockdown patches - OK

Issues / queries

  • One minor issue in the fwupd-efi SBAT. You claim to be using 1.8.16 in
    your code, but the upstream version for fwupd-efi is still listed as
    1.4. Not blocking on this as it's only informational, but please
    double-check and maybe fix this in future.

@steve-mcintyre
Copy link
Collaborator

One more review needed

@dennis-tseng99
Copy link
Collaborator

review for virtuozzo-shim-x86_64-20240401

  • code is reproducible (ok)
Step 20/20 : RUN sha256sum ./usr/share/shim/15.8-4.vl9/x64/shimx64.efi /shimx64.efi
 ---> Running in 28ac2fb843d2
fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  ./usr/share/shim/15.8-4.vl9/x64/shimx64.efi
fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  /shimx64.efi
Removing intermediate container 28ac2fb843d2
---> 6de0d9e05312
  • Hash is matched (ok)
#sha256sum shimx64.efi
fa7866274689d8cec54380183e6ebf46fa79c14bf39f4f88c6169a6c9de7834c  shimx64.efi
  • NX flag is disable: (ok)
#objdump -x shimx64.efi | grep -E 'SectionAlignment|DllCharacteristics'
SectionAlignment        00001000
DllCharacteristics      00000000
  • sbat seems ok for shim and grub
objcopy --only-section .sbat -O binary shimx64.efi /dev/stdout
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.virtuozzo,1,Virtuozzo,shim,15.8,security@virtuozzo.com

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md 
grub,3,Free Software Foundation,grub,2.02,https//www.gnu.org/software/grub/ 
grub.rh,2,Red Hat,grub2,2.06-70.el9,mailto:secalert@redhat.com 
grub.virtuozzo,1,Virtuozzo,grub2,2.06-70.vl9,mailto:security@virtuozzo.com
  • use an ephemeral key for signing kernel modules (good)

  • Certificate Validity: 10 years + 4096 bit size for RSA Public-Key (good)

openssl x509 -in secureboot-ca-x86_64.cer -inform der -noout -text
Certificate:
    .............
    Validity
        Not Before: May 25 01:54:15 2024 GMT
        Not After : May 23 01:54:15 2034 GMT
    ..............    
    Subject Public Key Info:
        Public Key Algorithm: rsaEncryption
            RSA Public-Key: (4096 bit)    
  • 3 propritary patches (ok)
    All patches are related to grub menu UTF-8 coding which might not impact reviewing.

  • Conclusion
    Everything looks fine. Hi @steve-mcintyre, could we put "accept" tag on it ?

@steve-mcintyre
Copy link
Collaborator

3 good reviews, accepting

@steve-mcintyre steve-mcintyre added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired labels Jun 18, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jul 29, 2024

@AVasiljev did you get a signed shim back?

@AVasiljev
Copy link
Author

AVasiljev commented Jul 29, 2024 via email

@THS-on THS-on closed this as completed Jul 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 contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

7 participants