rhboot/shim-review

Shim for Virtuozzo Linux

Closed this issue · 44 comments

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

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?
  • 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.

  • 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?

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/

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

Currently not. LEt me know if it is must.

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?

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

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?

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

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.

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.

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

@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 done. Sorry missed this thing.

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?

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).

@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?

@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?

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

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.

@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.

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

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 I think your SecurebootCA already expired,

            Not Before: Apr 21 02:22:50 2024 GMT
            Not After : May 21 02:22:50 2024 GMT

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

@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.

@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,

Verification mails sent now

Artem's Vasiliev:

aspire scull adulthood elicit Severus kittenish circumcising homesickness decays assuring

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

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

  • Ephemeral keys are used for signing kernel modules
  • Lockdown patches are included (keeps upstream RHEL kernel)
  • UKI's isn't provided, however they did provide what the SBAT will look like, will keep that logged at #397

LGTM, we will need one more reviewer

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

@SherifNagy Should we remove the BUG flag?

@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

@SherifNagy Ouch. Fixed. Sorry.

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.

One more review needed

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 ?

3 good reviews, accepting

@AVasiljev did you get a signed shim back?