Shim 15.8 for Trusted Disk

[ifankner]

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/Rohde-Schwarz/shim-review/tree/rohdeschwarz-shim-x64-20240219

---

What is the SHA256 hash of your final SHIM binary?

---

6a887c457f51cee59855c4b98834e9eab273a6400ab9137bcd0a99dbae1aaa60

---

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

---

Shim 15.6
Shim 15.4

[ifankner]

To verify our shim bootloader, please use the following command:

docker build https://github.com/Rohde-Schwarz/shim-review.git

[THS-on]

Review for rohdeschwarz-shim-x64-20240219

• Rohde & Schwarz Cybersecurity GmbH had already previously accepted shims
• Required for their full disk encryption product
• Note that no Linux kernel is booted instead their tdisk EFI binary is used
• Security contacts haven't changed
• HSM is used for key storage

Shim

• Based on 15.8
• Contains two custom patches (haven't changed since last submission)
  • msabi.patch
  • bypass_bootoptions.patch
• NX bit is not set
• Embedded CA hasn't changed
• SBAT entries are fine. If there is ever an issue specific to your old one the current one will also be revoked because they have the same SBAT version number.
• Shim is reproducible:

#20 [16/18] RUN sha256sum /build/shim-review/shimx64.efi
#20 0.357 6a887c457f51cee59855c4b98834e9eab273a6400ab9137bcd0a99dbae1aaa60  /build/shim-review/shimx64.efi
#20 DONE 0.4s

#21 [17/18] RUN sha256sum /build/shim-15.8/shimx64.efi
#21 0.403 6a887c457f51cee59855c4b98834e9eab273a6400ab9137bcd0a99dbae1aaa60  /build/shim-15.8/shimx64.efi
#21 DONE 0.4s

Questions

• Have you seen the issue with the load options also on newer Shims and have you tried to upstream the other patch?
• How ready is tdisk and Windows to be booted with a NX enabled Shim?

Besides the questions LGTM

[ifankner]

First of all, thanks for the review and for the tip about increasing the vendor specific sbat level.

We rebuilt our shim with a new sbat level and changed the submission text with the new hash and new tag:

rohdeschwarz-shim-x64-20240308

To your questions:

• Yes, we noticed the "invalid parameter" error on the same Dell notebooks (e.g. 7470, 7470, 7480 Touch) with the 15.8 shim code, and so we decided to apply the "bypass_bootoptions.patch" again.

• I think the other patch "msabi.patch" is too specific to upstream. We need it for our pre-boot authentication code, which is built with the Microsoft Visual Compiler. Unfortunately the Microsoft Visual Compiler is not able to use the linux System V ABI, and so we had to switch to MS ABI.

• Our boot chain with is not yet NX ready, but were are working on it.

[jsetje]

Did you dig into the load options issue at all? I wonder if that's a sign that something else might be going wrong. Did you see this behavior on all the systems you tried it on, or are those laptops just a subset?

Building shim with a non-linux toolchain is not well tested, and we may want to be cautious.

At the very least it would be a good idea to explicitly test self revocation. Just make sure you do this on a system that you can then easily disable secure boot on.

[ifankner]

Strangely, this behavior only occurs on a few models (e.g., Dell 7420) and only in combination with the DEFAULT_LOADER variable. Without deactivating the load options, the code overrides our hardcoded path and filename and produces a non-bootable name for the second stage loader.

make 'DEFAULT_LOADER=\\EFI\\RSCS\\RSCSBOOT.EFI'

Unfortunately, we haven't had the time yet to debug the error adequately. And since it only occurs in our specific case, and we don't need the load options, it was safer for us to completely deactivate the load options.

As you can see in our Dockerfile, our shim is built using a traditional Ubuntu Linux toolchain and well tested. Specifically, self-revocation is also tested. Only our pre-boot authentication EFI software (the second stage loader) is built using the Microsoft Visual Compiler.

[aronowski]

I checked the binary and it seems alright. The build reproduces, checksum matches, and characteristics are OK - please see rhboot/shim#634 in regard to the SBAT generation numbers provided in the application.

Regarding the aforementioned strange behavior, I recall seeing something related a while ago. I'm not a low-level developer and can't help with diagnosing the issue, but this comment may come in handy.
I also recommend reading the whole thread.

[ifankner]

Thx for the info @aronowski

Due to the latest information from rhboot/shim#634 and #355, we reverted our last commit so that our shim has a Vendor SBAT level of 1 again.

We also changed the submission text back to the old hash and old tag:

rohdeschwarz-shim-x64-20240219

I have read the entire thread, and it really seems to be the same error. But since we don't want this function, I think in our case it is the best option to completely deactivate the load options and force our hardcoded path and filename in DEFAULT_LOADER, regardless of whether there is an error in it or not.

[aronowski]

On 2024.04.03 06:34:20, Ingo Fankner wrote: Thx for the info @aronowski Due to the latest information from rhboot/shim#634 and #355, we reverted our last commit so that our shim has a Vendor SBAT level of 1 again. We also changed the submission text back to the old hash and old tag: `rohdeschwarz-shim-x64-20240219`

Considering that the current written consensus is what it is, and the "downstream generation number should never go down" consensus hasn't been yet written in the official shim's SBAT.md document, I myself can see this decision fine as of today. Just to inform, I operate in real-time with the information that is available at the time of writing the reply. It may change in the future and it did, a short time after the linked application.

I have read the entire thread, and it really seems to be the same error. But since we don't want this function, I think in our case it is the best option to completely deactivate the load options and force our hardcoded path and filename in `DEFAULT_LOADER`, regardless of whether there is an error in it or not.

Seems OK to me, but I likely won't be able to review such a patch, since I'm not a low-level programmer and won't be able to tell, if the final code would have no security issues. I do, however, trust the developer team at Rohde & Schwarz Cybersecurity GmbH, as there already were signed shims provided by Microsoft - so I'll leave a thorough peer-review of the patch for another accredited reviewer. However, ping me once the patch gets introduced - I'll take a quick look at the latest application tag and see if e.g. the build reproduces.

…

-- Reply to this email directly or view it on GitHub: #385 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[ifankner]

Is there anything else we could do or answer since the question tag has not been reset yet?

The patch "bypass_bootoptions.patch" only bypasses the bootoption parsing. Maybe @steve-mcintyre? Perhaps you could take a look at this again since you've already reviewed our last version 15.6 with this patch?

[aronowski]

Alright, we're talking about that simple patch! I got confused a bit. Looks like it's safe after all - it's been accepted by Microsoft as part of [application #257](#257). Just one more question. To [quote @jsetje](#385 (comment)),

At the very least it would be a good idea to explicitly test self revocation.

Have you checked, whether that works alright? In particular on the laptops where the bug was present. If so, then things look OK to me. For more information on the load options behaving differently across different machines I did take a look at the comments in the `parse_load_options` function in [`load-options.c`](https://github.com/rhboot/shim/blob/main/load-options.c)

[ifankner]

hi @aronowski

yes, we have tested the shim self-revocation feature on a wide range of systems, also on these problem laptops

[aronowski]

Thank you for the update. Accepting!