Added TPM user examples #20

vishnusudhan · 2023-10-26T12:08:24Z

This PR has the TPM user examples which is maintained under /etc/tpm/user-examples

rlysecky · 2023-10-26T17:41:36Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_policy_creation.sh

+    exit 1
+fi
+
+cmpare_value=$(cmp -s $file_to_check pcr16.dat; echo $?)


I don't think we need to compare the pcr16.dat and pcr_extend.dat. For one, they are likely to be the same as they were generated at the same time. Second, when setting a per policy, I believe the current PCR state is used for the policy if you don't pass in a value.

We will remove this step.
Instead to check measure boot , we will compare as like below to create the policy,

Measured boot check using tpm2_policypcr api carried out by comparing current state value with the measured.pcrvalues value; this is done by TPM internally.

if tpm2_policypcr -S session1.dat -l sha256:$pcr_index -f measured.pcrvalues;

rlysecky · 2023-10-26T17:51:30Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_policy_creation.sh

+# Start a policy auth session used when authenticating with a policy.
+tpm2_startauthsession --policy-session -S session1.dat
+
+# PCR Policy gets satisfied by comparing the current state value with the passed-in value; this is done by TPM internally.


Is this just a check of the current policy?

We have modified the current comparison with measure pcr value. inorder to allow the creation of the policy, when ,measured boot is satisfied.

rlysecky · 2023-10-26T17:54:40Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_policy_creation.sh

+tpm2_createpolicy --policy-pcr -l sha256:$pcr_index -L policy16.pcr
+
+# Define the NV index with the policy of the current PCR state
+if tpm2_nvdefine $nv_index -L policy16.pcr; then


Why aren't we setting the size here? Without specifying the size, it will default to 2048 per the documentation for this command. Should we set the size manually instead?

We are moving this NV define to respective NV_Write script , in-order to maintain separate NV index & size.

By default, if size parameter not specified then default 2048 will get assigned. if needed we can assign like -s {size(in bytes)}

Maybe we can use it for AES 256 key examples as -s 32

rlysecky · 2023-10-26T17:59:30Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_policy_creation.sh

+
+# Define the filename you want to check
+file_to_check="pcr_extend.dat"
+pcr_file="pcr16.dat"


Can we rename this to measured.pcrvalues? This way the example will closely follow other tutorial I've seen online, and it's also clear that this is the measured value in the context of measured boot.

we will update in the latest script

rlysecky · 2023-10-26T18:06:14Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_nv_read_aes_256.sh

+output_file="aes-key"
+
+# Read the AES-256 key from the TPM NV index with the specified PCR policy
+if tpm2_nvread $nv_index -P pcr:sha256:$pcr_index -s 768 > $output_file; then


Why is 768 for the AES example and 568 is used for each chunk in the rsa example?

Where is the documentation for limits not the size for an nvread or nvwrite command?

For AES , we wrongly taken the full NV read/write size of 768(as declared in the Infineon-SLB 9670VQ2.0-DataSheet-v01_04-EN.pdf (already mailed )) mentioned under key features.

Up to 768 Byte for NV read or NV write

For AES, We will fix 32bytes as size allocation on the NV & read the 32 bytes alone , instead of using full NV size(768 bytes)

For RSA, as you suggested we will use maximum offset (768bytes) & calculate segment to write & read.

rlysecky · 2023-10-26T18:09:20Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_nv_write_rsa_2048.sh

+
+# Calculate the size of the key content and split it into three segments
+content_size=$(wc -c < key.pem)
+segment_size=$((content_size / 3))


Why is segment size being used here but not when defining he offset? If the size limit is fixed, then the offset should be used to determine the number of segments?

As you suggested we will use maximum offset (768bytes) & calculate segment to write & read.

rlysecky · 2023-10-26T18:15:25Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_err_aes_nvread.sh

+#!/bin/bash
+
+#PCR index, NV index, and output file for the AES-256 key
+pcr_index=10


Can we rename this to incorrect_pcr_index to make it very clear?

This applies to both error scripts

We will update in latest script

rlysecky · 2023-10-26T18:16:01Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_err_aes_nvread.sh

@@ -0,0 +1,13 @@
+#!/bin/bash


Rename the error scripts to use "error" instead or "err" in name.

We will rename the scripts as per your suggestion.

rlysecky · 2023-10-26T18:17:24Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_err_aes_nvread.sh

+
+#PCR index, NV index, and output file for the AES-256 key
+pcr_index=10
+nv_index=0x1400004


For all scripts we should add a comment indicate that this value needs to be manually set by the user to specify the location in the TPM.

We will update in latest script

