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

KGPE-D16 Coreboot 4.11 + Measured Boot #867

Merged
merged 4 commits into from
Dec 2, 2020

Conversation

Tonux599
Copy link
Contributor

@Tonux599 Tonux599 commented Oct 23, 2020

Edit (tlaurion): TPM used by @Tonux599 : Asus 90-C1B0AU-00XBN0VZ

This is based upon #795 (credit to @tlaurion) but focuses on measured boot without vboot.

I would appreciate feedback from the community, notably that ./patches/coreboot-4.11/0020-kgpe-d16_measured-boot-support.patch is implemented correctly.

Also note that this error occurs when microcode is included in the CBFS:

TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x0
TPM: Asserting physical presence
TPM: command 0x4000000a returned 0x0
TPM: command 0x65 returned 0x0
TPM: flags disable=0, deactivated=0, nvlocked=1
TPM: setup succeeded
TPM: pcr 2 measure fffff800 @ 2048: 80909df13fd6a372cca9837b538971e375d3f4d4
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure ff000300 @ 1048576: dcfbe783c5d139c5481253b8c8ee8909b49297ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 213776: 6f279a6cc1687a5b3170d80cca7a3147e59037ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 1162424: 2ea6887b5d072fb004b2115a320621b8f32f2ac4
TPM: command 0x14 returned 0x0
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x26
TPM: Can't run startup command.
TPM: pcr 2 measure ff042988 @ 9895: 75fe16ebfce00b4becdc5b7f6f31feec791b87c6
TPM: command 0x14 returned 0x0
\_SB.PCI0.LPC.TPM: LPC TPM PNP: 004e.0
TPM: pcr 2 measure ff041b78 @ 3524: a1615718de98d6d619812c6b9fd3c3f5177bc137
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00090000 @ 4224: d74d67b4e9bc8dc6d3536082e78dcae391fa1503
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 01000000 @ 3772880: 35e974ae974845687c8128055661fe4005e5b970
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00040000 @ 180: 31fae3ad7f3a0f2ee6fa1afeaa370ba152dc2662
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00091000 @ 25: 1a0c772ebbab34217f5ee534c8e01c94fd8d3bd6
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 04000000 @ 4041216: 5cef02f227841f10622c4ca0adbb14843d6c4b97
TPM: command 0x14 returned 0x0

However when microcode is removed such as:

cbfstool coreboot.rom remove -n microcode_amd.bin
cbfstool coreboot.rom remove -n microcode_amd_fam15h.bin

The above error does not occur:

Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x0
TPM: Asserting physical presence
TPM: command 0x4000000a returned 0x0
TPM: command 0x65 returned 0x0
TPM: flags disable=0, deactivated=0, nvlocked=1
TPM: setup succeeded
TPM: pcr 2 measure fffff800 @ 2048: 80909df13fd6a372cca9837b538971e375d3f4d4
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure ff000300 @ 1048576: dcfbe783c5d139c5481253b8c8ee8909b49297ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 213776: 6f279a6cc1687a5b3170d80cca7a3147e59037ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 1162424: 2ea6887b5d072fb004b2115a320621b8f32f2ac4
TPM: command 0x14 returned 0x0
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x26
TPM: Can't run startup command.
TPM: pcr 2 measure ff042988 @ 9895: 75fe16ebfce00b4becdc5b7f6f31feec791b87c6
TPM: command 0x14 returned 0x0
\_SB.PCI0.LPC.TPM: LPC TPM PNP: 004e.0
TPM: pcr 2 measure ff041b78 @ 3524: a1615718de98d6d619812c6b9fd3c3f5177bc137
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00090000 @ 4224: d74d67b4e9bc8dc6d3536082e78dcae391fa1503
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 01000000 @ 3772880: 35e974ae974845687c8128055661fe4005e5b970
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00040000 @ 180: 31fae3ad7f3a0f2ee6fa1afeaa370ba152dc2662
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00091000 @ 25: 1a0c772ebbab34217f5ee534c8e01c94fd8d3bd6
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 04000000 @ 4041216: 5cef02f227841f10622c4ca0adbb14843d6c4b97
TPM: command 0x14 returned 0x0

I think this is because the TPM is not initialised early enough but will test more soon.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 23, 2020

