Update VerifyAttestation logic #209
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jkl73
changed the title
josephlr
approved these changes
Jun 25, 2022
josephlr
approved these changes
Jun 27, 2022
There was a problem hiding this comment.
@jkl73 @alexmwu I made some changes to how we do the hash algorithm validation.
Basically, this change just rejects all SHA-1 signatures, rather than using the opts.AllowSHA1 option to make it configurable. We only really need to allow SHA-1 for the PCRs, but we shouldn't ever need to accept SHA-1 signatures. TPM2s will always have SHA256 support for signing keys, even if a firmware issue leaves us with only SHA-1 TCG eventlog events.
PTAL and let me know if it looks good to you. I split this up into two separate commits to make review easier.
josephlr
force-pushed
the
fixcertkey
branch
2 times, most recently
from
4098f9e to
e430bd4
Compare
jessieqliu
approved these changes
Jun 27, 2022
alexmwu
approved these changes
Jun 27, 2022
josephlr
force-pushed
the
fixcertkey
branch
2 times, most recently
from
3ba3bee to
72ad2e3
Compare
|
LGTM |
We now only allow SHA-1 for PCRs and not for the signing algorithm. This does two things: - Reduces the risk of SHA-1 collision attacks - Reduces the scope of the AllowSHA1 flag - Simplifies the signing validation logic Signed-off-by: Joe Richey <joerichey@google.com>
Also splits out the cert/key verification stuff into its own functions. Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update VerifyAttestation to ignore pubkey if cert exists
Update hash validation on the signing hash algorithm in VerifyQuote
Signed-off-by: Jiankun Lu jiankun@google.com