-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add asymmetric encryption support to TPM provider #225
Conversation
94a11db
to
766c48c
Compare
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.
Thanks for adding support for asymmetric encryption!
// Test is ignored as TPMs do not support labels that don't end in a 0 byte | ||
// A resolution for this has not been reached yet, so keeping as is | ||
// See: https://github.com/parallaxsecond/parsec/issues/217 |
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.
We could also add a 0
at the end of the salt just to make the test pass on all providers?
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.
The one that does asym encrypt and decrypt with Parsec would work, but the other one won't unless we build a &str
label that ends with a 0 byte UTF-8 character (I actually tried that, it worked). Don't know which path is better, whether they should be ignored or not, @paulhowardarm was saying that we should have some tests that prove the problem and they should be disabled for now.
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.
ah I see. I was thinking it would be nice to have some tests activated (even if TPM specific) to make sure that it is working end-to-end.
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'll add an integration test just for that (the kind that only use the provider, not the whole service)
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 adding an integration test but I'll raise an issue to refactor those integration tests into the e2e_tests, think I have an easy way of doing it.
This commit adds support for asymmetric encryption to the TPM provider. Signed-off-by: Ionut Mihalcea <[email protected]>
766c48c
to
975cae2
Compare
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.
�Thanks for adding the test!
rsa = "0.3.0" | ||
rand = "0.7.3" | ||
sha2 = "0.9.1" |
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.
We should probably add that in a future PR to the README.md
This commit adds support for asymmetric encryption to the TPM provider.
Signed-off-by: Ionut Mihalcea [email protected]
Implements #217