I will use your branch and push it on my own github so that the built is made, since my github heads is followed by CircleCI which trigger builds for defined boards under .circleci/config.yml

I will do edit .circleci/config.yml for it to only build kgpe-d16 boards touched by your branch.

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 23, 2020

@Tonux599 actually, decided to start by just replicating your branch to trigger a build. (Rebasing resulted iun chaos)
https://app.circleci.com/pipelines/github/tlaurion/heads/534/workflows/67a1e987-9e7d-4ff4-b06b-a471466f701a/jobs/578

tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 23, 2020
@tlaurion
Copy link
Collaborator

tlaurion commented Oct 23, 2020

@Tonux599
Copy link
Contributor Author

@tlaurion so kgpe-d16_workstation built so, yay. Looks like kgpe-d16_workstation-usb_keyboard failed due to malformed make command.

tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 23, 2020
tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 24, 2020
@Tonux599
Copy link
Contributor Author

Tonux599 commented Oct 24, 2020

Success here. Will bring this PR on par with that.

Edit:
Success on this branch. Artefacts here.

tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 24, 2020
tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 24, 2020
@tlaurion
Copy link
Collaborator

tlaurion commented Oct 24, 2020

@Tonux599 https://app.circleci.com/pipelines/github/tlaurion/heads/541/workflows/9d19e4e0-a6ab-4d68-bfa1-f47d61b9b760/jobs/588 has passed.

The question here is to know if the microcodes are applied by QubesOS early enough / if the system requires them for some CPUs or not at this point.

You can cherry pick or rebase force push https://github.com/tlaurion/heads/tree/tonux_kgpe-d16_411_measured-boot_rebased on top of here and that would replace #795 :) or I can push my branch over #795 with your commits if build is successful.

I think we should remove microcodes, but I haven't tested the builds myself yet.

@Tonux599
Copy link
Contributor Author

@tlaurion I think we may have made the same changes in each of our branch as this PR now builds as is but I can still force push yours to this if you desire. Let me know :)

Regarding the microcode, gimme a few days as I'll see if I can fix that error. My concern being that if microcode is not measured and not included in the CBFS, an attacker could implement malicious microcode without the end user being aware.

@tlaurion
Copy link
Collaborator

Todo:

  • remove LOCAL_VERSION in coreboot config to be compliant with internally kept version scheme for FWUPD upgrades

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 24, 2020

@tlaurion I think we may have made the same changes in each of our branch as this PR now builds as is but I can still force push yours to this if you desire. Let me know :)

Regarding the microcode, gimme a few days as I'll see if I can fix that error. My concern being that if microcode is not measured and not included in the CBFS, an attacker could implement malicious microcode without the end user being aware.

@Tonux599 Well, microcode injection still requires AMD signatures on binaries pushed to CPU so I am not sure that is a real risk. My recommendation was to disable microcode inclusion in ROM. But if you think you can fix this, no prob. My intuition confirms your doubts that microcode measurements are not possible at the stage TPM is initialized. AFAIK microcode is being injected prior of the romstage, while kgpe-d16 is implementing measurements in romstage in 4.11. So that might not be possible without implementing TPM measurements into bootblock, which from past discussions, doesn't seem possible for that board in 4.11 unless a lot of 4.12 patches are brought back, but I have not investigated this.

https://github.com/tlaurion/heads/tree/tonux_kgpe-d16_411_measured-boot_rebased is just rebased on master with a more clean CircleCI config, that could be merged as is, since all baords were built successfully, while cache is still being made and uploaded as we speak (with same signature that will be reused for all modules/* and patches/* being the same).

@tlaurion
Copy link
Collaborator

@MrChromebox : what is the impact of those coreboot 4.11 patches on the librem_l1um ?

@tlaurion
Copy link
Collaborator

