| Message ID | 20230801181917.8535-1-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Measuring TPM update counter in IMA | expand |
On 8/1/23 14:19, Tushar Sugandhi wrote: > Entries in IMA log may be lost due to code bugs, certain error conditions I hope we don't have such bugs. And I guess the most critical ones would be between logging and PCR extensions > being met etc. This can result in TPM PCRs getting out of sync with the > IMA log. One such example is events between kexec 'load' and kexec > 'execute' getting lost from the IMA log when the system soft-boots into > the new Kernel using kexec[1]. The remote attestation service does not Though this particular condition I thought would go away with your kexec series. The other conditions would be an out-of-memory or TPM failure. The OOM would probably be more critical since something that was supposed to be logged couldn't be logged and now you cannot show this anymore and presumably not even an error condition could be logged. https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_queue.c#L179 > have any information if the PCR mismatch with IMA log is because of loss > of entries in the IMA log or something else. TPM 2.0 provides an update > counter which is incremented each time a PCR is updated [2]. Measuring the > TPM PCR update counter in IMA subsystem will help the remote attestation > service to validate if there are any missing entries in the IMA log, when > the system goes through certain important state changes (e.g. kexec soft > boot, IMA log snapshotting etc.) > > This patch series provides the required functionality to measure the > update counter through IMA subsystem by - > - introducing a function to retrieve PCR update counter in the TPM > subsystem. > - IMA functionality to acquire the update counter from the TPM subsystem. > - Measuring the update counter at system boot and at kexec Kernel > load. Then the bugs you mentioned above that may happen between system boot and kexec load are still going to confuse anyone looking at the log and quote. I don't think you should mention them. I thought you would provide a way to sync up on every step... Also, I thought you had a variable in your kexec series that would prevent all further logging and measuring once the log had been marshalled during kexec 'exec' stage and this wasn't necessary. Stefan > > > This patch series would be a prerequisite for the next version of kexec > load/execute series[1] and the future IMA log snapshotting patch series. > > [1] https://lore.kernel.org/all/20230703215709.1195644-1-tusharsu@linux.microsoft.com/ > ima: measure events between kexec load and execute > > [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf > Section 22.4.2, Page 206. > > Tushar Sugandhi (6): > tpm: implement TPM2 function to get update counter > tpm: provide functionality to get update counter > ima: get TPM update counter > ima: implement functionality to measure TPM update counter > ima: measure TPM update counter at ima_init > kexec: measure TPM update counter in ima log at kexec load > > drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++ > drivers/char/tpm/tpm.h | 3 ++ > drivers/char/tpm/tpm2-cmd.c | 48 ++++++++++++++++++++++++++++++ > include/linux/ima.h | 1 + > include/linux/tpm.h | 8 +++++ > kernel/kexec_file.c | 3 ++ > security/integrity/ima/ima.h | 2 ++ > security/integrity/ima/ima_init.c | 3 ++ > security/integrity/ima/ima_main.c | 29 ++++++++++++++++++ > security/integrity/ima/ima_queue.c | 16 ++++++++++ > 10 files changed, 141 insertions(+) >
Thanks Stefan for reviewing this series. Appreciate it. Re-sending this email. I accidentally had some HTML content, the email bounced back from integrity mailing list. On 8/3/23 06:37, Stefan Berger wrote: > > > On 8/1/23 14:19, Tushar Sugandhi wrote: >> Entries in IMA log may be lost due to code bugs, certain error >> conditions > > I hope we don't have such bugs. And I guess the most critical ones > would be > between logging and PCR extensions I hope so too, but in general, it’s hard to prove a negative. That’s why this patch series. :) > > >> being met etc. This can result in TPM PCRs getting out of sync with the >> IMA log. One such example is events between kexec 'load' and kexec >> 'execute' getting lost from the IMA log when the system soft-boots into >> the new Kernel using kexec[1]. The remote attestation service does not > > Though this particular condition I thought would go away with your > kexec series. Currently the events in-between kexec ‘load’ and ‘execute’ are always lost – because IMA log is captured at ‘load’. My kexec series addresses this scenario. But as you said, there is always a possibility that the events will still be lost during kexec because of error conditions, OOM etc. > > The other conditions would be an out-of-memory or TPM failure. The OOM > would > probably be more critical since something that was supposed to be logged > couldn't be logged and now you cannot show this anymore and presumably > not even > an error condition could be logged. > Precisely. As you can see in patch 5 of this series, I am logging the counter at ima_init (ima_init_tpm_update_counter). So we will get the baseline counter at the system boot, where memory pressure is hopefully low. Even if we are not able to log the counter later due to OOM, this baseline counter along with the number of events in the IMA log should help detect if IMA log is missing events. > https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_queue.c#L179 > > >> have any information if the PCR mismatch with IMA log is because of loss >> of entries in the IMA log or something else. TPM 2.0 provides an update >> counter which is incremented each time a PCR is updated [2]. >> Measuring the >> TPM PCR update counter in IMA subsystem will help the remote attestation >> service to validate if there are any missing entries in the IMA log, >> when > > > >> the system goes through certain important state changes (e.g. kexec soft >> boot, IMA log snapshotting etc.) >> >> This patch series provides the required functionality to measure the >> update counter through IMA subsystem by - >> - introducing a function to retrieve PCR update counter in the TPM >> subsystem. >> - IMA functionality to acquire the update counter from the TPM >> subsystem. >> - Measuring the update counter at system boot and at kexec Kernel >> load. > > Then the bugs you mentioned above that may happen between system boot > and kexec > load are still going to confuse anyone looking at the log and quote. I > don't > think you should mention them. I thought you would provide a way to sync I used the kexec load-execute bug as an example to demonstrate the value of measuring update counter. There could be other examples which I am not aware of. As we discussed above, even when I fix the kexec bug – there is still a possibility that events may go missing in error/OOM cases. I can remove the kexec example if it is causing confusion. Please let me know. > up on every step... I don’t fully understand what you mean by “provide a way to sync up on every step”. Could you please elaborate? > > Also, I thought you had a variable in your kexec series that would > prevent all further > logging and measuring once the log had been marshalled during kexec > 'exec' stage > and this wasn't necessary. > No, the variable suspend_ima_measurements[1] suspends the measurements while copying the kexec buffer during kexec execute to ensure consistency of the IMA measurements. It doesn’t prevent all future logging. The variable is reset and the IMA measurements resume when the system boots into the new Kernel image. [1] https://patchwork.kernel.org/project/linux-integrity/patch/20230703215709.1195644-10-tusharsu@linux.microsoft.com/ ~Tushar > Stefan > >> >> >> This patch series would be a prerequisite for the next version of kexec >> load/execute series[1] and the future IMA log snapshotting patch series. >> >> [1] >> https://lore.kernel.org/all/20230703215709.1195644-1-tusharsu@linux.microsoft.com/ >> ima: measure events between kexec load and execute >> >> [2] >> https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf >> Section 22.4.2, Page 206. >> >> Tushar Sugandhi (6): >> tpm: implement TPM2 function to get update counter >> tpm: provide functionality to get update counter >> ima: get TPM update counter >> ima: implement functionality to measure TPM update counter >> ima: measure TPM update counter at ima_init >> kexec: measure TPM update counter in ima log at kexec load >> >> drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++ >> drivers/char/tpm/tpm.h | 3 ++ >> drivers/char/tpm/tpm2-cmd.c | 48 ++++++++++++++++++++++++++++++ >> include/linux/ima.h | 1 + >> include/linux/tpm.h | 8 +++++ >> kernel/kexec_file.c | 3 ++ >> security/integrity/ima/ima.h | 2 ++ >> security/integrity/ima/ima_init.c | 3 ++ >> security/integrity/ima/ima_main.c | 29 ++++++++++++++++++ >> security/integrity/ima/ima_queue.c | 16 ++++++++++ >> 10 files changed, 141 insertions(+) >>
On 8/3/23 17:30, Tushar Sugandhi wrote: > > Thanks Stefan for reviewing this series. Appreciate it. > > On 8/3/23 06:37, Stefan Berger wrote: >> >> >> On 8/1/23 14:19, Tushar Sugandhi wrote: >>> Entries in IMA log may be lost due to code bugs, certain error conditions >> >> I hope we don't have such bugs. And I guess the most critical ones would be >> between logging and PCR extensions >> > I hope so too, but in general, it’s hard to prove a negative. > That’s why this patch series. :) >>> being met etc. This can result in TPM PCRs getting out of sync with the >>> IMA log. One such example is events between kexec 'load' and kexec >>> 'execute' getting lost from the IMA log when the system soft-boots into >>> the new Kernel using kexec[1]. The remote attestation service does not >> >> Though this particular condition I thought would go away with your kexec series. >> > Currently the events in-between kexec ‘load’ and ‘execute’ are always > lost – because IMA log is captured at ‘load’. My kexec series addresses > this scenario. But as you said, there is always a possibility that the > events will still be lost during kexec because of error conditions, OOM > etc. >> The other conditions would be an out-of-memory or TPM failure. The OOM would >> probably be more critical since something that was supposed to be logged >> couldn't be logged and now you cannot show this anymore and presumably not even >> an error condition could be logged. >> > Precisely. As you can see in patch 5 of this series, I am logging the > counter at ima_init (ima_init_tpm_update_counter). So we will get the > baseline counter at the system boot, where memory pressure is hopefully > low. Even if we are not able to log the counter later due to OOM, this > baseline counter along with the number of events in the IMA log should > help detect if IMA log is missing events. How do you account for updates to other PCRs than PCR 10 that a user may use for whatever reason? I think you would have to have the TPM driver count the updates for PCR 10. Form what I can see there's one global PCR update counter for all PCRs and all banks. Also, if we hit an oom condition when logging then the PCR is not extended, which is good since otherwise we would be hopelessly out-of-sync. > >> https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_queue.c#L179 >> >>> have any information if the PCR mismatch with IMA log is because of loss >>> of entries in the IMA log or something else. TPM 2.0 provides an update >>> counter which is incremented each time a PCR is updated [2]. Measuring the >>> TPM PCR update counter in IMA subsystem will help the remote attestation >>> service to validate if there are any missing entries in the IMA log, when >> >> >> >>> the system goes through certain important state changes (e.g. kexec soft >>> boot, IMA log snapshotting etc.) >>> >>> This patch series provides the required functionality to measure the >>> update counter through IMA subsystem by - >>> - introducing a function to retrieve PCR update counter in the TPM >>> subsystem. >>> - IMA functionality to acquire the update counter from the TPM subsystem. >>> - Measuring the update counter at system boot and at kexec Kernel >>> load. >> >> Then the bugs you mentioned above that may happen between system boot and kexec >> load are still going to confuse anyone looking at the log and quote. I don't >> think you should mention them. I thought you would provide a way to sync > I used the kexec load-execute bug as an example to demonstrate the value of > measuring update counter. There could be other examples which I am not > aware of. As we discussed above, even when I fix the kexec bug – there is > still a possibility that events may go missing in error/OOM cases. Yes, and we're not extending the PCRs then either, which would be catastrophic if we were. > > I can remove the kexec example if it is causing confusion.> Please let me know. I am not convinced we need this series ... :-( Your kexec series prevents further logging and especially PCR extensions after the frozen measurement log has been created and in ima_add_template_entry(), if we hit an oom condition, then we luckily do not extend the PCR either. If either the log was to have one more entry than number PCR extensions occurred or vice versa, then the remote attestation service will see this mismatch no matter what and all the PCR update counter won't help (and is generally not a good indicator for this purpose imo) for it to recover from this. It's better to declare the system as un-trusted/ corrupted in this case then. Stefan >> up on every step... >> > I don’t fully understand what you mean by “provide a way to sync up > on every step”. Could you please elaborate? > >> Also, I thought you had a variable in your kexec series that would prevent all further >> logging and measuring once the log had been marshalled during kexec 'exec' stage >> and this wasn't necessary. >> > No, the variable suspend_ima_measurements[1] suspends the measurements > while copying the kexec buffer during kexec execute to ensure consistency > of the IMA measurements. It doesn’t prevent all future logging. The > variable is reset and the IMA measurements resume when the system boots > into the new Kernel image. > > [1] https://patchwork.kernel.org/project/linux-integrity/patch/20230703215709.1195644-10-tusharsu@linux.microsoft.com/ > > ~Tushar > >> Stefan >> >>> >>> >>> This patch series would be a prerequisite for the next version of kexec >>> load/execute series[1] and the future IMA log snapshotting patch series. >>> >>> [1] https://lore.kernel.org/all/20230703215709.1195644-1-tusharsu@linux.microsoft.com/ >>> ima: measure events between kexec load and execute >>> >>> [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf >>> Section 22.4.2, Page 206. >>> >>> Tushar Sugandhi (6): >>> tpm: implement TPM2 function to get update counter >>> tpm: provide functionality to get update counter >>> ima: get TPM update counter >>> ima: implement functionality to measure TPM update counter >>> ima: measure TPM update counter at ima_init >>> kexec: measure TPM update counter in ima log at kexec load >>> >>> drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++ >>> drivers/char/tpm/tpm.h | 3 ++ >>> drivers/char/tpm/tpm2-cmd.c | 48 ++++++++++++++++++++++++++++++ >>> include/linux/ima.h | 1 + >>> include/linux/tpm.h | 8 +++++ >>> kernel/kexec_file.c | 3 ++ >>> security/integrity/ima/ima.h | 2 ++ >>> security/integrity/ima/ima_init.c | 3 ++ >>> security/integrity/ima/ima_main.c | 29 ++++++++++++++++++ >>> security/integrity/ima/ima_queue.c | 16 ++++++++++ >>> 10 files changed, 141 insertions(+) >>>
On Thu, 2023-08-03 at 18:09 -0400, Stefan Berger wrote: > > I can remove the kexec example if it is causing confusion.> Please let me know. > > I am not convinced we need this series ... :-( Your kexec series prevents > further logging and especially PCR extensions after the frozen measurement log > has been created and in ima_add_template_entry(), if we hit an oom condition, > then we luckily do not extend the PCR either. If either the log was to have one > more entry than number PCR extensions occurred or vice versa, then the remote > attestation service will see this mismatch no matter what and all the PCR update > counter won't help (and is generally not a good indicator for this purpose imo) > for it to recover from this. It's better to declare the system as un-trusted/ > corrupted in this case then. As previously mentioned, there is a patch set that doesn't carry any records across kexec, if the the measurement list is too large, and another proposal to trim the measurement list. In both of these cases including a new IMA mesaurement record, at least after the boot_aggregate, would help simplify detecting whether the measurement list has been trimmed/truncated.
On 8/3/23 18:36, Mimi Zohar wrote: > On Thu, 2023-08-03 at 18:09 -0400, Stefan Berger wrote: >>> I can remove the kexec example if it is causing confusion.> Please let me know. >> >> I am not convinced we need this series ... :-( Your kexec series prevents >> further logging and especially PCR extensions after the frozen measurement log >> has been created and in ima_add_template_entry(), if we hit an oom condition, >> then we luckily do not extend the PCR either. If either the log was to have one >> more entry than number PCR extensions occurred or vice versa, then the remote >> attestation service will see this mismatch no matter what and all the PCR update >> counter won't help (and is generally not a good indicator for this purpose imo) >> for it to recover from this. It's better to declare the system as un-trusted/ >> corrupted in this case then. > > As previously mentioned, there is a patch set that doesn't carry any > records across kexec, if the the measurement list is too large, and > another proposal to trim the measurement list. > > In both of these cases including a new IMA mesaurement record, at least > after the boot_aggregate, would help simplify detecting whether the > measurement list has been trimmed/truncated. > And if you can detect that I would log an event but not using the PCR update counter. Unless the state of PCRs is also logged, it's going to be unrecoverable for a log+quote verifier from there. Stefan
Entries in IMA log may be lost due to code bugs, certain error conditions being met etc. This can result in TPM PCRs getting out of sync with the IMA log. One such example is events between kexec 'load' and kexec 'execute' getting lost from the IMA log when the system soft-boots into the new Kernel using kexec[1]. The remote attestation service does not have any information if the PCR mismatch with IMA log is because of loss of entries in the IMA log or something else. TPM 2.0 provides an update counter which is incremented each time a PCR is updated [2]. Measuring the TPM PCR update counter in IMA subsystem will help the remote attestation service to validate if there are any missing entries in the IMA log, when the system goes through certain important state changes (e.g. kexec soft boot, IMA log snapshotting etc.) This patch series provides the required functionality to measure the update counter through IMA subsystem by - - introducing a function to retrieve PCR update counter in the TPM subsystem. - IMA functionality to acquire the update counter from the TPM subsystem. - Measuring the update counter at system boot and at kexec Kernel load. This patch series would be a prerequisite for the next version of kexec load/execute series[1] and the future IMA log snapshotting patch series. [1] https://lore.kernel.org/all/20230703215709.1195644-1-tusharsu@linux.microsoft.com/ ima: measure events between kexec load and execute [2] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf Section 22.4.2, Page 206. Tushar Sugandhi (6): tpm: implement TPM2 function to get update counter tpm: provide functionality to get update counter ima: get TPM update counter ima: implement functionality to measure TPM update counter ima: measure TPM update counter at ima_init kexec: measure TPM update counter in ima log at kexec load drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++ drivers/char/tpm/tpm.h | 3 ++ drivers/char/tpm/tpm2-cmd.c | 48 ++++++++++++++++++++++++++++++ include/linux/ima.h | 1 + include/linux/tpm.h | 8 +++++ kernel/kexec_file.c | 3 ++ security/integrity/ima/ima.h | 2 ++ security/integrity/ima/ima_init.c | 3 ++ security/integrity/ima/ima_main.c | 29 ++++++++++++++++++ security/integrity/ima/ima_queue.c | 16 ++++++++++ 10 files changed, 141 insertions(+)