TrenchBoot / trenchboot-issues

This repository is to centralize issues and development progress tracking for the TrenchBoot project.
3 stars 1 forks source link

Code rebase onto the most recent work implementing Secure Launch protocol being upstreamed to Linux and GRUB #17

Closed BeataZdunczyk closed 7 months ago

BeataZdunczyk commented 1 year ago

Is your feature request related to a problem? Please describe.

The current state of TrenchBoot support has diverged with what was developed for QubesOS AEM for Intel hardware with TPM 1.2. This task aims to update the work and align with the TrenchBoot boot protocol being upstreamed to GRUB and Linux kernel.

Is your feature request related to a new idea or technology that would benefit the project? Please describe.

This issue is required to ensure Qubes OS AEM supports the most recent TrenchBoot boot protocol upstreamed to GRUB and Linux kernel, which will provide improved security and functionality.

Describe the solution you'd like

Rebase the code to the most recent work implementing Secure Launch protocol and align with the TrenchBoot boot protocol being upstreamed to GRUB and Linux kernel.

Describe alternatives you've considered

N/A

Additional context

This feature request is part of Phase 3 in TrenchBoot as Anti Evil Maid project, as outlined in the documentation: https://docs.dasharo.com/projects/trenchboot-aem-v2/.

Relevant documentation you've consulted

N/A

rossphilipson commented 1 year ago

These commits add SLRT support to GRUB:

https://github.com/TrenchBoot/grub/commit/b0445fe4e17dee9ee302952229a840bc5accfaa5

https://github.com/TrenchBoot/grub/commit/dc2bccaa1e2021d0dce13b88570616fa9fd3988c

https://github.com/TrenchBoot/grub/commit/065790927e01916ba66b70244c49ef769ae1dbb4

And then you need the latest SLR table header here:

https://github.com/rossphilipson/travail/blob/master/trenchboot/include/slr_table_grub.h

SergiiDmytruk commented 10 months ago

Overall state

Commits on grub-sl-fc-38-beta/grub-sl-fc-38-dlstub branch have broken messages. The branch is past 2.06 release. i386/txt: Add Intel TXT core implementation commit seems to be absent there. Present commits somewhat differ.

Possible path of aligning branches

  1. Re-order commits on AEM branch to separate GRUB-changes pulled from upstream and SL-related commits.

  2. (maybe) Integrate commits with fixes for new code on grub-sl-fc-38-beta into corresponding parent commits.

  3. Match commits on both branches based on what changes they include in preparation for cherry-picking.

  4. For each group of matching patches:

    1. Cherry-pick a patch/patches from grub-sl-fc-38-beta, make sure it compiles and adjust the code if it doesn’t. Use a reasonable commit message.

    2. Cherry-pick corresponding patch(es) from AEM branch and sort out differences (types, casts, comments, equivalent code changes (like different NULL checks), use of different functions). This can result in none or multiple patches each being a relatively standalone change. Requires figuring out cause of differences (style guide compliance, previous version of SL, upstream GRUB changes, etc.).

  5. Add unique changes from each branch. This will have some conflicts as well as code won't match any pre-existing version at this point.

The end result should structurally match grub-sl-fc-38-beta version with AEM changes being interspersed among them. And commit history should be clean.

krystian-hebel commented 10 months ago

The branch is past 2.06 release.

We have to start from what https://github.com/QubesOS/qubes-grub2 uses, which is 2.06.

Present commits somewhat differ.

Maybe a result of review done on https://github.com/QubesOS/qubes-grub2/pull/13? In that case changes suggested in the review have priority over grub-sl-fc-38-beta.

  1. Re-order commits on AEM branch to separate GRUB-changes pulled from upstream and SL-related commits.

If changes on grub-sl-fc-38-beta don't depend on those post-2.06 changes, we should drop them, otherwise future bump to GRUB version used by Qubes would introduce unnecessary merge conflicts.

  1. (maybe) Integrate commits with fixes for new code on grub-sl-fc-38-beta into corresponding parent commits.

That would be the best idea, remember to keep signed-off-by lines, we don't want to just steal the code :slightly_smiling_face:

The end result should structurally match grub-sl-fc-38-beta version with AEM changes being interspersed among them. And commit history should be clean.

This is more or less what we have to do. In the end we will be uploading patches to https://github.com/QubesOS/qubes-grub2/pull/13, so it doesn't make sense to have commits that fix bugs in our previous commits, we can fix the original change instead.

SergiiDmytruk commented 10 months ago

We have to start from what https://github.com/QubesOS/qubes-grub2 uses, which is 2.06.

That's why I thought it's relevant, but it makes no difference.

Maybe a result of review done on QubesOS/qubes-grub2#13? In that case changes suggested in the review have priority over grub-sl-fc-38-beta.

Yes, that seems to be the cause of the changes. So there is nothing to move to our side and no need to rebase it.

That would be the best idea, remember to keep signed-off-by lines, we don't want to just steal the code 🙂

Turns out, those commits were already integrated.


Changes are on intel-txt-aem-slrt branch. Cherry-picking with minor adjustments was enough. Some changes looked incomplete and didn't compile or caused warnings, TPM log was always for TPM2.0 which looked like a bug. Will go through the changes again and send PR.

krystian-hebel commented 10 months ago

I've left some comments with focus on next steps in https://github.com/TrenchBoot/grub/pull/13. @rossphilipson some of them apply to your commits in case you're interested, but they are rather small issues that shouldn't change the logic.

I also created an issue for gathering changes to the specification in https://github.com/TrenchBoot/documentation/issues/23. I think it would be best to not add them immediately since we will be doing more significant changes (hopefully soon) anyway.

BeataZdunczyk commented 7 months ago

Closing this issue. The release was published here: https://github.com/TrenchBoot/qubes-antievilmaid/pull/8, and test results are available here: https://github.com/TrenchBoot/trenchboot-issues/issues/18.