tlaurion commented Oct 25, 2020

Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x0
TPM: Asserting physical presence
TPM: command 0x4000000a returned 0x0
TPM: command 0x65 returned 0x0
TPM: flags disable=0, deactivated=0, nvlocked=1
TPM: setup succeeded
TPM: pcr 2 measure fffff800 @ 2048: 80909df13fd6a372cca9837b538971e375d3f4d4
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure ff000300 @ 1048576: dcfbe783c5d139c5481253b8c8ee8909b49297ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 213776: 6f279a6cc1687a5b3170d80cca7a3147e59037ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 1162424: 2ea6887b5d072fb004b2115a320621b8f32f2ac4
TPM: command 0x14 returned 0x0
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x26
TPM: Can't run startup command.
TPM: pcr 2 measure ff042988 @ 9895: 75fe16ebfce00b4becdc5b7f6f31feec791b87c6
TPM: command 0x14 returned 0x0
_SB.PCI0.LPC.TPM: LPC TPM PNP: 004e.0
TPM: pcr 2 measure ff041b78 @ 3524: a1615718de98d6d619812c6b9fd3c3f5177bc137
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00090000 @ 4224: d74d67b4e9bc8dc6d3536082e78dcae391fa1503
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 01000000 @ 3772880: 35e974ae974845687c8128055661fe4005e5b970
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00040000 @ 180: 31fae3ad7f3a0f2ee6fa1afeaa370ba152dc2662
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00091000 @ 25: 1a0c772ebbab34217f5ee534c8e01c94fd8d3bd6
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 04000000 @ 4041216: 5cef02f227841f10622c4ca0adbb14843d6c4b97
TPM: command 0x14 returned 0x0

@Tonux599
I'm confused on why:

Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup

Is shown twice?

@tlaurion
Copy link
Collaborator

@Tonux599 see tlaurion@4d6cee7

@tlaurion
Copy link
Collaborator

This is based upon #795 (credit to @tlaurion) but focuses on measured boot without vboot.

I would appreciate feedback from the community, notably that ./patches/coreboot-4.11/0020-kgpe-d16_measured-boot-support.patch is implemented correctly.

Also note that this error occurs when microcode is included in the CBFS:

TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7b9e88 @ 12684: 1077e1af95dc35d5661d54b0ba72652fa0d5aeec
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
TPM: pcr 2 measure ff7bd088 @ 7876: 983867f34d8599e779cf9e1454f8f4518b5628ac
src/drivers/pc80/tpm/tis.c:521 unexpected TPM status 0xff
src/drivers/pc80/tpm/tis.c:717 failed sending data to TPM
TPM: command 0x14 send/receive failed: 0x10000001
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x0
TPM: Asserting physical presence
TPM: command 0x4000000a returned 0x0
TPM: command 0x65 returned 0x0
TPM: flags disable=0, deactivated=0, nvlocked=1
TPM: setup succeeded
TPM: pcr 2 measure fffff800 @ 2048: 80909df13fd6a372cca9837b538971e375d3f4d4
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure ff000300 @ 1048576: dcfbe783c5d139c5481253b8c8ee8909b49297ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 213776: 6f279a6cc1687a5b3170d80cca7a3147e59037ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 1162424: 2ea6887b5d072fb004b2115a320621b8f32f2ac4
TPM: command 0x14 returned 0x0
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x26
TPM: Can't run startup command.
TPM: pcr 2 measure ff042988 @ 9895: 75fe16ebfce00b4becdc5b7f6f31feec791b87c6
TPM: command 0x14 returned 0x0
\_SB.PCI0.LPC.TPM: LPC TPM PNP: 004e.0
TPM: pcr 2 measure ff041b78 @ 3524: a1615718de98d6d619812c6b9fd3c3f5177bc137
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00090000 @ 4224: d74d67b4e9bc8dc6d3536082e78dcae391fa1503
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 01000000 @ 3772880: 35e974ae974845687c8128055661fe4005e5b970
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00040000 @ 180: 31fae3ad7f3a0f2ee6fa1afeaa370ba152dc2662
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00091000 @ 25: 1a0c772ebbab34217f5ee534c8e01c94fd8d3bd6
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 04000000 @ 4041216: 5cef02f227841f10622c4ca0adbb14843d6c4b97
TPM: command 0x14 returned 0x0

However when microcode is removed such as:

cbfstool coreboot.rom remove -n microcode_amd.bin
cbfstool coreboot.rom remove -n microcode_amd_fam15h.bin

