Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix broken OEM re-ownership process #1357

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Conversation

ThePlexus
Copy link
Contributor

Problem
When using a custom password for TPM, the OEM re-ownership process is broken

Impact
The OEM re-ownership process breaks for any user setting a custom password and not just using 12345678

First appeared
6923fb5

Detail
on line 498, if blank, the TPM custom password is overwritten with TPM_PASS_DEF (eg, when no custom password is set by the user installing)

if [ "$TPM_PASS" == "" ]; then TPM_PASS=$TPM_PASS_DEF; fi

so far so good. $TPM_PASS should be used for all TPM interaction from this point. $TMP_PASS_DEF is now a disposed of variable.

we see that happens when resetting the TPM on line 712 (generate_checksums) is that $TPM_PASS is used (correctly)

if [ "$CONFIG_TPM" = "y" ]; then
  echo -e "\nResetting TPM...\n"
  tpmr reset "$TPM_PASS" >/dev/null 2>/tmp/error
---SNIP

The TPM now has either the custom password of the user, or the default of 12345678 depending on user selection.

On line 712, we duck into the generate_checksums sub, which for some reason reverts to TPM_PASS_DEF

    # create Heads TPM counter
    if [ "$CONFIG_TPM" = "y" ];then
	    if [ "$CONFIG_IGNORE_ROLLBACK" != "y" ]; then
		    tpmr counter_create \
			 -pwdo "$TPM_PASS_DEF" \
--SNIP

This then, rightly, fails due to

Authentication failed (Incorrect Password) (ox1) from TPM_CreateCounter

Problem
When using a custom password for TPM, the OEM re-ownership process is broken

Impact 
The OEM re-ownership process breaks for any user setting a custom password and not just using 12345678 

First appeared
linuxboot@6923fb5

Detail
on line 498, if blank, the TPM custom password is overwritten with TPM_PASS_DEF (eg, when no custom password is set by the user installing)

```
if [ "$TPM_PASS" == "" ]; then TPM_PASS=$TPM_PASS_DEF; fi
```
so far so good.  $TPM_PASS should be used for all TPM interaction from this point. $TMP_PASS_DEF is now a disposed of variable.

we see that happens when resetting the TPM on line 712 (generate_checksums) is that $TPM_PASS is used (correctly)

```## reset TPM and set password
if [ "$CONFIG_TPM" = "y" ]; then
  echo -e "\nResetting TPM...\n"
  tpmr reset "$TPM_PASS" >/dev/null 2>/tmp/error
---SNIP
```
The TPM now has either the custom password of the user, or the default of 12345678 depending on user selection.

On line 712, we duck into the generate_checksums sub, which for some reason reverts to TPM_PASS_DEF

```
    # create Heads TPM counter
    if [ "$CONFIG_TPM" = "y" ];then
	    if [ "$CONFIG_IGNORE_ROLLBACK" != "y" ]; then
		    tpmr counter_create \
			 -pwdo "$TPM_PASS_DEF" \
--SNIP
```

This then, rightly, fails due to 
```
Authentication failed (Incorrect Password) (ox1) from TPM_CreateCounter
```
@ThePlexus ThePlexus mentioned this pull request Mar 30, 2023
@tlaurion
Copy link
Collaborator

tlaurion commented Mar 30, 2023

Nasty.... And a reminder for testing things outside of qemu :/ (which fails injecting key in firmware because #1203)

Testing now. Not sure how that landed in master (my bad, sorry about that.)

@ThePlexus
Copy link
Contributor Author

Nasty.... And a reminder for testing things outside of qemu :/ (which fails injecting key in firmware because #1203)

Testing now. Not sure how that landed in master (my bad, sorry about that.)

As I say to my teams, cant test for every possibility of every combination of every change. we would not progress as we would be doing nothing but testing all the time. The tpm2 changes were a large change set, so small bugs are expected . keep up the good work sir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants