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

VMware Photon OS shim-15.8 #412

Closed
8 tasks done
nkkuntal opened this issue Apr 19, 2024 · 8 comments
Closed
8 tasks done

VMware Photon OS shim-15.8 #412

nkkuntal opened this issue Apr 19, 2024 · 8 comments
Labels
accepted Submission is ready for sysdev

Comments

@nkkuntal
Copy link

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/nkkuntal/shim-review/tree/vmware-shim-x86_64-20240418


What is the SHA256 hash of your final SHIM binary?


$ sha256sum shimx64.efi
3c644e2d1f4449fa761c540615f2b59178639ae2fa74c493de71b951afc80e11  shimx64.efi

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


Previous version, based on shim 15.4, was approved here #164


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 as security contacts are not verified recently.

@steve-mcintyre steve-mcintyre added the easy to review This submission might be a good place to start for an inexperienced reviewer label Apr 22, 2024
@BogdanAriton
Copy link

BogdanAriton commented May 18, 2024

I'm not an official reviewer, but I'm trying to help as much as I can. This review if for: vmware-shim-x86_64-20240418.

  • Shim binary can be reproduced using the Dockerfile and matches the Hash supplied in the README.md:

3c644e2d1f4449fa761c540615f2b59178639ae2fa74c493de71b951afc80e11 ./shimx64.efi

  • Shim sbat section looks good
    objcopy --only-section .sbat -O binary ./shimx64_new.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.photon,1,VMware Photon OS,shim,15.8-1.ph5,https://github.com/vmware/photon)
    objdump ./shimx64_new.efi -s -j .sbatlevel

        ./shimx64_new.efi:     file format pei-x86-64

        Contents of section .sbatlevel:
        84000 00000000 08000000 37000000 73626174  ........7...sbat
        84010 2c312c32 30323330 31323930 300a7368  ,1,2023012900.sh
        84020 696d2c32 0a677275 622c330a 67727562  im,2.grub,3.grub
        84030 2e646562 69616e2c 340a0073 6261742c  .debian,4..sbat,
        84040 312c3230 32343031 30393030 0a736869  1,2024010900.shi
        84050 6d2c340a 67727562 2c330a67 7275622e  m,4.grub,3.grub.
        84060 64656269 616e2c34 0a00               debian,4..   
  • NX bit not set and section alignment looks OK:
    objdump -fhp ./shimx64.efi | egrep '(SectionAlignment|DllCharacteristics)'
    SectionAlignment        00001000
    DllCharacteristics      00000000
  • photon_sb2020.der is loaded directly as part of shim via VENDOR_CERT_FILE valid until Jan 1 06:59:59 2030 GMT, 2048 bit RSA
    openssl x509 -inform der -in photon_sb2020.der -text -noout
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            1f:54:cd:7b:51:42:53:99:4c:d5:9a:fc:74:93:e6:0e
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = "VMware, Inc.,O=VMware, Inc.,L=Palo Alto,S=California,C=US"
        Validity
            Not Before: Jul  8 01:53:00 2020 GMT
            Not After : Jan  1 06:59:59 2030 GMT
        Subject: CN = "VMware, Inc.,O=VMware, Inc.,L=Palo Alto,S=California,C=US"
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:9e:24:c6:a5:8d:fe:95:58:47:45:c8:26:51:46:
                    f9:5e:2e:f0:89:28:52:5d:d0:aa:cd:7a:e8:58:6f:
                    4c:50:6b:88:be:90:3d:7c:de:90:74:a9:ad:13:ed:
                    87:5a:4d:bc:59:aa:64:73:8c:64:2a:22:00:c4:4c:
                    2c:53:64:21:a2:09:ed:8f:aa:f8:14:45:53:e9:69:
                    0b:8b:b1:b3:d7:64:a4:de:6c:30:72:27:02:8a:f8:
                    f2:86:d9:04:0f:2f:31:fb:a5:1a:18:ff:7a:2d:ea:
                    ab:2d:b1:52:4a:04:60:fd:87:e3:2d:37:6c:62:ee:
                    99:d7:9f:3d:c3:43:51:f3:91:c5:68:b5:33:ec:b9:
                    90:b5:6b:97:dd:10:25:d8:8a:fa:49:a9:2e:05:44:
                    fc:b5:14:ec:41:28:59:86:18:96:61:cc:61:7c:fb:
                    1e:ab:e0:fb:a8:a6:10:21:cb:5a:89:47:83:0f:7c:
                    32:a7:91:96:d5:73:e1:9e:7a:12:83:ba:b8:0d:2a:
                    ab:3b:0e:71:b7:7a:b1:c2:54:1d:27:d6:0b:ab:24:
                    11:c4:4d:f8:55:07:86:67:67:29:28:88:62:4d:4d:
                    c4:49:fb:cb:88:82:b8:69:7c:a7:66:bf:6b:10:a0:
                    c1:ae:f2:5a:ed:f2:d1:67:89:8f:d2:64:d9:03:d2:
                    5b:17
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Key Identifier: 
                5F:07:9E:DA:AA:73:76:13:3F:04:5B:9A:D7:29:97:DD:F3:77:78:58
    Signature Algorithm: sha256WithRSAEncryption
    Signature Value:
        3d:8d:99:ee:5a:8c:1f:94:72:5d:aa:f1:c0:67:66:7c:d1:1e:
        e4:50:f7:15:d1:c5:e8:92:4e:7c:4e:de:a8:cf:26:09:d0:15:
        d4:46:27:c3:ad:a4:7c:47:f4:7a:cb:db:49:20:a2:95:57:9b:
        98:49:22:50:91:5d:4c:84:8b:8e:1c:61:86:df:f9:d3:f2:1f:
        3a:4b:57:25:bf:4f:7e:5f:09:5d:1c:52:39:50:14:04:a0:69:
        33:a5:f1:8e:b1:11:72:83:77:a8:50:82:1d:e3:ba:e6:10:a9:
        3a:9f:9f:95:fd:9a:7f:76:43:f3:23:b9:18:9b:2a:26:16:c1:
        30:bb:01:b2:8f:2b:99:63:09:da:bc:20:8d:5c:d6:78:55:00:
        8e:87:3d:54:5d:da:15:7c:26:14:a2:a3:31:33:9c:22:a2:04:
        95:d2:6a:6b:dd:d7:44:a9:bd:88:45:f0:39:65:f7:ac:66:d4:
        66:f7:b2:27:10:e8:f0:3d:b1:fe:b2:60:1b:7c:e2:bd:93:0e:
        35:0e:5a:b9:7a:3e:61:62:36:ba:fd:c7:01:13:e8:50:7b:2a:
        e8:73:04:52:a9:3a:c4:d9:03:90:c8:d6:4f:1c:aa:c3:14:d9:
        9e:3c:dc:3e:a5:f3:c1:69:e3:6c:51:07:93:74:a9:b4:c6:f4:
        91:09:6f:f6
  • There are additional 3 Shim patches.

    • 0001-Add-provision-to-disable-netboot-and-httpboot-in-shi.patch is clear and I don't see any issues here
    • 0001-Enforce-SBAT-presence-in-every-image.patch is clear and I don't see any issues here
    • In regards to patch "0001-Introduce-support-for-revocations-build.patch", could you explain (at least for my benefit) how this works?
  • Linux version v6.1.90 includes the necessary security patches

  • I'm in the process of reviewing the 3 security patches that help with secure boot and kernel lock-down for version 6.1.90 (specified in the readme), I couldn't find other references for these patches outside of VMWare Linux Photon. There are a lot more patches under the linux/generic folder and not sure if we should review all of these and whether if all the patches are still relevant (since some of them seem to be valid for older Linux versions)

    • 0001-kernel-lockdown-when-UEFI-secure-boot-enabled.patch - Looks like the new added definition CONFIG_LOCK_DOWN_KERNEL_IN_SECUREBOOT helps identify if secure boot is present and then calls the lock_kernel_down kernel function (this is achieved pretty much the same way by CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY but this will not check for secure boot and will lock down kernel either way)
    • I don't see issues with either of these patches 0002-Add-.sbat-section.patch and 0003-Verify-SBAT-on-kexec.patch , but maybe someone else could also take a glance.

@nkkuntal
Copy link
Author

Thank you @BogdanAriton for reviewing the shim-review.

The patch 0001-Introduce-support-for-revocations-build.patch utilizes the following code in shim.c: the function load_revocations_file() reads the provided revocations.efi, if it exists. In the case of valid .sbata and .sbatl section data, it updates the SbatLevel EFI variable by calling set_sbat_uefi_variable().

Consider a use case where Photon OS wants to revoke its grub2/kernel using SBAT. We could inject the aforementioned sections into revocations.efi generated with this patch. During the next boot, shim will update the SbatLevel and enforce the newly updated levels.

By utilizing this mechanism, we avoid changing SbatLevel in the shim source include/sbat_var_defs.h and avoid getting a new shim signed to revoke vendor binaries grub.photon and linux.photon.

@BogdanAriton
Copy link

Thank you @BogdanAriton for reviewing the shim-review.

The patch 0001-Introduce-support-for-revocations-build.patch utilizes the following code in shim.c: the function load_revocations_file() reads the provided revocations.efi, if it exists. In the case of valid .sbata and .sbatl section data, it updates the SbatLevel EFI variable by calling set_sbat_uefi_variable().

Consider a use case where Photon OS wants to revoke its grub2/kernel using SBAT. We could inject the aforementioned sections into revocations.efi generated with this patch. During the next boot, shim will update the SbatLevel and enforce the newly updated levels.

By utilizing this mechanism, we avoid changing SbatLevel in the shim source include/sbat_var_defs.h and avoid getting a new shim signed to revoke vendor binaries grub.photon and linux.photon.

@nkkuntal - thanks for clarifying that for me

@dennis-tseng99
Copy link
Collaborator

Hi @nkkuntal, Because the global generation number has been bumped to 4 from 1, may I ask you a question about whether the product-specific minimum generation number is 1 or 2 in README.md:

grub2

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.photon,2,VMware Photon OS,grub2,2.06-16.ph5,https://github.com/vmware/photon

I know your photon OS has been upgraded from 3.0 to 5.0 which can perfectly configure MACVLAN, VxLAN, macvtap .... I'm not sure this is your reason for 2.

In your previous shim-15.4 review:

grub2

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,1,Free Software Foundation,grub,2.06~rc1,https//www.gnu.org/software/grub/
grub.photon,1,VMware Photon OS,grub2,2.06~rc1-1.ph4,https://github.com/vmware/photon/tree/4.0/SPECS/grub2/

Please correct me if I'm wrong. Thank you very much.

@vbrahmajosyula1
Copy link

Hi @dennis-tseng99

Thanks for taking time to review.

We chose to keep grub.photon to gen 2 as an indicator that our grub2 has transitioned from upstream grub2 to fedora downstream grub2.

@dennis-tseng99
Copy link
Collaborator

Review for VMware Photon OS shim-15.8 #412 based on tag vmware-shim-x86_64-20240418

  • Binaries are producible by running "docker build ." (Ok)
    .............
    ---> 2e97eb30b7a5
    Successfully built 2e97eb30b7a5
  • Hash value is matched (ok)
    3c644e2d1f4449fa761c540615f2b59178639ae2fa74c493de71b951afc80e11 /shimx64.efi
    3c644e2d1f4449fa761c540615f2b59178639ae2fa74c493de71b951afc80e11 /usr/share/shim/shimx64.efi
  • sbat looks good
    objcopy --only-section .sbat -O binary /usr/share/shim/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.photon,1,VMware Photon OS,shim,15.8-1.ph5,https://github.com/vmware/photon
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.photon,2,VMware Photon OS,grub2,2.06-16.ph5,https://github.com/vmware/photon
  • ephemeral key is used for kernel signing (good)

  • NX is disabled (Ok)
    objdump -x shimx64.efi | grep -E 'SectionAlignment|DllCharacteristics'
    SectionAlignment 00001000
    DllCharacteristics 00000000

  • Certificate Validity (Ok)
    10 years is good enough
    2048 bits long is good enough before 2030. NITST deems RSA 2048 sufficient until 2030.

openssl x509 -in photon_sb2020.der -inform der -noout -text

Validity
            Not Before: Jul  8 01:53:00 2020 GMT
            Not After : Jan  1 06:59:59 2030 GMT
Subject: CN = "VMware, Inc.,O=VMware, Inc.,L=Palo Alto,S=California,C=US"
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                RSA Public-Key: (2048 bit)
  • comments:
    Marked as an easy to review
    This issue also reviewed by BogdanAriton
    Previous approved reviewing is here - VMware Photon OS shim-15.4 #164
    The usage of patch 0001-Introduce-support-for-revocations-build.patch is explained.
    VMware has done a good job for reviewing.
    Accepting.

@dennis-tseng99 dennis-tseng99 added accepted Submission is ready for sysdev and removed easy to review This submission might be a good place to start for an inexperienced reviewer labels May 29, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jul 30, 2024

@nkkuntal did you get a signed shim back?

@vbrahmajosyula1
Copy link

vbrahmajosyula1 commented Jul 30, 2024

Sorry we forgot to mention it here. We ran into a logistical issue and were stuck for sometime. A couple of weeks ago we got the signed shim back. Thanks every one who helped review this.

@THS-on THS-on closed this as completed Jul 30, 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
Projects
None yet
Development

No branches or pull requests

6 participants