The above error does not occur:

Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x0
TPM: Asserting physical presence
TPM: command 0x4000000a returned 0x0
TPM: command 0x65 returned 0x0
TPM: flags disable=0, deactivated=0, nvlocked=1
TPM: setup succeeded
TPM: pcr 2 measure fffff800 @ 2048: 80909df13fd6a372cca9837b538971e375d3f4d4
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure ff000300 @ 1048576: dcfbe783c5d139c5481253b8c8ee8909b49297ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 213776: 6f279a6cc1687a5b3170d80cca7a3147e59037ca
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00e00000 @ 1162424: 2ea6887b5d072fb004b2115a320621b8f32f2ac4
TPM: command 0x14 returned 0x0
Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup
TPM: command 0x99 returned 0x26
TPM: Can't run startup command.
TPM: pcr 2 measure ff042988 @ 9895: 75fe16ebfce00b4becdc5b7f6f31feec791b87c6
TPM: command 0x14 returned 0x0
\_SB.PCI0.LPC.TPM: LPC TPM PNP: 004e.0
TPM: pcr 2 measure ff041b78 @ 3524: a1615718de98d6d619812c6b9fd3c3f5177bc137
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00090000 @ 4224: d74d67b4e9bc8dc6d3536082e78dcae391fa1503
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 01000000 @ 3772880: 35e974ae974845687c8128055661fe4005e5b970
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00040000 @ 180: 31fae3ad7f3a0f2ee6fa1afeaa370ba152dc2662
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 00091000 @ 25: 1a0c772ebbab34217f5ee534c8e01c94fd8d3bd6
TPM: command 0x14 returned 0x0
TPM: pcr 2 measure 04000000 @ 4041216: 5cef02f227841f10622c4ca0adbb14843d6c4b97
TPM: command 0x14 returned 0x0

I think this is because the TPM is not initialised early enough but will test more soon.

@miczyg1 @pietrushnic any idea on modyfying https://github.com/osresearch/heads/blob/0782f77b9b6f93168decb95f7f4bb09453482272/patches/coreboot-4.11/0020-kgpe-d16_measured-boot-support.patch and https://github.com/osresearch/heads/blob/0782f77b9b6f93168decb95f7f4bb09453482272/patches/coreboot-4.11/0021-kgpe-d16_c-environment_bootblock.patch so that TPM is initialized early enough to measure microcode and to fix TPM being initialized twice?

@MrChromebox
Copy link
Contributor

@MrChromebox : what is the impact of those coreboot 4.11 patches on the librem_l1um ?

the only one which affects anything outside of the KGPE-D16 itself is the C environment patch, and that shouldn't be problematic. I'd want to test and verify to be sure though.

More generally though, feel like this 53-commit patch set could be squashed to ~5 commits and make reviewing a bit easier

@Tonux599
Copy link
Contributor Author

@Tonux599
I'm confused on why:

Found TPM SLB9635 TT 1.2 by Infineon
TPM: Startup

Is shown twice?

@tlaurion romstage is called three times when the board is booted, with the first two times a soft reset is called. It's likely that the romstage doesn't reach the TPM initialisation on its first iteration and does so on the second and third. This behaviour has been persistent throughout my work on this board.

@miczyg1
Copy link
Contributor

miczyg1 commented Nov 5, 2020

[...]The hardcoded bootblock size and base address look a little bit ugly. Wouldn't it be better to locate it with CBFS API?

One issue with trusting the external structure is if an attacker can modify the CBFS struct to point to a clean copy, which is not used by the reset vector since it will use the 0xFFFFFFF0 legacy address, then the measurement might not reflect the reality of what was booted.

Although, an attacker in that position could also modify the bootblock unless there is an ACM that does the PCR0 measurement, so it is probably acceptable.

If CBFS is modified then it doesn't matter if you use legacy reset vector or CBFS API or hardcoded addresses. The measured code will not be the one which was booted in all cases... Using hardcoded 0xFFFFF800 isn't better either. Somebody can place a modified bootblock there too. The CBFS API will return this address if nothing was modified. How do you trust the CBFS API to locate next stages then? How do you trust the romstage or ramstage that is executed next after bootblock?

If the flash is not protected in any way (either ACM or WP pin) what is the trust anchor here? Or what are the measurements for? If somebody replaces this bootblock, it should be reflected by the measurements. Maybe I am biased with how coreboot is developed: "if some functionality in the coreboot exists, use it instead of hardcoding or reimplementing the wheel". @Tonux599 I am sorry if my advice was bad.

