-
Notifications
You must be signed in to change notification settings - Fork 81
WIP: supports mutable IV in GcmParams, close #225 #226
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
Conversation
so that some PKCS11 implementation (like AWS CloudHSM) could write random IV into it Signed-off-by: Konge <[email protected]>
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 not using this scheme personally but the underlying issue seems real and the trade-offs in this solution reasonable.
Wdyt @hug-dev @ionut-arm @keldonin et al?
Although the trait constraint for
I open another branch 43c8863 that make all operations use the owned It's not necessary, but I think it can give us (and the compiler) more restrictions on when we can get things wrong. Any ideas about that? And is there anyone prefer passing the mutable reference instead of owned data? |
Wait, I thought mutable ref would allow the PKCS11 implementation to set these fields that are to be read by the caller, which would be impossible when using owned data? (unless the owned object is being returned from the function too). Btw, I think we can resume work on this given recent changes in Sorry for the delays @zkonge 🙇 |
Hello, I'm curious if there's any update on this one? I know this PR is out of date with the current release, 0.9.0, but I was able to make the same changes to an updated branch and I have success with AES GCM on AWS CloudHSM. I've verified that the mutable reference allows the IV to be provided back to the caller. This is with the V5 cloudhsm pkcs11 client. The changes don't seem to affect anything with SoftHSM tests, and my Luna SA works with it as well. |
Hmm... good question. We regularly have "contributor missing-in-action" situations here but it looks like all checks passed and the PR is merge-able so we could take it as is. @hug-dev what do you think about merging it as is? |
I think it's fine since we agreed in #225 that the approach of passing Although I would have liked a bit more comments on the reason of the mutability I think this is fine! We should complete this at some point moving all operations to Better to merge this as is if more people can benefit from it :) |
Yep, totally agreed on all points. Thanks for your time, Hugues 🙇 |
so that some PKCS11 implementation (like AWS CloudHSM) could write random IV into it.
Fixes #225