diff mbox series

[isar-cip-core,10/10] initramfs-crypt-hook: invalidate PCR7 after unlocking partitions

Message ID cc43a620895a8ddcf6745c7eb4564344ecdf74af.1733151072.git.jan.kiszka@siemens.com (mailing list archive)
State New
Headers show
Series Various initramfs hook improvements | expand

Commit Message

Jan Kiszka Dec. 2, 2024, 2:51 p.m. UTC 
From: Jan Kiszka <jan.kiszka@siemens.com>

This avoids that the running Linux can still access the partition keys
and possibly leak them. In the future, we may better address that by
measure boot.

Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
 .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Heinisch, Alexander Dec. 5, 2024, 10:05 a.m. UTC | #1 
> 
> This avoids that the running Linux can still access the partition keys and possibly leak them. In the future, we may better address that by measure boot.

Makes sense to lock the key once we leave initramfs

> 
> Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> index 834dea22..4bcb4277 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> @@ -258,6 +258,9 @@ for partition_set in $partition_sets; do
>  	finalize_tpm2_encryption "$part_device"
>  done
>  
> +# invalidate PCR7 to lock access to the disk keys tpm2_pcrextend 
> +7:sha1=1111111111111111111111111111111111111111,sha256=1111111111111111
> +111111111111111111111111111111111111111111111111
> +

From TCG Platform Firmware Profile Spec [1]: 

"Integrity measurements of the pre-OS environment MUST NOT be extended to the PCRs allocated to the
operating system. Any Host Platform configuration change that occurs after the Host Platform transfers
control to an operating system, or while resuming from other power states, is the purview of the operating
system and measurements of those events are extended to the PCRs allocated to the operating system.
PCR[0-7] are exclusively for the use of platform firmware. Operating system and OS present software
should never use those PCRs"

Since systemd uses PCR 11 to measure things done in initramfs, including events like
enter-initrd and leave-initrd, I recommend binding to PCR 11 in addition to PCR 7 to lock.

Working on a patch...

BR Alexander