@miczyg1
Copy link
Contributor

miczyg1 commented Nov 5, 2020

For example one may modify just a single letter in CBFS file header, for example file name fallback/ramstage -> fallback/rumstage and CBFS API won't find your ramstage. The boot process will simply stop. And it can be done without modifying a single bit in the ramstage code.

@Tonux599
Copy link
Contributor Author

Tonux599 commented Nov 5, 2020

[...] @Tonux599 I am sorry if my advice was bad.

@miczyg1 There is no need to apologise as I hold no difference in value of your advice or @osresearch's advice. At this stage I'm merely nothing more than someone who has some free time to attempt to bring a board that they own into support with heads.

I will, however, make adjustments to my contributions (unless I have a moral objection) in line with projects maintainers wishes (which is a little unclear at the moment, clarification on @tlaurion's and @osresearch's role in the project could be laid out in a CONTRIBUTING.md, which would also aid in the effort of onboard new devs into this project).

But back to the subject, I would encourage @miczyg1, @osresearch and really anyone whom have a good understanding coreboot to continue arguing, at which point I would expect maintainers to weigh each side of the argument and reach a decision. I would then implement that.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 5, 2020

@miczyg1 @osresearch : I'm kinda confused right now.

Consequently PCRs measurements for each PCRs are:

  • 2: Boot block, ROM stage, RAM stage, Heads linux kernel and initrd
  • 4: Boot mode (0 during /init, then recovery or normal-boot)
  • 5: Heads Linux kernel modules
  • 6: Drive LUKS headers
  • 7: Heads user-specific config files

Any problem/improvements that can happen here if there is no ACM?

@miczyg1
Copy link
Contributor

miczyg1 commented Nov 5, 2020

@miczyg1 @osresearch : I'm kinda confused right now.

These references are not really related to the discussion. The mentioned mechanisms are heads specific and it is all about heads how they will be handled. They do not have any particular impact on the coreboot stage localization.

If I am not mistaken the CBFS API responsible for locating coreboot stage during boot are intact. So for me it is pointless to hardcode the bootblock address and size if the CBFS API can locate it and return the size. The security threat does not change by using CBFS API here, if someone modifies the flash on the fly, it doesn't matter what method was used (hardcoding or CBFS API). The same CBFS API is used to locate coreboot stages during boot, someone may put a fake/malicious ramstage or other
stage to execute own code. What is important is that everything is measured. As long as the measurements are ok, one should be fine (at least in this particular case). If the SPI flash content was replaced, the measurements should reflect that.

Consequently PCRs measurements for each PCRs are:

  • 2: Boot block, ROM stage, RAM stage, Heads linux kernel and initrd
  • 4: Boot mode (0 during /init, then recovery or normal-boot)
  • 5: Heads Linux kernel modules
  • 6: Drive LUKS headers
  • 7: Heads user-specific config files

No particular comments with that. It looks good.

@pietrushnic
Copy link

Bootblock is not protected by any means more to that IIRC it measures itself. Bootblock can easily lie about itself and this is an attack vector.

One issue with trusting the external structure is if an attacker can modify the CBFS struct to point to a clean copy, which is not used by the reset vector since it will use the 0xFFFFFFF0 legacy address, then the measurement might not reflect the reality of what was booted.

@osresearch if I understand the only way to do that is somehow trick SPI reading (spispy?), but do we really have to use such a sophisticated mechanism when we have no S-CRTM?

Also, I think @miczyg1 got it right and it doesn't matter if we hardcode or use CBFS API if this CBFS API is the problem we should distrust it in all cases not only in bootblock. Of course, hardcoding all addresses will eventually cause maintenance issues that's why if there is no difference in threat model we should use a more convenient mechanism.

@Tonux599
Copy link
Contributor Author

Tonux599 commented Nov 10, 2020

So just to note, I've been using kgpe-d16_workstation for about a week now with an nvidia card and Qubes 4.0 and can confirm that all appears stable and heads specific function (i.e. adding PGP keys to flash) work without error.

@osresearch I'm inclined to keep @miczyg1's implementation of locating bootblock unless you have anything further to argue? Your input on what @miczyg1 and @pietrushnic have said would really be appreciated.

