| Message ID | 20211119114745.1560453-1-ilias.apalodimas@linaro.org |
|---|---|
| Headers | show |
| Series | measure initrd data loaded by the EFI stub | expand |
Hi Ard, On Sun, Nov 21, 2021 at 05:12:00PM +0100, Ard Biesheuvel wrote: > On Fri, 19 Nov 2021 at 12:48, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi! > > > > This is a respin of [1]. > > This enables initrd measuring when loaded directly by the kernel EFI stub. > > It ensures that the initrd observed and used by the OS is the same one that > > got measured into the TPM, which is difficult to guarantee in the current > > otherwise. > > > > There's a couple of changes compared to the original RFC: > > - Ard fixed the x86 assembly for providing the extra arguments needed and I > > rebased it on top of > > commit 22aa45cb465b ("x86/efi: Restore Firmware IDT before calling ExitBootServices()") > > - Instead of EV_IPL we are now using EV_EVENT_TAG. EV_IPL was marked > > as deprecated up until the latest PC client spec [2] (and deprecated in > > older revisions [3]). It's current description is: > > "It may be used by Boot Manager Code to measure events." > > > > EV_EVENT_TAG on the other hand seems more appropriate as it's defined as: > > "Used for PCRs defined for OS and application usage. Defined for use by > > Host Platform Operating System or Software." > > - We are only measuring the initrd if it was loaded using the LOAD_FILE2 > > protocol. This is not what we probably want in the long run, but let's > > only enforce the measurement on the new way of loading an initrd for now. > > > > Questions: > > - Since GRUB seems to be using PCRs 8/9 for it's EV_IPL events, maybe we should > > use something completely different? > > > > Thanks for continuing to work on this. You're welcome! > > I think using PCRs 8/9 is fine - if one upgrades to a new version of > the kernel that implements this change, the PCRs will change in any > case. The reasoning here is leave distros unaffected. Yes the PCRs will change regardless in a kernel update. However distros might already have infrastructure to seal keys factoring in PCRs 8 and 9. Keeping in mind the initrd is likely to change without changing GRUB, we could allow them to opt-in on the initrd measurement using PCR10 (up to 15). > > The only thing that is unclear to me is whether we should measure some > kind of separator event if no initrd is loaded at all - IIRC, this > came up before but I don't remember what the conclusion was in the > end. > I think we ended up saying along the lines of "We need more discussion. Let's check what Windows is doing". I did have a look on that Eventlog James included, but my windows understanding is mediocre at best. OTOH separators are used for PCRs 0-7 and they either indicate errors or delimit actions during the boot process. We already have separators before EBS so I skipped it for the initrd. Moreover I found nothing relevant to the spec wrt to tagged events and separators (apart from a mention to a Specification for Conventional BIOS). Delimiting actions during the boot process is useful for example when you want a key sealed against specific PCRs extended by the firmware, while not allowing later boot stages access it. I can't think of such a usage for the initrd. Obviously if anyone can and it makes sense I'll go add it. Thanks! /Ilias > > > I did test this on arm64 and x86_64 (incl mixed mode). Here's a snip of the > > EventLog > > <snip> > > - EventNum: 23 > > PCRIndex: 9 > > EventType: EV_EVENT_TAG > > DigestCount: 4 > > Digests: > > - AlgorithmId: sha1 > > Digest: "53fe403e0d763f6a4e3384e8112d6463e7ddf12b" > > - AlgorithmId: sha256 > > Digest: "28f24eab8cb433e4b8cbce0f96b7ad0aa541a4b905f748491139e42f0adc8026" > > - AlgorithmId: sha384 > > Digest: "848389ab172267dcf9a0b873c7534b3d969e915b525c9fe2b57db47f4ecd8283b18d6e0cff84099893d589447c2bea55" > > - AlgorithmId: sha512 > > Digest: "438b254c92e6716a5a1ba0338f5e751f3dd27782481e5698911b4cd33a98efdd776459d56781c5ae4a8d0b9945246d04ab243d4dbe03f49542e2455ac66db584" > > EventSize: 21 > > Event: "ec223b8f0d0000004c696e757820696e6974726400" > > <snip> > > > > [1] https://lore.kernel.org/linux-efi/20201102170634.20575-1-ardb@kernel.org/ > > [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf > > [3] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientSpecPlat_TPM_2p0_1p04_pub.pdf > > > > Ard Biesheuvel (3): > > efi/libstub: add prototype of > > efi_tcg2_protocol::hash_log_extend_event() > > efi/libstub: x86/mixed: increase supported argument count > > efi/libstub: consolidate initrd handling across architectures > > > > Ilias Apalodimas (1): > > efi/libstub: measure loaded initrd info into the TPM > > > > arch/x86/boot/compressed/efi_thunk_64.S | 14 +++- > > arch/x86/include/asm/efi.h | 14 +++- > > arch/x86/platform/efi/efi_thunk_64.S | 14 +++- > > .../firmware/efi/libstub/efi-stub-helper.c | 73 ++++++++++++++++--- > > drivers/firmware/efi/libstub/efi-stub.c | 10 +-- > > drivers/firmware/efi/libstub/efistub.h | 30 +++++++- > > drivers/firmware/efi/libstub/x86-stub.c | 26 +++---- > > 7 files changed, 134 insertions(+), 47 deletions(-) > > > > -- > > 2.33.1 > >
On Sun, 21 Nov 2021 at 19:37, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Ard, > > On Sun, Nov 21, 2021 at 05:12:00PM +0100, Ard Biesheuvel wrote: > > On Fri, 19 Nov 2021 at 12:48, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > Hi! > > > > > > This is a respin of [1]. > > > This enables initrd measuring when loaded directly by the kernel EFI stub. > > > It ensures that the initrd observed and used by the OS is the same one that > > > got measured into the TPM, which is difficult to guarantee in the current > > > otherwise. > > > > > > There's a couple of changes compared to the original RFC: > > > - Ard fixed the x86 assembly for providing the extra arguments needed and I > > > rebased it on top of > > > commit 22aa45cb465b ("x86/efi: Restore Firmware IDT before calling ExitBootServices()") > > > - Instead of EV_IPL we are now using EV_EVENT_TAG. EV_IPL was marked > > > as deprecated up until the latest PC client spec [2] (and deprecated in > > > older revisions [3]). It's current description is: > > > "It may be used by Boot Manager Code to measure events." > > > > > > EV_EVENT_TAG on the other hand seems more appropriate as it's defined as: > > > "Used for PCRs defined for OS and application usage. Defined for use by > > > Host Platform Operating System or Software." > > > - We are only measuring the initrd if it was loaded using the LOAD_FILE2 > > > protocol. This is not what we probably want in the long run, but let's > > > only enforce the measurement on the new way of loading an initrd for now. > > > > > > Questions: > > > - Since GRUB seems to be using PCRs 8/9 for it's EV_IPL events, maybe we should > > > use something completely different? > > > > > > > Thanks for continuing to work on this. > > You're welcome! > > > > > I think using PCRs 8/9 is fine - if one upgrades to a new version of > > the kernel that implements this change, the PCRs will change in any > > case. > > The reasoning here is leave distros unaffected. Yes the PCRs will > change regardless in a kernel update. However distros might already > have infrastructure to seal keys factoring in PCRs 8 and 9. Keeping > in mind the initrd is likely to change without changing GRUB, we > could allow them to opt-in on the initrd measurement using PCR10 (up > to 15). > > > > > The only thing that is unclear to me is whether we should measure some > > kind of separator event if no initrd is loaded at all - IIRC, this > > came up before but I don't remember what the conclusion was in the > > end. > > > > I think we ended up saying along the lines of "We need more > discussion. Let's check what Windows is doing". I did have a look on > that Eventlog James included, but my windows understanding is > mediocre at best. OTOH separators are used for PCRs 0-7 and they > either indicate errors or delimit actions during the boot process. We > already have separators before EBS so I skipped it for the initrd. > Moreover I found nothing relevant to the spec wrt to tagged events and > separators (apart from a mention to a Specification for Conventional > BIOS). > Delimiting actions during the boot process is useful for example when > you want a key sealed against specific PCRs extended by the firmware, > while not allowing later boot stages access it. I can't think of > such a usage for the initrd. Obviously if anyone can and it makes > sense I'll go add it. > Thanks. I've queued these up so they should appear in -next shortly. > > > > > I did test this on arm64 and x86_64 (incl mixed mode). Here's a snip of the > > > EventLog > > > <snip> > > > - EventNum: 23 > > > PCRIndex: 9 > > > EventType: EV_EVENT_TAG > > > DigestCount: 4 > > > Digests: > > > - AlgorithmId: sha1 > > > Digest: "53fe403e0d763f6a4e3384e8112d6463e7ddf12b" > > > - AlgorithmId: sha256 > > > Digest: "28f24eab8cb433e4b8cbce0f96b7ad0aa541a4b905f748491139e42f0adc8026" > > > - AlgorithmId: sha384 > > > Digest: "848389ab172267dcf9a0b873c7534b3d969e915b525c9fe2b57db47f4ecd8283b18d6e0cff84099893d589447c2bea55" > > > - AlgorithmId: sha512 > > > Digest: "438b254c92e6716a5a1ba0338f5e751f3dd27782481e5698911b4cd33a98efdd776459d56781c5ae4a8d0b9945246d04ab243d4dbe03f49542e2455ac66db584" > > > EventSize: 21 > > > Event: "ec223b8f0d0000004c696e757820696e6974726400" > > > <snip> > > > > > > [1] https://lore.kernel.org/linux-efi/20201102170634.20575-1-ardb@kernel.org/ > > > [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf > > > [3] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientSpecPlat_TPM_2p0_1p04_pub.pdf > > > > > > Ard Biesheuvel (3): > > > efi/libstub: add prototype of > > > efi_tcg2_protocol::hash_log_extend_event() > > > efi/libstub: x86/mixed: increase supported argument count > > > efi/libstub: consolidate initrd handling across architectures > > > > > > Ilias Apalodimas (1): > > > efi/libstub: measure loaded initrd info into the TPM > > > > > > arch/x86/boot/compressed/efi_thunk_64.S | 14 +++- > > > arch/x86/include/asm/efi.h | 14 +++- > > > arch/x86/platform/efi/efi_thunk_64.S | 14 +++- > > > .../firmware/efi/libstub/efi-stub-helper.c | 73 ++++++++++++++++--- > > > drivers/firmware/efi/libstub/efi-stub.c | 10 +-- > > > drivers/firmware/efi/libstub/efistub.h | 30 +++++++- > > > drivers/firmware/efi/libstub/x86-stub.c | 26 +++---- > > > 7 files changed, 134 insertions(+), 47 deletions(-) > > > > > > -- > > > 2.33.1 > > >
Hi! This is a respin of [1]. This enables initrd measuring when loaded directly by the kernel EFI stub. It ensures that the initrd observed and used by the OS is the same one that got measured into the TPM, which is difficult to guarantee in the current otherwise. There's a couple of changes compared to the original RFC: - Ard fixed the x86 assembly for providing the extra arguments needed and I rebased it on top of commit 22aa45cb465b ("x86/efi: Restore Firmware IDT before calling ExitBootServices()") - Instead of EV_IPL we are now using EV_EVENT_TAG. EV_IPL was marked as deprecated up until the latest PC client spec [2] (and deprecated in older revisions [3]). It's current description is: "It may be used by Boot Manager Code to measure events." EV_EVENT_TAG on the other hand seems more appropriate as it's defined as: "Used for PCRs defined for OS and application usage. Defined for use by Host Platform Operating System or Software." - We are only measuring the initrd if it was loaded using the LOAD_FILE2 protocol. This is not what we probably want in the long run, but let's only enforce the measurement on the new way of loading an initrd for now. Questions: - Since GRUB seems to be using PCRs 8/9 for it's EV_IPL events, maybe we should use something completely different? I did test this on arm64 and x86_64 (incl mixed mode). Here's a snip of the EventLog <snip> - EventNum: 23 PCRIndex: 9 EventType: EV_EVENT_TAG DigestCount: 4 Digests: - AlgorithmId: sha1 Digest: "53fe403e0d763f6a4e3384e8112d6463e7ddf12b" - AlgorithmId: sha256 Digest: "28f24eab8cb433e4b8cbce0f96b7ad0aa541a4b905f748491139e42f0adc8026" - AlgorithmId: sha384 Digest: "848389ab172267dcf9a0b873c7534b3d969e915b525c9fe2b57db47f4ecd8283b18d6e0cff84099893d589447c2bea55" - AlgorithmId: sha512 Digest: "438b254c92e6716a5a1ba0338f5e751f3dd27782481e5698911b4cd33a98efdd776459d56781c5ae4a8d0b9945246d04ab243d4dbe03f49542e2455ac66db584" EventSize: 21 Event: "ec223b8f0d0000004c696e757820696e6974726400" <snip> [1] https://lore.kernel.org/linux-efi/20201102170634.20575-1-ardb@kernel.org/ [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf [3] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientSpecPlat_TPM_2p0_1p04_pub.pdf Ard Biesheuvel (3): efi/libstub: add prototype of efi_tcg2_protocol::hash_log_extend_event() efi/libstub: x86/mixed: increase supported argument count efi/libstub: consolidate initrd handling across architectures Ilias Apalodimas (1): efi/libstub: measure loaded initrd info into the TPM arch/x86/boot/compressed/efi_thunk_64.S | 14 +++- arch/x86/include/asm/efi.h | 14 +++- arch/x86/platform/efi/efi_thunk_64.S | 14 +++- .../firmware/efi/libstub/efi-stub-helper.c | 73 ++++++++++++++++--- drivers/firmware/efi/libstub/efi-stub.c | 10 +-- drivers/firmware/efi/libstub/efistub.h | 30 +++++++- drivers/firmware/efi/libstub/x86-stub.c | 26 +++---- 7 files changed, 134 insertions(+), 47 deletions(-)