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

Implement and test Linux kernel code #34

Closed
BeataZdunczyk opened this issue Jul 5, 2024 · 17 comments
Closed

Implement and test Linux kernel code #34

BeataZdunczyk opened this issue Jul 5, 2024 · 17 comments
Assignees
Labels
P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: task Type: task. An action item that is neither a bug nor an enhancement. W: in progress Workflow: in progress. The issue is actively being worked on.

Comments

@BeataZdunczyk
Copy link
Member

Brief summary

Implement Linux kernel code for AMD with necessary fixes for the most up-to-date TrenchBoot boot protocol. The code should be pushed to TrenchBoot Linux repository.

@BeataZdunczyk BeataZdunczyk added T: task Type: task. An action item that is neither a bug nor an enhancement. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. W: todo Workflow: todo. The issue is in the initial to do state. labels Jul 5, 2024
@SergiiDmytruk SergiiDmytruk added W: in progress Workflow: in progress. The issue is actively being worked on. and removed W: todo Workflow: todo. The issue is in the initial to do state. labels Oct 12, 2024
@SergiiDmytruk
Copy link
Member

Rebased linux-sl-5.13-amd onto linux-sl-master-9-12-24-v11 (from linux-sl-master-8-21-24-v10).

Cherry-picked commits from linux-sl-5.13-amd onto linux-sl-master-9-12-24-v11 to make linux-sl-amd-refresh.

  • "x86: Secure Launch main header file" (small bits contain AMD definitions)
  • "x86: Secure Launch main header file AMD support"
  • "x86: Implement AMD support for Secure Launch"
  • "x86: Prepare CPUs for post SKINIT launch"
  • "x86: Extend PCR measurements in TPM during late initialization"

Made things compile and make more sense (e.g., dropped landing-zone stuff).

Made sl_stub.S determine absolute address of kernel's entry point instead of relying on the value of %ebx provided by TXT.

Currently need to pass SLRT address into slmodule on AMD:

  • How to pass information from compressed environment to the decompressed one? Old way was for GRUB to pass SKL base in boot params, but that's likely undesirable and I think we don't have that code in GRUB anymore.
  • I guess I just add a new parameter for startup_64 of arch/x86/kernel/head_64.S in addition to boot_params pointer. That should work and can be changed later if anyone has a better idea. Extending boot_params structure (or setup_header) is an obvious one, but don't know if upstream will welcome it (only 40 bytes of padding are left there).

@SergiiDmytruk
Copy link
Member

Modified the code to pass SLRT address and adjusted/brought back some related code.

Started testing on hardware, it just resets at the moment. The reset happens surprisingly fast in sl_stub.S, but the entry point gets to run, just not for long.

@SergiiDmytruk
Copy link
Member

Found a couple of issues, it now stops in sl_main.c on checking magic number of SLRT.

While looking at assembly realized that call 1f method has some issues. The reason is that base address is necessary to setup GDT and then %ss and %esp, but at the entry point only %cs and %ds are guaranteed to be adequate, so we can't really use the stack at least on Intel. Knowing that SKL setup the stack, we can probably utilize it on AMD after using cpuid.

@krystian-hebel
Copy link
Member

at the entry point only %cs and %ds are guaranteed to be adequate

Actually, when working on Xen rebase I read through 315168_TXT_MLE_DG_rev_017_4 again and I couldn't find a definite statement that %ds is good on entry to MLE. The closest I could find is table in Appendix E Platform State upon SINIT Exit and Return to MLE, which states that for BSP (aka ILP) %ds is undefined. A.3 Authentication and Execution of AC Module says that CS and DS are initialized, but this applies to entry to ACM, not MLE.