@Tonux599
Copy link
Contributor Author

Force push due to email change (email updated on both GitHub and PGP key). HEAD is verified but commits before appear unverified. When clean-up + squash take place all commits will be signed and verified correctly.

Tonux599 pushed a commit to Tonux599/heads that referenced this pull request Nov 28, 2020
Changing CONFIG_USB_BOOT_DEV to sdc1, adding back CONFIG_BOOT_STATIC_IP to 192.168.2.3, adding dual console to OpenBMC and tty0 in attempt to have QubesOS graphic installer which complains with no networking when attempting to start VNC

Adding dual console to OpenBmc and tty0

putting kgpe-d16-coreboot.conf in defconfig format

NO_HZ wasn't included in kernel config. Adding it.

Wasn't able to have both console firing up QubesOS gui installer, complaining about hvc1 console errors. Splitting up Workstation and server config. This one works for Worstation

Removing serial configuration and static IP stuff since we have a workstation here.

Seperate Workstation and Server board configurations until dual console truely works through QubesOS gui installation. kgpe-d16 board config removed until then.

Placing files in good directories

Corrrect flashrom options for kgpe-d16 server and workstation boards

kgpe-d16 linux: NO_HZ_IDLE instead of NO_HZ

kgpe-d16: seperate board for workstation to be AST and gui-init based, while kgpe-d16-> kgpe-d16_server

kgpe-d16_server: boots, shows ASpeed text on VGA, controllable through BMC via SSH.

kgpe-d16_workstation on ASpeed console. WIP. (Includes CIs configs to build server and workstation)

kgpe-d16_workstation in defconfig format

kgpe-d16 boards: pass from GPG to GPG2 board definitions

kgpe-d16_workstation : Adding Cairo and FbWhpitail in board config for gui-init to work in FB mode

kgpe-d16: removing plymouth.ignore-serial-consoles to fix server terminal output

kgpe-d16: bring par with staging branch https://gitlab.com/tlaurion/heads/commits/kgpe-d16_staging

kgpe-d16 : expressively export CONFIG_TPM=n

kgpe-d16_wokstation gui-init variables were missing

kgpe-d16 boards: add CONFIG_LINUX_USB_COMPANION_CONTROLLER so that usb is recognized

linux-kgpe-d16*: add support for Pike

kgpe-d16_workstation-usb_keyboard board support addition