[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-2.pdf
[2] https://github.com/systemd/systemd/blob/main/docs/TPM2_PCR_MEASUREMENTS.md

>  if [ -n "$watchdog_pid" ]; then
>  	kill "$watchdog_pid"
>  fi
> diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
> index 71ee44db..2145a6e5 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
> @@ -42,7 +42,7 @@ HOOK_ADD_MODULES = " \  HOOK_COPY_EXECS = " \
>      openssl mke2fs grep awk expr seq sleep basename uuidparse mountpoint \
>      e2fsck resize2fs cryptsetup \
> -    tpm2_pcrread tpm2_testparms tpm2_flushcontext \
> +    tpm2_pcrread tpm2_pcrextend tpm2_testparms tpm2_flushcontext \
>      /usr/lib/*/libgcc_s.so.1"
>  
>  HOOK_COPY_EXECS:append:clevis = " \
> --
> 2.43.0
>
Jan Kiszka Dec. 6, 2024, 8:24 a.m. UTC | #2 
On 05.12.24 11:05, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
>>
>> This avoids that the running Linux can still access the partition keys and possibly leak them. In the future, we may better address that by measure boot.
> 
> Makes sense to lock the key once we leave initramfs
> 
>>
>> Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
>>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>> index 834dea22..4bcb4277 100644
>> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>> @@ -258,6 +258,9 @@ for partition_set in $partition_sets; do
>>  	finalize_tpm2_encryption "$part_device"
>>  done
>>  
>> +# invalidate PCR7 to lock access to the disk keys tpm2_pcrextend 
>> +7:sha1=1111111111111111111111111111111111111111,sha256=1111111111111111
>> +111111111111111111111111111111111111111111111111
>> +
> 
> From TCG Platform Firmware Profile Spec [1]: 
> 
> "Integrity measurements of the pre-OS environment MUST NOT be extended to the PCRs allocated to the
> operating system. Any Host Platform configuration change that occurs after the Host Platform transfers
> control to an operating system, or while resuming from other power states, is the purview of the operating
> system and measurements of those events are extended to the PCRs allocated to the operating system.
> PCR[0-7] are exclusively for the use of platform firmware. Operating system and OS present software
> should never use those PCRs"
> 
> Since systemd uses PCR 11 to measure things done in initramfs, including events like
> enter-initrd and leave-initrd, I recommend binding to PCR 11 in addition to PCR 7 to lock.
> 
> Working on a patch...
> 

Looking forward!

Thanks,
Jan

> BR Alexander
> 
> [1] https://trustedcomputinggroup.org/wp-content/uploads/TCG-PC-Client-Platform-Firmware-Profile-Version-1.06-Revision-52_pub-2.pdf
> [2] https://github.com/systemd/systemd/blob/main/docs/TPM2_PCR_MEASUREMENTS.md
> 
>>  if [ -n "$watchdog_pid" ]; then
>>  	kill "$watchdog_pid"
>>  fi
>> diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
>> index 71ee44db..2145a6e5 100644
>> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
>> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
>> @@ -42,7 +42,7 @@ HOOK_ADD_MODULES = " \  HOOK_COPY_EXECS = " \
>>      openssl mke2fs grep awk expr seq sleep basename uuidparse mountpoint \
>>      e2fsck resize2fs cryptsetup \
>> -    tpm2_pcrread tpm2_testparms tpm2_flushcontext \
>> +    tpm2_pcrread tpm2_pcrextend tpm2_testparms tpm2_flushcontext \
>>      /usr/lib/*/libgcc_s.so.1"
>>  
>>  HOOK_COPY_EXECS:append:clevis = " \
>> --
>> 2.43.0
>>
Jan Kiszka Dec. 6, 2024, 8:59 a.m. UTC | #3 
On 06.12.24 09:24, Jan Kiszka wrote:
> On 05.12.24 11:05, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
>>>
>>> This avoids that the running Linux can still access the partition keys and possibly leak them. In the future, we may better address that by measure boot.
>>
>> Makes sense to lock the key once we leave initramfs
>>
>>>
>>> Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
>>>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>> index 834dea22..4bcb4277 100644
>>> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>> @@ -258,6 +258,9 @@ for partition_set in $partition_sets; do
>>>  	finalize_tpm2_encryption "$part_device"
>>>  done
>>>  
>>> +# invalidate PCR7 to lock access to the disk keys tpm2_pcrextend 
>>> +7:sha1=1111111111111111111111111111111111111111,sha256=1111111111111111
>>> +111111111111111111111111111111111111111111111111
>>> +
>>
>> From TCG Platform Firmware Profile Spec [1]: 
>>
>> "Integrity measurements of the pre-OS environment MUST NOT be extended to the PCRs allocated to the
>> operating system. Any Host Platform configuration change that occurs after the Host Platform transfers
>> control to an operating system, or while resuming from other power states, is the purview of the operating
>> system and measurements of those events are extended to the PCRs allocated to the operating system.
>> PCR[0-7] are exclusively for the use of platform firmware. Operating system and OS present software
>> should never use those PCRs"
>>
>> Since systemd uses PCR 11 to measure things done in initramfs, including events like
>> enter-initrd and leave-initrd, I recommend binding to PCR 11 in addition to PCR 7 to lock.
>>
>> Working on a patch...
>>
> 
> Looking forward!
> 

There is one catch, though: We will have to extend the key binding from
PCR7 to 7+11. That will not work for existing deployments using only
PCR7. We would have to make this configurable, I suppose.

Jan
Jan Kiszka Dec. 6, 2024, 9:45 a.m. UTC | #4 
On 06.12.24 09:59, Jan Kiszka wrote:
> On 06.12.24 09:24, Jan Kiszka wrote:
>> On 05.12.24 11:05, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
>>>>
>>>> This avoids that the running Linux can still access the partition keys and possibly leak them. In the future, we may better address that by measure boot.
>>>
>>> Makes sense to lock the key once we leave initramfs
>>>
>>>>
>>>> Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
>>>>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
>>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>> index 834dea22..4bcb4277 100644
>>>> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>> @@ -258,6 +258,9 @@ for partition_set in $partition_sets; do
>>>>  	finalize_tpm2_encryption "$part_device"
>>>>  done
>>>>  
>>>> +# invalidate PCR7 to lock access to the disk keys tpm2_pcrextend 
>>>> +7:sha1=1111111111111111111111111111111111111111,sha256=1111111111111111
>>>> +111111111111111111111111111111111111111111111111
>>>> +
>>>
>>> From TCG Platform Firmware Profile Spec [1]: 
>>>
>>> "Integrity measurements of the pre-OS environment MUST NOT be extended to the PCRs allocated to the
>>> operating system. Any Host Platform configuration change that occurs after the Host Platform transfers
>>> control to an operating system, or while resuming from other power states, is the purview of the operating
>>> system and measurements of those events are extended to the PCRs allocated to the operating system.
>>> PCR[0-7] are exclusively for the use of platform firmware. Operating system and OS present software
>>> should never use those PCRs"
>>>
>>> Since systemd uses PCR 11 to measure things done in initramfs, including events like
>>> enter-initrd and leave-initrd, I recommend binding to PCR 11 in addition to PCR 7 to lock.
>>>
>>> Working on a patch...
>>>
>>
>> Looking forward!
>>
> 
> There is one catch, though: We will have to extend the key binding from
> PCR7 to 7+11. That will not work for existing deployments using only
> PCR7. We would have to make this configurable, I suppose.
> 

This likely needs some more thoughts and possibly controls. I'm dropping
this patch for now so that we are not rushing something into the
upcoming release which will cause more headaches than benefits.

Jan
Heinisch, Alexander Dec. 6, 2024, 10:58 a.m. UTC | #5 
>On 06.12.24 09:59, Jan Kiszka wrote:
>> On 06.12.24 09:24, Jan Kiszka wrote:
>>> On 05.12.24 11:05, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
>>>>>
>>>>> This avoids that the running Linux can still access the partition keys and possibly leak them. In the future, we may better address that by measure boot.
>>>>
>>>> Makes sense to lock the key once we leave initramfs
>>>>
>>>>>
>>>>> Suggested-by: Fabian Bläse <fabian.blaese@siemens-healthineers.com>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>>  .../initramfs-crypt-hook/files/local-top-complete              | 3 +++
>>>>>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb           | 2 +-
>>>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git 
>>>>> a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete 
>>>>> b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>>> index 834dea22..4bcb4277 100644
>>>>> --- 
>>>>> a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-comple
>>>>> +++ te
>>>>> @@ -258,6 +258,9 @@ for partition_set in $partition_sets; do
>>>>>  	finalize_tpm2_encryption "$part_device"
>>>>>  done
>>>>>  
>>>>> +# invalidate PCR7 to lock access to the disk keys tpm2_pcrextend
>>>>> +7:sha1=1111111111111111111111111111111111111111,sha256=11111111111
>>>>> +11111
>>>>> +111111111111111111111111111111111111111111111111
>>>>> +
>>>>
>>>> From TCG Platform Firmware Profile Spec [1]: 
>>>>
>>>> "Integrity measurements of the pre-OS environment MUST NOT be 
>>>> extended to the PCRs allocated to the operating system. Any Host 
>>>> Platform configuration change that occurs after the Host Platform 
>>>> transfers control to an operating system, or while resuming from other power states, is the purview of the operating system and measurements of those events are extended to the PCRs allocated to the operating system.
>>>> PCR[0-7] are exclusively for the use of platform firmware. Operating 
>>>> system and OS present software should never use those PCRs"
>>>>
>>>> Since systemd uses PCR 11 to measure things done in initramfs, 
>>>> including events like enter-initrd and leave-initrd, I recommend binding to PCR 11 in addition to PCR 7 to lock.
>>>>
>>>> Working on a patch...
>>>>
>>>
>>> Looking forward!

The patch is actually done and works for bookworm,
still testing the clevis part for bullseye and buster!

>>>
>> 
>> There is one catch, though: We will have to extend the key binding 
>> from
>> PCR7 to 7+11. That will not work for existing deployments using only 
>> PCR7. We would have to make this configurable, I suppose.

For existing deployments this addition won't apply.
But at least we don't break them!
So new deployments will benefit from that change, while
old ones remain intact as they are now.
>> 
>
>This likely needs some more thoughts and possibly controls. I'm dropping this patch for now so that we are not rushing something into the upcoming release which will cause more headaches than benefits.

The whole tpm handling needs to be extended to support desired changes of 
pcr measures like updating the secure boot keys, but also for scenarios where 
people want to bind to multiple measures e.g. specific ukis.

That way, the addition is still an improvement, although use cases like the one 
above and limitations of systemd-cryptenroll/systemd-cryptsetup (also incl. 
current versions) have to be considered!

>
>Jan
>
>--
>Siemens AG, Technology
>Linux Expert Center
>

BR Alexander
diff mbox series

Patch

diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
index 834dea22..4bcb4277 100644
--- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
+++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
@@ -258,6 +258,9 @@  for partition_set in $partition_sets; do
 	finalize_tpm2_encryption "$part_device"
 done
 
+# invalidate PCR7 to lock access to the disk keys
+tpm2_pcrextend 7:sha1=1111111111111111111111111111111111111111,sha256=1111111111111111111111111111111111111111111111111111111111111111
+
 if [ -n "$watchdog_pid" ]; then
 	kill "$watchdog_pid"
 fi
diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
index 71ee44db..2145a6e5 100644
--- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
+++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.5.bb
@@ -42,7 +42,7 @@  HOOK_ADD_MODULES = " \
 HOOK_COPY_EXECS = " \
     openssl mke2fs grep awk expr seq sleep basename uuidparse mountpoint \
     e2fsck resize2fs cryptsetup \
-    tpm2_pcrread tpm2_testparms tpm2_flushcontext \
+    tpm2_pcrread tpm2_pcrextend tpm2_testparms tpm2_flushcontext \
     /usr/lib/*/libgcc_s.so.1"
 
 HOOK_COPY_EXECS:append:clevis = " \