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.

Sign up for GitHub

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

Jump to bottom

Shim 15.8 for Mariner 2.0 #387

Closed
8 tasks done
ddstreetmicrosoft opened this issue Feb 21, 2024 · 21 comments
Closed
8 tasks done

Shim 15.8 for Mariner 2.0 #387

ddstreetmicrosoft opened this issue Feb 21, 2024 · 21 comments
Labels
accepted Submission is ready for sysdev

Comments

@ddstreetmicrosoft
Copy link

ddstreetmicrosoft commented Feb 21, 2024 •
edited
Loading

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/microsoft/shim-review/tree/cbl-mariner-shim-x64-20240221

Add missing shim sbat in README: https://github.com/microsoft/shim-review/tree/cbl-mariner-shim-x64-20240222

What is the SHA256 hash of your final SHIM binary?

82ede1584e7de1347446a241f09c05cf3efc134206c446be3d548a748e4752e3

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

#223
@aronowski
Copy link
Collaborator

Note for reviewers: please see rhboot/shim#634 in regard to the SBAT generation numbers provided in the application.

@eduardacatrinei
Copy link

Hey @ddstreetmicrosoft, it looks like you forgot to mention shim sbat in section of the readme.

@ddstreetmicrosoft
Copy link
Author

Hey @ddstreetmicrosoft, it looks like you forgot to mention shim sbat in section of the readme.

Ah ok sorry, I had interpreted "...booting or planning to boot directly through shim" as meaning "...booting directly through shim, or planning to boot directly through shim" but I assume it actually was intended to mean "...booting (or planning to boot) directly from UEFI firmare or directly from shim". Might be clearer to just say "Please provide exact SBAT entries for all shim binaries as well as all SBAT binaries that shim will directly boot."

I added the shim sbat and pushed a new tag (with today's date). thanks!

@ddstreetmicrosoft
Copy link
Author

Hey @ddstreetmicrosoft, it looks like you forgot to mention shim sbat in section of the readme.

Ah ok sorry, I had interpreted "...booting or planning to boot directly through shim" as meaning "...booting directly through shim, or planning to boot directly through shim" but I assume it actually was intended to mean "...booting (or planning to boot) directly from UEFI firmare or directly from shim". Might be clearer to just say "Please provide exact SBAT entries for all shim binaries as well as all SBAT binaries that shim will directly boot."

I added the shim sbat and pushed a new tag (with today's date). thanks!

opened #388 to clarify the question wording (assuming I understand it correctly now)

@ddstreet
Copy link
Contributor

Hello, just checking if anything is needed for this review. Thanks!

@aronowski aronowski self-assigned this Mar 21, 2024
@es-fabricemarie
Copy link

es-fabricemarie commented Apr 8, 2024 •
edited
Loading

I'm not an official reviewer, just trying to help with the official reviewers load.

  • build reproduces fine
  • SHA256 of shim matches (82ede1584e7de1347446a241f09c05cf3efc134206c446be3d548a748e4752e3)
  • Issues with the certificates (they are not part of the shim or shim-review repos):
    • Found mentioned expired certificate cbl-mariner-ca-20211013.pem
    • Could not find new cbl-mariner-ca-20230216.der/pem certificate to inspect it.

Expired cbl-mariner-ca-20211013.pem certificate:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            33:00:00:04:ee:7f:94:de:8d:43:27:a5:1f:00:01:00:00:04:ee
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = US, ST = Washington, L = Redmond, O = Microsoft Corporation, CN = Microsoft Testing PCA 2010
        Validity
            Not Before: Oct 14 17:28:05 2021 GMT
            Not After : Oct 13 17:28:05 2022 GMT
        Subject: C = US, ST = Washington, L = Redmond, O = Microsoft Corporation, CN = Mariner Secure Boot(Production Signing)
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:c5:e3:98:53:1c:f4:00:03:bc:9c:ea:0d:e2:56:
                    ea:9c:40:a1:79:d7:29:42:ca:78:d4:c9:26:2a:a9:
                    08:5a:da:c4:33:13:bb:5b:27:67:a0:56:45:b8:71:
                    f6:35:51:b2:6c:9b:ae:62:db:f5:46:23:53:8b:66:
                    26:b8:b1:1c:17:44:de:ed:a1:12:b9:68:49:58:7e:
                    9b:20:84:c8:06:11:a0:92:3e:ce:bd:d7:90:55:a8:
                    ee:16:5f:48:f1:5e:cc:2a:45:9d:82:3c:7f:9e:bc:
                    d0:82:85:09:ce:aa:d8:ef:14:2a:a9:ed:a7:90:e3:
                    86:ff:0e:96:66:68:c8:9c:2f:f0:f0:6c:8e:3d:db:
                    91:d6:19:26:1a:fb:f3:29:d4:5b:37:c8:68:4f:e5:
                    7e:f9:f9:cf:6f:c3:88:09:e4:aa:31:70:e1:51:99:
                    01:2d:d1:ce:e5:39:c9:4e:8b:93:69:e6:3c:ac:ee:
                    5a:eb:76:63:cf:6e:3a:00:2c:44:41:6a:d8:97:8f:
                    cc:3b:d8:b5:19:bb:8f:88:f7:bd:3f:ea:3a:8d:12:
                    41:de:1b:1a:95:48:f4:51:34:81:19:b9:b8:ef:44:
                    87:e0:f2:e5:0b:af:c9:3f:2a:3f:13:6f:39:c2:5a:
                    81:ae:69:3f:fd:ca:85:5b:1e:1a:94:29:03:09:c7:
                    11:6b
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Extended Key Usage: 
                Code Signing
            X509v3 Subject Key Identifier: 
                B4:2D:6B:9E:28:09:B7:B9:09:7C:FF:9E:9B:01:94:B8:6E:80:BF:95
            X509v3 Subject Alternative Name: 
                DirName:/OU=Microsoft Corporation/serialNumber=460897\+468597
            X509v3 Authority Key Identifier: 
                BF:65:A2:AB:6F:75:A3:4E:45:96:57:05:CF:39:87:F4:C0:15:1C:1C
            X509v3 CRL Distribution Points: 
                Full Name:
                  URI:http://www.microsoft.com/pkiops/crl/Microsoft%20Testing%20PCA%202010(1).crl
            Authority Information Access: 
                CA Issuers - URI:http://www.microsoft.com/pkiops/certs/Microsoft%20Testing%20PCA%202010(1).crt
            X509v3 Basic Constraints: critical
                CA:FALSE
    Signature Algorithm: sha256WithRSAEncryption
    Signature Value:
        b2:6e:eb:fa:92:68:53:db:2f:7b:14:e2:d1:96:32:c2:2e:f4:
        25:04:bf:c4:77:12:8f:0d:83:45:84:70:8a:21:47:79:e7:4c:
        94:a1:45:bc:5c:8c:52:61:af:83:9b:1f:f5:f8:d6:12:e0:dc:
        6a:97:b7:9c:b9:1e:20:f7:ee:22:d0:33:a6:36:36:0e:35:8f:
        1e:a4:fb:29:22:98:d4:77:d3:80:88:a3:4a:25:2b:36:df:4d:
        e2:d9:34:28:8d:74:1c:65:57:93:3b:cf:5b:2b:7a:57:fa:ca:
        68:00:21:ae:11:51:2e:59:27:4b:ab:ea:0f:0d:86:70:35:32:
        b2:a1:b8:fd:c0:76:0e:e9:65:c9:7d:c7:4b:3e:56:19:82:10:
        e3:47:f5:8d:3b:96:f3:bf:3a:62:da:79:ff:73:83:98:bc:c8:
        a1:2c:da:b4:e6:e3:4e:d0:80:7f:ce:ab:b2:01:a0:8a:6e:2d:
        79:bf:f3:ea:62:8b:35:06:c4:fe:61:fb:78:cd:ff:2b:cb:5e:
        f2:15:50:48:a8:93:1a:d8:09:fa:19:ec:7b:48:d5:a3:8e:3d:
        52:ee:e0:60:6b:7a:19:71:31:ef:47:cc:6f:dd:f9:9b:c1:f4:
        0c:22:b6:51:99:9a:ea:f3:19:32:4b:14:15:ef:17:9a:d2:c1:
        3b:5f:fa:49
  • did not review/comment on grub2 sbat levels/patches.
  • shim sbat version looks correct:
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.mariner,1,Microsoft,shim,15.8-1.cm2,https://github.com/microsoft/CBL-Mariner

  • NX compat is disabled:

    objdump -p shimx64.efi  | grep DllCharacteristics

    DllCharacteristics      00000000

@aronowski
Copy link
Collaborator

aronowski commented Apr 8, 2024 via email

@ddstreetmicrosoft
Copy link
Author

I'll send verification emails soon

The verification email to me decrypted to the words:

serotonin aflame surgery calorie cosmos subdivide attempt implicate calculate
nursing

@dennis-tseng99
Copy link
Collaborator

dennis-tseng99 commented Apr 9, 2024 •
edited
Loading

@ddstreet Just remind you that the pem format of certificate file cbl-mariner-ca-20211013.pem found in the review of @es-fabricemarie is not allowed according to https://github.com/rhboot/shim-review/pull/402/files

Besides, I found shim-15.7 is also included in db.x64.esl file. Is it useful ?

sig-list-to-certs db.x64.esl db.x64
-rw-r--r-- 1 dennis users 1290 Apr  9 16:04 db.x64-0.der
-rw-r--r-- 1 dennis users 1722 Apr  9 16:04 db.x64-1.der
-rw-r--r-- 1 root   root  3100 Apr  9 16:03 db.x64.esl
drwxr-xr-x 1 dennis users  224 Apr 11  2023 shim-15.7

@aronowski aronowski added the contact verification needed Contact verification is needed for this review label Apr 9, 2024
@ddstreetmicrosoft
Copy link
Author

@ddstreet Just remind you that the pem format of certificate file cbl-mariner-ca-20211013.pem found in the review of @es-fabricemarie is not allowed according to https://github.com/rhboot/shim-review/pull/402/files

That isn't in the shim, so unless I'm missing something, it's not at all relevant to this shim review.

Besides, I found shim-15.7 is also included in db.x64.esl file. Is it useful ?

sig-list-to-certs db.x64.esl db.x64
-rw-r--r-- 1 dennis users 1290 Apr  9 16:04 db.x64-0.der
-rw-r--r-- 1 dennis users 1722 Apr  9 16:04 db.x64-1.der
-rw-r--r-- 1 root   root  3100 Apr  9 16:03 db.x64.esl
drwxr-xr-x 1 dennis users  224 Apr 11  2023 shim-15.7

Again, unless I'm missing something, it's not in the esl; probably you just had it in your filesystem already?

$ ls -la
total 12
drwxrwxr-x 2 ddstreet ddstreet 4096 Apr  9 08:44 .
drwxrwxr-x 3 ddstreet ddstreet 4096 Apr  9 08:44 ..
-rw-r----- 1 ddstreet ddstreet 3100 Apr  9 08:44 db.x64.esl

$ sig-list-to-certs db.x64.esl cert
X509 Header sls=1334, header=0, sig=1290
file cert-0.der: Guid f4de3b90-399b-4eb0-aa3f-041c434a2de3
Written 1290 bytes
X509 Header sls=1766, header=0, sig=1722
file cert-1.der: Guid f4de3b90-399b-4eb0-aa3f-041c434a2de3
Written 1722 bytes

$ ls -la
total 20
drwxrwxr-x 2 ddstreet ddstreet 4096 Apr  9 08:44 .
drwxrwxr-x 3 ddstreet ddstreet 4096 Apr  9 08:44 ..
-rw-rw-r-- 1 ddstreet ddstreet 1290 Apr  9 08:44 cert-0.der
-rw-rw-r-- 1 ddstreet ddstreet 1722 Apr  9 08:44 cert-1.der
-rw-r----- 1 ddstreet ddstreet 3100 Apr  9 08:44 db.x64.esl

@ddstreetmicrosoft
Copy link
Author

I'll send verification emails soon, since the contact pair has changed. I'll
need to do this for both contacts, as that's the process.

Just FYI, @christopherco has just left for extended vacation so unfortunately it's unlikely he will be able to handle the verification email until he is back; if the review does require verification from him also (as well as the verification I already provided) can we at least get through all the rest of the review so this can be approved right away once he gets back?

@aronowski
Copy link
Collaborator

Looks like some errors happened that shall be clarified.

The comment I sent earlier through an e-mail did not get formatted by GitHub as Markdown properly. Despite that, the text itself mentions that the ESL certificates match the ones provided as part of the application, i.e. they are DER-formatted. Simply running binwalk, like I mentioned, shall print a hint about them:

[...]
DECIMAL       HEXADECIMAL     DESCRIPTION
--------------------------------------------------------------------------------
44            0x2C            Certificate in DER format (x509 v3), header length: 4, sequence length: 1286
1378          0x562           Certificate in DER format (x509 v3), header length: 4, sequence length: 1718

Just FYI, @christopherco has just left for extended vacation so unfortunately it's unlikely he will be able to handle the verification email until he is back; if the review does require verification from him also (as well as the verification I already provided) can we at least get through all the rest of the review so this can be approved right away once he gets back?

Yes, I'll kindly do further review and learn how the ESL embedding into a shim binary behaves, but that will take me a while to recreate a laboratory I made some time ago.

I understand that the PGP key hasn't changed, but I'm sticking to the current process, where both contacts need to be verified like here - I was supposed to type the decrypted random words, although my key has been and still is the same.

@ddstreetmicrosoft ddstreetmicrosoft mentioned this issue Apr 10, 2024
Merged
12 tasks
@aronowski
Copy link
Collaborator

@ddstreetmicrosoft, as promised, I'm preparing a laboratory to check the ESL things out in the context of the whole bootloader chain working.

However, I couldn't get Secure Boot to work - when attempting to boot, I immediately get a security violation error.

As part of recreating this setup, I generated two separate CAs with their corresponding certificates and private keys (used a date hooking library to more-or-less recreate their application-equivalent actual validity periods). I merged them into an .esl file and built an unsigned shim, which has this file embedded into the binary.

I then manually signed the shim binary with my own db-related assets akin to:

$ sudo sbsign --cert db.crt --key db.key shimx64.efi

and then attempted to rebuild the RPM with a self-signed shim, which should work with my own EFI signature lists enrolled into the firmware.

However, in order to rebuild the RPM, a command similar to the following shall be ran:

$ rpmbuild -D 'pe_signing_token NSS Certificate DB' -D 'pe_signing_cert XXX' -ba *.spec

Question: what should be present in place of the placeholder entry XXX? A nickname that represents the certificate bound to the newer CA, or something else? Is this kind of signing assets sufficient, or do the further components contain multiple signatures for this to work? Maybe you have a completely different build process, which handles this scenario well, that I'm not aware of?

@aronowski aronowski added the question Reviewer(s) waiting on response label Apr 11, 2024
@ddstreetmicrosoft
Copy link
Author

ddstreetmicrosoft commented Apr 11, 2024 •
edited
Loading

I then manually signed the shim binary with my own db-related assets akin to:

$ sudo sbsign --cert db.crt --key db.key shimx64.efi

The esl is what gets embedded inside the shim, none of the keys in the esl are used to sign the shim. For production use, after a successful review here, the shim itself will be signed by the Microsoft UEFI signing cert (i.e. the one that's included by default inside most UEFI firmware DB across the world), and that Microsoft UEFI signing cert should not be embedded inside the shim.

Conversely, to accurately test the normal secure boot configuration, none of the certs that are embedded inside the shim should be added to the UEFI DB.

However, in order to rebuild the RPM, a command similar to the following shall be ran:

$ rpmbuild -D 'pe_signing_token NSS Certificate DB' -D 'pe_signing_cert XXX' -ba *.spec

If you're trying to build the Mariner 2.0 unsigned shim rpm, then the pe_signing* defines are almost certainly not going to work, as we don't use pesign during any RPM build process.

I would suggest first starting with the srpm that's included in the review repo: https://github.com/microsoft/shim-review/raw/cbl-mariner-shim-x64-20240222/shim-unsigned-x64-15.8-1.cm2.src.rpm

You can install that on any rpm-based distro (e.g. Fedora) and then you simply need to replace the db.x64.esl file with whatever esl file you've locally created (with your own locally-created certs), e.g.:

$ rpm -i shim-unsigned-x64-15.8-1.cm2.src.rpm 
$ cp db.esl rpmbuild/SOURCES/db.x64.esl

You'll need to install the proper deps (which are slightly different on Fedora than Mariner) and then build it normally (but with --nodeps since you won't have vim-extra installed):

$ sudo dnf install dos2unix xxd efivar-devel
...
$ rpmbuild --nodeps -bb rpmbuild/SPECS/shim-unsigned-x64.spec
...

That will produce an rpm with the unsigned shim efi, that has your esl embedded inside it. You can extract this unsigned shim efi from the rpm:

$ sudo dnf install rpmdevtools
...
$ mkdir shim-files
$ rpmdev-extract -C shim-files rpmbuild/RPMS/x86_64/shim-unsigned-x64-15.8-1.fc39.x86_64.rpm
shim-unsigned-x64-15.8-1.fc39.x86_64/usr/share/licenses/shim-unsigned-x64
shim-unsigned-x64-15.8-1.fc39.x86_64/usr/share/licenses/shim-unsigned-x64/COPYRIGHT
shim-unsigned-x64-15.8-1.fc39.x86_64/usr/share/shim-unsigned-x64/shimx64.efi
$ cp shim-files/shim-unsigned-x64-15.8-1.fc39.x86_64/usr/share/shim-unsigned-x64/shimx64.efi

Now sign the shimx64.efi manually with your local self-signed cert (not a cert that you put into the esl that you embedded; instead use the cert that you added to your local system UEFI DB).

Note that you will also need to resign (or add a signature to) your boot loader (e.g. grubx64.efi) and your kernel - using one of the certs you put in your local esl file - and your grub must be at the SBAT level that the shim requires, of course.

@ddstreetmicrosoft
Copy link
Author

Oh also I forgot to point out that our shim builds with no EFIDIR= so you will also need to place your grubx64.efi into the same dir as the shim, i.e. you should have:

/boot/efi/EFI/BOOT/bootx64.efi
/boot/efi/EFI/BOOT/grubx64.efi

Once the shim loads grub, it's up to your particular grub build to determine where grub will look for its config file(s).

@aronowski
Copy link
Collaborator

The esl is what gets embedded inside the shim, none of the keys in the esl are used to sign the shim. For production use, after a successful review here, the shim itself will be signed by the Microsoft UEFI signing cert (i.e. the one that's included by default inside most UEFI firmware DB across the world), and that Microsoft UEFI signing cert should not be embedded inside the shim.

Conversely, to accurately test the normal secure boot configuration, none of the certs that are embedded inside the shim should be added to the UEFI DB.

Exactly that! What I did were these two separate actions:

The first is that I built an unsigned shim with the .esl file I got from merging the two self-generated CAs (the ones I generated with the time-hooking library to simulate the validity period of your cert-0.der and cert-1.der CAs)

The second is that I created my own lists that I enrolled into my firmware (since I myself am not Microsoft and don't own the private part used for signing binaries), equivalent to the following:

  • Microsoft Corporation KEK CA 2011
  • Microsoft Corporation UEFI CA 2011

Later on I manually signed the unsigned shim binary (having the ESL embedded into it) with my equivalent of Microsoft Corporation UEFI CA 2011 (which in my case are just the db.crt and db.key files).

Then I wanted to build an RPM with a signed shim (signed by my equivalent of Microsoft Corporation UEFI CA 2011), what I normally do by running rpmbuild and defining the macros pe_signing_token and pe_signing_cert, using my equivalent of the certificate issued by the "newer CA" (my equivalent of the one by Microsoft Mariner RSA Root CA 2023).

Since that's just a laboratory, I was not using a real HSM or a smartcard, only the internal NSS certificate database.

your grub must be at the SBAT level that the shim requires, of course
[...]
Oh also I forgot to point out that our shim builds with no EFIDIR= so you will also need to place your grubx64.efi into the same dir as the shim

I might have missed something - thank you! I'll take a closer look soon.

@ddstreetmicrosoft
Copy link
Author

Then I wanted to build an RPM with a signed shim (signed by my equivalent of Microsoft Corporation UEFI CA 2011), what I normally do by running rpmbuild and defining the macros pe_signing_token and pe_signing_cert, using my equivalent of the certificate issued by the "newer CA" (my equivalent of the one by Microsoft Mariner RSA Root CA 2023).

For reference, the current spec file (not updated yet for 15.8) for Mariner 2.0 that takes the signed shim and builds an rpm is here, however you really don't need to do that, you can just copy the signed shim into place on your test system, to /boot/efi/EFI/BOOT/bootx64.efi (i.e. replacing the previous shim since that's the standard location). The rpm package, at least for Mariner 2.0, doesn't do anything at all other than carry the externally-signed shim at that location. There is no additional signing of anything.

@aronowski
Copy link
Collaborator

I've finished the laboratory - it does work! However, I did it in a slightly different way than suggested, e.g. putting the self-signed shim binary in /boot/efi/EFI/fedora/ and having the further Fedora bootloader-related components rebuilt with appropriate SBAT entries and certificates.

If anyone wants, I can write a guide on how to recreate the setup.

As mentioned in #387 (comment), waiting for a reply with the decrypted random words. Then we can celebrate!

@aronowski aronowski removed the question Reviewer(s) waiting on response label Apr 17, 2024
@christopherco
Copy link
Contributor

christopherco commented Apr 23, 2024 •
edited
Loading

As mentioned in #387 (comment), waiting for a reply with the decrypted random words. Then we can celebrate!

@aronowski just returned from vacation and here are the decrypted words for my verification. Thanks for doing this review! Please let us know if there is any more information we can help provide.

change swimwear bondless alkalize custard irritant clicker chop cotton idealism

@aronowski
Copy link
Collaborator

Thank you for the update. Accepting!

@aronowski aronowski added accepted Submission is ready for sysdev and removed contact verification needed Contact verification is needed for this review labels Apr 24, 2024
@aronowski aronowski removed their assignment Apr 24, 2024
@ddstreetmicrosoft ddstreetmicrosoft mentioned this issue Jun 6, 2024
Open
8 tasks
@christopherco
Copy link
Contributor

Update - We have received the signed shim. This issue can now be closed. Thanks!

@THS-on THS-on closed this as completed Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted Submission is ready for sysdev
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@ddstreet @THS-on @christopherco @dennis-tseng99 @es-fabricemarie @aronowski @eduardacatrinei @ddstreetmicrosoft