kgpe-d16_server-whiptail: Add board and dependencies to have gui-init in whiptail (console mode, not FbWhiptail based

GitlabCI: kgpe-d16 fixes and upstream merge of change

kgpe-d16* board: add statement to fixate coreboot version to 4.8.1 for the moment

kgpe-d16: add missing config/linux-kgpe-d16_server-whiptail.config file

KGPE-D16: community work migration to coreboot 4.11 to fix issue linuxboot#740

KGPE-D16 boards: Adding VBOOT+measured boot, musl-cross patch and 4.11 patch brought up per linuxboot#709

kgpe-d16* boards: add VBOOT Kconfig patch per @miczyg1 recommendation under linuxboot#795 (comment)

KGPE-D16* coreboot configs: Add S3NV as a Runtime data whitelist (so that it is not measured at term) per @miczyg1 recommendation under linuxboot#795 (comment)

kgpe-d16 coreboot 4.11: add https://review.coreboot.org/c/coreboot/+/36908 patch

kgpe-d16 boards: add Linux kernel version where missing.

CircleCI: Add debug output on fail for kgpe-d16 board builds to bring par with upstream after rebasing on master

coreboot module: typo correction (tabs vs spaces)

CircleCI: trying to address "g++: fatal error: Killed signal terminated program cc1plus." happening under coreboot 4.11 and coreboot 4.12 builds

CircleCI: remove past addition to test recommendation from CircleCI: "resource_class: large"

CircleCi: Ok.... lets output dmesg content prior of other logs.... I'm out of ideas. Next step, ask CircleCI for support

At this stage:
- job's "make --load" is supposed to guarantee that the number of thread doesn't exhaust pass of a load of 2 (medium, free class, CircleCI has 32 cores so possibility of a load of 32)
- "--max_old_space_size=4096" in CircleCI environement is supposed to limit memory consumption to 4096Mb of memory, the max of a medium class free tier CircleCI node

CircleCI: remove verbose build (no more V=1), in case of failed build, find all logs modified in last minute and output each of them on console.

coreboot module: implement load average respect inside of problematic CI build for coreboot 4.11+ being killed in the action (32 cores with 4Gb ram get gcc OOM)

coreboot module: replace nproc by number of Gb actually available as number of CPUs, since each thread is expected to have 1Gb of ram.

CircleCI & coreboot config: fix merge conflict rebasing on master

coreboot 4.11 kgpe-d16 vboot patches addendum, credits goes to @Tonux599

Fix merge conflicts and make sure all boards are inside of CircleCI builds. PoC build for linuxboot#867
@Tonux599
Copy link
Contributor Author

@tlaurion please see the latest four commits here which is squashed based more on authors as opposed to features (things ended up getting messy). If you're happy with this I can force push into this PR

tlaurion and others added 4 commits December 2, 2020 15:56
Changing CONFIG_USB_BOOT_DEV to sdc1, adding back CONFIG_BOOT_STATIC_IP to 192.168.2.3, adding dual console to OpenBMC and tty0 in attempt to have QubesOS graphic installer which complains with no networking when attempting to start VNC

Adding dual console to OpenBmc and tty0

putting kgpe-d16-coreboot.conf in defconfig format

NO_HZ wasn't included in kernel config. Adding it.

Wasn't able to have both console firing up QubesOS gui installer, complaining about hvc1 console errors. Splitting up Workstation and server config. This one works for Worstation

Removing serial configuration and static IP stuff since we have a workstation here.

Seperate Workstation and Server board configurations until dual console truely works through QubesOS gui installation. kgpe-d16 board config removed until then.

Placing files in good directories

Corrrect flashrom options for kgpe-d16 server and workstation boards

kgpe-d16 linux: NO_HZ_IDLE instead of NO_HZ

kgpe-d16: seperate board for workstation to be AST and gui-init based, while kgpe-d16-> kgpe-d16_server

kgpe-d16_server: boots, shows ASpeed text on VGA, controllable through BMC via SSH.

kgpe-d16_workstation on ASpeed console. WIP. (Includes CIs configs to build server and workstation)

kgpe-d16_workstation in defconfig format

kgpe-d16 boards: pass from GPG to GPG2 board definitions

kgpe-d16_workstation : Adding Cairo and FbWhpitail in board config for gui-init to work in FB mode

kgpe-d16: removing plymouth.ignore-serial-consoles to fix server terminal output

kgpe-d16: bring par with staging branch https://gitlab.com/tlaurion/heads/commits/kgpe-d16_staging

kgpe-d16 : expressively export CONFIG_TPM=n

kgpe-d16_wokstation gui-init variables were missing

kgpe-d16 boards: add CONFIG_LINUX_USB_COMPANION_CONTROLLER so that usb is recognized

linux-kgpe-d16*: add support for Pike

kgpe-d16_workstation-usb_keyboard board support addition

kgpe-d16_server-whiptail: Add board and dependencies to have gui-init in whiptail (console mode, not FbWhiptail based

GitlabCI: kgpe-d16 fixes and upstream merge of change

kgpe-d16* board: add statement to fixate coreboot version to 4.8.1 for the moment

kgpe-d16: add missing config/linux-kgpe-d16_server-whiptail.config file

KGPE-D16: community work migration to coreboot 4.11 to fix issue linuxboot#740

KGPE-D16 boards: Adding VBOOT+measured boot, musl-cross patch and 4.11 patch brought up per linuxboot#709

kgpe-d16* boards: add VBOOT Kconfig patch per @miczyg1 recommendation under linuxboot#795 (comment)

KGPE-D16* coreboot configs: Add S3NV as a Runtime data whitelist (so that it is not measured at term) per @miczyg1 recommendation under linuxboot#795 (comment)

kgpe-d16 coreboot 4.11: add https://review.coreboot.org/c/coreboot/+/36908 patch

kgpe-d16 boards: add Linux kernel version where missing.

CircleCI: Add debug output on fail for kgpe-d16 board builds to bring par with upstream after rebasing on master

coreboot module: typo correction (tabs vs spaces)

CircleCI: trying to address "g++: fatal error: Killed signal terminated program cc1plus." happening under coreboot 4.11 and coreboot 4.12 builds

CircleCI: remove past addition to test recommendation from CircleCI: "resource_class: large"

CircleCi: Ok.... lets output dmesg content prior of other logs.... I'm out of ideas. Next step, ask CircleCI for support

At this stage:
- job's "make --load" is supposed to guarantee that the number of thread doesn't exhaust pass of a load of 2 (medium, free class, CircleCI has 32 cores so possibility of a load of 32)
- "--max_old_space_size=4096" in CircleCI environement is supposed to limit memory consumption to 4096Mb of memory, the max of a medium class free tier CircleCI node

CircleCI: remove verbose build (no more V=1), in case of failed build, find all logs modified in last minute and output each of them on console.

coreboot module: implement load average respect inside of problematic CI build for coreboot 4.11+ being killed in the action (32 cores with 4Gb ram get gcc OOM)

coreboot module: replace nproc by number of Gb actually available as number of CPUs, since each thread is expected to have 1Gb of ram.

CircleCI & coreboot config: fix merge conflict rebasing on master

coreboot 4.11 kgpe-d16 vboot patches addendum, credits goes to @Tonux599

Fix merge conflicts and make sure all boards are inside of CircleCI builds. PoC build for linuxboot#867
Bring patches/coreboot-4.11 on par with master

Removed patches/coreboot-4.11/0020-kgpe-d16-vboot.patch
Removed Vboot options from KGPE-D16 coreboot configs

Enabled TPM in kgpe-d16 board configs
Enabled measured boot in kgpe-d16 coreboot configs.

Added support for video cards that require nouveau, radeon and amdgpu drivers in linux-kgpe-d16_workstation.config

`nouveau.config=NvForcePost=1` to be added to kexec'd kernels for better Nvidia card support.
…t defined, describe better board configs applications
	cp config/linux.. ./build/linux*/.config
	cd build/linux*
	make savedefconfig
	cp defconfig ../../config/linux..

Resulting in only linux-kgpe-d16_workstation.config being updated.

For KGPE-D16 workstation boards:
Remove `console=tty0` from `CONFIG_BOOT_KERNEL_ADD` as was blocking Qubes graphical installer (CLI installer was launched).
Comment out `export CONFIG_BOOT_KERNEL_REMOVE="plymouth.ignore-serial-consoles"` to provide a more desktop like experience.

Removed 0001-cpu-x86-smm-Use-PRIxPTR-to-print-uintptr_t.patch as already exists as 0000-cpu-x86-smm-Use-PRIxPTR-to-print-uintptr_t.patch

Added 0020-kgpe-d16_measured-boot-support.patch for coreboot 4.11

Fix TPM errors when microcode is measured by initialising TPM earlier and loading the microcode later.
Thanks to Michał Żygowski <miczyg1> for condition suggestion: `if (CONFIG(MEASURED_BOOT) && CONFIG(LPC_TPM) && boot_cpu())`

Locate bootblock location and size with CBFS API. Credit to: Michał Żygowski <miczyg1>
@Tonux599 Tonux599 changed the title (WIP) KGPE-D16 Coreboot 4.11 + Measured Boot KGPE-D16 Coreboot 4.11 + Measured Boot Dec 2, 2020
@Tonux599
Copy link
Contributor Author

Tonux599 commented Dec 2, 2020

Rebased. Ready imo @tlaurion

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 2, 2020

Reported functional for workstation. Lets just remember that past working commit was 014e592 prior of moving the kgpe-d16 boards to 4.11!

Awesome work @Tonux599 !!!!

@tlaurion tlaurion merged commit 1661e5d into linuxboot:master Dec 2, 2020
@Tonux599 Tonux599 deleted the kgpe-d16_411_measured-boot branch December 2, 2020 23:55
@blobless
Copy link

blobless commented Dec 3, 2020

I can confirm it works with CONFIG_TPM=n. Excellent job @Tonux599

@Tonux599
Copy link
Contributor Author

Tonux599 commented Dec 3, 2020

I can confirm it works with CONFIG_TPM=n. Excellent job @Tonux599

Thanks @blobless! There are TPM's available if you choose to use one. I use 'Asus 90-C1B0AU-00XBN0VZ' with no issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants