Measuring TPM update counter in IMA
Subsystems (8)
cover Measuring TPM update counter in IMA
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/[email protected]/
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(+)
Signature · 1 line
2.25.1
PATCH 1/6 tpm: implement TPM2 function to get update counter
The TPM2_PCR_Read command returns TPM2_PCR_Read Response struct[1]. It contains pcrUpdateCounter member which contains the current value of TPM PCR update counter. The update counter provides the number of times the PCRs are updated, which is essential for tracking changes and verifying system integrity. Thus, subsystems (like IMA) should measure pcrUpdateCounter value. Although tpm2_pcr_read_out struct is returned by tpm2_pcr_read(), it is not used by it's caller function tpm_pcr_read(). Further, TPM2_PCR_Read Response struct and pcrUpdateCounter is not available in tpm1_pcr_read().
PcrUpdateCounter is only needed in a specific case (IMA for measurements). Changing tpm_pcr_read() and tpm2_pcr_read() function signature to return tpm2_pcr_read_out struct would be a more disruptive change, since these functions are used elsewhere too. Creating separate functions to get pcrUpdateCounter when needed would be a cleaner approach.
Add a function, 'tpm2_pcr_get_update_counter()' to retrieve the update counter for a given PCR index and algorithm ID on a TPM2 chip.
This function complements existing TPM functionalities such as reading and extending PCRs, and enhances the ability to monitor PCR status in the Linux Kernel.
[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf
Section 22.4.2, Page 206.
--- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm2-cmd.c | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+)
drivers/char/tpm/tpm.h +3 -0
@@ -288,6 +288,9 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)int tpm2_get_timeouts (struct tpm_chip *chip);int tpm2_pcr_read (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digest,u16 *digest_size_ptr); +int tpm2_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id, +u32 *update_counter);int tpm2_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests);int tpm2_get_random (struct tpm_chip *chip,u8 *dest,size_t max);
drivers/char/tpm/tpm2-cmd.c +48 -0
54 lines not on this page · raw message
Signature · 1 line
2.25.1
Jarkko Sakkinen
maintainerRe: [PATCH 1/6] tpm: implement TPM2 function to get update counter
The short summary is cryptic to say the least.
"update counter" does not map it to have anything to do with PCRs.
Why not "tpm: Read pcrUpdateCounter field from TPM2_PCR_Read"?
Quoted 37 lines · Tushar Sugandhi
The
TPM2_PCR_Readcommand returnsTPM2_PCR_ReadResponse struct[1]. It contains pcrUpdateCounter member which contains the current value of TPM PCR update counter. The update counter provides the number of times the PCRs are updated, which is essential for tracking changes and verifying system integrity. Thus, subsystems (like IMA) should measure pcrUpdateCounter value. Althoughtpm2_pcr_read_outstruct is returned bytpm2_pcr_read(), it is not used by it's caller functiontpm_pcr_read(). Further,TPM2_PCR_ReadResponse struct and pcrUpdateCounter is not available intpm1_pcr_read().PcrUpdateCounter is only needed in a specific case (IMA for measurements). Changing
tpm_pcr_read()andtpm2_pcr_read()function signature to returntpm2_pcr_read_outstruct would be a more disruptive change, since these functions are used elsewhere too. Creating separate functions to get pcrUpdateCounter when needed would be a cleaner approach.Add a function, '
tpm2_pcr_get_update_counter()' to retrieve the update counter for a given PCR index and algorithm ID on a TPM2 chip.This function complements existing TPM functionalities such as reading and extending PCRs, and enhances the ability to monitor PCR status in the Linux Kernel.
[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf
Section 22.4.2, Page 206.Signed-off-by Tushar Sugandhi <[email protected]>--- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm2-cmd.c | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+)drivers/char/tpm/tpm.h +0
-0
drivers/char/tpm/tpm.h static inline void tpm_add_ppi(struct tpm_chip *chip) · line 293
Quoted 6 lines
drivers/char/tpm/tpm.h +3
-0@@ -288,6 +288,9 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)int tpm2_get_timeouts (struct tpm_chip *chip);int tpm2_pcr_read (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digest,u16 *digest_size_ptr); +int tpm2_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id, +u32 *update_counter);
tpm_pcr_read_update_cnt()
Quoted 64 lines
drivers/char/tpm/tpm.h +0
-0int tpm2_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests);int tpm2_get_random (struct tpm_chip *chip,u8 *dest,size_t max);drivers/char/tpm/tpm2-cmd.c +48
-0@@ -216,6 +216,54 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,return rc; } +/** +* tpm2_pcr_get_update_counter() - gets an update counter value for a PCR bank +* @chip: TPM chip to use +* @pcr_idx: PCR index used to retrieve the update counter +* @alg_id: alg id used to retrieve the update counter +* @update_counter: output update counter value +* +* Return: Same as with tpm_transmit_cmd. +*/ +int tpm2_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id,u32 *update_counter) +{ +int rc; +struct tpm_buf buf; +struct tpm2_pcr_read_out *read_out; +u8 pcr_select[TPM2_PCR_SELECT_MIN ] = {0 }; + +if (pcr_idx >=TPM2_PLATFORM_PCR ) +return -EINVAL ; + +if (!update_counter) +return -EINVAL ; + + rc =tpm_buf_init (&buf,TPM2_ST_NO_SESSIONS ,TPM2_CC_PCR_READ ); +if (rc) +return rc; + + pcr_select[pcr_idx >>3 ] =1 << (pcr_idx &0x7 ); + +tpm_buf_append_u32 (&buf,1 ); +tpm_buf_append_u16 (&buf, alg_id); +tpm_buf_append_u8 (&buf,TPM2_PCR_SELECT_MIN ); +tpm_buf_append (&buf, (const unsigned char *)pcr_select, +sizeof (pcr_select)); + + rc =tpm_transmit_cmd (chip, &buf,0 ,"attempting to read a pcr value" ); +if (rc) + goto out; + + read_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE ]; + + *update_counter =be32_to_cpu (read_out->update_cnt); + +out: +tpm_buf_destroy (&buf); +return rc; +} +struct tpm2_null_auth_area {__be32 handle;__be16 nonce_size;Signature · 1 line
2.25.1
BR, Jarkko
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Thanks for the response Jarkko.
Quoted 1 line · Jarkko Sakkinen
The short summary is cryptic to say the least.
Do you mean the patch subject line, or the description below?
Quoted 1 line
"update counter" does not map it to have anything to do with PCRs.
Agreed. I noticed that when I was testing the patches. The update counter is same for all PCRs. It was also the same for the two hash algo's I tested it for (SHA1 and SHA256). But the spec description and Kernel implementation requires to pass the pcr_idx and hash algo to PCR_Read command to get the update counter.
Quoted 1 line
Why not "tpm: Read pcrUpdateCounter field from
TPM2_PCR_Read"?
As I said in the patch description below, update counter is only needed for IMA measurements. None of the other code that calls tpm2_pcr_read() use the update counter.
I was not sure if you were okay changing the function signature and implementation of tpm2_pcr_read(). It felt disruptive.
But I can update tpm2_pcr_read() if you are ok with it. Please let me know.
I also have a few more thoughts on this in the comment below.
Quoted 33 lines · Tushar Sugandhi
Quoted 32 lines
The
TPM2_PCR_Readcommand returnsTPM2_PCR_ReadResponse struct[1]. It contains pcrUpdateCounter member which contains the current value of TPM PCR update counter. The update counter provides the number of times the PCRs are updated, which is essential for tracking changes and verifying system integrity. Thus, subsystems (like IMA) should measure pcrUpdateCounter value. Althoughtpm2_pcr_read_outstruct is returned bytpm2_pcr_read(), it is not used by it's caller functiontpm_pcr_read(). Further,TPM2_PCR_ReadResponse struct and pcrUpdateCounter is not available intpm1_pcr_read().PcrUpdateCounter is only needed in a specific case (IMA for measurements). Changing
tpm_pcr_read()andtpm2_pcr_read()function signature to returntpm2_pcr_read_outstruct would be a more disruptive change, since these functions are used elsewhere too. Creating separate functions to get pcrUpdateCounter when needed would be a cleaner approach.Add a function, '
tpm2_pcr_get_update_counter()' to retrieve the update counter for a given PCR index and algorithm ID on a TPM2 chip.This function complements existing TPM functionalities such as reading and extending PCRs, and enhances the ability to monitor PCR status in the Linux Kernel.
[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf
Section 22.4.2, Page 206.Signed-off-by Tushar Sugandhi <[email protected]>--- drivers/char/tpm/tpm.h | 3 +++ drivers/char/tpm/tpm2-cmd.c | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+)
Quoted 12 lines
drivers/char/tpm/tpm.h static inline void tpm_add_ppi(struct tpm_chip *chip) · line 293
Quoted 11 lines
drivers/char/tpm/tpm.h +3
-0@@ -288,6 +288,9 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)int tpm2_get_timeouts (struct tpm_chip *chip);int tpm2_pcr_read (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digest,u16 *digest_size_ptr); +int tpm2_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id, +u32 *update_counter);
tpm_pcr_read_update_cnt()
I can rename 'get' -> 'read'
About 'tpm2' -> 'tpm':
I already have tpm_pcr_get_update_counter() defined in patch 2.
I was following the existing pattern here in patch 1 and 2 i. e.
- Implementing the tpm1/tpm2 specific functionality in drivers/char/tpm/tpm.h, drivers/char/tpm/tpm2-cmd.c, drivers/char/tpm/tpm1-cmd.c.
- And combining that functionality in drivers/char/tpm/tpm-interface.c and exposing it to other subsystems (like IMA) through include/linux/tpm.h (patch 2 of this series)
BTW, if I understand correctly, the update counter is not available in TPM 1.2.
Please let me know if you want me to expose the functionality directly from drivers/char/tpm/tpm2-cmd.c and getting rid of patch #2 of this series.
~Tushar
Quoted 65 lines
Quoted 64 lines
drivers/char/tpm/tpm.h +0
-0int tpm2_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests);int tpm2_get_random (struct tpm_chip *chip,u8 *dest,size_t max);drivers/char/tpm/tpm2-cmd.c +48
-0@@ -216,6 +216,54 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,return rc; } +/** +* tpm2_pcr_get_update_counter() - gets an update counter value for a PCR bank +* @chip: TPM chip to use +* @pcr_idx: PCR index used to retrieve the update counter +* @alg_id: alg id used to retrieve the update counter +* @update_counter: output update counter value +* +* Return: Same as with tpm_transmit_cmd. +*/ +int tpm2_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id,u32 *update_counter) +{ +int rc; +struct tpm_buf buf; +struct tpm2_pcr_read_out *read_out; +u8 pcr_select[TPM2_PCR_SELECT_MIN ] = {0 }; + +if (pcr_idx >=TPM2_PLATFORM_PCR ) +return -EINVAL ; + +if (!update_counter) +return -EINVAL ; + + rc =tpm_buf_init (&buf,TPM2_ST_NO_SESSIONS ,TPM2_CC_PCR_READ ); +if (rc) +return rc; + + pcr_select[pcr_idx >>3 ] =1 << (pcr_idx &0x7 ); + +tpm_buf_append_u32 (&buf,1 ); +tpm_buf_append_u16 (&buf, alg_id); +tpm_buf_append_u8 (&buf,TPM2_PCR_SELECT_MIN ); +tpm_buf_append (&buf, (const unsigned char *)pcr_select, +sizeof (pcr_select)); + + rc =tpm_transmit_cmd (chip, &buf,0 ,"attempting to read a pcr value" ); +if (rc) + goto out; + + read_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE ]; + + *update_counter =be32_to_cpu (read_out->update_cnt); + +out: +tpm_buf_destroy (&buf); +return rc; +} +struct tpm2_null_auth_area {__be32 handle;__be16 nonce_size;Signature · 1 line
2.25.1BR, Jarkko
Jarkko Sakkinen
maintainerRe: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 5 lines · Tushar Sugandhi
Thanks for the response Jarkko.
Quoted 1 line · Jarkko Sakkinen
The short summary is cryptic to say the least.
Do you mean the patch subject line, or the description below?
It is in the process documentation:
https://www.kernel.org/doc/html/v6.3/process/submitting-patches.html#the-canonical-patch-format
Quoted 6 lines
Quoted 1 line
"update counter" does not map it to have anything to do with PCRs.
Agreed. I noticed that when I was testing the patches. The update counter is same for all PCRs. It was also the same for the two hash algo's I tested it for (SHA1 and SHA256). But the spec description and Kernel implementation requires to pass the
pcr_idxand hash algo toPCR_Readcommand to get the update counter.
I was referring to the fact that TPM2_PCR_Read does not have a field called "update counter" in its response but it has a field called "pcrUpdateCounter". Please refer to thigs that actually exist.
In the long description you are in some occasions referring to the same object as:
1. "update counter"
2. "pcrUpdateCounter"
3. "PcrUpdateCounter"
This is ambiguous and wrong.
From long description I see zero motivation to ack this change, except some heresay about IMA requiring it. Why does IMA need update_cnt and why this is not documented to the long description?
Quoted 2 lines
But I can update
tpm2_pcr_read()if you are ok with it. Please let me know.
You can add "u32 *update_cnt".
BR, Jarkko
Re: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 9 lines · Jarkko Sakkinen
Quoted 5 lines · Tushar Sugandhi
Thanks for the response Jarkko.
Quoted 1 line · Jarkko Sakkinen
The short summary is cryptic to say the least.
Do you mean the patch subject line, or the description below?
It is in the process documentation:
https://www.kernel.org/doc/html/v6.3/process/submitting-patches.html#the-canonical-patch-format
Sounds good. I will cleanup both the summary phrase and the patch description.
Quoted 10 lines
Quoted 6 lines
Quoted 1 line
"update counter" does not map it to have anything to do with PCRs.
Agreed. I noticed that when I was testing the patches. The update counter is same for all PCRs. It was also the same for the two hash algo's I tested it for (SHA1 and SHA256). But the spec description and Kernel implementation requires to pass the
pcr_idxand hash algo toPCR_Readcommand to get the update counter.I was referring to the fact that
TPM2_PCR_Readdoes not have a field called "update counter" in its response but it has a field called "pcrUpdateCounter". Please refer to thigs that actually exist.
Quoted 8 lines
In the long description you are in some occasions referring to the same object as:
1. "update counter"
2. "pcrUpdateCounter"
3. "PcrUpdateCounter"This is ambiguous and wrong.
Thanks. I will consistently use pcrUpdateCounter going forward.
Quoted 3 lines
Quoted 1 line
From long description I see zero motivation to ack this change, except
some heresay about IMA requiring it. Why does IMA need
update_cntand why this is not documented to the long description?
Since patch 2 of this series exposes the functionality to IMA, it is described in the long description of patch 2.
But I can add the description here as well for completeness.
Quoted 3 lines
Quoted 2 lines
But I can update
tpm2_pcr_read()if you are ok with it. Please let me know.You can add "u32
*update_cnt".
Sounds good. Will do.
Btw, the function tpm2_pcr_read is not exposed directly to the other subsystems (like IMA). It is exposed via tpm_pcr_read.
Do you want to expose tpm2_pcr_read directly,
or do you want me to update the function signature of tpm_pcr_read as well?
Updating the function signature of tpm_pcr_read as well - to return "u32 *update_cnt" seems like the right approach. In that case, I can set *update_cnt to say 0 or -1 for TPM1 (because pcrUpdateCounter is not available for TPM1).
Please let me know what do you think.
I will make the changes accordingly.
I will also wait for IMA/Kexec maintainers to take a look at the remaining patches in this series, incorporate their feedback, and send the V2 of this series.
Thanks again for your feedback. Really appreciate it.
~Tushar
Quoted 2 lines
BR, Jarkko
Jarkko Sakkinen
maintainerRe: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 5 lines · Tushar Sugandhi
Btw, the function
tpm2_pcr_readis not exposed directly to the other subsystems (like IMA). It is exposed viatpm_pcr_read.Do you want to expose
tpm2_pcr_readdirectly,
or do you want me to update the function signature oftpm_pcr_readas well?
As long as you mention that 'update_cnt' causes divegence in the in-kernel API, and therefore tpm[12]_pcr_read() cannnot be under the same orchestrator.
If you take this path, please implement a precursory patch, which replace the existing call sites with some sequence of tpm[12]_pcr_read() and tpm_is_tpm2() calls.
BR, Jarkko
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 9 lines · Jarkko Sakkinen
Quoted 5 lines · Tushar Sugandhi
Btw, the function
tpm2_pcr_readis not exposed directly to the other subsystems (like IMA). It is exposed viatpm_pcr_read.Do you want to expose
tpm2_pcr_readdirectly,
or do you want me to update the function signature oftpm_pcr_readas well?As long as you mention that '
update_cnt' causes divegence in the in-kernel API, and therefore tpm[12]_pcr_read() cannnot be under the same orchestrator.
Yup. I will mention that in the description/comment.
Quoted 4 lines
If you take this path, please implement a precursory patch, which replace the existing call sites with some sequence of tpm[12]_pcr_read() and
tpm_is_tpm2()calls.
Sure. I will add a precursory patch which will replace the existing call sites.
Thanks for confirming the approach.
~Tushar
Quoted 1 line
BR, Jarkko
Mimi Zohar
maintainerRe: [PATCH 1/6] tpm: implement TPM2 function to get update counter
In reply to Jarkko SakkinenQuoted 4 lines · Jarkko Sakkinen
From long description I see zero motivation to ack this change, except some heresay about IMA requiring it. Why does IMA need
update_cntand why this is not documented to the long description?
The motivation is to detect whether the IMA measurement list has been truncated, for whatever reason. A new IMA record should be defined containing the "pcrCounter" value. (I have not had a chance to review this patch set.)
This new record would be a pre-req for both Tushar's "ima: measure events between kexec load and execute" patch set and Sush's proposal to trim the measurement list. (I haven't looked at it yet either.)
Signature · 9 lines
thanks, Mimi _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Jarkko Sakkinen
maintainerRe: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 6 lines · Mimi Zohar
Quoted 4 lines · Jarkko Sakkinen
From long description I see zero motivation to ack this change, except some heresay about IMA requiring it. Why does IMA need
update_cntand why this is not documented to the long description?
Quoted 8 lines
The motivation is to detect whether the IMA measurement list has been truncated, for whatever reason. A new IMA record should be defined containing the "pcrCounter" value. (I have not had a chance to review this patch set.)
This new record would be a pre-req for both Tushar's "ima: measure events between kexec load and execute" patch set and Sush's proposal to trim the measurement list. (I haven't looked at it yet either.)
Please describe the story in a bit more understandable form. In the commit messages it is not good to have some redundancy in patch sets.
BR, Jarkko
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/6] tpm: implement TPM2 function to get update counter
Quoted 10 lines · Jarkko Sakkinen
Quoted 9 lines · Mimi Zohar
Quoted 3 lines · Jarkko Sakkinen
From long description I see zero motivation to ack this change, except
some heresay about IMA requiring it. Why does IMA needupdate_cntand why this is not documented to the long description?The motivation is to detect whether the IMA measurement list has been truncated, for whatever reason. A new IMA record should be defined containing the "pcrCounter" value. (I have not had a chance to review this patch set.)
Quoted 7 lines
Quoted 3 lines
This new record would be a pre-req for both Tushar's "ima: measure events between kexec load and execute" patch set and Sush's proposal to trim the measurement list. (I haven't looked at it yet either.)
Please describe the story in a bit more understandable form. In the commit messages it is not good to have some redundancy in patch sets.
BR, Jarkko
Thanks Jarkko. I had provided the overall context in the cover letter. But I understand the cover letter will be lost when the patches are merged. I will describe the story in the patch descriptions as well, in the next version of this series.
~Tushar
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/6] tpm: implement TPM2 function to get update counter
In reply to Mimi ZoharQuoted 4 lines · Mimi Zohar
Quoted 3 lines · Jarkko Sakkinen
From long description I see zero motivation to ack this change, except
some heresay about IMA requiring it. Why does IMA needupdate_cntand why this is not documented to the long description?
Quoted 9 lines
The motivation is to detect whether the IMA measurement list has been truncated, for whatever reason. A new IMA record should be defined containing the "pcrCounter" value. (I have not had a chance to review this patch set.)
This new record would be a pre-req for both Tushar's "ima: measure events between kexec load and execute" patch set and Sush's proposal to trim the measurement list. (I haven't looked at it yet either.)
Thanks Mimi. Take your time.
PATCH 2/6 tpm: provide functionality to get update counter
In reply to Tushar SugandhiThe IMA subsystem needs to measure pcrUpdateCounter value present in TPM2_PCR_Read Response struct [1]. However,the pcrUpdateCounter value is not exposed outside of the TPM subsystem by any of the existing functions.
Implement a new function 'tpm_pcr_get_update_counter()', which provides a way to retrieve the PCR update counter values from subsystems outside of TPM. If the input tpm_chip is not a TPM2 chip, return an error as the functionality is currently only implemented for TPM2 chips.
This function improves TPM capabilities in the Linux kernel by facilitating access to the PCR update counter.
[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part3_Commands_pub.pdf
Section 22.4.2, Page 206.
--- drivers/char/tpm/tpm-interface.c | 28 ++++++++++++++++++++++++++++ include/linux/tpm.h | 8 ++++++++ 2 files changed, 36 insertions(+)
drivers/char/tpm/tpm-interface.c +28 -0
@@ -300,6 +300,34 @@ int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx, }EXPORT_SYMBOL_GPL (tpm_pcr_read); +/** +* tpm_pcr_get_update_counter - gets an update counter value for a PCR bank +* @chip: a &struct tpm_chip instance, %NULL for the default chip +* @pcr_idx: PCR index used to retrieve the update counter +* @alg_id: alg id used to retrieve the update counter +* @update_counter: output update counter value +* +* Return: same as with tpm_transmit_cmd() +*/ +int tpm_pcr_get_update_counter (struct tpm_chip *chip,u32 pcr_idx, +u16 alg_id,u32 *update_counter) +{ +int rc; + + chip =tpm_find_get_ops (chip); +if (!chip) +return -ENODEV ; + +if (chip->flags &TPM_CHIP_FLAG_TPM2 ) + rc =tpm2_pcr_get_update_counter (chip, pcr_idx, alg_id, + update_counter); +else + rc = -ENODEV ; + +tpm_put_ops (chip); +return rc; +} +EXPORT_SYMBOL_GPL (tpm_pcr_get_update_counter);/** * tpm_pcr_extend - extend a PCR value in SHA1 bank. * @chip: a &struct tpm_chip instance, %NULL for the default chip
include/linux/tpm.h +8 -0
@@ -424,6 +424,8 @@ extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf, size_t min_rsp_body_length, const char *desc);extern int tpm_pcr_read (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digest); +extern int tpm_pcr_get_update_counter (struct tpm_chip *chip,u32 pcr_idx, +u16 alg_id,u32 *update_counter);extern int tpm_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests);extern int tpm_send (struct tpm_chip *chip,void *cmd,size_t buflen);
@@ -440,6 +442,12 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, {return -ENODEV ; } +static inline int tpm_pcr_get_update_counter (struct tpm_chip *chip, +u32 pcr_idx,u16 alg_id, +u32 *update_counter) +{ +return -ENODEV ; +}static inline int tpm_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests)
Signature · 1 line
2.25.1
PATCH 3/6 ima: get TPM update counter
In reply to Tushar SugandhiMeasuring the TPM PCR update counter 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.). Detecting such missing entries would help the remote attestation service functionality to be more robust. It should also help the system administrators with manual investigations when TPM PCR quotes go out of sync with IMA measurements.
Implement a new function, 'ima_tpm_get_update_counter()', which uses the 'tpm_pcr_get_update_counter()' function from the TPM driver interface to retrieve the PCR update counter of the TPM chip in use.
--- security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_queue.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+)
security/integrity/ima/ima.h +1 -0
@@ -167,6 +167,7 @@ void ima_init_template_list(void);int __init ima_init_digests (void );int ima_lsm_policy_change (struct notifier_block *nb,unsigned long event,void *lsm_data); +int ima_tpm_get_update_counter (u32 *cpu_update_counter);/* * used to protect h_table and sha_table
security/integrity/ima/ima_queue.c +16 -0
@@ -135,6 +135,22 @@ unsigned long ima_get_binary_runtime_size(void)return binary_runtime_size +sizeof (struct ima_kexec_hdr ); } +int ima_tpm_get_update_counter (u32 *update_counter) +{ +int result; + +if (!update_counter) +return -EINVAL ; + + result =tpm_pcr_get_update_counter (ima_tpm_chip, +CONFIG_IMA_MEASURE_PCR_IDX ,TPM_ALG_SHA1 , update_counter); + +if (result !=0 ) +pr_err ("Failed to get TPM PCR update counter, result: %d\n" , result); + +return result; +} +static int ima_pcr_extend (struct tpm_digest *digests_arg,int pcr) {int result =0 ;
Signature · 1 line
2.25.1
PATCH 4/6 ima: implement functionality to measure TPM update counter
In reply to Tushar SugandhiCurrently TPM update counter is not available external to the system, for instance, a remote attestation service. It is a problem because the service cannot easily determine if the IMA log entries are missing. The IMA functionality needs to be extended to measure the TPM update counter from various subsystems in Linux kernel to help detect if the IMA log entries are missing.
Implement a function, 'ima_measure_update_counter()' which would retrieve the TPM update counter using the previously defined function 'ima_tpm_get_update_counter()'. Format it as a string with the value "update_counter=<N>;", and measure it using the function 'ima_measure_critical_data()'.
The function takes an event name as input, and the update counter value is measured as part of this event.
--- include/linux/ima.h | 1 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_main.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+)
include/linux/ima.h +1 -0
@@ -40,6 +40,7 @@ extern int ima_measure_critical_data(const char *event_label,const char *event_name, const void *buf, size_t buf_len, bool hash, u8 *digest, size_t digest_len); +int ima_measure_update_counter (const char *event_name);#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM extern void ima_appraise_parse_cmdline (void );
security/integrity/ima/ima.h +1 -0
@@ -168,6 +168,7 @@ int __init ima_init_digests(void);int ima_lsm_policy_change (struct notifier_block *nb,unsigned long event,void *lsm_data);int ima_tpm_get_update_counter (u32 *cpu_update_counter); +int ima_measure_update_counter (const char *event_name);/* * used to protect h_table and sha_table
security/integrity/ima/ima_main.c +28 -0
@@ -1071,6 +1071,34 @@ int ima_measure_critical_data(const char *event_label, }EXPORT_SYMBOL_GPL (ima_measure_critical_data); +#define IMA_TPM_UPDATE_CTR_BUF_SIZE 128 +int ima_measure_update_counter (const char *event_name) +{ +int result; +u32 update_counter =0 ; +char buf[IMA_TPM_UPDATE_CTR_BUF_SIZE ]; +int buf_len; + +if (!event_name) +return -ENOPARAM ; + + result =ima_tpm_get_update_counter (&update_counter); + +if (result !=0 ) +return result; + +scnprintf (buf,IMA_TPM_UPDATE_CTR_BUF_SIZE ,"update_counter=%u;" , + update_counter); + + buf_len =strlen (buf); + + result =ima_measure_critical_data ("tpm_pcr_update_counter" , event_name, + buf, buf_len, false,NULL ,0 ); + +return result; +} +EXPORT_SYMBOL_GPL (ima_measure_update_counter); +static int __init init_ima (void ) {int error;
Signature · 1 line
2.25.1
Mimi Zohar
maintainerRe: [PATCH 4/6] ima: implement functionality to measure TPM update counter
Quoted 13 lines · Tushar Sugandhi
Currently TPM update counter is not available external to the system, for instance, a remote attestation service. It is a problem because the service cannot easily determine if the IMA log entries are missing. The IMA functionality needs to be extended to measure the TPM update counter from various subsystems in Linux kernel to help detect if the IMA log entries are missing.
Implement a function, '
ima_measure_update_counter()' which would retrieve the TPM update counter using the previously defined function 'ima_tpm_get_update_counter()'. Format it as a string with the value "update_counter=<N>;", and measure it using the function 'ima_measure_critical_data()'.
Quoted 4 lines
The function takes an event name as input, and the update counter value is measured as part of this event.
Signed-off-by Tushar Sugandhi <[email protected]>
Explicit TPM2 quote commands do not return the quoted PCR values or the pcrCounter value. Define and include a new IMA measurement record containing the pcrCounter, other TPM info, and IMA info in the IMA measurement list to help simplify detecting a trimmed/truncated measurement list.
Quoted 52 lines
--- include/linux/ima.h | 1 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_main.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+)include/linux/ima.h +1
-0@@ -40,6 +40,7 @@ extern int ima_measure_critical_data(const char *event_label,const char *event_name, const void *buf, size_t buf_len, bool hash, u8 *digest, size_t digest_len); +int ima_measure_update_counter (const char *event_name);#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM extern void ima_appraise_parse_cmdline (void );security/integrity/ima/ima.h +1
-0@@ -168,6 +168,7 @@ int __init ima_init_digests(void);int ima_lsm_policy_change (struct notifier_block *nb,unsigned long event,void *lsm_data);int ima_tpm_get_update_counter (u32 *cpu_update_counter); +int ima_measure_update_counter (const char *event_name);/* * used to protect h_table and sha_table security/integrity/ima/ima_main.c +14
-0@@ -1071,6 +1071,34 @@ int ima_measure_critical_data(const char *event_label, }EXPORT_SYMBOL_GPL (ima_measure_critical_data); +#define IMA_TPM_UPDATE_CTR_BUF_SIZE 128 +int ima_measure_update_counter (const char *event_name) +{ +int result; +u32 update_counter =0 ; +char buf[IMA_TPM_UPDATE_CTR_BUF_SIZE ]; +int buf_len; + +if (!event_name) +return -ENOPARAM ; + + result =ima_tpm_get_update_counter (&update_counter); + +if (result !=0 )
security/integrity/ima/ima_main.c int ima_measure_critical_data(const char *event_label, · line 1097
Quoted 10 lines
security/integrity/ima/ima_main.c +9
-0+return result; + +scnprintf (buf,IMA_TPM_UPDATE_CTR_BUF_SIZE ,"update_counter=%u;" , + update_counter); + + buf_len =strlen (buf); + + result =ima_measure_critical_data ("tpm_pcr_update_counter" , event_name, + buf, buf_len, false,NULL ,0 );
The new record should contain everything needed to verify the pcrCounter. For example, each IMA measurement record updates the pcrCounter for each TPM bank enabled. So the number of enabled TPM banks and number of IMA measurements should also be included in this record.
Perhaps include a version number as well, so that if we ever want to include other information, we could.
Mimi
Quoted 8 lines
security/integrity/ima/ima_main.c +5
-0+ +return result; +} +EXPORT_SYMBOL_GPL (ima_measure_update_counter); +static int __init init_ima (void ) {int error;
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 4/6] ima: implement functionality to measure TPM update counter
Thanks for the review Mimi.
Quoted 17 lines · Mimi Zohar
Quoted 16 lines · Tushar Sugandhi
Currently TPM update counter is not available external to the system, for instance, a remote attestation service. It is a problem because the service cannot easily determine if the IMA log entries are missing. The IMA functionality needs to be extended to measure the TPM update counter from various subsystems in Linux kernel to help detect if the IMA log entries are missing.
Implement a function, '
ima_measure_update_counter()' which would retrieve the TPM update counter using the previously defined function 'ima_tpm_get_update_counter()'. Format it as a string with the value "update_counter=<N>;", and measure it using the function 'ima_measure_critical_data()'.The function takes an event name as input, and the update counter value is measured as part of this event.
Quoted 6 lines
Quoted 1 line
Signed-off-by Tushar Sugandhi <[email protected]>Explicit TPM2 quote commands do not return the quoted PCR values or the pcrCounter value. Define and include a new IMA measurement record containing the pcrCounter, other TPM info, and IMA info in the IMA measurement list to help simplify detecting a trimmed/truncated measurement list.
Sounds good.
Quoted 59 lines
Quoted 59 lines
--- include/linux/ima.h | 1 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_main.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+)include/linux/ima.h +1
-0@@ -40,6 +40,7 @@ extern int ima_measure_critical_data(const char *event_label,const char *event_name, const void *buf, size_t buf_len, bool hash, u8 *digest, size_t digest_len); +int ima_measure_update_counter (const char *event_name);#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM extern void ima_appraise_parse_cmdline (void );security/integrity/ima/ima.h +1
-0@@ -168,6 +168,7 @@ int __init ima_init_digests(void);int ima_lsm_policy_change (struct notifier_block *nb,unsigned long event,void *lsm_data);int ima_tpm_get_update_counter (u32 *cpu_update_counter); +int ima_measure_update_counter (const char *event_name);/* * used to protect h_table and sha_table security/integrity/ima/ima_main.c +21
-0@@ -1071,6 +1071,34 @@ int ima_measure_critical_data(const char *event_label, }EXPORT_SYMBOL_GPL (ima_measure_critical_data); +#define IMA_TPM_UPDATE_CTR_BUF_SIZE 128 +int ima_measure_update_counter (const char *event_name) +{ +int result; +u32 update_counter =0 ; +char buf[IMA_TPM_UPDATE_CTR_BUF_SIZE ]; +int buf_len; + +if (!event_name) +return -ENOPARAM ; + + result =ima_tpm_get_update_counter (&update_counter); + +if (result !=0 ) +return result; + +scnprintf (buf,IMA_TPM_UPDATE_CTR_BUF_SIZE ,"update_counter=%u;" , + update_counter); + + buf_len =strlen (buf); +
Quoted 8 lines
security/integrity/ima/ima_main.c int ima_measure_critical_data(const char *event_label,
Quoted 3 lines
security/integrity/ima/ima_main.c +2
-0+ result =ima_measure_critical_data ("tpm_pcr_update_counter" , event_name, + buf, buf_len, false,NULL ,0 );The new record should contain everything needed to verify the pcrCounter. For example, each IMA measurement record updates the pcrCounter for each TPM bank enabled. So the number of enabled TPM banks and number of IMA measurements should also be included in this record.
Agreed. That should be valuable information. How does the below format look like for the buf above?
version=<N>.<N>.<N>;num_enabled_pcr_banks=<N>;pcrUpdateCounter=<N>;num_ima_measurements=<N>;
Quoted 3 lines
Perhaps include a version number as well, so that if we ever want to include other information, we could.
By version number, do you mean kernel_version, or a new version number specific to this record? Or something else?
~Tushar
Quoted 11 lines
Mimi
Quoted 8 lines
security/integrity/ima/ima_main.c +5
-0+ +return result; +} +EXPORT_SYMBOL_GPL (ima_measure_update_counter); +static int __init init_ima (void ) {int error;
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Mimi Zohar
maintainerRe: [PATCH 4/6] ima: implement functionality to measure TPM update counter
Quoted 13 lines · Tushar Sugandhi
Quoted 5 lines
Quoted 5 lines
security/integrity/ima/ima_main.c +5
-0+scnprintf (buf,IMA_TPM_UPDATE_CTR_BUF_SIZE ,"update_counter=%u;" , + update_counter); + + buf_len =strlen (buf); +Quoted 8 lines
security/integrity/ima/ima_main.c int ima_measure_critical_data(const char *event_label, · line 1096
Quoted 3 lines
security/integrity/ima/ima_main.c +2
-0+ result =ima_measure_critical_data ("tpm_pcr_update_counter" , event_name, + buf, buf_len, false,NULL ,0 );The new record should contain everything needed to verify the pcrCounter. For example, each IMA measurement record updates the pcrCounter for each TPM bank enabled. So the number of enabled TPM banks and number of IMA measurements should also be included in this record.
Quoted 4 lines
Agreed. That should be valuable information. How does the below format look like for the buf above?
version=<N>.<N>.<N>;num_enabled_pcr_banks=<N>;pcrUpdateCounter=<N>;num_ima_measurements=<N>;
Refer to comment in 5/6.
Quoted 4 lines
Quoted 2 lines
Perhaps include a version number as well, so that if we ever want to include other information, we could.
By version number, do you mean
kernel_version, or a new version number specific to this record? Or something else?
This is a record version type number. The record format shouldn't change, but we should be prepared for it changing. A single number should be fine.
Signature · 9 lines
thanks, Mimi _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 4/6] ima: implement functionality to measure TPM update counter
Quoted 14 lines · Mimi Zohar
Quoted 13 lines · Tushar Sugandhi
Quoted 13 lines
security/integrity/ima/ima_main.c int ima_measure_critical_data(const char *event_label, · line 1096
Quoted 8 lines
security/integrity/ima/ima_main.c +7
-0+scnprintf (buf,IMA_TPM_UPDATE_CTR_BUF_SIZE ,"update_counter=%u;" , + update_counter); + + buf_len =strlen (buf); + + result =ima_measure_critical_data ("tpm_pcr_update_counter" , event_name, + buf, buf_len, false,NULL ,0 );The new record should contain everything needed to verify the pcrCounter. For example, each IMA measurement record updates the pcrCounter for each TPM bank enabled. So the number of enabled TPM banks and number of IMA measurements should also be included in this record.
Quoted 5 lines
Quoted 4 lines
Agreed. That should be valuable information. How does the below format look like for the buf above?
version=<N>.<N>.<N>;num_enabled_pcr_banks=<N>;pcrUpdateCounter=<N>;num_ima_measurements=<N>;
Refer to comment in 5/6.
Responded.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Perhaps include a version number as well, so that if we ever want to include other information, we could.
By version number, do you mean
kernel_version, or a new version number specific to this record? Or something else?This is a record version type number. The record format shouldn't change, but we should be prepared for it changing. A single number should be fine.
Sounds good.
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
PATCH 5/6 ima: measure TPM update counter at ima_init
In reply to Tushar SugandhiIMA log entries can be lost due to a variety of causes, such as code bugs or error conditions, leading to a mismatch between TPM PCRs and the IMA log. Measuring TPM PCR update counter during ima_init would provide a baseline counter for the number of times the TPM PCRs are updated. The remote attestation service can compare this baseline counter with a subsequent measured one (e.g., post-kexec soft-boot) to identify if there are any lost IMA log events.
Measure the TPM update counter at ima init.
--- security/integrity/ima/ima_init.c | 3 +++ security/integrity/ima/ima_main.c | 1 + 2 files changed, 4 insertions(+)
security/integrity/ima/ima_init.c +3 -0
@@ -154,5 +154,8 @@ int __init ima_init(void)UTS_RELEASE ,strlen (UTS_RELEASE ), false, NULL, 0); +/* Measures TPM update counter at ima_init */ +ima_measure_update_counter ("ima_init_tpm_update_counter" ); +return rc; }
security/integrity/ima/ima_main.c +1 -0
@@ -1035,6 +1035,7 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) buf, size,"kexec-cmdline" ,KEXEC_CMDLINE ,0 , NULL, false, NULL, 0);fdput (f); + }/**
Signature · 1 line
2.25.1
Mimi Zohar
maintainerRe: [PATCH 5/6] ima: measure TPM update counter at ima_init
Quoted 9 lines · Tushar Sugandhi
IMA log entries can be lost due to a variety of causes, such as code bugs or error conditions, leading to a mismatch between TPM PCRs and the IMA log. Measuring TPM PCR update counter during
ima_initwould provide a baseline counter for the number of times the TPM PCRs are updated. The remote attestation service can compare this baseline counter with a subsequent measured one (e.g., post-kexec soft-boot) to identify if there are any lost IMA log events.Measure the TPM update counter at ima init.
No need for separate patches for one line changes like this. Either merge patches 5/6 and 6/6 or all three 4/6, 5/6, 6/6 together.
Quoted 12 lines
Signed-off-by Tushar Sugandhi <[email protected]>--- security/integrity/ima/ima_init.c | 3 +++ security/integrity/ima/ima_main.c | 1 + 2 files changed, 4 insertions(+)security/integrity/ima/ima_init.c +0
-0
security/integrity/ima/ima_init.c int __init ima_init(void) · line 159
Quoted 6 lines
security/integrity/ima/ima_init.c +3
-0@@ -154,5 +154,8 @@ int __init ima_init(void)UTS_RELEASE ,strlen (UTS_RELEASE ), false, NULL, 0); +/* Measures TPM update counter at ima_init */ +ima_measure_update_counter ("ima_init_tpm_update_counter" ); +
With "ima_policy=critical_data" on the boot command line, the IMA measurement list record looks like:
6e190cc643ff0b718485966a0300473baedface735 ima_init_tpm_update_counter 7570646174655f636f756e7465723d3330383b
Please change the "ima_init_tpm_update_counter" to something shorter and the hex encoded ascii string and pcr counter to something readable. Perhaps name this critical-data "tpm" and "tpm-info", similar to the SELinux "selinux" and "selinux-state". Then again, if this is TPM critical-data we should rethink what other info should be included.
Quoted 7 lines
security/integrity/ima/ima_init.c +0
-0return rc; }security/integrity/ima/ima_main.c +0
-0
security/integrity/ima/ima_main.c void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) · line 1041
Quoted 7 lines
security/integrity/ima/ima_main.c +1
-0@@ -1035,6 +1035,7 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) buf, size,"kexec-cmdline" ,KEXEC_CMDLINE ,0 , NULL, false, NULL, 0);fdput (f); + }/**
Unnecessary change.
Signature · 9 lines
thanks, Mimi _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 5/6] ima: measure TPM update counter at ima_init
Quoted 9 lines · Mimi Zohar
Quoted 8 lines · Tushar Sugandhi
IMA log entries can be lost due to a variety of causes, such as code bugs or error conditions, leading to a mismatch between TPM PCRs and the IMA log. Measuring TPM PCR update counter during
ima_initwould provide a baseline counter for the number of times the TPM PCRs are updated. The remote attestation service can compare this baseline counter with a subsequent measured one (e.g., post-kexec soft-boot) to identify if there are any lost IMA log events.
Quoted 4 lines
Quoted 1 line
Measure the TPM update counter at ima init.
No need for separate patches for one line changes like this. Either merge patches 5/6 and 6/6 or all three 4/6, 5/6, 6/6 together.
Sounds good.
I will merge 4/6, 5/6, 6/6 together.
Quoted 20 lines
Quoted 11 lines
Signed-off-by Tushar Sugandhi <[email protected]>--- security/integrity/ima/ima_init.c | 3 +++ security/integrity/ima/ima_main.c | 1 + 2 files changed, 4 insertions(+)security/integrity/ima/ima_init.c +0
-0security/integrity/ima/ima_init.c int __init ima_init(void) · line 159
Quoted 6 lines
security/integrity/ima/ima_init.c +3
-0@@ -154,5 +154,8 @@ int __init ima_init(void)UTS_RELEASE ,strlen (UTS_RELEASE ), false, NULL, 0); +/* Measures TPM update counter at ima_init */ +ima_measure_update_counter ("ima_init_tpm_update_counter" ); +With "ima_policy=critical_data" on the boot command line, the IMA measurement list record looks like:
Quoted 4 lines
6e190cc643ff0b718485966a0300473baedface735ima_init_tpm_update_counter7570646174655f636f756e7465723d3330383bPlease change the "
ima_init_tpm_update_counter" to something shorter and the hex encoded ascii string and pcr counter to something readable.
I believe you are seeing the above line in ascill_runtime_measurements log.
The ascii logging format is consistent with other event data for critical_data event e.g. kernel_version.
10 8f449175bbf88bc55fc1127466628c39a3957d15 ima-buf sha1:4acab4fbb08db663b7b7b4528e8729187d726782 kernel_version 362e332e302d7263332b
10 f10678b63c4b2529339dff02282e63d9c6bb0385 ima-buf sha1:d8c187524412f74a961f2051a9529c009e700337 ima_init_tpm_update_counter 7570646174655f636f756e7465723d3133303b
Entries in the binary runtime measurements look readable to me.
ima_init_tpm_update_counter update_counter=130;
...kexec_load_tpm_update_counte rupdate_counter=133;
Please let me know if you still want me to change the format.
Quoted 1 line
Perhaps name this critical-data "tpm" and "tpm-info", similar to the
From patch 4/6:
+ result = ima_measure_critical_data("tpm_pcr_update_counter", event_name, + buf, buf_len, false, NULL, 0);
The critical_data event_label value is currently set to "tpm_pcr_update_counter". I can rename event_label to "tpm-info", so that the admins can filter the event in IMA policy based on the label if needed.
As you know, event_label doesn't appear in IMA log, it can appear in IMA policy. Whereas event_name appears in IMA log.
I was thinking of using event_name to identify when was the info captured. (e.g. ima_init, kexec_load, or at some other event in future).
We can either do
(a)event_label = "tpm-info" event_name = "tpm-info-ima-init" | "tpm-info-kexec-load" | ...
-or-
(b)event_label = "tpm" event_name = "tpm-info"
and event_data to describe the where/when this info was captured.
e.g. version=<N>.<N>.<N>;num_enabled_pcr_banks=<N>;pcrUpdateCounter=<N>;num_ima_measurements=<N>;event=kexec_load;
Let me know if you would prefer option (a) or (b) or something else.
Quoted 2 lines
SELinux "selinux" and "selinux-state". Then again, if this is TPM critical-data we should rethink what other info should be included.
As you suggested in Patch 4/6, I will add version, number of enabled pcr banks, pcrUpdateCounter, and num_ima_measurements. I think we should include the TPM version as well (1 v/s 2).
Please let me know if you think of any other attribute to record.
Quoted 13 lines
Quoted 13 lines
security/integrity/ima/ima_init.c +0
-0return rc; }security/integrity/ima/ima_main.c +1
-0@@ -1035,6 +1035,7 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) buf, size,"kexec-cmdline" ,KEXEC_CMDLINE ,0 , NULL, false, NULL, 0);fdput (f); + }
Quoted 3 lines
security/integrity/ima/ima_main.c void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) · line 1041
Quoted 1 line
security/integrity/ima/ima_main.c +0
-0/** Unnecessary change.
oops. Thanks for catching. Will fix.
~Tushar
Mimi Zohar
maintainerRe: [PATCH 5/6] ima: measure TPM update counter at ima_init
Quoted 11 lines · Tushar Sugandhi
Quoted 11 lines
security/integrity/ima/ima_init.c int __init ima_init(void) · line 159
Quoted 8 lines
+++
b/security/integrity/ima/ima_init.csecurity/integrity/ima/ima_init.c +3
-0@@ -154,5 +154,8 @@ int __init ima_init(void)UTS_RELEASE ,strlen (UTS_RELEASE ), false, NULL, 0); +/* Measures TPM update counter at ima_init */ +ima_measure_update_counter ("ima_init_tpm_update_counter" ); +With "ima_policy=critical_data" on the boot command line, the IMA measurement list record looks like:
Quoted 5 lines
Quoted 4 lines
6e190cc643ff0b718485966a0300473baedface735ima_init_tpm_update_counter7570646174655f636f756e7465723d3330383bPlease change the "
ima_init_tpm_update_counter" to something shorter and the hex encoded ascii string and pcr counter to something readable.I believe you are seeing the above line in
ascill_runtime_measurementslog.
Yes, the ascii_runtime_measurements are suppose to be readable to the end user.
Quoted 2 lines
The ascii logging format is consistent with other event data for
critical_dataevent e.g.kernel_version.
Then you got it wrong.
Quoted 8 lines
10
8f449175bbf88bc55fc1127466628c39a3957d15ima-buf sha1:4acab4fbb08db663b7b7b4528e8729187d726782kernel_version362e332e302d7263332b
10f10678b63c4b2529339dff02282e63d9c6bb0385ima-buf sha1:d8c187524412f74a961f2051a9529c009e700337ima_init_tpm_update_counter7570646174655f636f756e7465723d3133303bEntries in the binary runtime measurements look readable to me.
You've inverted the meaning of the ascii and binary runtime measurement lists. For comparison look at the ima-ng/ima-sig records.
Quoted 5 lines
ima_init_tpm_update_counterupdate_counter=130;
...kexec_load_tpm_update_counterupdate_counter=133;Please let me know if you still want me to change the format.
OI course the ascii measurement list should be human readable.
Quoted 8 lines
Quoted 1 line
Perhaps name this critical-data "tpm" and "tpm-info", similar to the
From patch 4/6:
+ result = ima_measure_critical_data("tpm_pcr_update_counter",event_name,
+ buf,buf_len, false, NULL, 0);The
critical_dataevent_labelvalue is currently set to "tpm_pcr_update_counter".
Why is the string so long? Long strings or variables don't make the
code or logs more understandable. Please shorten both the strings and variables.
Quoted 2 lines
I can rename
event_labelto "tpm-info", so that the admins can filter the event in IMA policy based on the label if needed.
The new record needs to be self containd and verifiable. The additional info I suggested were just examples. Please take the time to consider what needs to be included in the new record. Decide whether this is a TPM security critical data record. Only then, decide on the naming.
Signature · 10 lines
thanks, Mimi _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 5/6] ima: measure TPM update counter at ima_init
Quoted 13 lines · Mimi Zohar
Quoted 11 lines · Tushar Sugandhi
Quoted 11 lines
security/integrity/ima/ima_init.c int __init ima_init(void) · line 159
Quoted 8 lines
+++
b/security/integrity/ima/ima_init.csecurity/integrity/ima/ima_init.c +3
-0@@ -154,5 +154,8 @@ int __init ima_init(void)UTS_RELEASE ,strlen (UTS_RELEASE ), false, NULL, 0); +/* Measures TPM update counter at ima_init */ +ima_measure_update_counter ("ima_init_tpm_update_counter" ); +With "ima_policy=critical_data" on the boot command line, the IMA measurement list record looks like:
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
6e190cc643ff0b718485966a0300473baedface735ima_init_tpm_update_counter7570646174655f636f756e7465723d3330383bPlease change the "
ima_init_tpm_update_counter" to something shorter and the hex encoded ascii string and pcr counter to something readable.I believe you are seeing the above line in
ascill_runtime_measurementslog.Yes, the
ascii_runtime_measurementsare suppose to be readable to the end user.
We were passing string literals to 'buf' param in ima_measure_critical_data(). I believe we need to convert them first.
Quoted 3 lines
Quoted 2 lines
The ascii logging format is consistent with other event data for
critical_dataevent e.g.kernel_version.Then you got it wrong.
I see. I will fix it for tpm in this patch series. I think I should spin up another series to fix it for selinux, kernel info, DM etc.
Quoted 10 lines
Quoted 8 lines
10
8f449175bbf88bc55fc1127466628c39a3957d15ima-buf sha1:4acab4fbb08db663b7b7b4528e8729187d726782kernel_version362e332e302d7263332b
10f10678b63c4b2529339dff02282e63d9c6bb0385ima-buf sha1:d8c187524412f74a961f2051a9529c009e700337ima_init_tpm_update_counter7570646174655f636f756e7465723d3133303bEntries in the binary runtime measurements look readable to me.
You've inverted the meaning of the ascii and binary runtime measurement lists. For comparison look at the ima-ng/ima-sig records.
Yup. Agreed.
Quoted 6 lines
Quoted 5 lines
ima_init_tpm_update_counterupdate_counter=130;
...kexec_load_tpm_update_counterupdate_counter=133;Please let me know if you still want me to change the format.
OI course the ascii measurement list should be human readable.
Yup. I will make the changes as I mentioned above.
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
Perhaps name this critical-data "tpm" and "tpm-info", similar to the
From patch 4/6:
+ result = ima_measure_critical_data("tpm_pcr_update_counter",event_name,
+ buf,buf_len, false, NULL, 0);
Quoted 5 lines
Quoted 2 lines
The
critical_dataevent_labelvalue is currently set to "tpm_pcr_update_counter".Why is the string so long? Long strings or variables don't make the
code or logs more understandable. Please shorten both the strings and variables.
Agreed. I will name this "tpm" and "tpm-info" or something similar.
Quoted 7 lines
Quoted 2 lines
I can rename
event_labelto "tpm-info", so that the admins can filter the event in IMA policy based on the label if needed.The new record needs to be self containd and verifiable. The additional info I suggested were just examples. Please take the time to consider what needs to be included in the new record. Decide whether this is a TPM security critical data record. Only then, decide on the naming.
Yes. I will explore what other potential attributes can be added to this record. And I'll share them here for the community feedback.
PATCH 6/6 kexec: measure TPM update counter in ima log at kexec load
In reply to Tushar SugandhiIMA measurements snapshot occurs at kexec 'load', but any additional measurements between 'load' and kexec 'execute' aren't carried over post kexec soft-reboot.[1] This may lead to TPM PCRs extending with events that are not reflected in the new Kernel's IMA log. By measuring the TPM update counter at kexec 'load' and at ima_init after the kexec soft-reboot, the remote attestation service can identify potentially lost events by comparing the log event count with the counter difference.
Measure the TPM update counter at kexec image load.
[1] https://lore.kernel.org/all/[email protected]/
ima: measure events between kexec load and execute
--- kernel/kexec_file.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/kexec_file.c +3 -0
@@ -246,6 +246,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, image->cmdline_buf_len - 1); } +/* Measures TPM update counter at kexec load. */ +ima_measure_update_counter ("kexec_load_tpm_update_counter" ); +/* IMA needs to pass the measurement list to the next kernel. */ ima_add_kexec_buffer (image);
Signature · 1 line
2.25.1
Quoted 1 line · Tushar Sugandhi
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
Quoted 4 lines
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
Quoted 11 lines
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.)
Quoted 7 lines
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
Quoted 31 lines
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/[email protected]/
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 atima_init
kexec: measure TPM update counter in ima log at kexec loaddrivers/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.
Quoted 7 lines · Stefan Berger
Quoted 1 line · Tushar Sugandhi
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. :)
Quoted 8 lines
Quoted 6 lines
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.
Quoted 6 lines
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.
Quoted 22 lines
https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_queue.c#L179
Quoted 11 lines
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.)
Quoted 7 lines
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.
Quoted 3 lines
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.
Quoted 1 line
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?
Quoted 5 lines
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/[email protected]/
~Tushar
Quoted 33 lines
Stefan
Quoted 31 lines
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/[email protected]/
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 atima_init
kexec: measure TPM update counter in ima log at kexec loaddrivers/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(+)
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 31 lines · Tushar Sugandhi
Thanks Stefan for reviewing this series. Appreciate it.
Quoted 8 lines · Stefan Berger
Quoted 1 line · Tushar Sugandhi
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. :)
Quoted 7 lines
Quoted 4 lines
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.
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 22 lines
Quoted 22 lines
https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_queue.c#L179
Quoted 18 lines
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.
Quoted 7 lines
Quoted 3 lines
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.
Quoted 2 lines
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
Quoted 15 lines
Quoted 2 lines
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?
Quoted 4 lines
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.
Quoted 37 lines
[1] https://patchwork.kernel.org/project/linux-integrity/patch/[email protected]/
~Tushar
Quoted 33 lines
Stefan
Quoted 31 lines
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/[email protected]/
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 atima_init
kexec: measure TPM update counter in ima log at kexec loaddrivers/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(+)
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Mimi Zohar
maintainerQuoted 11 lines · Stefan Berger
Quoted 1 line
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.
Signature · 9 lines
thanks, Mimi _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Quoted 13 lines · Mimi Zohar
Quoted 11 lines · Stefan Berger
Quoted 1 line
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.
Quoted 8 lines
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
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec