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.
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
Support for multi-subject attestations using different hash algorithms #361
base: main
Are you sure you want to change the base?
Support for multi-subject attestations using different hash algorithms #361
Changes from all commits
27e9c05
afa1b7b
38c9e8f
33d31c1
c110832
d089013
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another idea for an optimization is to create the mapping between subject and the algorithms and expected digests in one pass. With this proposed solution, we make two passes, one to build the digest algorithms and one to verify the digests, which would be a performance regression from the previous solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that would require shifting a lot of logic into the already tested
getHashFunctions
(if I understand correctly), I'm not too concerned about the performance penalty of iterating through the digests here again. I do think it's worth revisiting when/if we implement #363.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be a constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slices are mutable and therefore cannot be declared as constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we err out if digest is empty? This is the current behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interpreting the spec to say the subject is invalid if the digest is the empty string seems pretty defensible to me / 👍 to erroring out on empty digests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little iffy on this, because verification may still pass if there exists a subject with the given digest, even if a subject exists that does not contain a digest. I'm not sure it's the responsibility of the verifier to make sure every subject contains a digest.