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: use tpm2 hash algorithm constants and allow non-SHA-256 PCRs #7811

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

uhthomas
Copy link
Contributor

@uhthomas uhthomas commented Sep 29, 2023

Pull Request

What? (description)

The conversion from TPM 2 hash algorithm to Go crypto algorithm will fail for uncommon algorithms like SM3256. This can be avoided by checking the constants directly, rather than converting them. It should also be fine to allow some non SHA-256 PCRs.

Why? (reasoning)

Fixes: #7810

Acceptance

Please use the following checklist:

  • you linked an issue (if applicable)
  • you included tests (if applicable)
  • you ran conformance (make conformance)
  • you formatted your code (make fmt)
  • you linted your code (make lint)
  • you generated documentation (make docs)
  • you ran unit-tests (make unit-tests)

See make help for a description of the available targets.

@frezbo
Copy link
Member

frezbo commented Oct 2, 2023

i think i'd still fail if sha256 is not found, since it would fail elsewhere. I;ll clean that up

@uhthomas
Copy link
Contributor Author

uhthomas commented Oct 2, 2023

@frezbo Is it possible to build a version of Talos with this change? I can test it on the affected system. I believe it should be fine as there will be sha256 banks available - the original code would fail if there were any non-sha256 banks present at all, which I believe was the main problem.

@frezbo
Copy link
Member

frezbo commented Oct 2, 2023

@uhthomas Could you try this image: ghcr.io/frezbo/installer:v1.6.0-alpha.0-60-g733aa4df2-amd64-secureboot?

This would work with a fresh boot only, an upgrade won't work

@frezbo
Copy link
Member

frezbo commented Oct 2, 2023

/ok-to-test

@frezbo
Copy link
Member

frezbo commented Oct 2, 2023

i think i'd still fail if sha256 is not found, since it would fail elsewhere. I;ll clean that up

this makes no sense, since it loops over all banks

@frezbo
Copy link
Member

frezbo commented Oct 2, 2023

/promote integration-trusted-boot

@uhthomas
Copy link
Contributor Author

uhthomas commented Oct 3, 2023

@frezbo Do you also have an ISO I could use? I just tried with the installer you provided, but it had the same error (which indicates to me the code hasn't changed, and the ISO probably needs to be updated too).

@frezbo
Copy link
Member

frezbo commented Oct 3, 2023

@frezbo Do you also have an ISO I could use? I just tried with the installer you provided, but it had the same error (which indicates to me the code hasn't changed, and the ISO probably needs to be updated too).

Ahh I see the problem, the first code runs from iso 😅 . I'll generate one and sent over

@uhthomas
Copy link
Contributor Author

uhthomas commented Oct 3, 2023

@frezbo Thanks for building that ISO for me. I've been able to install Talos with secureboot and TPM encryption.

@frezbo
Copy link
Member

frezbo commented Oct 3, 2023

/promote integration-trusted-boot

The conversion from TPM 2 hash algorithm to Go crypto algorithm will fail for
uncommon algorithms like SM3256. This can be avoided by checking the constants
directly, rather than converting them. It should also be fine to allow some non
SHA-256 PCRs.

Fixes: siderolabs#7810

Signed-off-by: Thomas Way <thomas@6f.io>
Signed-off-by: Noel Georgi <git@frezbo.dev>
@frezbo
Copy link
Member

frezbo commented Oct 3, 2023

/promote integration-trusted-boot

1 similar comment
@frezbo
Copy link
Member

frezbo commented Oct 3, 2023

/promote integration-trusted-boot

@frezbo
Copy link
Member

frezbo commented Oct 3, 2023

/m

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

Successfully merging this pull request may close these issues.

unsupported hash algorithm: 18
3 participants