Probably the best way to do this is to change SKL to be as close to TXT as possible (because we won't be able to change how TXT does things) and instead not depend on %ebx value on MB2 entry, as we're doing on Intel anyway. With https://lore.kernel.org/xen-devel/20241007141539.1899350-1-frediano.ziglio@cloud.com/T/#m794a192186ed86a5a8adb51aaa45128a45885e5c merged, it will be possible to link early C with external symbols, which should make it easier to work with.

@SergiiDmytruk
Copy link
Member

Found a couple of issues, it now stops in sl_main.c on checking magic number of SLRT.

The code was using old register with the start of SKL, goes further now and resets somewhere when dealing with event log.

Actually, when working on Xen rebase I read through 315168_TXT_MLE_DG_rev_017_4 again and I couldn't find a definite statement that %ds is good on entry to MLE. The closest I could find is table in Appendix E Platform State upon SINIT Exit and Return to MLE, which states that for BSP (aka ILP) %ds is undefined. A.3 Authentication and Execution of AC Module says that CS and DS are initialized, but this applies to entry to ACM, not MLE.

The code itself doesn't seem to rely on %ds, then only the comment is wrong.

Probably the best way to do this is to change SKL to be as close to TXT as possible (because we won't be able to change how TXT does things) and instead not depend on %ebx value on MB2 entry, as we're doing on Intel anyway.

I guess so, if Intel won't change something about TXT in the future.

@krystian-hebel
Copy link
Member

The code itself doesn't seem to rely on %ds, then only the comment is wrong.

It does, these two lines implicitly use %ds to get the address of GDT descriptor from (%eax) (address in the descriptor itself is always linear and not segment-relative). It should be enough to change those lines to:

	addl	%eax, %cs:2(%eax)
	lgdt	%cs:(%eax)

@SergiiDmytruk
Copy link
Member

I missed how addl updates the address and was under impression that lgdt doesn't care about segments. The code builds fine with %cs: added.

@SergiiDmytruk
Copy link
Member

addl %eax, %cs:2(%eax) doesn't work because code segments are read-only.

After making one more Intel-specific part conditional, the code now crashes in slmodule because something is wrong with passing SKL base into decompressed kernel.

@SergiiDmytruk
Copy link
Member

Trusting printk() was a mistake, I don't know why, but it prints nonsense. The SLRT address is passed fine now and the current issue is that slmodule.c unlike sl_main.c thinks it's dealing with TPM1, which was part of the reason I thought SLRT address was still wrong. Maybe TPM2 signature is accidentally overwritten, because mapping event log seems fine.

@SergiiDmytruk
Copy link
Member

Yay, it boots:

root@tb:~# tpm2_pcrread | grep -e sha -e '1[789]:\|2[012]:'
sha1:
  17: 0x7308FF607AECCC64F0CF00CE51EC6D9A7EE0A017
  18: 0x870470B90632C7A28BC3589CFCB77EBC1F9FDEE0
  19: 0x0000000000000000000000000000000000000000
  20: 0x0000000000000000000000000000000000000000
  21: 0x0000000000000000000000000000000000000000
  22: 0x0000000000000000000000000000000000000000
sha256:
  17: 0x10507B82E7C4F6AE46E952274C1D7AEA3CED3D6663AE2CC06A78C0967F226F91
  18: 0x2F704BC465EBC7A688F773002C06E6327EEFB675A7ECF4F1902D12BF1A35A8FA
  19: 0x0000000000000000000000000000000000000000000000000000000000000000
  20: 0x0000000000000000000000000000000000000000000000000000000000000000
  21: 0x0000000000000000000000000000000000000000000000000000000000000000
  22: 0x0000000000000000000000000000000000000000000000000000000000000000

The digests don't match output of tpm2_eventlog /sys/kernel/security/slaunch/eventlog yet though.

Trusting printk() was a mistake, I don't know why, but it prints nonsense.

Turns out it's a security feature to not print pointer values with %p. Adding no_hash_pointers to kernel parameters solves it.

Maybe TPM2 signature is accidentally overwritten, [...]

That was it, because neither log nor SLB were reserved.

@SergiiDmytruk
Copy link
Member

Cleaned up the code.

The digests don't match output of tpm2_eventlog /sys/kernel/security/slaunch/eventlog yet though.

SHA1 hash for PCR-18 matches, but not for PCR-17. The latter matches if I compute the expected PCR value manually excluding TXT_EVTYPE_SLAUNCH_{START,END} events. So they are messing up results of tpm2_eventlog and not providing digests confuses __calc_tpm2_event_size() into thinking that an event log entry is malformed.

SHA256 hash matches for PCR-17 the same way, but doesn't match for PCR-18, which looks really weird.

@SergiiDmytruk
Copy link
Member

SHA256 hash matches for PCR-17 the same way, but doesn't match for PCR-18, which looks really weird.

The code for extending PCRs of TPM2 looked weird and that is where the bug was: TrenchBoot/linux@3beecd1

Hashes now match the event log if you take into account:

The latter matches if I compute the expected PCR value manually excluding TXT_EVTYPE_SLAUNCH_{START,END} events. So they are messing up results of tpm2_eventlog

It behaves the same with Intel TXT, so this is not exactly in scope, although it will likely break OSFV tests.


Next: run OSFV tests on this.

@SergiiDmytruk
Copy link
Member

Tests might need some update:

  • automating booting with SeaBios is problematic, I had to hard-code boot sequence
  • there is actually no SLRT entry on AMD because only Intel-specific part of it is ever measuredd
  • there is no MB2 entry because we're not booting using Multiboot2
  • START/END events confuse tpm2_eventlog and this will likely require a workaround (not a big deal though)

Otherwise the tests pass at least on APU2 (APU3 has weird serial which mixes output).

@SergiiDmytruk
Copy link
Member

Made DRTM tests pass by computing expected PCR values without tpm2_eventlog.

Then tried using kexec which didn't work initially because of a bug (INIT redirection wasn't really disabled). It works now, but PCRs stay as if DRTM is on. Not sure if something needs to be done here.

@SergiiDmytruk
Copy link
Member

SergiiDmytruk commented Oct 28, 2024

@SergiiDmytruk
Copy link
Member

Found out that OSFV tests didn't work when some banks were disabled, fixed.

Rebased meta-trenchboot PR onto zarhus/meta-trenchboot#49.

Saw that PCR extension is stilll broken in Linux, fixed.

@SergiiDmytruk
Copy link
Member

Another related PR: Dasharo/osfv-results#14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: task Type: task. An action item that is neither a bug nor an enhancement. W: in progress Workflow: in progress. The issue is actively being worked on.
Projects
None yet
Development

No branches or pull requests

3 participants