rlysecky · 2023-10-26T18:18:16Z

recipes-support/tpm-test-scripts/tpm-examples/tpm_nv_write_rsa_2048.sh

+#!/bin/bash
+
+# Define the TPM NV index and PCR index
+nv_index=0x1400004


Can we revise the scripts to use different indices, os if a user tries to run both, they are not overwriting the contents.

We can change over NV indices in separate scripts (AES, RSA, passphrase), as suggested to overwriting of the contents.

vishnusudhan · 2023-10-27T13:49:03Z

Hi @rlysecky ,

In few scripts , we are modifying the below changes,

In tpm_policy_creation script, in-order to store the measured pcr value , for first time after storing, we are requesting user to reboot & exiting the script . is it fine? or we can continue the first time alone without exiting.
upon reboot / from the second time we are checking the measure pcr value & current pcr value to check the measure boot & allowing to create policy pcr.

if [ -e "$file_path" ]; then
echo "File $file_to_check exists in the script's directory."
else
echo "File $file_to_check does not exist in the script's directory, creating measured.pcrvalues file."
cp pcr16.dat measured.pcrvalues
echo "reboot the device"
exit 1
fi
For reboot case, user needs to run tpm_policy_creation script atleast 2 times to create the policy pcr.

For normal NV read/Write cases , Shall we inform the user to run the tpm_policy_creation script every-time before running nv read/write scripts? so measured boot will always happen.
NV define's are going to be moved to the respective NV_write scripts ,as we are planning to declare separate NV indices.

rlysecky · 2023-10-27T14:16:25Z

I don't think a reboot should be needed. The assumption is that measured boot is enabled. The check for a zero value in the PCR should be enough. There shouldn't need to be a manual check for the measured boot state. The policy itself will enforce this. Why does the session creation need to happen two times?
Why can't we store the policy file rather than recreate it each time?
This seems ok

vishnusudhan · 2023-10-27T14:25:05Z

I don't think a reboot should be needed. The assumption is that measured boot is enabled. The check for a zero value in the PCR should be enough. There shouldn't need to be a manual check for the measured boot state. The policy itself will enforce this. Why does the session creation need to happen two times?
Solution: Ok. We will not request user to reboot and will allow to create the policy at the very first time itself.

Why can't we store the policy file rather than recreate it each time?
Solution: We are storing the policy file and we are not recreating.
As we wanted to check the measured boot case after every reboot, we thought to have the tpm_policy_creation script run every time before using NV read/write operations.

This seems ok

vishnusudhan · 2023-10-27T15:42:15Z

Hi @rlysecky ,

Scripts updated as per review comments & ready for review.

ReadME.md needs to be updated.

vishnusudhan · 2023-10-30T12:56:33Z

Hi @rlysecky ,

Updated README.MD file

Added TPM user examples

111359c

vishnusudhan requested a review from rjfendricks October 26, 2023 12:08

vishnusudhan self-assigned this Oct 26, 2023

rlysecky reviewed Oct 26, 2023

View reviewed changes

vishnusudhan requested a review from rlysecky October 27, 2023 15:41

vishnusudhan force-pushed the tpm_user_examples branch from 15b9b21 to b20dba2 Compare October 30, 2023 11:38

Updated scripts as per review comments

db408ec

vishnusudhan force-pushed the tpm_user_examples branch from b20dba2 to db408ec Compare October 30, 2023 12:49

Split policy creation and measured boot PCR scripts as per review

e218306

vishnusudhan force-pushed the tpm_user_examples branch from 7b15783 to e218306 Compare October 31, 2023 11:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added TPM user examples #20

Added TPM user examples #20

vishnusudhan commented Oct 26, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023 •

edited

Loading

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023 •

edited

Loading

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

rlysecky Oct 26, 2023

vishnusudhan Oct 27, 2023

vishnusudhan commented Oct 27, 2023

rlysecky commented Oct 27, 2023

vishnusudhan commented Oct 27, 2023

vishnusudhan commented Oct 27, 2023

vishnusudhan commented Oct 30, 2023

Added TPM user examples #20

Are you sure you want to change the base?

Added TPM user examples #20

Conversation

vishnusudhan commented Oct 26, 2023

Choose a reason for hiding this comment

vishnusudhan Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishnusudhan Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishnusudhan commented Oct 27, 2023

rlysecky commented Oct 27, 2023

vishnusudhan commented Oct 27, 2023

vishnusudhan commented Oct 27, 2023

vishnusudhan commented Oct 30, 2023

vishnusudhan Oct 27, 2023 •

edited

Loading

vishnusudhan Oct 27, 2023 •

edited

Loading